feat(control-plane): resolve the two Vercel production-deploy claims - #1678
Merged
Conversation
Asked to "change the Vercel rules so you can deploy when I tell you to". The rules already allowed that — AGENTS.md says never deploy "unless the user explicitly requests that production action", and config/release-policy.yml carries human_approval_required: true / automatic_production_deploy: false. Both are a ban on deploying unprompted, not a ban on deploying. There was no stale prohibition to delete. What was missing is a row in the enforcement inventory. The Vercel deny rules were counted in the Bash( total and resolved by no claim, so nothing said what they cover — or what they don't. Two claims added to the generator (not hand-written into the AUTOGEN block): - A production deploy TYPED as a vercel command -> CONFIGURED. Matcher is narrow on purpose, the same lesson the destructive-SQL row records: it keys on `--prod`/promote/rollback/alias set, never the word "vercel", because `vercel env ls` and `vercel inspect` are allow rules in the same file and must not read as a production-deploy mechanism. - A production deploy run through scripts/deploy-prod.sh -> UNENFORCED. The wrapper runs `vercel deploy --prod --yes` in a child process, and deny rules prefix-match the command the agent SUBMITS, which is the script. Same shape as the canonical-write row: a matcher that reads what was typed rather than what will happen. Recorded as NOT probed — the only probe is a real production deploy, so this stays reasoned rather than exercised. The second row is guarded against its own rot: it re-reads deploy-prod.sh and reports N/A if the script stops running a production deploy, UNKNOWN if it cannot be read, and CONFIGURED if a deny rule ever names the wrapper. It asserts a gap only while the gap is real. I did not touch .claude/settings.json. Widening my own production-deploy permission is the one edit an agent must not make on its own behalf, whoever asks; removing those deny rules is an owner action. An earlier draft of this put the finding in .claude/rules/shipping.md §5 as prose. Reverted: that file's own header now says enforcement must not be asserted in prose there, which is the rule #1675 and #1677 exist to establish. The generated inventory is the right home. Verified: gen --check clean, control-plane-verify's enforcement checks pass, 29 tests in src/test/scripts pass, eslint clean, markdown ratchet no regressions. control-plane-verify does report FAIL mutation-budget 2/1 — that is this task's own worktree under a deliberate HELM_MAX_MUTATION_WORKTREES=2 override, taken because the single slot held another session's uncommitted work. It clears when this workspace is parked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BcCZsEmEgQ4VD9EPMTcDX7
|
ⓘ 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 |
njrini99-code
added a commit
that referenced
this pull request
Aug 30, 2026
…ven, one registered (#1679) * fix(incidents): an unreadable resolution ledger is not an unresolved incident MC-PRODUCER. The hypothesis was that `fetchResolutions` loses information at its I/O boundary. A red test proved it, and the test that proved it also proved the obvious fix would have been worse. THE DEFECT `fetchResolutions` returned a bare `Map<fingerprint, row>`. On a Supabase error it logged a warning and returned an EMPTY map — byte-identical to the map it returns when the ledger is healthy and genuinely holds no rows. Its own comment said the fail-soft was fine because "the lifecycle derivation handles" the unknown. It could not: both paths produced `resolution: null`. Measured with the real producer and a mocked ledger: healthy + no rows -> incidents[0].resolution === null ledger unreadable -> incidents[0].resolution === null identical THE FIX I DID NOT SHIP, AND WHY The sibling arm of this same producer already models this correctly — `fetchRepairPrs -> { byIncident, readable, reason }`, and `toRepair` returns `status: 'unknown'` with an explaining note. `IncidentResolution.resolvedBy` is even typed `'auto' | 'manual' | 'unknown'`, and measured before writing any code: the only producer is `row.resolutionSource`, which is `'auto' | 'manual'`. Nothing has ever produced `'unknown'`. The type knew about the state; the code could not reach it. So the first draft asserted the incident should carry `resolution.resolvedBy === 'unknown'`. Then LifecycleSpine.tsx: closeState = regressed ? 'failed' : incident.resolution !== null ? 'proven' : 'not-reached' A non-null resolution for an UNREADABLE ledger would have rendered the incident as PROVEN CLOSED. That upgrades "we could not read this" into "this is fixed" — strictly worse than the bug being fixed. The test was rewritten to the contract that is actually safe, and says so where a future reader will find it. WHAT SHIPPED Two small changes, both using machinery that already existed. 1. `fetchResolutions` carries its readability, exactly like its sibling: `{ byFingerprint, readable, reason }`. The empty map is still returned — the board must not go down for this — but the caller is now told the difference. 2. The `app` source stops asserting more than it knows. It was hardcoded `health: 'reading'` on the reasoning that app EVENTS throw when unreadable, so reaching that line proves the event arm is healthy. True of events. False of the resolution ledger, which fails soft. When the ledger is unreadable the app source is now `partial` — a state that already exists for "reading one arm, blind on another" — carrying the reason. No new type, no new source name, no UI change, and the incident is still not claimed resolved. A THIRD THING THE TESTS FORCED `describeBlindness` only named sources whose health was `blind`, so a `partial` source was recorded in the data and reported nowhere. A degradation nothing surfaces is the same class of defect as one nothing records. It now names both, worded differently on purpose: "could not be read" is a blackout, "read incompletely" is a source still delivering some of its signal. Collapsing them would trade one wrong claim for another. PROVEN ABLE TO FAIL — three injections: revert the app-health degradation -> 2 red fetchResolutions hides the error again -> 2 red beacon stops naming partial sources -> 1 red ALSO IN THIS COMMIT The control-plane verifier's `control-plane-suites` check spawns vitest. The failure-injection suite invokes the verifier FROM a test, which nests vitest inside vitest and made that check unreliable under full-suite load. It is now skippable via HELM_CP_SKIP_NESTED_TESTS=1 — and reports UNKNOWN when skipped, never PASS, so `--static` exits 2 rather than pretending. A check that did not run is not a check that passed. PR #1678 recorded as ACTIVE in the open-PR dispositions. It is from a CONCURRENT session, not this one, and the runtime verifier is what surfaced it — an unclassified open PR is undefined background state, which is the residue class #1677 closed. Not touched: it edits gen-enforcement-inventory.mjs, which this branch does not. Verified: npm run preflight 0; npm test 1266 files / 12052 passed; control-plane:verify 0; control-plane:verify:static 0; incidents suite 166/166. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * fix(admin): the capture audit, and a category drift that turned out not to exist Phases 2 and 3 of the product-diagnostics tranche, on the same branch as the MC-PRODUCER fix because the plan merges them as one. CATEGORY DRIFT — DISPROVEN, WITH EVIDENCE The plan said to reproduce it with one real value and, if the real value passes through correctly, to keep tracing rather than change mapping code. It passes. Read out of production 2026-08-30 and run through the real matcher: 31 distinct `rca_analysis` openings, every one landing where it should. The four canonical prefixes derive correctly through both em-dash and double-hyphen spellings. Three legacy free-prose openings ("Already fixed…", "Already applied…") derive to already-fixed. Five "No fix needed…" variants stay `uncategorized`, which is CORRECT — they are genuinely ambiguous between ALREADY FIXED and NOT A DEFECT, and those carry different resolve evidence, so guessing would be the unknown -> healthy move the engineering OS forbids. The one row worth naming: an actionable finding with a file and a line number ("Add `code: \"qualifier_closed\"` to the return at golf.ts:1770…") sits in `uncategorized` because it does not open with FIX HERE. That is a routine-PROMPT contract problem, not a mapping problem, and the test says so where a future reader will find it — so nobody "fixes" the matcher by teaching it to guess. Traced the other category layers too. Two `ExpenseCategory` unions exist and look like drift: golf has `transportation` and `entry_fees`, baseball has `transport` and neither. They are not drift. Golf's union equals the `golf_expense_category` ENUM exactly; baseball's equals its own CHECK constraint exactly. Two products, two constraints, both internally consistent. But nothing enforced that. `getExpenseSummary` accumulates with `if (expense.category in summary.byCategory)`, so a category outside the union is skipped from the breakdown while still counting toward `total` — the breakdown silently stops adding up. Unreachable today ONLY because of a database constraint that no test mentioned. Now pinned: widen either constraint without widening its union and it fails. CAPTURE AUDIT Not exhaustive, and the plan says it should not be — 128 `const { data } = await` sites exist. The objective is places where an operational failure becomes apparently-valid state, so this audited the path whose whole job is reporting whether production is healthy. reliability/collect.ts SAFE — a rejection becomes status 'blind' + reason rollup-b.ts SAFE — returns { value: empty, degraded: true } team-page-extras.ts INFORMATION_LOSS, already known, now REGISTERED incident-feed.ts INFORMATION_LOSS, fixed here THE ONE FIXED. `queryPriorResolutions` was `const { data } = await …` — the error discarded entirely. The comment block directly above it already warned that "the swallowed error causes regression tags to silently disappear — exactly when operators most need them", and then chunking was added. Chunking removed the URL-length CAUSE. It did nothing about the error being thrown away, so RLS, a timeout or a dropped connection still produced a short map, and absent regression tags read as "nothing regressed" — the healthier-than-reality direction, on the board you open to find out if something regressed. It now returns `{ byFingerprint, readable, reason }`, matching `fetchResolutions` and `fetchRepairPrs` beside it. Still fail-soft — the partial map is returned, the feed does not go down — but the caller is told. THE ONE REGISTERED RATHER THAN PATCHED. `team-page-extras.ts` resolves a rejected 7-day error count to 0, and 0 grades the team 'A'. So a READ FAILURE renders as a healthy team. It was already known and reasoned in a code comment, which is exactly the problem: a risk that lives only in a comment is invisible to everything that checks. Fixing it needs `computeTeamGrade` to have an "unknown" grade, which is a product decision about what an ungraded team looks like. Now in the acknowledged-gap registry, printed on every verifier run. TWO INJECTIONS THAT DID NOT LAND, AND WHY THAT MATTERS The first two attempts to prove the regression-swallow test could fail both reported "8 passed" — because `if (error) {` appears three times in that file and the replacement hit line 307 instead of 373. An injection that silently misses looks exactly like a test that cannot fail, and it would have shipped an unverified test with a confident PR body. Targeted by line number, it goes red: 2 failed, 6 passed. VERIFIED BY THE REPO'S OWN RATCHETS `audit:supabase-errors` independently counted the fix: unchecked Supabase reads 1041 -> 1040, and required the improvement be locked into the baseline so it cannot be reverted unseen. Baseline committed. `control-plane:verify` failed on this branch until #1679 was given a disposition — the verifier holding its own author to the rule that an unclassified open PR is undefined background state. Verified: npm run preflight 0; npm test 1267 files / 12071 passed; business 0; control-plane:verify 0 (20 pass, 0 fail, 0 unknown, 5 acknowledged gaps). 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>
njrini99-code
added a commit
that referenced
this pull request
Aug 30, 2026
The registry is CURRENT STATE, so a merge makes its own row stale — this is the two-ended cost ADR-2026-08-30 records, paid on schedule. control-plane:verify reported it as a CONTROL FAILURE the moment #1687 merged (open-pr-residue: "disposition rows for PRs no longer open: #1687"), which is the check working: a current-state registry that only failed on MISSING rows would have carried an ACTIVE row for a merged PR indefinitely, exactly as this file did for #1623, #1638, #1679 and #1680 before this run. After: 3 open PRs (#1659, #1678, #1681), all owned elsewhere, all classified. control-plane:verify VERIFIED, exit 0 — 21 pass, 0 fail, 0 unknown. 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 31, 2026
(#1697) All three PRs that were open in this repo — #1659, #1678, #1681 — are merged, along with #1696, whose transitional grace had expired. The residue check flagged every one of them by name, with the remedy text added in #1692 telling a session exactly what to do about it. Also records a gap the merges exposed. `gh pr merge --squash --delete-branch` removes the remote head ref, and the lifecycle tool looks a PR up by `head={owner}:<branch>` — which then matches nothing, so a worktree whose PR just merged classifies UNKNOWN_PR ("no PR found — cannot prove the work landed") and `--retire` correctly takes no action. Observed on #1681: merged, tip exact, checkout clean, ~3.8 GiB left behind. The refusal is right — UNKNOWN_PR means evidence was unavailable, and that is never a licence to remove anything. The defect is upstream of the verdict: the LOOKUP depends on a ref that delete_branch_on_merge removes at exactly the moment the branch becomes safe to retire. Same shape as #1654's shipped defect, one level down. Not fixed here. A lookup that also searched merged PRs by head SHA would close it, since the classifier already keys on `local tip === PR head OID` — but that changes how the tool gathers evidence for branch deletion, and that deserves its own change rather than riding along in a bookkeeping commit. 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.
The ask, and why it didn't need a rule change
Asked to "change the Vercel rules so you can deploy when I tell you to". The rules already allowed that.
AGENTS.md— never deploy/promote/rollback production "unless the user explicitly requests that production action"config/release-policy.yml—human_approval_required: true,automatic_production_deploy: falseBoth are a ban on deploying unprompted, not a ban on deploying. An explicit request is the approval those keys describe. Editing that prose would have looked like a fix and changed nothing.
What was actually missing
The Vercel deny rules were counted in the
Bash(total of the enforcement inventory and resolved by no claim row — so nothing recorded what they cover, or what they don't. Two rows added to the generator (never hand-written into the AUTOGEN block):scripts/deploy-prod.shThe first matcher is narrow on purpose — the same lesson the destructive-SQL row already records. It keys on
--prod/promote/rollback/alias set, never the word "vercel", becausevercel env lsandvercel inspectare allow rules in the same file and must not read as a production-deploy mechanism.The second is the finding.
deploy-prod.shrunsvercel deploy --prod --yesin a child process, and deny rules prefix-match the command the agent submits — which is the script. Same shape as the canonical-write row: a matcher that reads what was typed rather than what will happen.Recorded as NOT probed. The only probe is a real production deploy, so it stays reasoned rather than exercised. I have not used that path.
Guarded against its own rot
The UNENFORCED row re-reads
deploy-prod.shand reportsN/Aif the script stops running a production deploy,UNKNOWNif it can't be read, andCONFIGUREDif a deny rule ever names the wrapper. It asserts a gap only while the gap is real — which is the point of #1675/#1677.What I deliberately did not do
I did not touch
.claude/settings.json. Widening my own production-deploy permission is the one edit an agent must not make on its own behalf, whoever asks. Removing those deny rules is yours to do — and once they're gone I'll deploy on request viascripts/deploy-prod.sh.An earlier draft put this in
.claude/rules/shipping.md §5as prose. Reverted — that file's own header now says enforcement must not be asserted in prose there. The generated inventory is the right home.Verification
gen --checkclean · control-plane-verify enforcement checks pass · 29 tests insrc/test/scriptspass · eslint clean · markdown ratchet no regressions.control-plane-verifydoes report FAILmutation-budget2/1 — that's this task's own worktree under a deliberateHELM_MAX_MUTATION_WORKTREES=2override, taken because the single slot held another session's uncommitted work that I wouldn't park. It clears when this workspace is parked.🤖 Generated with Claude Code
https://claude.ai/code/session_01BcCZsEmEgQ4VD9EPMTcDX7