feat(worktree): park is not retire, and a checkout is not a node_modules - #1676
Merged
Conversation
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Three leaks with one cause: worktree, branch, PR and dependency tree were treated as one object with one lifetime. Splitting them is the whole change, and the plan is explicit that separate PRs for parking, branch GC and dependency bloat would generate exactly the residue being fixed. 1 — REMOVING A CHECKOUT NO LONGER ABANDONS ITS BRANCH The old model made "remove worktree" imply "abandon branch", so a checkout could only go once its PR merged. #1659 has been open for days awaiting a physical-iPhone test and held ~3.8 GiB the entire time for no reason: its work was pushed, and the checkout was reproducible from a ref. PARK remove the disposable checkout, KEEP the branch (no PR needed) RETIRE park, AND delete a branch proven merged Parking asks a different question than retiring. Not "has this work landed" but "is this checkout reproducible from a pushed ref". That is the only place a remote tip is required, and it is required in the safe direction. 2 — BRANCH DELETION, PROVEN BY PR HEAD OID Nothing deleted branches, so they accumulated invisibly: `gh pr merge --delete-branch` fails while a worktree holds the branch, and the worktree removal path never touched refs. Ten local branches for merged PRs were sitting in this repo. The evidence is `PR MERGED` + `local tip === PR head OID`, and deliberately NOT a remote tip. `delete_branch_on_merge: true` removes origin/<branch> at merge time, so requiring it is guaranteed false for exactly the branches that are safe — #1654's shipped defect, whose first dry run printed "Nothing retirable" with 11.6 GiB in front of it. Verified before writing the classifier: for #1669/#1670/#1671/#1672 the remote branch is gone, GitHub still reports .head.sha, and it matches the local tip exactly in all four. The counterexample that earns its own verdict: someone commits to a branch AFTER its PR merged. The PR proves the old tip landed and says nothing about the new commits, which exist nowhere else once the remote is gone. KEEP_DIVERGED_AFTER_PR. `git branch -D` is unavoidable here — squash-merge means `-d` always refuses — which is precisely why it is gated behind that proof, re-verified immediately before deleting, and never exposed as a bare command a caller can aim. 3 — A CHECKOUT IS NO LONGER A ~3.8 GiB COMMITMENT new-worktree.sh installed dependencies unconditionally. Most control-plane, docs and config work never runs a test. That coupling is why six worktrees in one day took the volume to ZERO BYTES FREE, where nothing runs at all because writing a command's output needs disk. Installs are now opt-in (--install) or on demand (scripts/ensure-worktree-deps.mjs), behind a two-number policy: HELM_DISK_RESERVE_GIB 12 the floor that keeps the MACHINE usable HELM_INSTALL_BUDGET_GIB 5 conservative estimate of one install An install needs reserve+budget = 17 GiB. The measured cost was 3.8 GiB and the budget is deliberately larger: one day's observation is not a promise about the next. Unknown free space fails closed. new-worktree.sh's own precheck drops to the reserve alone, because creating a checkout no longer costs GiB. ONE AUTHORITY, TWO LAYERS scripts/lib/worktree-lifecycle.mjs decides; scripts/worktree-lifecycle.mjs gathers. Pure classifier means every verdict is covered without a git fixture per case, and a wrong verdict is a logic bug rather than a shell-quoting bug. retire-worktrees.sh becomes a forwarding shim — two implementations of a rule that deletes things is how they drift. A NEAR MISS, FOUND BY MY OWN TEST The first draft resolved the repository from `import.meta.url`. A test that built a throwaway git fixture and ran the CLI with cwd set to it therefore operated on the REAL repository, and --park removed a live worktree (keyboard-covers-distance, PR #1659). Nothing was lost, and the reason is the design rather than luck: parking keeps the branch, and PARKABLE requires the tip already equal its pushed remote. Verified after the fact — local == remote == 03a1307, PR still open, branch intact. The checkout was recreatable the whole time. It was still wrong. A tool that deletes things must take its target from where the caller is standing, not from where the script lives. Fixed, and proven by running it inside a fixture and seeing only the fixture. The bug also cost 140s of test time — the CLI was scanning 40 real branches through `gh` on every case. The suite now runs in 2s. TWO SUITES MIGRATED, NOT DELETED Both encoded the previous model and failed correctly: retire-worktrees.test.ts a clean pushed worktree with an OPEN PR was KEEP. It is now PARK. That IS the redesign. All 13 refusals are preserved in the new suite; the HELM_PR_LOOKUP seam is kept, now carrying a head SHA so exact-match cases are expressible. new-worktree-precheck.test.ts asserted the refusal names retire-worktrees.sh and that ENOSPC cleanup lives in new-worktree.sh. Both moved. Each assertion has a successor and got one; none was dropped. AGENTS.md says the standing grant RELIES on these refusals firing, so quietly weakening either file would quietly weaken that authorization. 30 lifecycle cases + 5 precheck cases. The classifier layer covers all four worktree verdicts and all nine branch verdicts, including the ones that only appear when evidence is MISSING — a failed lookup is UNKNOWN_PR and never "no PR", which is #1668's whole subject. DOCS AGENTS.md and autonomy.md both enumerated five guarantees including "an isolated dependency install". That is now four, and the install is separate. Shipping the tool without that edit would have shipped the contradiction this program exists to remove. DELIBERATELY NOT DONE HERE: the sweep across the 38 local branches. The plan sequences it after this merges, report-only first, every branch re-verified. Being right about the logic is not the same as being right about the branches. CI caught one more: the Lint ratchet, a gate distinct from `npm run lint`, went +1 on an unused `HERE` constant left over from an earlier draft of ensure-worktree-deps.mjs. `npm run preflight` is the blocking static set and runs it; I had run the individual gates instead. Fixed, and preflight is green. Verified: npm run preflight 0; npm test 1274 files / 12042 passed; typecheck 0; lint 0; check:cycles 0; enforcement:check 0; shellcheck 0; repo:doctor 0; markdown:ratchet 0; both drift gates 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
njrini99-code
force-pushed
the
agent/lifecycle-convergence
branch
from
August 29, 2026 21:50
8f997d3 to
7843291
Compare
njrini99-code
added a commit
that referenced
this pull request
Aug 29, 2026
) Two days of repairs left the pieces better and unable to check each other. This makes drift observable, so the next forensic pass is a command instead of a week. THE PATTERN, STATED ONCE control written -> documented -> assumed -> stops running -> docs remain -> agent trusts docs -> failure Observed, all of it: guard-bash.sh existed, unwired, deleted guard-sql.sh existed, unwired, deleted retire-worktrees.sh existed, never invoked four rule files claimed hooks that were gone worktree GC keyed on a ref GitHub deletes on merge Docs Regen opened a PR that sat open while main went stale a fixture test operated on the real repository Not one was a missing control. Every one was a control that stopped running while its documentation stayed put. WHAT THIS ADDS npm run control-plane:verify static + runtime npm run control-plane:verify:static CI-safe, wired as its own required job READ-ONLY by construction. It regenerates nothing, deletes nothing, mutates no config. The previous docs gate re-ran the generator and diffed, so the only way to learn the docs were stale was to have already fixed them — a verifier that repairs what it inspects cannot tell you what was wrong. 0 VERIFIED passed, or a named acknowledged gap 1 CONTROL FAILURE a control that should hold does not 2 UNKNOWN a required state could not be established UNKNOWN outranks FAIL and never becomes PASS. If something could not be established, "these specific things are broken" would imply everything else was checked. ACKNOWLEDGED GAP != UNKNOWN. Four gaps carry an id, an owner, a date, a reason and a closing condition in config/control-plane-gaps.json, and every run prints them so they cannot fade. MUTATION BUDGET, ENFORCED BEFORE ALLOCATION HELM_MAX_MUTATION_WORKTREES defaults to 1. Refused before `git worktree add` and before any install, so a refusal costs nothing. Classification FAILS TOWARD MUTATION: an unreadable or undeclared workspace counts. A declared release workspace escapes the budget only if it is ALSO detached — on a branch it can still be committed to. The disk reserve stopped a catastrophe; it never stopped waste. Six worktrees in one day was a concurrency problem that only became visible as a disk problem. BRANCH-GC AUTHORIZATION, RECORDED IN THE TOOL ITSELF Verdict renamed DELETE_SAFE -> DELETE_MERGED_EXACT so it states its own evidence, and the grant lives in the authority, not in a paragraph elsewhere: PR MERGED + local tip === PR head OID + not protected + not checked out Nine verdicts are explicitly excluded. The sharpest is new: NO_UPSTREAM_UNIQUE_WORK. Measured — ten branches hold up to 19 commits with no upstream, existing nowhere else. Folding those into UNKNOWN_PR conflated "the lookup failed" with "this is the only copy", and only one of those is permanent. THE GATE-LIE FROM #1672, ACTUALLY CLOSED #1672 raised the timeout and its own PR said that makes the unknown RARER, not DISTINGUISHABLE. The eleven repo sweeps now run in a dedicated `guards` project under scripts/repo-guards.mjs: PASS ran, found nothing POLICY_FAILURE ran, found violations exit 1 INFRASTRUCTURE_FAILURE did not finish — UNKNOWN exit 2 Proven end to end by injecting a real 200s hang into a real guard: exit 2, never reported as a violation. The guards are UNTOUCHED. They are 116-357 lines of bespoke matching logic each; rewriting them to fix a REPORTING defect would risk weakening the guards themselves. The lie was at the reporting boundary and is fixed there. Worth recording: vitest's JSON reporter does not carry the timeout text — a real timeout arrives as `Error: STACK_TRACE_ERROR`. A first draft scraped the human-readable stream to recover it. That did not match, and it was the wrong idea: parsing a tool's console output to make a safety decision is the brittle-regex pattern this repo has already deleted guards for. The guarantee is the DEFAULT — an unrecognised failure is INFRASTRUCTURE_FAILURE — not a pattern. TOOL AUTHORITY, AND EVIDENCE THAT EXPIRES docs/TOOL_AUTHORITY_MATRIX.md, generated. Five words that are not synonyms: CONFIGURED, CONNECTED, EXPOSED, ALLOWED, EXERCISED. ET-4 lost two days reading the first as the last. Probed live, not inferred: Sentry BOTH namespaces connected, find_organizations byte-identical. Redundant -> mcp__claude_ai_Sentry__* is authority, plugin denied. Reversible: one deny line restores it. Vercel plugin exposes ONLY authenticate -> CONFIGURED, not CONNECTED. Account connector is authority; dead namespace denied. GitHub the gh CLI is the lifecycle path, exercised three ways: MERGED #1676, OPEN #1659, and a branch with no PR. A failed lookup stays UNKNOWN_PR (#1668). Supabase sanctioned mcp__supabase__* is installed but NOT connected. Every observation carries a fingerprint of the configuration that produced it. Change a deny rule and the matching EXERCISED claims become STALE on the next regeneration. Nobody has to remember to invalidate them. Two bugs found by running it: Sentry and GitHub produced an IDENTICAL fingerprint (both slices empty), which would have made drift undetectable for both; and a service governed by nothing now says `ungoverned` rather than implying coverage it does not have. SUPABASE — A STRUCTURAL FINDING Starting the sanctioned server's OAuth flow shows its requested scopes: organizations:read projects:read database:read analytics:read secrets:read edge_functions:read environment:read storage:read Every scope is :read. That is connector-enforced capability restriction — real evidence, not a URL string, and obtainable without any write probe. It also means mcp__supabase__apply_migration almost certainly CANNOT function under this grant, while an explicit owner authorization says it may. Surfaced as gap SUPABASE_MIGRATION_GRANT_VS_READ_SCOPES rather than resolved, because resolving it means attempting a production migration to answer a documentation question. USER-GLOBAL, UNDER EXPLICIT AUTHORIZATION AND WITH A BACKUP removed mcp__plugin_supabase_supabase__{execute_sql,apply_migration} — no such plugin is installed; a grant waiting to activate on install is a trap, not a capability removed mcp__claude_ai_Supabase__apply_migration — account-WIDE mutation nobody authorized kept mcp__supabase__apply_migration — the owner's explicit grant kept mcp__claude_ai_Supabase__execute_sql as TEMPORARY_FALLBACK, named UNENFORCED, because it is the only working query path until the sanctioned one connects fixed the autoMode prose claiming a PreToolUse hook blocks destructive SQL. It never did; it had been copied from this repo's own docs. SANDBOX — MEASURED, NOT ASSUMED, AND NOT ENABLED Ten disposable probes: writes inside the project, outside it, into .claude/settings.json and .claude/hooks, reads of .env.local, and ../ traversal ALL SUCCEEDED — including paths the config lists under denyRead and denyWithinAllow. sandbox.filesystem.disabled is true and authoritative. Network sandboxing IS enforced (observed: deny network-outbound analytics.brew.sh:443). NOT enabled. The configured allowWrite covers /private/tmp/helmv3-*, ~/.npm and ~/Library/Caches/deno — not the repository — so flipping it as configured would block every worktree write, user-globally, for every project and any concurrent session. Recorded as gap SANDBOX_FILESYSTEM_DISABLED with the closing condition. No fake Bash parser was added as compensation. DOCS REGEN — created != integrated #1623 existed because main carried a stale route count (234 vs 235, after /admin/self-heal shipped) and the fix lived in a bot PR that nothing merged. That workflow's own header recorded that GITHUB_TOKEN PRs may receive no required CI, so it could not be merged confidently either. regen-docs.mjs gains a genuinely NON-MUTATING --check that computes expected blocks, diffs them, names the first differing line, and leaves the tree byte-identical. It is now a required PR-time gate. The auto-PR workflow is verification-only and creates nothing. #1623's diff is byte-for-byte what regeneration produces here, so it closes as superseded. FAILURE INJECTION — 15 cases, every one in a disposable COPY delete a configured hook -> FAIL point a hook at a nonexistent script -> FAIL break the blocking hook's matcher -> FAIL reintroduce a false safety claim -> FAIL the same claim, QUOTED -> PASS (corrections stay writable) declare a namespace never observed -> FAIL move the budget check after allocation -> FAIL remove the budget check -> FAIL change a service's config -> its observations STALE change one service -> others NOT staled a service governed by nothing -> ungoverned, not fresh inject a real 200s guard hang -> INFRASTRUCTURE_FAILURE, exit 2 inject a real banned import -> POLICY_FAILURE, exit 1 Plus a SENTINEL proving the injections cannot reach the live checkout. That is the direct regression for #1676, where a fixture test resolved its target back to the real repository and parked a live worktree. THREE THINGS THE VERIFIER CAUGHT IN ITS OWN AUTHORS' WORK 1. It reported mutation-budget-enforced FAIL against a correctly-ordered script, because indexOf matched `git worktree add` in the script's own HELP TEXT at line 15. Substring is not mechanism — the same error the enforcement generator made on its first run. Now line-based and comment-aware. 2. The matrix rendered the whole Supabase namespace as DENIED when only its ten mutators are, and folded DENIED_BY_POLICY and NOT_EXERCISED into "FAILED" — which reads as "we tried and it broke", untrue of both. 3. #1672's own test asserted `unit` must be no stricter than its siblings, because `unit` held the sweeps. They moved. The failure was the premise announcing itself, and the test now pins the invariant that survives: whichever project runs the sweeps must be the most generous. DELIBERATELY NOT DONE No UNKNOWN or no-upstream branch deleted. No production SQL write as a probe. No regex Bash/SQL pseudo-guard. #1638 not merged. sandbox.filesystem not enabled. The Repair LaunchAgent not re-armed — a green control plane is not evidence about a different unresolved runtime problem. Verified: npm run preflight 0; npm test 1265 files / 12049 passed; control-plane:verify 0 (20 pass, 0 fail, 0 unknown, 1 acknowledged gap); control-plane:verify:static 0; enforcement:check 0; docs:inventory-check 0; tool-authority:check 0; guards 32/32; check:cycles 0; shellcheck 0; repo:doctor 0. Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 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.
Park is not retire, and a checkout is not a
node_modulesThree leaks, one cause: worktree, branch, PR and dependency tree were treated
as one object with one lifetime. Splitting them is the whole change. The plan
is explicit that separate PRs for parking, branch GC and dependency bloat would
generate exactly the residue being fixed.
1 — Removing a checkout no longer abandons its branch
The old model made "remove worktree" imply "abandon branch", so a checkout
could only go once its PR merged. #1659 has been open for days awaiting a
physical-iPhone test and held ~3.8 GiB the whole time — its work was pushed
and the checkout was reproducible from a ref.
Parking asks a different question: not "has this work landed" but "is this
checkout reproducible from a pushed ref". That's the only place a remote tip is
required, and it's required in the safe direction.
2 — Branch deletion, proven by PR head OID
Nothing deleted branches, so they accumulated invisibly:
gh pr merge --delete-branchfails while a worktree holds the branch, and worktree removalnever touched refs. Ten local branches for merged PRs were sitting in this
repo.
Evidence is
PR MERGED+local tip === PR head OID, and deliberately not aremote tip.
delete_branch_on_merge: trueremovesorigin/<branch>at mergetime, so requiring it is guaranteed false for exactly the safe branches —
#1654's shipped defect, whose first dry run printed "Nothing retirable" with
11.6 GiB in front of it.
Verified before writing the classifier — for #1669/#1670/#1671/#1672 the remote
branch is gone, GitHub still reports
.head.sha, and it matched the local tipexactly in all four.
The counterexample that earns its own verdict: someone commits after the PR
merged. The PR proves the old tip landed and says nothing about the new commits,
which exist nowhere else once the remote is gone →
KEEP_DIVERGED_AFTER_PR.git branch -Dis unavoidable (squash-merge means-dalways refuses), which isprecisely why it's gated behind that proof, re-verified immediately before
deleting, and never exposed as a bare command a caller can aim.
3 — A checkout is no longer a ~3.8 GiB commitment
new-worktree.shinstalled dependencies unconditionally. Most control-plane,docs and config work never runs a test. That coupling is why six worktrees in
one day took the volume to zero bytes free, where nothing runs at all because
writing a command's output needs disk.
HELM_DISK_RESERVE_GIBHELM_INSTALL_BUDGET_GIBAn install needs 17 GiB. Measured cost was 3.8 GiB; the budget is
deliberately larger, because one day's observation is not a promise about the
next. Unknown free space fails closed.
A near miss, found by my own test
The first draft resolved the repository from
import.meta.url. A test that builta throwaway git fixture and ran the CLI with
cwdset to it therefore operatedon the real repository, and
--parkremoved a live worktree(
keyboard-covers-distance, PR #1659).Nothing was lost, and that was the design rather than luck: parking keeps the
branch, and PARKABLE requires the tip already equal its pushed remote. Verified
after the fact — local == remote ==
03a13075d, PR still open, branch intact.It was still wrong. A tool that deletes things must take its target from where
the caller is standing, not from where the script lives. Fixed, and proven by
running it inside a fixture and seeing only the fixture. The same bug cost 140 s
of test time — the CLI was scanning 40 real branches through
ghon every case.The suite now runs in 2 s.
Two suites migrated, not deleted
Both encoded the previous model and failed correctly:
retire-worktrees.test.ts— a clean pushed worktree with an OPEN PR wasKEEP; it is nowPARK. That is the redesign. All 13 refusals preserved;the
HELM_PR_LOOKUPseam kept, now carrying a head SHA so exact-match casesare expressible.
new-worktree-precheck.test.ts— asserted the refusal namesretire-worktrees.sh, and that ENOSPC cleanup lives innew-worktree.sh. Bothmoved. Each assertion has a successor and got one; none was dropped.
AGENTS.mdsays the standing grant relies on these refusals firing, soquietly weakening either file would quietly weaken that authorization.
35 cases. The pure-classifier layer covers all four worktree verdicts and all
nine branch verdicts — including the ones that only appear when evidence is
missing: a failed lookup is
UNKNOWN_PR, never "no PR", which is #1668's wholesubject.
Docs
AGENTS.mdandautonomy.mdboth enumerated five guarantees including "anisolated dependency install". That's now four, and the install is separate.
Shipping the tool without that edit would have shipped the contradiction this
program exists to remove.
Deliberately not done here
The sweep across the 38 local branches. The plan sequences it after this merges,
report-only first, every branch re-verified. Being right about the logic is not
the same as being right about the branches.
Verified
npm test1274 files / 12042 passed · typecheck 0 · lint 0 · check:cycles 0· enforcement:check 0 · shellcheck 0 · repo:doctor 0 · markdown:ratchet 0 · both
drift gates 0
🤖 Generated with Claude Code
https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH