Preserve Unicode in automatically derived trail titles - #8
Merged
Merged
Conversation
Entire-Checkpoint: 01M228EVE8JJQ9J34JESFQX7BH
Entire-Checkpoint: 01M22923KW09CSPHDJCH64VAJ2
Entire-Checkpoint: 01M246V9SGBJ57VSN2D99HBNYJ
Entire-Checkpoint: 01M24Q8GF7HC3PPHQPN85NHXDA
Entire-Checkpoint: 01M2625NJXB8TT0E95YH1RDGE5
summary_generation.provider takes names out of the agent registry, and every name in that registry is also a valid `entire enable --agent` value, so writing the agent you code with into it looks right. For opencode and factoryai-droid -- registered agents with no GenerateText -- the whole diagnosis was "agent opencode does not support summary generation": no accepted values, and no pointer to where the value lives. Neither writer can produce such a value (configure --summarize-provider validates, and the picker's candidates are capability-filtered, a filter present since the feature's first commit), so it always arrives by hand editing. And nothing said so: Validate() checks only model-without-provider, `entire status` never mentions summary generation, and the error surfaces at `checkpoint explain --generate` / `dispatch` / `runner setup`, possibly weeks after the edit. The three "does not support summary generation" sites now route through unsupportedSummaryProviderError, which lists the capable agents derived from the registry so a stale name cannot outlive the capability. It is deliberately not $PATH-filtered: it answers which names the field accepts, and the PATH condition keeps its own error. entire doctor gains checkSummaryProvider, which reports the condition without fixing it -- the value may live in a committed settings.json where a rewrite changes everyone's configuration. An unregistered name and an off-$PATH binary are deliberately not reported; see the call site for why. Both walks, the doctor check and the error paths share one capability probe, summaryCapableAgent, so the predicate cannot drift between them -- AsTextGenerator consults CapabilityDeclarer for external agents, which is the part an inline copy gets wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26FGE98QA6Z4V1MZ9CT8PPK
Addresses the four review comments on trail 1272. Rename `selected` to `explicitRemote`. The field holds only the SyncRemoteSourceConfig tier, but the codebase already says "elected" for the resolver's all-five-tier value, so `selected` read cold at fanOutMatters invited reading it as "wherever checkpoints go" — the reading that makes the deliberate `observed` suppression look like a bug. Document the states that leave it empty. It is not only "nothing explicit is configured": ResolveCheckpointSyncRemote also errors when checkpoint_push_remote names a remote that is not configured (fail-closed, checkpoint sync disabled entirely) or when settings are unreadable, and all three collapsed into one empty string with only the first named. Cover the selected-and-pinned interaction. The table exercised `selected` against fan-out in four combinations but never set `pinned`, so the term that suppresses the warning had no case. A dedicated checkpoint_remote only takes effect when origin and every push URL of the push remote share the checkpoint repo's owner (checkpointRemoteIsInherited), so the case needs owner-aligned remotes rather than the fork-of-upstream shape the other cases use. Guarded against passing vacuously by asserting the fixture really pins a fanning-out fork; removing the !d.pinned term from fansOut() makes the case fail with the spurious two-URL warning. Use t.Context() for the call under test, which had opted out of the cancellation bound the setup probe in the same block already kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26PA598PAPF0XF162PV0Q5T
Addresses the review findings on this branch. TestFetchCheckpointRefFrom_InheritedDedicatedDoesNotRetry passed on unfixed code, and pre-fix it passed by a different mechanism than its name describes: the lead-less ownership vote ACCEPTS the dedicated store (origin and the checkpoint repo share an owner), so the probe failed against the store rather than the fork. "An error that is not ErrReferenceNotFound" is true either way, so the assertion could not tell them apart. Naming the remote in the error discriminates in both subtests — verified by deleting the fix hunk, where it now fails reporting acme/checkpoints.git. The bash git transport shim existed twice, in different packages, and had already diverged: one matched ls-remote|fetch and mapped URLs, the other matched ls-remote|fetch|fetch-pack|push and mapped bare remote names too. dupl cannot see it, since a heredoc is one string literal. Both now call testutil.GitTransportShim, parameterised by the subcommand set and the rewrite table. The table maps spellings to environment variable NAMES rather than paths: the unit fixture repoints CHECKPOINT_TEST_UPSTREAM after the shim is written to force a transport failure, so the indirection is load-bearing. Entries are sorted, so the generated script does not vary with map iteration order. The fetchCheckpointRefFrom comment did not name the distinction it was drawing. Both branches keep a single target; what the lead changes is how that target is RESOLVED. It also read as though the lead becomes the fetch target, which it does not in the common case — it joins FetchURL's ownership vote, and becomes the target only when that vote vetoes the store as inherited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27BCA5GJJ227MTQRY0W71XZ
The fetch-side ownership vote resolved the elected remote with GetRemoteURL —
one FETCH url — while the push side votes with GetPushURLs, every push url. A
remote can fetch from one owner and push to another (remote.<name>.pushurl),
and there the two disagreed: the push vote saw {acme, contributor} and vetoed
the acme/checkpoints store, so writes went to the fork, while the fetch vote
saw {acme, acme} and accepted it, so reads came from a store that never had
them. That is the asymmetry this branch exists to close, one topology over.
The candidate's push destinations now join the vote. ADDED to its fetch url
rather than substituted for it: the rule is that EVERY identity must be owned
by the checkpoint repo's owner, so widening the set can only turn accept into
veto — no read that falls back to the candidate today can start using the
store, and the blast radius across FetchURL's other callers is bounded in one
direction. Substituting would drop an identity and could do the reverse.
A failure to resolve the push urls is an error rather than a skip, matching
the fetch url beside it: an identity whose owner cannot be determined counts
as inherited, because dropping it fails OPEN.
gitremote gains GetPushURLsInDir, the push-side counterpart of the existing
GetRemoteURLInDir. Without it the vote's two halves would describe different
repositories whenever FetchURLOptions.WorktreeRoot is set — fetch url from the
named worktree, push urls from the process working directory.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M27C86X8G9FMP2Z7RGF65VS1
GitTransportShim generates a bash script by interpolating three caller-controlled values UNQUOTED — the subcommand list and the rewrite keys land in `case` patterns, the rewrite values land as `$NAME` — then chmods it 0755 while the caller prepends its directory to PATH. Every git invocation in the test therefore executes it, so a key such as `x) ; curl … | sh ;;` was arbitrary command execution. This was self-inflicted by the previous commit. The two duplicated scripts it replaced carried all three values as LITERALS and could not be fed anything; sharing them is what made them feedable, and putting the result in testutil — which carries no build tags so that every test package can import it — widened that from two files to the repository. Inputs are now validated fail-closed against allowlists that exclude every shell metacharacter by construction: git subcommands, remote names and URLs, and environment variable names. The check is a pure function so the property is directly testable rather than only reachable through a fixture, and the test drives thirteen vectors — pattern breaks, command substitution, backticks, quotes, globs, newlines, brace expansion — plus the ordinary remote names and URLs the fixtures actually pass. The allowlists are deliberately narrower than what git accepts. A fixture needing something outside them should write its own script rather than widen them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27D0430CKGAQXD52G3YEB1K
Reverts the shared shim helper entirely rather than keeping it hardened. It generated a bash script by interpolating three caller-controlled values unquoted, chmod 0755'd it, and the caller put its directory first on PATH — so every git call in the test executed it and a rewrite key such as `x) ; curl ... | sh ;;` was arbitrary command execution. Validation closed that, but the helper should not have existed. The two scripts it replaced carry all three values as LITERALS and cannot be fed anything; sharing them is what created a path for input to reach generated shell, and putting it in testutil — no build tags, importable by every test package — widened that from two files to the repository. Removing it restores a state where the property holds by construction instead of by a regex, and it was out of scope besides: the task was to review findings, and the duplication finding was the one recommended for deferral. The duplication it addressed is back to being a known low: two copies of the shim, in different packages, already differing in their matched subcommands and whether bare remote names are mapped. dupl cannot see it, since a heredoc is one string literal. A third copy is the point at which extraction earns its risk — done then with a literal script per caller, not a generator. Kept from that commit, both being fixes to real findings: the assertion naming the remote an inherited-veto error came from, without which the test passed on unfixed code; and the regression test for a lead whose fetch and push URLs have different owners. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27DH4KH97KQBYKN9C6D3ZQV
The previous commit added the read candidate's push urls to the fetch-side
vote but left origin exempt: fetchOwnershipURLs returned nil for it, on the
grounds that the caller already counts origin's FETCH url. Its PUSH urls were
never counted, and origin can carry a remote.origin.pushurl naming another
owner like any remote.
So the same asymmetry survived one branch over. With origin fetching from
acme/app and pushing to contributor/app, the push side voted {acme, other} and
vetoed acme/checkpoints, while the fetch side voted {acme} and accepted it —
writes to the push destination, reads from a store that never had them.
origin's push urls now join the vote, leaving only the no-candidate case
returning nil, where there is no read candidate to ask about. The resolution
is shared with the non-origin path (pushOwnershipURLs) so the two cannot
diverge again, and a failure stays an error rather than a skip: an identity
whose owner cannot be determined counts as inherited, because dropping it
fails OPEN.
Verified the new test fails with origin exempt again, where it probes
acme/checkpoints.git instead of reading origin.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M27DYAPV6ZCADFCB0J6BZX2S
…ilure When the ownership vote vetoes a configured checkpoint_remote, the elected remote is where the single-remote gate already routed writes — so it is the checkpoint destination, and a missing ref on it means the ref does not exist. It was being probed as non-authoritative, which turned that into "refusing to treat this as absence": callers could not tell "no such checkpoint" from "could not reach it". fetchURLResolved now reports the veto alongside authoritative, and the candidate-aware entry point probes a vetoed target as authoritative. Three things this deliberately does not do. It does not retry the legacy origin tier. The target stays single, so a stale tip on origin can never be installed as the canonical local ref on the strength of an elected-remote miss — which is also why this needs no repo-level veto verdict: the earlier reading of this defect implied checking later candidates, and there each candidate re-runs its own vote, so `origin` re-accepts the store and the chain probes it instead. It does not touch the lead-less FetchCheckpointRef, which must keep refusing: its fallback may be a remote that never hosts the store's refs and it has nothing else to consult (TestFetchCheckpointRef_FallbackTargetNeverClassifies Absence, on main, pins this). It does not change authoritative, which answers a different question — "is the dedicated store what serves reads". fetchURLAuthoritative stays a thin wrapper so FetchURL and ReadsDedicatedStore see no change; a vetoed store must not report as serving reads. The test becomes a two-case table, because the two now differ: a miss on the vetoed target is absence, an unreachable one is not. Verified it discriminates — with the change reverted the absence case fails and the transport case still passes. It flips an assertion this branch had written, at the reviewer's explicit direction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27EK7H2W9X01X7VCAP1NCTA
The earlier change added the read candidate's push destinations ALONGSIDE its fetch url. That made the read vote stricter than the push vote, which is not safer — just asymmetric in the other direction. A remote fetching from another owner but pushing to the checkpoint owner had the push side accept the store and route writes there, while the read side vetoed it and fell back to the fetch repo. The identity sets are now identical by construction: origin plus the candidate's PUSH destinations, the same pair PushURL hands checkpointRemoteIsInherited. The candidate's fetch url is not a voter. This is what the original finding said to do. The additive variant was chosen on the reasoning that widening an every-identity-must-match check can only turn accept into veto, so it could not make reads less safe. That reasoning answered the wrong question: the invariant is reads landing where writes went, not maximal vetoing. Both directions are now pinned, each verified to fail against the other implementation: a candidate pushing away from the checkpoint owner vetoes the store, and one pushing to it keeps the store even when its fetch url points elsewhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27EXAYYAM6Y7PAZ5S62DJ75
Four findings from the review of the remote picker: - Do not offer the picker when the local settings layer is rejected. The choice is saved to .entire/settings.local.json, so a tracked local file made every real answer abort `entire enable` after the user had already answered — and, on fresh setup, discard the agent selection. - Treat huh.ErrUserAborted as the picker's own "Keep current destination" option instead of failing the command with a raw `huh: user aborted`. A cancelled context still aborts. - Run the picker after the up-front flag validation, so an invocation that was always going to be rejected (a bad --checkpoint-backend, conflicting --local/--project) never opens a prompt first. The deferred destination report keeps its place at the top: it needs the choice pointer, not the answer, so it still runs last. - Restore the multi-remote ambiguity note on every path where no picker ran. The gate keyed on "the prepare step ran", which is every `entire enable`, so non-interactive runs lost the fanout warning, the remote count paragraph and the checkpoint_remote pin advice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27EZT3YTAQNRY0E6D7VS2CW
Three review findings on the new check. The remedy printed `--summarize-provider <a|b|c>`, which a shell reads as a redirect and a pipe, so the obvious copy-paste fails before it runs. It now prints one runnable command and lists the alternatives on their own line, where they are data rather than something to paste. The remedy also omitted --local. `entire configure` with no layer flag writes the PROJECT file whenever one exists (settingsTargetFile), so a provider coming from settings.local.json was "fixed" in a file the local layer still overrode: the command reported success and doctor still reported the fault. Verified against a real repo before and after. The diagnosis now names the file supplying the value and adds --local when that file is the local layer. Finally, doctor's enumerated "Checks performed" list did not mention the check at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27T4SBPE8KJ2XZ9HGYRQS4G
Two review findings on the layer detection added in the previous commit. summaryProviderSourceLayer read .entire/settings.local.json directly, so it could not see that the loader drops a TRACKED local file wholesale. With the same provider in both files it attributed the value to the local layer and advised --local, sending the user to edit a file the loader ignores while the project-level fault stayed put -- the same wrong-file advice the --local fix was meant to end. It now consults LocalLayerRejection first, and takes the merged settings rather than a bare string so that verdict is available to it. The tests reached that code with the process still pointing at the developer's own repository, so they read the real .entire/settings.local.json and their output depended on whose machine they ran on. The isolation now lives in the shared helper, since any test reaching the fault branch needs it. The layer test runs against real settings loading and the real registry: opencode is genuinely incapable so the registry needs no stub, and the tracked-local case cannot be stubbed at all -- localLayerRejection is unexported, so only a real Load over a real tracked file produces it. Verified non-vacuous by removing the guard and watching that case fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27WS23ZXSD65P9ZV8C3NEY8
`entire graph` downloaded, verified and placed entire-graph.exe under pkg/graph/ correctly, then linked a 0-byte entry into bin/ that Windows refused to execute: "The filename, directory name, or volume label syntax is incorrect." Two defects in materializeManagedEntry combined. os.Root.Symlink on Windows stores an absolute target verbatim, without the `\??\` prefix that CreateSymbolicLinkW adds, so the link is created (Go re-enables the symlink privilege itself, so an elevated shell gets no error) and then cannot be followed. And os.Root.Link resolves both operands inside the root, so the hardlink fallback, handed an absolute source, failed as a path escape on every platform and never ran once. Windows now skips the symlink entirely; the source is converted to a root-relative name (managedTreeName) so the hardlink works for every remote install, and the copy remains the fallback for local-dev sources outside the tree. Unix keeps symlink-first. checkManagedPluginRunnable treats an unfollowable symlink and an empty file as reinstall-fixable, so entries left by earlier builds get the `plugin install graph --force` remedy instead of a raw fork/exec error. The materialize tests no longer skip on Windows and read THROUGH the entry, which is the assertion that would have caught this; withPluginDir releases the memoized plugin-dir root so TempDir cleanup succeeds on Windows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M27XP9BBGJ2F8D4TKZX3TM33
plumbing.Hash is a struct carrying an object-format field alongside its bytes, so == compares that field too. FromHex stamps sha256 on a 64-char hash and leaves the field unset on a 40-char one, which means == holds only while go-git's tree decoder makes the same choice. Probed against the pinned v6.0.0-alpha.5.0.20260812155443, it does — sha1 gives "" on both sides, sha256 gives "sha256" on both — so this works today by coincidence. If a decoder ever stamped "sha1", every candidate would compare dirty, the phantom carry-forward entireio#2336 fixed would return, and nothing would fail, because no test pins the format field. Equal ignores it by construction and is already what the fallback branch three lines down uses. Also records two decisions reviewers reached independently. hash-object takes its paths as argv rather than --stdin-paths because hash-object has no -z, so --stdin-paths is newline-delimited and splits a legal path containing a newline into two nonexistent ones; the chunking that supports argv is therefore not scaffolding to be removed. And TestGitWorktreeDiffCallSitesDoNotRefreshTheIndex drops the gitInvocationMarkers filter that TestGitStatusCallSitesPassNoOptionalLocks keeps because the failure direction inverts between them: a wrapped argv separates the flag from "status" (loud false positive) but would separate the marker from "diff" (silent false negative). Follow-up to entireio#2336; resolves findings on trail 1277. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27XQH2W43GWB5ZX4V0VT0VW
Signing in from a fresh CI runner hits GitHub's new-device verification, which emails a code and cannot be automated. GitHub skips it for accounts with 2FA enabled, so the test account uses an authenticator app and the browser step now fills the RFC 6238 code computed from E2E_GH_TOTP_SECRET, required alongside the username and password. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2G8TWAEMQ6T2ZB2AEP0NK0F
The repository secrets now carry the same names as the test's variables. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2G908725NNX3QRF5EZRV6T1
Entire-Checkpoint: 01M2GB520NW38WGV1WVWRFJZFY
managedEntryUnrunnable printed a bare `entire plugin install <name> --force` for every repairable bin/ entry, while doctor had moved to entryRepairFix: the recorded URL with --pin/--allow-unverified for a release install, rebuild-or-remove for a manifest-less local-dev entry. The dispatcher now reuses the helper, so `entire graph` failing on a broken entry prints the repair doctor would, and the two cannot drift. Also from the review of 69136b4: the non-executable-link problem text drops "dangling or" (a dangling link is caught by the stat first), the unreadable-manifest branch of entryRepairFix gains a test, and the stale duplicate comment on the stat guard in checkManagedBinaryIntegrity is removed. external-commands.md describes the manifest-aware repair. Findings: 01M2GASRMQ, 01M2GASV6G, 01M2GASZN7, 01M2GAT1MT. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2GBKXA9KYS83GR3M0BXC6D0
entire-api's repo_full_name is the bare pair for both forges — Core builds a native repo's full name as project/repo, never et/-prefixed — so the et/-qualified comparand made verifyResponseIdentity reject every native --repo read. newAPICheckpointReader now derives both the forge-qualified display ref and the bare identity comparand from one (forge, owner, repo) triple, so a caller cannot pair them inconsistently again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 01M2GXQGRR2RFGSHCJYCGP6YG8
…d pages TestMain logs out after the run so the shared test account does not accumulate a live login per CI run. The clonable poll treats a by-name not-found from `repo get` as not ready yet instead of failing the test. The browser stepper reports the page a login is stuck on after a minute without navigation rather than waiting for the context deadline. The TOTP key is decoded unpadded as GitHub displays it, and the task description names the TOTP secret it requires. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2J6DNWZ0D54WQ1NB7S3KGY1
The branch trigger was a temporary aid for validating the workflow from its own branch; the suite is green there, so drop it before merge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2J6M23316W98E65XV3DBBKT
Each call to runEntireWithTimeout creates its own context, so cleanup getting a fresh deadline holds by construction, and the fixture never spawned the descendant that WaitDelay exists for. It also ran only after a production login and pinned the package to a single binary override. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2J6XHN1QAAJBY9AYVXZFBJ6
…explain fix checkpoint explain for native repos
totpCode is support code for the suite's login, which exercises it end to end on every run; a second layer of tests around it is not worth keeping. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2J8GFB19SGNZAS19KZ7VP75
The package has one test, so a regex over it selects everything or nothing. The mise task keeps its filter argument for local use. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2J95ZV95H4S1066PAY6PWBX
GitHub shows the key unpadded, but a base32 key copied from elsewhere carries trailing "=", which the unpadded decoder rejected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2J9SPZP6J9RBTKE4QREAPM3
…utput A create that succeeded but printed unusable JSON failed the test before its cleanup was registered and left the resource behind on the shared account. Cleanups now register by name as soon as the create returns and switch to the ULID once decoded, since an already-deleted ULID exits 0 while a name that no longer resolves is an error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2JAMC63BH2AGBW1HWNZ7ZGC
…cleanup t.Cleanup cannot run when the process is killed: a package timeout, a cancelled job, or a lost runner. With a three-org quota on the test account, one such run blocked every later one until someone deleted the leftovers by hand. Each run now starts by deleting e2e-cp-* orgs older than 30 minutes, with their projects and repos; the age gate leaves a run in progress elsewhere alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2JB5PS1Y58CFQ6PN2N1QGXE
…dows fix: install the managed plugin bin entry on Windows without a symlink
On Windows, answering a plugin confirmation needed a second keypress before the confirmed action started. Bubble Tea only builds a cancellable console reader for os.Stdin; the separately opened CONIN$ handle the prompt uses gets a fallback whose Cancel is a no-op, so the read its loop issued after the answer stays pending, and Go's os.File.Close on Windows waits for every pending operation — which a console completes only on a keypress. PromptTTY.Close now cancels pending I/O on the input (CancelIoEx) before closing it, and the plugin confirmation opens its terminal through interactive.OpenPromptTTY instead of tea.OpenTTY so it goes through that Close. The login key prompt already did, so it is fixed by the same change. The cancelled read reports io.EOF, so the close stays after the form has returned. Verified on Windows 11 (conhost) with a minimal program: Close blocked until a key was injected before, returns immediately after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WyJLbQP5sdFssuU1f1gSMU Entire-Checkpoint: 01M2JCVA9BSMQX48YMV809YT1N
filterToUncommittedFiles decided "already committed" by comparing the working tree's raw bytes against the HEAD blob. Under core.autocrlf — the Git for Windows default, and what both testutil.InitRepo and e2e/testutil/repo.go set — the working tree holds CRLF while the blob holds LF, so every committed text file compared unequal and the filter never dropped anything. That matters because the filter is what stops an agent's mid-turn commit from being checkpointed twice. With it broken, turn-end saw a file that PostCommit had already condensed, missed the "no changes, skip" gate, and minted a fresh shadow branch on the *new* HEAD seconds after PostCommit deleted the old one. Nothing condenses that branch away — the session ends and no further commit arrives — so it outlives the session. On the nightly install smoke that surfaced as copilot-cli failing TestMultiSessionSequential on windows-latest with "shadow branches should be cleaned up within 10s after commit", every night since 09-11. The condition was latent from the start and only became reachable when copilot-cli's turn-end file list stopped being empty. The last green nightly ran the same broken filter, so its list was genuinely empty rather than correctly filtered; the only extraction change in that window is entireio#2341's restrictedProperties.filePaths fallback. That fallback follows Copilot moving the field out of properties, so both halves are needed to explain the flip — and neither is at fault: reporting a path the agent later committed is correct at that layer, and narrowing it to what is still uncommitted is this function's job. The three other Windows agents produce no turn-end list at all and so never reached the comparison. Compare through gitrepo.HashWorktreeFiles (git hash-object) instead, which applies Git's path-specific clean filters. This also fixes .gitattributes eol/text rules and clean filters such as Git LFS, which the byte comparison got wrong for the same reason. Symlinks stay on the raw path: hash-object follows the link and hashes the target's content, while a Git symlink blob stores the target path. Anything Git cannot hash falls back to the previous comparison, so the function still fails open — toward keeping a file rather than dropping one. Both call sites benefit: turn-end and the subagent task path, whose own comment already described this exact failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2JJHKDTXB4SAFGYRC6FWRWJ
…one-liner docs: install Homebrew casks by fully qualified name
…ision checkpoint: accept case-folded shard directories in ParseRef
Checking only the HEAD entry's mode was not enough. git hash-object
follows a working-tree symlink and hashes the target's *content*, so a
tracked regular file replaced by a link to identical content hashes equal
to its HEAD blob and was dropped as "already committed" — while git status
calls it a typechange (" T"). The HEAD entry is still a regular file, so
the tree mode cannot catch it. Verified directly: after replacing a
committed foo.txt with a symlink to a file of equal content,
`git hash-object -- foo.txt` returns the HEAD blob hash exactly.
That drops a real change, which is the direction this function must never
fail in. A FIFO is worse than wrong: hash-object blocks reading it, and on
a hook path that costs the caller its whole budget.
strategy already had this rule and a test asserting it, so rather than add
a third copy the pair moves to worktreedir.HashableEntry, where both
callers reach it. It cannot live beside HashWorktreeFiles in gitrepo:
worktreedir's own test imports testutil, which imports gitrepo, so that
edge is an import cycle in the test binary. Its tests move with it, and
content_overlap_test keeps the half that is still its own — what the
confined fallback answers for a symlink.
Reported by Cursor Bugbot and Copilot on entireio#2482, both pointing at
strategy.requiresConfinedWorktreeHash as the precedent. They were right.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M2JNBMD9081P0M1ZTX1KKXM2
…ails PromptTTY.Close returned before closing either console handle when CancelIoEx failed with anything but ERROR_NOT_FOUND, and every caller discards Close's error, so both handles leaked silently. The release failure is now joined with the close errors instead of replacing them. The Windows regression test also asserts that the released read reports io.EOF. Without that, a Close that ran before the read was pending still passed: CancelIoEx found nothing, the read then hit a closed handle, and both selects succeeded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2JNSSVQD3DSS99TVQRT0EP9
…-autocrlf fix(lifecycle): compare committed files through Git's clean filters
…er-interactive fix: release the pending console read before closing a prompt terminal (Windows double Enter)
…luster-hint git-remote-entire: delete the wrong-cluster hint, which cannot fire
The sweep of leftover orgs is now best effort: a leftover it cannot list or delete is logged and skipped instead of failing the current run, and "swept" is logged only once the org delete succeeded. The device login runs in the isolated state dir rather than the checkout, a stalled about:blank page is named instead of printed as an empty host, and the sweep no longer reuses one output variable across nested listings. Restores the offline RFC 6238 vector test, now also covering a padded key: the production workflow runs only on main and manual dispatch, so this is the only check of the algorithm that runs on every PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2JRD2XPQH9Y8VSHBTC88VNX
Add control-plane E2E tests with device-flow login
MuskanPaliwal
pushed a commit
that referenced
this pull request
Sep 24, 2026
…ionFile #5 external.go reimplemented SessionStore.Name's relative branch verbatim — same IsAbs/VolumeName pairing, same ToSlash(Clean(FromSlash(…))) — without the comment explaining why the pairing is needed, and it had to know that the store checks a name only after Name has produced one, which is WriteFile's private prologue. Name's relative branch is now a named primitive both callers share, the store exposes ValidateExternalSessionRef (every rule needing no store, plus whether the ref is filesystem-shaped) and ValidateExternalWriteRef (the store-backed half), and ValidateWritePath is unexported. #6 The preflight ran after marshalling and was gated as a whole on RepoPath != "". Both current callers set RepoPath, which is what makes that a check that disappears silently for the next one; the lexical rules need no repo, so only the store-backed half is conditional now. The remaining asymmetry — an opaque relative ref is forwarded as given while an absolute one is resolved — is the protocol's, not this function's, and is left alone deliberately. #7 ErrOutsideSessionStore said "path is outside the agent's session directory" for an ID that never resolved anywhere, which names the wrong problem. Malformed names now report ErrUnsafeSessionName; a path that genuinely left the store still reports ErrOutsideSessionStore. validateWriteName no longer claims to "inspect" anything — it touches no filesystem. #8 The volume-separator check existed in both validators, worded identically, while the shared reason helper omitted it. It moves in. ValidateSessionID keeps its separator check ahead of the helper so a Windows absolute path still reports the separators rather than the colon. The ResolveSessionFile contract this branch wrote into agent.go had two live violations: copilotcli's resolveTranscriptRef and cursor's, both passing a raw payload ID to a resolver that puts it in a directory position. Both now resolve through SessionStore.SessionFile, and a GitGrepGuard fails the build on the next one. Two guards in buildAgentStop go: isSubagentAgentStop already returns false for an unsafe ID via SessionFile, and ExtractModelFromTranscript reads a path and never touches the ID, so gating it only dropped model attribution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2RM92E1D4CK1JJ6NDSMTMHC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/MuskanPaliwal/cli/trails/8
This draft pull request was opened by Entire after CI was requested for the linked trail. Feel free to edit the title or body — the link above is what keeps the trail and PR connected.