fix(lifecycle): an OPEN PR's checkout needs its owner's consent, not lsof's silence - #1683
Merged
Conversation
…lsof's silence Reproduced 2026-08-30: `npm run worktrees:retire` removed a concurrent session's checkout (agent/round-type-reclassify, PR #1681, OPEN). Every mechanical signal said disposable — clean, tip identical to its pushed remote, and no process whose cwd `lsof` could see. Nothing was lost, because parking is defined to keep the branch and PARKABLE already required the tip to match its remote. But the checkout had an owner and the tool could not tell, so this is a classifier defect, not a slip. The unsound step is reading silence as absence. `lsof +D` samples one instant: hasLiveProcess === true proof of activity — a sound veto hasLiveProcess === false NOT proof of inactivity — an agent session between two tool calls has no visible cwd classifyWorktree's own header stated the defect as design — "parking does NOT consult the PR at all" — so the comment is rewritten in the same commit. A file whose prose contradicts its code is the failure this program exists to remove. WHAT CHANGED New worktree verdict KEEP_PR_OWNER_INTENT_REQUIRED, deliberately neither ACTIVE (nothing proved anyone is using it) nor UNKNOWN (the PR read fine; what is missing is a decision). It is in REQUIRES_HUMAN_VERDICTS, so the standing authorization already excludes it. config/open-pr-dispositions.json gains `worktree_policy`, because the disposition LABEL was being asked to imply an ACTION readers had to infer: KEEP never parked automatically PARK_IF_REPRODUCIBLE park once clean and pushed; the branch stays A missing row, an unrecognised policy, or a disposition of ACTIVE/UNKNOWN all KEEP. #1659 — an open PR waiting on a physical-device test — is released explicitly by its owner and stays parkable, which is the case the park/retire split exists for. The registry is now current state in BOTH directions. It carried ACTIVE rows for #1623, #1638, #1679 and #1680 after they closed or merged; control-plane -verify now fails on a stale row as well as a missing one, and on a worktree_policy outside the vocabulary. That vocabulary check lives in the verifier because the lifecycle tool must fail SAFE on a malformed entry (unrecognised policy => KEEP) and would therefore never report one. #1681's row is recorded ACTIVE/KEEP, transcribed from observable state — PR OPEN, worktree checked out — by this session and not by its owner, who should confirm it. Recorded rather than left blank because leaving it blank is exactly what let --retire park it. WHAT THIS DOES NOT COVER A worktree whose branch has NO PR is still parked on reproducibility alone — the same defect class, with no PR to key a disposition on. Requiring recorded intent for every branch would make --park unable to act at all, which defeats the reason parking exists. Registered as WORKTREE_PARK_NO_PR_OWNERSHIP rather than quietly accepted. VERIFIED preflight 0 · 1269 files / 12,101 tests 0 · docs:check 0 · knowledge:check 0 · markdown ratchet 30515, unchanged · control-plane:verify VERIFIED exit 0 (was CONTROL FAILURE on open-pr-residue) · lifecycle suite 60 passed. Failure injection: disabling the ownership gate turns 8 of them red, including the end-to-end CLI case where --park refuses a fixture worktree whose PR is OPEN with no disposition. The lib was restored from a hashed backup and re-verified byte-identical. Assumption stated once: this ran in the canonical checkout rather than a worktree. HELM_MAX_MUTATION_WORKTREES=1 and the other session holds the only mutation workspace, so new-worktree.sh refuses by design; AGENTS.md permits a single session in canonical, and the concurrency hazard the worktree rule addresses does not apply because the other session is isolated in its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
|
ⓘ 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 |
…cost Opening #1683 made control-plane:verify red on open-pr-residue immediately, because the key set must now equal the live open-PR set exactly. That is the chosen tradeoff, not a surprise: a PR number does not exist until the PR does, so the row can only ever land in a follow-up commit. KEEP rather than PARK_IF_REPRODUCIBLE: this work runs in the canonical checkout, which classifyWorktree holds ACTIVE before consulting anything else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
njrini99-code
added a commit
that referenced
this pull request
Aug 30, 2026
…said otherwise (#1684) * docs(authority): one owner per kind of truth, and six documents that said otherwise Helm already has every primitive a knowledge system needs. What it did not have was a statement of which one owns which kind of truth — and six current-state documents were quietly asserting things that had stopped being true. All six were verified against the live repository before being touched. THE ADR memory/decisions/ADR-2026-08-30-helm-knowledge-authority.md records the hierarchy, the projection rule, identity normalization, the three-feature-map split, and the alternatives rejected (a fourth registry, merging the two feature maps, a KNOWN_ISSUES.md, deleting the superseded documents, a prose contradiction detector). The decisions directory's own README names this exact reconciliation as the case it exists for. The load-bearing rule: a projection may summarise an authority; it may never become a second copy with its own update path. WHAT WAS FALSE, AND VERIFIED FALSE memory/system/golfhelm-engineering-os.md — the runtime contract every session loads — claimed a guard-feature-context PreToolUse hook DENIES governed edits without loaded context. There is no such file on disk and no such hook in .claude/settings.json; the only wired PreToolUse hook matches Write|Edit| MultiEdit and refuses canonical writes. It also credited a guard-bash.sh rule deleted 2026-08-27. Corrected to POST-HOC DETECTION, which is what docs/CONTROL_PLANE_ENFORCEMENT.md has recorded the whole time. docs/README.md opened with "Everything in docs/ is hand-written prose. None of it is generated" while two generated files sat in that directory. Its Start Here list routed readers to an issue ledger whose own header reads STATUS: SUPERSEDED, and has since 2026-07-10. memory/ledgers/README.md documented a ledgers/operations/ directory that has never existed — no directory, no file, no writer, no reference. Removed rather than created: incident aftermath already has a per-feature home, and a fifth slot with no distinct lifecycle is a place for entries to go unread. memory/operations/release-queue.yml said it was "empty until then, not a placeholder for hand-written entries" directly above five hand-written repair units. The header now describes how it is actually populated. memory/README.md said collapsing the two feature-doc generations was "work still owed" and that CLAUDE.md routes to the losing one — CLAUDE.md no longer does. Reframed; the actual collapse is the next PR. docs/OBSERVABILITY.md implied every server failure reaches both Sentry and admin_events. Its scope is narrowed to EMISSION and it now points at docs/OBSERVABILITY_AUTHORITY.md for what each surface knows. .claude/rules/shipping.md answered "worktree or canonical?" two different ways in one file while AGENTS.md carried the real rule. AGENTS.md owns workspace policy; this file links. Three prose counts removed under shipping.md §1 — "there are 22", "the other 44", "192 commits stale". An index that breaks that rule about itself is not one to trust about anything else. A GATE THAT COULD NOT SEE A NAMESPACE Writing the ADR turned docs:schema-drift red on `golf_round_lifecycle` — a memory/registry.yml FEATURE ID, not a table. The gate greps memory/** for golf_*/baseball_* tokens, and feature ids share that shape, so the first .md under memory/ to name one in prose failed as a phantom table. Same substring-is-not-a-mechanism error this repo has now made three times. Fixed by excluding DECLARED registry keys only — parsed from the registry, not pattern-matched — so a misspelled table is still caught, and printed on every run so the exemption is never silent. Five tests; injection (forcing the exemption set empty) turns two of them red. docs/HELM_OS.md lands here rather than later because the ADR and the docs index both need to point somewhere, and a dangling forward reference is what docs:path-drift exists to refuse. VERIFIED preflight 0 · 1270 files / 12,106 tests 0 · docs:check 0 · knowledge:check 0 · markdown ratchet 30515, unchanged · control-plane:verify VERIFIED exit 0. Also removes #1683's disposition row, now stale — the closing half of the cost the ADR states: every PR turns the verifier red twice, once on open and once on merge, because the registry must equal the live open-PR set exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * chore(dispositions): record #1684 The open-half of the cost the ADR states. Row lands after the PR exists because there is no other order in which it can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> 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
1 participant
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.
Reproduced invariant violation, fixed at the classifier.
On 2026-08-30
npm run worktrees:retireremoved a concurrent session'scheckout (
agent/round-type-reclassify, PR #1681, OPEN). Every mechanicalsignal said disposable: clean, tip identical to its pushed remote, no process
whose cwd
lsofcould see. Nothing was lost — parking keeps the branch — butthe checkout had an owner and the tool could not tell.
The unsound step is reading silence as absence:
classifyWorktree's own header stated the defect as design ("parking does NOTconsult the PR at all"). It is rewritten in the same commit.
What changed
KEEP_PR_OWNER_INTENT_REQUIRED— deliberately neither ACTIVE(nothing proved anyone is using it) nor UNKNOWN (the PR read fine; a decision
is what's missing). Added to
REQUIRES_HUMAN_VERDICTS.worktree_policyon each disposition —KEEP|PARK_IF_REPRODUCIBLE.The label was being asked to imply an action readers had to infer.
ACTIVE/UNKNOWNall KEEP. fix(golf): stop the keyboard covering the distance field you're typing into #1659 staysparkable because its owner released it explicitly — that is the case the
park/retire split exists for.
docs: regen inventory blocks #1623, docs: the agent lifecycle in this repo, traced from the configs themselves #1638, fix(admin): three information-loss boundaries — one fixed, one disproven, one registered #1679, feat(control-plane): close a gap by measuring it, register one found by looking #1680 after they closed or merged.
control-plane:verifynow fails on a stale row and on an invalid policy, not only a missing row.
Not covered, and registered
A worktree whose branch has no PR is still parked on reproducibility alone.
Requiring recorded intent for every branch would make
--parkunable to act atall. Registered as
WORKTREE_PARK_NO_PR_OWNERSHIP.Verified
preflight 0 · 1269 files / 12,101 tests 0 · docs:check 0 · knowledge:check 0 ·
markdown ratchet 30515 unchanged ·
control-plane:verifyVERIFIED exit 0(was CONTROL FAILURE) · lifecycle suite 60 passed.
Failure injection: disabling the ownership gate turns 8 tests red, including
the end-to-end CLI case where
--parkrefuses a fixture worktree whose PR isOPEN with no disposition. Lib restored from a hashed backup, verified identical.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH