Post-push cleanup deletes an uncondensed shadow ref when session state is malformed - #4
Draft
MuskanPaliwal wants to merge 123 commits into
Draft
MuskanPaliwal wants to merge 123 commits into
MuskanPaliwal wants to merge 123 commits into
Conversation
Droid produced no checkpoint on Windows: commits landed and not one agent
hook fired. The Windows artifact from run 34127634577 shows only git hooks
in entire.log plus "prepare-commit-msg: no active sessions", while
claude-code on the same runner, with byte-identical sh wrappers, fires all
of its own.
The difference is who runs the command. Droid's Windows build hands each
hook command to cmd.exe (spawn("cmd.exe",["/c",H]) at 0.109.1, shell:
"cmd.exe" at 0.178.0, %ComSpec% /d /s /c at 0.215.1); only its non-Windows
build runs them under sh. cmd.exe reads our wrapper's `>/dev/null` and
`2>&1` as its own redirections and `&` as a separator, so the line is cut
apart and `entire` is never reached. No session, so a commit has nothing to
condense, and nothing says why.
This is what UseWindowsProductionHooks exists for, but its probe is the
wrong gate here: it only establishes that `sh -c 'exit 0'` runs, a command
with no cmd.exe metacharacters, so a host with Git Bash reports success and
keeps the sh wrapper. Droid never uses sh on Windows at any version, so add
HookHostIsWindows — the plain host check over the same test seam — and gate
droid on that. Codex and Cursor keep the probe: they resolve a shell
themselves, so for them a working sh really does mean the sh wrapper runs.
Also adds WrapProductionPlainTextWarningHookCommandForOS, the missing
selector for the one wrapper shape droid uses that had no ForOS variant.
Migration needs no new code — dropStaleEntireHooks already replaces a
managed hook that is not the wanted one, and IsManagedHookCommand
recognises both Windows shapes; the tests pin that, since it is what stops
a repo carrying both entries and double-firing. Both new droid tests fail
against the probe-gated predicate and are named so the test-windows CI job
runs them on a real Windows runner.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M236D0NE731AP0HYE49YZG11
Three pre-existing droid tests hardcoded the sh wrapper, so on a Windows host they now assert a command InstallHooks no longer writes there: FreshInstall (all 8 hooks), PreservesUserHooksOnSameType, and ReplacesLegacyLocalDevHook, which looks the installed command up before seeding the legacy one over it. Reported from a real Windows box, and confirmed there against main — on main these fail only with TempDir cleanup noise, with no assertion failures. CI would not have caught it: ci.yml's windows job runs -run '(Windows|MSYS)' and none of those three names match. Only a human running the full suite on Windows sees them, which is exactly who hit it. Route them through droidHookCommand / droidStopHookCommand, which apply the same host-keyed selector InstallHooks uses, so there is no longer a form to drift. The two tests that assert WHICH wrapper is chosen keep naming it outright — they pin the host with SetWindowsHookProbeForTesting, and reusing the selector there would restate the implementation and pass either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EyeD2x5atUN41dQgjDFSWX Entire-Checkpoint: 01M25AZKBSNBA1C4P285KGXR6H
The exclusion was for the hook-path defect this branch fixes: droid's Windows build runs hook commands through cmd.exe, which shredded the sh wrapper, so no agent hook fired and no checkpoint was ever recorded. Restoring takes the three things the exclusion comment named — the name in AGENTS, the installer, and its bin directory on PATH. That directory is %USERPROFILE%\bin, not the .local\bin the other Windows installers use; droid's install.ps1 picks it and updates only the stored user PATH, which later steps do not re-read. The added iex needs no $LASTEXITCODE guard for the same reason the claude one does not: droid's installer shadows Write-Error with a function that exits 1, so every failure path ends the step. Note this workflow installs the PUBLISHED nightly and checks out the tag it resolves from `entire version`, so it does not test this branch. It first exercises the fix on the scheduled run after a nightly carrying it ships — tagging at 06:00 precedes the 09:00 smoke, so the two stay in step. A manual dispatch before then would install a pre-fix nightly and fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EyeD2x5atUN41dQgjDFSWX Entire-Checkpoint: 01M25B35SXVGGEZPN5BF7ZN0RF
…ate's doc Review findings on entireio#2349. The HookHostIsWindows doc claimed Codex and Cursor "resolve a shell themselves". That is contradicted by three places in this repo that say their runners go through cmd.exe as well, so it was a reassurance the code does not support. What actually separates them is evidence: Codex passes the Windows nightly with the sh wrapper installed, so whatever its composition does the wrapper survives it; Cursor is excluded from that matrix for want of tmux and may have this same defect. Say that instead, and say how to settle it. Droid's composition differs from Codex's in the two ways that decide whether a wrapper survives — `/d /s /c`, and the command appended verbatim rather than quoted — so it nests one cmd.exe inside another and leans on /s's strip-first-and-last-quote rule. That was checked by hand on a Windows box; runWindowsWrapper already executes wrappers for real under the `(Windows|MSYS)` CI filter, so add droid's runner beside it and pin the behaviour before merge instead. Both runners now share one body. Record the version scope too: 0.109.1 spawned without windowsVerbatimArguments, so libuv backslash-escapes the nested wrapper's inner quotes into something cmd.exe does not understand. It stays broken, as it already was under the sh wrapper. The bare shape would likely fix it; that is not taken here because it would trade the shape verified on current droid for one that is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EyeD2x5atUN41dQgjDFSWX Entire-Checkpoint: 01M25DM3FD4PA0RNV373YW2F5V
CI lint went red on 005b9ea: maintidx put InstallHooks at MI 19 against a threshold of 20. The version-scope comment I added inside the body is what tipped it — maintidx's LOC term counts comment lines — and the function was one line from the threshold before that, at cyclomatic 29. Trimming the prose would buy one more comment's worth of room, so move the seven wrapper constructions to silentHookCommand / stopHookCommand instead. That takes the explanation with them, to a place where it documents the thing it is about, and leaves InstallHooks doing only what its name says. The verbs become the HookName* constants rather than seven repeated literals, and the test helpers now delegate to the same two functions, so the wrapper choice is stated once. Reported by review on entireio#2349. My own lint run missed it because I passed --new-from-rev origin/main: the issue anchors on line 64, the unchanged func signature, so the baseline suppressed it. Neither mise-tasks/lint/go nor golangci-lint-action applies that baseline — a comment can fail lint on a line the diff never touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EyeD2x5atUN41dQgjDFSWX Entire-Checkpoint: 01M25E0EX7WC46WMHNP9MJBTJY
Entire-Checkpoint: 01M26DK1CD43X78H24HFGQBBZT
`entire status` reports what sessions exist, and reporting should not change what it reports. Today status goes through List, which deletes any record that has gone stale — so asking the question mutates the answer. Split the stale cleanup behind a parameter and expose ListReadOnly for the passive path. List and Load keep their existing contract: doctor and the session sweeper own cleanup and are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27BBHCNZ7H36WTWRFT6TQB1
Two defects in how status reports sessions, both fixed by the same change. `--json` collapsed active_sessions to one entry per agent, so a developer running two sessions for the same agent — one per worktree, a normal setup — saw only one of them, and there was no way to recover the other from the output. Each session is now its own entry, carrying session_id. Consumers wanting a per-agent view can group on the agent field; they could not do the reverse. Both the text and JSON paths also read through List, which deletes stale records as a side effect, and ran finalizeExitedSessions. So looking at status changed what status reported. Both now read through ListReadOnly and finalize nothing. Cleanup is unchanged for the callers that own it: doctor and the session sweeper still call List and still finalize. Replaces TestRunStatusJSON_DeduplicatesSessions, which pinned the old shape, with a test asserting each session is reported separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27BPPKF4WMPH5FJRK1ZV126
Ports the two integration regressions: two sessions for one agent across worktrees both appear with their worktree_path, and a dead-owner session is reported as exited with its state file and transcript byte- and mtime-identical afterwards. Adds worktree_path and branch to the per-session JSON entries — without them two entries for the same agent are reported but not distinguishable, which was the point. CLAUDE.md and sessions-and-checkpoints.md both stated that finalizeExitedSessions sweeps inside `entire status`. That is no longer true and is now recorded as a deliberate property: doctor and the sweeper own cleanup, status observes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27C71EV3V1V1MQYXE4ZQ6S1
Entire-Checkpoint: 01M27FNYWAG56R5H9CV0BCZSZK
Entire-Checkpoint: 01M27XEBHFT7E8K000W92QZ05E
Entire-Checkpoint: 01M282EWYTJR6CC1QPSK5CW3PJ
agent-guide.md's InstallHooks template built its hook command with an unconditional agent.WrapProductionSilentHookCommand, with no mention of Windows, of either gate, or of the *ForOS selectors — and agent-integration-checklist.md points every new integration at that template. Three built-ins now choose per host (codex and cursor on UseWindowsProductionHooks, droid on HookHostIsWindows), so the guide was teaching the one shape this branch exists to fix, on the platform where it fails at exit 0. Adds a "Choosing the hook wrapper" section naming both gates, when each applies, and who uses them, plus the rule that decides it: read the agent's runner rather than reaching for the probe, because the probe only establishes that a metacharacter-free command runs and so passes on a Git Bash host that is still broken. The template itself now selects per host. That needed two adjacent corrections to make the snippet coherent: InstallHooks takes a context.Context (the interface has since gained one, and UseWindowsProductionHooks needs it), and the root helper is paths.WorktreeRoot(ctx) — paths.RepoRoot does not exist. Docs only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M286YRZARVSHMNM8B7Z2Z5P2
Step 8's InstallHooks template did filepath.Join(repoRoot, ".youragent", "settings.json") and read it directly — the pre-HookConfigFile shape. CLAUDE.md's Root Anchors section names an agent's hook config as one of the eight trees with an owner and says never to assemble a path into one and hand it to os.ReadFile: a symlinked .youragent arriving with the checkout is resolved before any boundary exists, and this is the file naming the command Entire executes on every agent turn. The template now opens through agent.OpenHookConfig and reads and writes through the handle, and shows HookConfigRelPath alongside it, since OpenHookConfig takes the same string HookConfigLocator returns and doctor's symlink diagnosis reads the path from there rather than from a hand-kept list. The checklist gains the matching item, naming the guard test that requires the interface. Docs only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M28768DHS7GEBAAM8TZF6Q1X
The droid Windows fix on this branch has no pre-merge check: verifying it needed a throwaway edit to this workflow, which hardcoded claude-code in four places, so the run that proved it is not reproducible from the branch. An `agent` input defaulting to claude-code costs nothing on the push path or the existing caller, and makes that run repeatable via workflow_dispatch. The bootstrap invocation, the --agent flag and the artifact name parameterize by substitution; the installer does not, so there is a step per agent — each ships its own script and lands its binary somewhere different (droid writes %USERPROFILE%\bin, not .local\bin), and neither installer's PATH update is visible to later steps, which is why both append to GITHUB_PATH. Both keys reach the run: droid's e2e runner writes ANTHROPIC_API_KEY into its BYOK settings, and FACTORY_API_KEY is its own auth. The concurrency group gains the agent, or a droid run would cancel a claude-code one. e2e.yml forwards the agent and admits droid to the Windows leg; an unset agent still means claude-code, so the push path is unchanged. actionlint is clean on e2e-windows.yml; e2e.yml's three reports are pre-existing on main, on lines this does not touch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2879F3SRVWXF6C7QVQ8P63P
Entire-Checkpoint: 01M287AR8Y7EZMJ3CRXDGFQYV8
Resolve conflicts in explain_summary_provider.go (keep main's comment on why binary names are spelled out, keep this branch's message that lists opencode) and go.mod (take main's layout; `go mod tidy` re-adds hujson at the same pinned version). Follow-ups from main's summary-provider guard rails now that opencode can generate text: - TestSummaryCapableProviderNames_MatchesTheBuiltInAgents expects opencode - README lists opencode as an accepted provider and drops it from the "cannot generate summaries" note - doctor's help text and the real-registry doctor test use factoryai-droid as the incapable example instead of opencode - a shared openCodeBinary constant replaces three "opencode" literals (goconst) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Entire-Checkpoint: 01M2AW38T6Y7300KGQ45Z0VPHS
…1634) `entire api --to cell` and `-j <jurisdiction>` always aimed at production: the cell client discovered a login against api.BaseURL(), which with no ENTIRE_API_BASE_URL is the entire.io apex, so a staging (partial.to) login selected via --context was refused with "API host entire.io does not accept the login selected by --context" and every staging cell was unreachable through the CLI. `--to core` already followed the context; only the cell path was wrong. The cell subject now follows the selected context (--context, else $ENTIRE_CONTEXT, else current_context) whenever no data host is configured, reusing the resolver `auth token --jurisdiction` already used for the same reason. That context's core is both the login and the core whose cluster catalog supplies the cell apiUrl, so a staging login lists staging's catalog and lands on a staging cell; production behaviour is unchanged because a prod context resolves to the same catalog it did before. An explicit ENTIRE_API_BASE_URL still discovers a login against the named data host — that is the user naming a host, and its trusted-issuer document decides which saved login may authenticate it. `-j <slug>` with no matching cell now fails naming the core consulted and the jurisdictions it serves, still unwrapping to ErrNoCellForJurisdiction for the callers that fall back on it. api.BaseURLOverride reports whether the data host was set, so the production default can no longer stand in for a choice the user never made. ClientFor's routing is split into cellBaseURLFor so the decision is testable without a network round trip. Tests cover prod and staging contexts, $ENTIRE_CONTEXT and --context selection, -j against both environments, the unknown-jurisdiction error, not-logged-in parity with --to core, and that an ENTIRE_API_BASE_URL override still takes the discovery path. No shared context-resolution code (contexts.Active / activeContext) was touched, to stay clear of COR-1630. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GHN3ARZSA20QXJ09E2P1NC
Review cleanups on the COR-1634 change, no behaviour change: - resolveStoredCellSubject is now the three-line policy (selected context unless ENTIRE_API_BASE_URL is set) with the discovery half extracted into resolveDiscoveredCellSubject, so the two leaves are symmetric. - resolveCellAPIBaseURL normalizes coreURL once, and the unknown-jurisdiction message builds its served list lazily on the error path via servedJurisdictions, which folds rows through NormalizeJurisdiction (the one label rule) instead of a second ToLower/TrimSpace, and sorts with slices rather than a bespoke map helper. - The COR-1634 history is told once, on resolveStoredCellSubject; the other doc comments and CLAUDE.md point at it instead of repeating it. - Tests: the two environments are envFixture values keyed by context name, so the routing table derives the expected core, catalog and JWT from the selected login instead of carrying a redundant column; isolateCellClientEnv takes the data-host override so the override test and the two older JurisdictionToken tests share it (ENTIRE_TOKEN is unset, not blanked — a blank value is fail-closed); catalogTransport records only catalog requests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GJFWFGGA08BFEG0DGJ48PQ
Comment-only. The resolveStoredCellSubject doc now says what discovering against the default data host would do and why that fails (COR-1634) in the present tense instead of narrating the old behaviour; ErrNoCellForJurisdiction and resolveCellAPIBaseURL docs cover explicit --jurisdiction and the login core they consult rather than only the home-jurisdiction case; the test fixture and helper docs match what the code has. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GJVRQ52YZM096QSZ2D2X5M
…ironment Review follow-ups on the COR-1634 change. A login core is not a cell. cellSubject's dataOrigin, which meant "the data host the CLI is pointed at", was being filled with the login core's origin on the context and env-token paths, and resolveTargetCellBaseURL's verbatim carve-outs for a direct-cell or loopback origin then applied to it: a local-dev login (core on localhost, no ENTIRE_API_BASE_URL) dialed entire-core as if it were entire-api and 404'd, where main failed closed at discovery. The field is now dataHost — the explicit ENTIRE_API_BASE_URL origin or "" — and only an explicit host is ever kept verbatim; with none, the catalog of the login core decides, so a local-dev login lands on the cell its core advertises. The audience/core templates already fell back to the core's environment family, so JurisdictionToken is unchanged. `--to cell` honours ENTIRE_TOKEN first, as `--to core` and `auth status` do: resolveCellClientSubject (was resolveStoredCellSubject) is env token, else selected context, else discovery under an explicit data host. A CI runner with only ENTIRE_TOKEN set no longer gets "not logged in" from the cell half of one command. activity/recap fall back from the cell to the data API only when auth.DataAPIServesSelectedLogin says both are in the same environment. The cell leg now follows a staging or local login while the data API still defaults to production, so an unconditional fallback would either be refused there or auto-select a saved prod login and render prod data under a staging context; the cell error is reported instead. recap keeps its tolerant path for a missing login, since then no environment has been selected to stay in. Docs: upstream-host-resolution.md's account-selection rule now scopes cell routing's host matching to the explicit-data-host case and describes the default path; CLAUDE.md and `api --help` name ENTIRE_TOKEN. Tests: loopback login resolves through the catalog, env token drives the cell path, the fallback predicate across prod/staging/custom/env-token/override, and the four older cell-client tests share isolateCellClientEnv so a developer's exported ENTIRE_CONTEXT or ENTIRE_TOKEN cannot steer them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GMHE3SPZ9AEYDSG1PR1P6S
Comment-only. The environment-mismatch reason for skipping the data-API fallback lives once, on DataAPIServesSelectedLogin; activity and recap point at it. BaseURLOverride names the current resolver, catalogTransport's doc no longer claims a loopback core is kept verbatim, and two test comments that restated their assertion or the cmp.Or they sat on are gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GNFAVA8ZH7Q2EWE91F8BY7
… fallbacks Bugbot findings on entireio#2414. DataAPIServesSelectedLogin returned true when ENTIRE_TOKEN was set but unparseable, so activity/recap fell back to the data API — which never reads the env token — and could answer from a stored production login instead of surfacing the token error; it now returns false. recap exempted ErrNotLoggedIn from the environment check, which let a selected staging login whose refresh failed fall through to an auto-selected production login; the predicate already returns true when no login is selected, so the exemption only opened the hole and is gone. The `api` help text says "control plane" rather than the internal "core". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GPX0ZPT41CHAABZD4WFFP2
Bugbot follow-up on entireio#2414: the invalid-env-token check ran after the ENTIRE_API_BASE_URL override returned true, so with both set a malformed token still let activity/recap fall back. The rule is simpler than an ordering fix: the data-API path never reads ENTIRE_TOKEN, so no fallback can act as the env-token login — valid or not, override or not — and DataAPIServesSelectedLogin is false whenever the variable is set. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LgYNrWHSxSK6GcKsuQwqD8 Entire-Checkpoint: 01M2GQ8QM8S0FPW78CX57VPNF7
Entire-Checkpoint: 01M2H3G37P1G0DTK9CVAZ2TYRW
Entire-Checkpoint: 01M2H4AVDYGD7X17NQK4SMT1ZT
`auth.Contexts()` resolved the acting context internally and then returned only its name, discarding the pointer. Every caller needing the object had to re-find it by looping over the slice, so the same lookup grew independently in three places — and two of the three dropped the guard that treats a context with no CoreURL as "none acting" rather than dialing an empty host. The identity recovery added in the previous commit was the third copy. Add `auth.ActiveContext`, which returns the context `Contexts` already resolved, and switch `resolveEntireIdentityProfile` onto it. That removes the loop and picks up the CoreURL guard: a contexts.json entry with a blank CoreURL now asks the user to log in, where before it minted a token against an empty core and surfaced the transport error instead. `resolveStatusTarget` and `hasImportSyncLogin` still carry their own copies. They are left alone here — moving them touches `auth status`, `logout`, `api` and `repo mirror`, which is a wider blast radius than this fix needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2H7HZN51YJRZVKHPT1CPFA6
…ssions fix(status): list every session without mutating state
The wrapcheck ignore entry and its rationale, the complexity-report feature mapping, and a test-plan clause all named code that no longer exists. A comment reflowed during the removal also left an orphan line. Entire-Checkpoint: 01M2P9CH2V8C17ZSN76MFP65GP
…ason into the error The six formatGitPushError tests pass in isolation whether or not anything calls it; the line that was wrong the first time is the PushWithOptions call site that threw the combined output away. Drive a real refused push through a bare repo's pre-receive hook so reverting that line fails with the original symptom, "git push: exit status 1" without the remote's reason. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2P9MP7W0DQ2K5K70M5D87WX
The guard exists for the fixture someone just wrote and has not yet staged, which a plain git grep never sees; --untracked covers it. || true swallowed git's own failures (exit 128 outside a repo) as a clean pass, so branch on the status instead: 1 is no matches, anything higher is git failing. The self-exclusion went too: the pattern cannot match its own text, so the pathspec only created a blind spot. Also a blank line so the awsKeyFixture comment no longer swallows the supabase prefix functions' doc comment. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Entire-Checkpoint: 01M2P9YT06B1YF7GXDV1P0A1MW
Merge the split event branch in the agent hook dispatcher, move the pre-push refs lookup next to its loop, and drop the OpenCurrent caller census, which was already stale. The agent-help comments and tests no longer use the deleted policy command as their mixed-group example. Entire-Checkpoint: 01M2PBGV249KWNBK88MYXHD6NB
Every listed child of the group is read-only, so the group's own audience now agrees with them instead of rendering as task-driven. Entire-Checkpoint: 01M2PC56AV9KQYFJR5TCH53Z4R
explain --generate runs a summary provider and writes the result, so the group renders as "read-only except: explain" rather than read-only. The backend doc states the mixed-fleet limitation in terms of current behavior: reading refs checkpoints needs a CLI with git-refs support. Entire-Checkpoint: 01M2PVNF3FZ6G872JBJX6C9V9M
Entire-Checkpoint: 01M2PYB3JR2GJ2E6RZSZ0Z3DM7
…-push-rejection fix(checkpoint): surface the remote's reason when a checkpoint push is refused
…t-literal test(redact): assemble the AWS key fixture instead of inlining it
factoryai-droid: install the cmd.exe hook wrappers on Windows
Main moved the grant family under its nouns while this branch renamed the control-plane verbs, so both touched the same surface. - repo now registers grant alongside the renamed view/edit and the new remote and access subtrees. - grant.go takes main's generic grantTarget rewrite wholesale; this branch's only change there was swapping a Use literal for cmdListRepo, which main's version builds from the noun instead. cmdListRepo stays in names.go for repo access list and repo protection list. - The deprecated-strings guard bans both sets of removed spellings: the renamed control-plane verbs and the old top-level grant paths. - Docs carry both: org and project gain grant, repo keeps the renamed verbs and gains grant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2Q5G823CMCX64T5EBWK6T6C
Removing a verb made its old spelling exit 0: the groups have no RunE, so cobra returned flag.ErrHelp before it validated arguments, printed the group's help and reported success. `entire auth use prod && entire repo delete ...` therefore deleted under the previous identity, and `repo visibility set <repo> private` reported success while the repo stayed public. On main each of those exited 1. requireSubcommand gives a group both NoArgs and a RunE. Both halves are needed: cobra rejects an unknown subcommand only on a parentless command, and the missing RunE short-circuits ahead of NoArgs, so NoArgs alone is a no-op. A bare group still prints its help and exits 0. This restores no compatibility: the removed spellings fail, they just say so instead of reporting success. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2Q8HKH45BGCEV768SPABRM1
docs: slim repository instructions into task-specific references
Four review findings, all on the ref handling and docs the rename left behind. bareRefSuggestions promised "one per grammar that would actually accept it, so a suggestion is never itself a ref that would fail next", and the GitHub-only mirror verbs broke it: `mirror add octocat/hello-world` offered /et/octocat/hello-world, which the same call then refused. It now takes the forges the caller serves; naming none still means both, which is repo clone. `repo mirror get` rejected the /gh/<owner>/<repo> form every sibling takes, because isOwnerRepoRef cuts on the first slash and a leading "/" left an empty owner, dropping the ref into the entire:// parser. It now resolves the forge-qualified form through the same by-name lookup, and a native ref gets the subtree's own refusal. The bare <owner>/<repo> still works; whether to drop it for consistency is a separate call. `repo access` does not say GitHub in its name, so a native ref is a reasonable thing to type at it. It now names `entire repo grant list`. Doc drift: a core-client comment read "mirror access list", which is no command, and still showed the positional cluster host; a clone comment said `mirror add <url>` where the argument is `<repo>`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QAGXDFBFG6P7DXRFWT378Z
`repo mirror get` was the last verb with its own ref grammar: it took the bare <owner>/<repo> every sibling rejects, and rejected the /gh/<owner>/<repo> every sibling takes. It now routes a ULID and an entire:// clone URL as before, and hands everything else to the same parser the rest of the subtree uses, so a bare pair is answered with the forge-qualified ref it should have been. The directory prints what the verbs accept. `mirror list` NAME, the detail view's Name, and the --json `repo` field now carry /gh/<owner>/<repo>, so a value copied from a row or piped out of --json is a reference rather than something to prepend a forge to by hand. The server-side filter still receives the bare pair; the forge is the CLI's grammar, not the API's. --owner reads the owner back out of the qualified name. isOwnerRepoRef is gone with its last caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QBV00PSX5DMGYK2QPKHK7T
Removing the checkpoint policy feature deleted both production callers (checkpoint_policy.go and strategy/checkpoint_policy.go), leaving the exported accessor and its two per-platform constants exercised only by their own tests. This PR removes the feature end to end, and that was the last piece of it still standing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QC2HT2JQNBQVJEHBGT0CAX
The mixed-fleet bullet was updated to drop the checkpoint_min_version upgrade nudge, but the paragraph below it still justified not dual-writing on the grounds that an old client failing to read the new format "is a feature, not something to paper over" — which it was only because the nudge turned it into an explicit prompt. With the nudge gone the case is silent, so the doc says so rather than arguing from a mechanism that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QC2YR5A9PNPBKH7NAP51FE
Moving OpenRepository into the OPFRun branch put it between perf.Start and RewriteUnpushedV1WithOPF, so the span began covering an open it had never measured — and that open is not free: on a reftable repo gitrepo routes reference reads back through the git CLI. Hook-perf triage attributes by span name, so the span would have quietly come to mean something different from every trace recorded before it. The open gets its own open_repository span instead, the name manual_commit_hooks.go and manual_commit_git.go already use, so the error it can fail with is still recorded somewhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QC2ZMAS78NPX7NFNKNDTDZ
Removing `checkpoint policy` left the checkpoint group with no task-driven child, and classifying `checkpoint explain` task-driven is the fail-safe way to restore one: --generate writes a summary onto the checkpoint and spends tokens with the summary provider, the audience axis has no flag granularity, and this table's stated direction is an agent declining to run something it could have rather than running something it should not have. What that costs is an agent declining the drill-down `checkpoint list` and `checkpoint search` exist to feed, over a flag it was never going to pass — and the only record of the distinction was a trailing comment in the table. It now reaches both audiences where CLAUDE.md says each belongs: agentHelpGuidance for the agent-facing "when to use this", cobra Long for the part a human wants too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QC3GDYKX6CQX6BJJFC3660
agentHelpAudienceNote falls back to a group's own audience whenever its classified children agree with each other — which its comment described as the children agreeing with the GROUP. Those are not the same thing, and the gap is how a group starts lying: delete its last dissenting child and the listing keeps rendering the group's audience while every child now says the opposite. That is what removing `checkpoint policy` did here — four read-only children under a group still classified task-driven — so this is a trap with one confirmed victim rather than a hypothetical, and the previous commit had to answer it by hand. The fallback itself is right: the group's audience is a claim about the whole group, including the hidden children the note never counts (`checkpoint resume` switches branches). So the disagreement is caught at build time instead, and which side moves stays a human's call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QC3V3ZK18KYGJDZT164W0P
A repository is named /<forge>/<a>/<b> and no other way. A GitHub URL is unambiguous about its forge, so it was accepted, but that left two spellings for one repo. It is now recognised only to name the ref it should have been, which is what `repo clone` already did: invalid <repo> "github.com/octocat/hello-world": pass GitHub repositories as /gh/octocat/hello-world Every mirror verb and repo clone now take exactly /gh/<owner>/<repo>, with /et/... refused by the GitHub-only verbs and served by clone. Reading a clone's own git remote is untouched: that goes through gitremote.ParseURL on the configured URL, not through this grammar, so a bare `entire repo remote use` in a GitHub clone still resolves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QCNRHAAKZ0RZKPQZMZ87SF
Main split CLAUDE.md into a short index plus references under docs/development/, so the one conflict was structural rather than textual: this branch edited the command-layout section in place while main moved it. Took main's index wholesale and ported this branch's command-layout changes into docs/development/cli-conventions.md: auth switch, repo view/edit/list --project, the mirror/remote/access subtrees, and the single repository-reference grammar. That last paragraph is rewritten rather than copied, because the branch has since narrowed it further: a repository is named /<forge>/<a>/<b> and no other way, so a bare pair and a GitHub URL are both refused and answered with the ref they should have been. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QD3RHE82M2FY4XKHBZ50DK
Two conflicts, both from the removal meeting main head-on. cmd/entire/cli/checkpointpolicy/remote.go: modify/delete. 7693ebb fixed checkpointpolicy.Push to stop embedding git's raw output in its own error, now that the wrapped error carries the collapsed, remote-redacted copy. The fix is entirely inside the package this branch deletes and Push goes with it, so the deletion stands and there is nothing to port. The same commit's changes outside the package — the tail-weighted elision and the nonInteractiveSSHAuthFailure warning — merged cleanly and are untouched. CLAUDE.md: d16dce0 slimmed it from 2143 lines to 199 and moved the detail into docs/development/. Taking main's version wholesale would have silently dropped this branch's three edits, so they are re-applied at their new homes instead: the experimental command list in cli-conventions.md, the OpenCurrentOrCwd paragraph in git-safety.md, and the checkpoint-policy clauses of the read-election bullet in checkpoint-implementation.md. Verified by grep: no reference to checkpointpolicy, `checkpoint policy`, or OpenCurrentOrCwd survives anywhere under docs/ or in CLAUDE.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QF6ZN71V16Y83GFDP5X2JC
These verbs refuse every /et/ ref, but they parsed it first, so how the project and repo were spelled decided which answer came back: /et/my-project/my-repo -> does not support Entire repository … GitHub mirrors only /et/p/r -> project "p" is not a name the server accepts: 3-32 … /et/foo -> expected /et/<project>/<repo> (2 names after et, got 1) One outcome, three answers, and the last two send the reader to fix a name this verb rejects however it is spelled. `repo access list` was the sharpest: its pointer at `repo grant list` rode on the syntax error and echoed the same bad name back. Declaring the native forge is now the whole answer. The name still matters in `repo grant`, which can act on it, and still reports there. My commit 3610242 claimed "/et/... gets the subtree's own GitHub-only refusal", which held only for a well-formed ref; the test used one, so nothing pinned the other half. It now covers a short name, an unnamed repo, a missing segment, and the optional leading slash, and asserts the name rule is never quoted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M2QFG12BQSV5JQXFW0H6FNZB
Remove the checkpoint policy feature
…-commands-name Unify control-plane command names and repository references
Entire-Checkpoint: 01M2QK1KZNWNVHDXW8KSTQS688
Entire-Checkpoint: 01M2QY2F115T6ZE98GKCMPQ0N8
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
9 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/3
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.