fix(lifecycle): a checkout is disposable only if it says so itself - #1689
Merged
Conversation
The #1681 fix made an OPEN PR's checkout unparkable without recorded owner intent. It never covered the window BEFORE a PR exists — and that window is where a session actually starts. A worktree five minutes old is clean, pushed, has no PR to key a disposition on, and is invisible to `lsof` between two tool calls: every signal the old rule read as "disposable", and every one of them wrong. That residue was registered as WORKTREE_PARK_NO_PR_OWNERSHIP. Disposability now comes from the workspace's own identity, in the marker scripts/new-worktree.sh already wrote: .helm/workspace.json -> { "parkPolicy": "KEEP" } at creation, always "PARK_IF_REPRODUCIBLE" only if a human sets it No new store, no new registry — one field on a file that existed. Releasing a checkout is a positive act; everything else KEEPS: no marker, no key, an unknown value, a file that will not parse. Verdict KEEP_WORKSPACE_INTENT_REQUIRED, which sits outside the standing autonomous authorization. The gate runs BEFORE the reproducibility checks and independently of any PR, so both gates must permit. An OPEN PR its owner released still cannot override a workspace KEEP. The two questions stay separate, because conflating them is what caused this: workspace identity may this CHECKOUT go? PR state may this BRANCH be deleted? Branch retirement is untouched: PR MERGED plus an exact head-OID match, exactly as before. CLOSED DIFFERENTLY FROM THE CONDITION IT WAS OPENED WITH, deliberately. That asked for a session id checked for LIVENESS — the same unsound negative-evidence inference in a new costume, since a session between two tool calls looks dead. Declared intent needs no probe. WHAT THIS CHANGES FOR THE ONE LIVE WORKTREE: nothing observable, and the reason matters. agent/round-type-reclassify (#1681) predates the marker entirely, so it now reports KEEP_WORKSPACE_INTENT_REQUIRED with "no .helm/workspace.json" rather than KEEP_PR_OWNER_INTENT_REQUIRED. Still kept, for a stronger reason. worktree_policy in open-pr-dispositions.json is NOT removed here, though it is now the weaker of two gates. #1659 is PARKED — there is no checkout on disk to carry a marker — so deleting the field would delete the only recorded authorization for it. The file says so, with the condition for removing it. Verified: three injections, each red for its own reason — unwiring the gate (8 tests), letting a missing marker mean released (5), and moving the gate after the reproducibility checks (1) — with the library restored byte-identical (sha256). The end-to-end tests run real `--park` against real git worktrees with the marker absent, KEEP, and released; the fixture repo gained the `.helm/` gitignore line the real repo has at .gitignore:257, without which the marker itself reads as uncommitted work. preflight 0, 220 tests across src/test/scripts, control-plane:verify VERIFIED. 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
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 |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH
…orkflow #1688 shipped the transitional merge-identity exception so a PR's own row could survive its own merge without a cleanup push to main. That exception is scoped to exactly one commit: it holds while main's HEAD IS the merge commit, and stops the moment anything else lands. From this branch, HEAD is this branch's tip, so #1688's row is stale again and the check says so, naming the merge SHA it no longer matches. Clearing it here IS the workflow — the next ordinary PR removes the previous row while adding its own. First exercise of it, and it behaved exactly as designed rather than needing a bypass, which was the whole point. 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
The designed workflow, second exercise: the next ordinary PR clears the previous PR's transitionally-closed row while adding its own. No push to main. 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
… reporting (#1690) * feat(knowledge): map the CRM surface, and find a P0 the red check was reporting Three tasks, one thread: the permanently-red `Supabase Preview` check turned out to be telling the truth. 1. A COMMITTED P0 MIGRATION IS NOT APPLIED TO PRODUCTION supabase/migrations/20260819200000_preserve_golf_history_on_account_deletion.sql carries the owner decision of 2026-08-18 — "There should be no deletion of golf shot history" — and measures that 93 of 94 players reach the cascade. Verified read-only against production 2026-08-30, it is not live: golf_players.user_id still NOT NULL (migration: nullable) golf_players_user_id_fkey ON DELETE CASCADE (migration: SET NULL) golf_player_anonymize_on_unlink does not exist So DELETE /api/account/delete still destroys that player's rounds, holes, shots, reviews and stats cache. Filed as memory/incidents/golf_round_lifecycle/INC-2026-08-30-account-deletion-still-cascades-golf-history.md. NOT fixed here: applying a production migration needs explicit owner authorization, and `supabase db push` / `migration up` are denied by permissions.deny, correctly. The wider divergence is registered as MIGRATIONS_REPO_PRODUCTION_LEDGER_DIVERGENCE: 846 ledger versions against 337 tracked files. Most of that gap is explainable — 240 versions predate the squashed baseline, and applying via dashboard/MCP stamps its own timestamp, so 46 of 54 "missing" August versions are the same migration under a different version. Two of this session's own first guesses were WRONG for the same reason (baseball_event_telemetry is a migration name, not a table; the table is baseball_timeline_event_acks), and the incident records that rather than hiding it. Supabase Preview is therefore NOT decommissioned: it reports a real condition, and turning it off would silence a true alarm. 2. THE CRM SURFACE IS MAPPED memory/features/crm_outreach.md, written from verified code — 3 routes, 20 action files (two of which break the crm- naming convention and are listed individually), 11 API routes, 20 owned tables, 6 integrations. Registered as a feature and now owns the crm_recruiting_pipeline runtime key, closing the last feature_awareness_gap. KNOWLEDGE ROUTING ONLY, and checked rather than assumed: src/lib/admin/ feature-registry.ts carries an owner directive that CRM is never wrapped, tagged, or shown on the Bridge board. That row is untouched — still excluded: 'crm', still actions: {}. Verified that memory/registry.yml is read only by scripts/knowledge/**, scripts/repo-doctor and the drift gates, and that src/lib/reliability/collect.ts resolves feature ids from a hardcoded route table because it "cannot read a build-time artifact". Nothing here reaches the Bridge. Also recorded, not silently fixed: sending is human-triggered by omission — process-sequences is fully wired but deliberately absent from vercel.json, so adding it to "fix a missing schedule" would start sending cold email. 3. TWO STALE CLAIMS CORRECTED FORWARD DATABASE_STATUS.md's "NOT FIXED — needs a product decision" heading sat above a section that had since grown an "✅ APPLIED" block with role-impersonation evidence. Re-verified against live pg_policies (not the migration file — trap G8): baseball_coaches_select is org-scoped. Already fixed. And the convergence report said "two items marked NOT FIXED". There was one, plus one separately-declared deliberate gap — a count written into prose, wrong, in the run whose thesis is that stale counts read as current forever. 4. THE GATE THAT CAUGHT THE INCIDENT docs:schema-drift failed on the incident, correctly: it names four identifiers that are not in the schema. That is the incident's SUBJECT — deleting them deletes the finding — and baselining them would be wrong twice, since the baseline is for known-bad references that should shrink. So a document may now DECLARE, per identifier, that it names something because it is absent: <!-- schema-drift-absent: name_one, name_two --> Exempt only those names, only in that file, printed on every run. A typo two lines away is still caught; another document's declaration covers nothing. Injection: making the declaration file-global turns that test red, script restored byte-identical (sha256). Verified: preflight 0, helm-os:check 0, control-plane:verify VERIFIED (6 registered gaps), 223 tests across src/test/scripts, ratchet 30514 unchanged. All production reads were read-only through the account MCP. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * chore(control-plane): record #1690, clear #1689's transitional row The designed workflow, second exercise: the next ordinary PR clears the previous PR's transitionally-closed row while adding its own. No push to main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * fix(knowledge): regenerate the inventory AFTER staging, not before CI caught this and local runs did not, for a reason worth writing down: the generator enumerates `git ls-files`, which reports the INDEX. Running it before `git add` of a new document counts the tree without that document — 1693 instead of 1694 — and the local --check then agrees with itself, because both halves read the same stale index. CI checks out the merge commit, where the file IS tracked, and disagrees. So the generator is deterministic across checkouts (the convergence run proved that by reading CI's own log); what is order-dependent is WHEN you run it relative to staging. Regenerate after `git add`, not before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * fix(incident): verify the CONSEQUENCE against the route, not the migration header The schema half was verified first-hand — user_id NOT NULL, FK ON DELETE CASCADE, no anonymize function — and that proves the migration is unapplied. But the impact sentence ("account deletion still destroys the history", "93 of 94 players reach the cascade") came from the MIGRATION'S OWN HEADER, written 2026-08-19. Eleven days and many merges later that is a quoted comment, not evidence, in a record filed at R4 for irreversible data loss. G8 inverted, and in the worst possible place. Read the route as it stands today. It holds: route.ts:150 USER_BLOCKING_TABLES is still exactly the three attribution tables — nothing blocks on golf_rounds or golf_shots route.ts:308 still ends in admin.from('users').delete().eq('id', user.id) whole file names golf_rounds / golf_shots / golf_players NOWHERE, so no application-side preservation was added either So the finding stands, now on first-hand evidence with file:line anchors instead of a quotation. The 93-of-94 ratio is re-attributed as 2026-08-18's measurement, explicitly not re-measured here — it is a live-data figure this session cannot cheaply reproduce, and pretending otherwise would be the same error one level down. 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.
Closes
WORKTREE_PARK_NO_PR_OWNERSHIP.The #1681 fix made an OPEN PR's checkout unparkable without recorded owner intent. It never reached the window before a PR exists — which is where a session starts. A worktree five minutes old is clean, pushed, has no PR to key a disposition on, and is invisible to
lsofbetween two tool calls: every signal the old rule read as "disposable", and every one wrong.The mechanism
One field on a file
new-worktree.shalready wrote. No new store. Releasing a checkout is a positive act; everything else KEEPs — no marker, no key, unknown value, unparseable file. VerdictKEEP_WORKSPACE_INTENT_REQUIRED, outside the standing authorization.Runs before the reproducibility checks and independently of any PR, so both gates must permit — an OPEN PR its owner released cannot override a workspace KEEP.
Branch retirement is untouched: PR MERGED + exact head OID.
Closed differently from how it was opened
The gap's closing condition asked for a session id checked for liveness. That is the same unsound negative-evidence inference in a new costume — a session between two tool calls looks dead. Declared intent needs no probe.
What changes for the one live worktree: nothing observable
agent/round-type-reclassify(#1681) predates the marker, so it now reportsKEEP_WORKSPACE_INTENT_REQUIRED— "no .helm/workspace.json" — instead ofKEEP_PR_OWNER_INTENT_REQUIRED. Still kept, for a stronger reason.worktree_policyis deliberately NOT removed yetIt is now the weaker of two gates, but #1659 is parked — no checkout on disk to carry a marker — so deleting the field would delete the only recorded authorization for it. The file records the condition for removing it.
Verification
Three injections, each red for its own reason: unwiring the gate (8 tests), letting a missing marker mean released (5), moving the gate after the reproducibility checks (1). Library restored byte-identical (sha256).
End-to-end
--parkruns against real git worktrees with the marker absent / KEEP / released. The fixture repo gained the.helm/gitignore line the real repo has at.gitignore:257— without it the marker itself reads as uncommitted work, and the fixture would disagree with production about what "clean" means.preflight 0 · 220 tests in
src/test/scripts·control-plane:verifyVERIFIED, 5 registered gaps.🤖 Generated with Claude Code
https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH