feat(admin): the board can finally say the two error surfaces disagree - #1691
Merged
Conversation
Closes ERROR_SURFACES_DISAGREE. Also records why sandbox.filesystem stays off.
THE FAILURE, measured 2026-08-30:
admin_events 0 errors in 48h
Sentry 12 unresolved, several last seen 2-10h ago
and the board rendered one surface's zero as production health. Both numbers
were correct. The single verdict was not.
The two surfaces have different jobs — admin_events is graded, Helm-handled
faults; Sentry is every captured runtime error, including ones Helm never saw —
so they are ALLOWED to differ. What is not allowed is one of them standing in
for production health while the other contradicts it. `partial` is the state
that could not previously be expressed, and it is the common one: two surfaces
disagreeing without either being broken.
reconciliation.ts is pure — no fetch, no React, no Supabase — so every arm is
testable without a fixture. Order is fail-safe: unreadability is decided BEFORE
agreement, because two surfaces cannot agree when one did not speak. A blind
source can never produce `healthy`, the same rule canClaimAllClear applies
page-wide, and an unread count renders as an em-dash rather than 0.
TWO DECISIONS THE CONTRACT DID NOT MAKE, now written down where they are made:
- The counts are ASYMMETRIC. Application events count error-or-worse, because
admin_events is graded and the grading is its value. Sentry incidents count
ungraded, because the contract forbids silencing an issue on Helm's opinion of
a fault Helm never handled. The Sentry number is CORRELATED INCIDENTS, so it
will not match the Sentry dashboard's issue count — twelve events of one fault
are one thing wrong.
- The block renders even when the surfaces AGREE, unlike BlindnessBeacon which
returns null when quiet. Silence is right for a warning and wrong for a
reconciliation: without a visible row, "the two agree" and "nobody compared
them" look identical again, which is the failure being closed.
Nothing was mirrored between the surfaces and no Sentry issue was silenced —
both explicitly forbidden by docs/OBSERVABILITY_AUTHORITY.md, which was the
definition of done and is now marked shipped.
SANDBOX: ADR-2026-08-30-sandbox-filesystem-allowwrite records the design and why
it is NOT enabled. Three blockers, and the third is the real one:
allowWrite is user-global so making it safe for Helm means enumerating every
other project on the machine; a read-only canonical contradicts AGENTS.md's
single-session policy and re-couples every task to a ~3.8 GiB worktree; and git
itself writes the working tree, so `git checkout` and `git pull` stop working.
The protection wanted is "an agent may not edit source in canonical"; the
mechanism available is "no process may write under this path". Those are not the
same rule. The gap stays open with its measurement rather than closed by
flipping a switch whose blast radius is every project here.
Verified: preflight 0, typecheck 0, next build 0, helm-os:check 0,
control-plane:verify VERIFIED, 257 tests across the touched trees, ratchet 30514
unchanged. Injections: rendering disagreement as healthy turns 4 tests red,
treating a blind surface as zero turns 2 red, module restored byte-identical
(sha256).
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
njrini99-code
added a commit
that referenced
this pull request
Aug 30, 2026
…'s, and say why Two coordination fixes, both from the residue checker reporting honestly. **#1691's row is removed.** It was TRANSITIONALLY closed — merged at a03ef84 while HEAD sat on that exact commit, which is the narrow exception that stops a PR carrying its own row from failing main the second it merges. The grace ends the moment any other commit lands, and it has: this branch added its own. The row going stale is the exception working exactly as designed, not a defect. **The failure now names the remedy.** It previously read only "merged at <sha>, HEAD has moved past it", which a session meeting it cold cannot distinguish from an ordinary stale row left behind by carelessness. The two have the same symptom and different histories, and only one of them has a one-line fix. It now says to delete the row in the PR already being opened. That is the whole coordination cost of the transitional exception, and it belongs in the tool's own output rather than in a comment block someone has to already know to read. **#1692's row is added** in the same change, which is the pattern the file's own $comment prescribes: the next ordinary PR clears the expired row and registers itself at the same time. Verified: the notice renders — classifyDispositionResidue exercised directly with a MERGED row whose merge sha differs from HEAD emits the new text. guards 32/32, residue suite 14/14, JSON parses. Expect one red `open-pr-residue` run between the PR opening and this commit reaching its head: in-flight arrival is proven by reading the row at the PR's own head, and the head does not carry it until this lands. That is the mechanism, not a regression. 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
… red I wrote, earlier in this run, that `Supabase Preview` "has been red on every main commit for so long that it reads as background noise." That is wrong. Measured across eight PRs (#1679, #1680, #1686–#1691), its conclusion is SKIPPED every time — never FAILURE. It is also not one of main's six required contexts, so it could not block a merge even if it did fail. A check that never runs is invisible in a way a red one is not, which makes the real situation worse than the one I described, not better. The skipping is legitimate: Supabase only builds a preview branch for a PR that touches supabase/migrations/**, and most do not. Branching is enabled and works — #1681 touches migrations and its preview branch reached FUNCTIONS_DEPLOYED today. What is genuinely unaddressed is one level down: the Supabase branch record for `main` carries status MIGRATIONS_FAILED with created_at == updated_at == 2026-07-03T21:11:11Z, so it has not been refreshed since it was written. Whether that is a live verdict on today's migrations or a stale setup artifact is NOT established — both readings fit the timestamps, and picking one would be a guess. Recorded as unresolved rather than resolved in the wrong direction. This changes the shape of the "make Supabase Preview green" task: the check is inert, not failing, so the decision is whether an inert check should be required, repaired, or removed — not how to turn it green. One consequence worth having: a preview branch is an isolated project, NOT production, which makes it the environment the plan's "do not use production for destructive behavioral tests" rule points toward. With Docker down it is the only remaining route for exercising the golf-history migration. Not taken from here — provisioning one is a real resource on the owner's account. 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
…ify every unledgered migration (#1692) * fix(golf): make the four notification fan-outs tolerate a null player user_id Preparation for 20260819200000 (preserve golf history on account deletion), which changes golf_players.user_id from NOT NULL to nullable so a deleted account leaves its rounds, shots and holes intact instead of cascading them away. How the four sites were found, rather than guessed: the generated types are built from production, where user_id is still NOT NULL, so tsc cannot see the change yet. I hand-edited a disposable copy of the golf_players Row/Insert/ Update types to make user_id nullable, ran typecheck, and took the errors as the complete compile-visible breakage list. Four, all the same shape — a golf_players -> users notification fan-out that puts user_id straight into an `in` list. The file was then restored and verified byte-identical by sha256. These four are stragglers, not a new problem: golf.ts already does exactly this filter in three other fan-outs (2869, 3459, 4793). The patch makes the four match, using the same `.filter((id): id is string => Boolean(id))`. Ships BEFORE the migration on purpose. user_id is NOT NULL in production today, so the filter removes nothing and the change is a no-op — which is what decouples this from the lockstep deploy the migration header warns about. What else the audit found, and did not change: - No code path anywhere sets golf_players.user_id to null. Verified across src/ and supabase/. The trigger's `old.user_id is not null and new.user_id is null` condition is therefore reachable only through the FK's ON DELETE SET NULL — no ordinary write can trip the anonymization by accident. - Every column the trigger nulls (first_name, last_name, email, phone, avatar_url, hometown, state, high_school_name, graduation_year, gpa) is ALREADY nullable, so no rendering path needs a change. - The two remaining user_id consumers tsc did not flag are ownership checks (round-drafts.ts:690, generate-review/route.ts:75). Both compare with !== / === against user.id, so a null denies. Fail-closed, correct. - An anonymized player keeps its golf_team_members row and would still appear on an active roster with a blank name. That is a product decision, not a compile break, and is registered in the incident rather than decided here. Verified: typecheck 0 against today's types AND 0 against the nullable types; lint 0; the three notification tests pass (8/8). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * docs(migrations): classify all 42 unledgered migrations against the live catalog Closes the open question inside MIGRATIONS_REPO_PRODUCTION_LEDGER_DIVERGENCE: the gap recorded that the repo and production's ledger disagree, but not what was actually missing. Method, because the method is the finding. A ledger row is a claim about history, not evidence about the schema — so each of the 42 files with no production ledger row was parsed for the objects it DECLARES (tables, columns, functions, policies, indexes, triggers, enum values, constraints) and those objects were checked against the live catalog in bulk. Not filenames. 22 applied — every declared object present 5 partial 2 unapplied 11 UNKNOWN by construction — GRANT/REVOKE and data backfills leave no catalog object to look for, and "probably applied" is not a verdict 2 self-declared no-ops The headline: 20260819200000_preserve_golf_history_on_account_deletion is the ONLY golf-facing migration with genuinely missing effects. 1 of its 5 objects exists, and the FK that does exist is still confdeltype='c' (CASCADE). Two things this corrected that a filename read would have got wrong: - Three "absent" policy sets are name divergences, not missing RLS. Production covers the same table and verbs under its own names (baseball_event_acknowledgements_* vs baseball_event_acks_*). One of the three is genuinely short a DELETE policy; the other two are equivalent or a superset. Predicate equivalence is NOT claimed — only verb coverage was measured. - All seven ncaa_division enum values and all four avatar storage policies read ABSENT until re-queried case-insensitively; the extractor lowercases SQL and production stores them uppercase/capitalised. Two migrations would have been reported as production gaps that are not. A case-folding bug in a classifier is indistinguishable from a missing object unless you go and look. Also confirms the plan's hard stop empirically rather than assuming it: `supabase db push` would propose all 42, not the one migration the owner authorised. `db push --dry-run` is itself covered by the permissions.deny prefix rule and was not attempted; `supabase migration list --linked` answers the same question and is not denied. The incident gains the measured blast radius (104 players, all 104 with a linked user; 521 rounds / 36,943 shots / 9,162 holes — the migration header's 2026-08-18 baseline has grown by about half again), the application compatibility audit, and an explicit note that the local exercise is BLOCKED: Docker Desktop was launched but its daemon never came up this session. Verified: docs:schema-drift 0, docs:path-drift 0, docs:inventory-check 0, markdown ratchet at baseline (net 0). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * fix(announcements): do not report "nobody to notify" as a delivery failure Follow-on to the nullable-user_id compatibility patch. The recipient guard fires on `userRows.length === 0` and logs a delivery FAILURE. After 20260819200000 a batch whose targeted players are all anonymized legitimately resolves to zero users — nobody to notify is not a failed lookup, and logging it as one is the failure-vs-empty conflation this repo keeps removing, pointed the other way. Only this site needed it. The other three fan-outs already no-op on empty without logging: tasks.ts logs only on a real `userRowsError`, golf.ts maps over an empty array, and player-notify.ts returns early on `!userRows?.length`. Verified: typecheck 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * chore(control-plane): clear #1691's expired transitional row, add #1692's, and say why Two coordination fixes, both from the residue checker reporting honestly. **#1691's row is removed.** It was TRANSITIONALLY closed — merged at a03ef84 while HEAD sat on that exact commit, which is the narrow exception that stops a PR carrying its own row from failing main the second it merges. The grace ends the moment any other commit lands, and it has: this branch added its own. The row going stale is the exception working exactly as designed, not a defect. **The failure now names the remedy.** It previously read only "merged at <sha>, HEAD has moved past it", which a session meeting it cold cannot distinguish from an ordinary stale row left behind by carelessness. The two have the same symptom and different histories, and only one of them has a one-line fix. It now says to delete the row in the PR already being opened. That is the whole coordination cost of the transitional exception, and it belongs in the tool's own output rather than in a comment block someone has to already know to read. **#1692's row is added** in the same change, which is the pattern the file's own $comment prescribes: the next ordinary PR clears the expired row and registers itself at the same time. Verified: the notice renders — classifyDispositionResidue exercised directly with a MERGED row whose merge sha differs from HEAD emits the new text. guards 32/32, residue suite 14/14, JSON parses. Expect one red `open-pr-residue` run between the PR opening and this commit reaching its head: in-flight arrival is proven by reading the row at the PR's own head, and the head does not carry it until this lands. That is the mechanism, not a regression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * docs(incident): correct the Supabase Preview claim — it is inert, not red I wrote, earlier in this run, that `Supabase Preview` "has been red on every main commit for so long that it reads as background noise." That is wrong. Measured across eight PRs (#1679, #1680, #1686–#1691), its conclusion is SKIPPED every time — never FAILURE. It is also not one of main's six required contexts, so it could not block a merge even if it did fail. A check that never runs is invisible in a way a red one is not, which makes the real situation worse than the one I described, not better. The skipping is legitimate: Supabase only builds a preview branch for a PR that touches supabase/migrations/**, and most do not. Branching is enabled and works — #1681 touches migrations and its preview branch reached FUNCTIONS_DEPLOYED today. What is genuinely unaddressed is one level down: the Supabase branch record for `main` carries status MIGRATIONS_FAILED with created_at == updated_at == 2026-07-03T21:11:11Z, so it has not been refreshed since it was written. Whether that is a live verdict on today's migrations or a stale setup artifact is NOT established — both readings fit the timestamps, and picking one would be a guess. Recorded as unresolved rather than resolved in the wrong direction. This changes the shape of the "make Supabase Preview green" task: the check is inert, not failing, so the decision is whether an inert check should be required, repaired, or removed — not how to turn it green. One consequence worth having: a preview branch is an isolated project, NOT production, which makes it the environment the plan's "do not use production for destructive behavioral tests" rule points toward. With Docker down it is the only remaining route for exercising the golf-history migration. Not taken from here — provisioning one is a real resource on the owner's account. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH * chore(docs): regenerate the document authority inventory The Supabase Preview correction changed a STATE_SNAPSHOT column and the inventory went stale again. CI caught it, correctly. Third time this run, same root cause, worth naming: this generator reads the git INDEX, and `--check` in CI runs against the COMMIT. Regenerating before the last content edit produces a locally-green, remotely-red result every time. The regeneration has to be the final step before the commit, not a step during it. 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
ERROR_SURFACES_DISAGREE(Task 10) and records thesandbox.filesystemdecision (Task 13). Also clears #1690's transitional disposition row — the designed workflow.The failure
The board rendered one surface's zero as production health. Both numbers were correct; the single verdict was not.
The surfaces have different jobs —
admin_eventsis graded Helm-handled faults, Sentry is every captured runtime error including ones Helm never saw — so they are allowed to differ. What is not allowed is one standing in for production health while the other contradicts it.reconciliation.tsis pure — no fetch, no React, no Supabase. Order is fail-safe: unreadability is decided before agreement, because two surfaces cannot agree when one did not speak. A blind source can never producehealthy(the rulecanClaimAllClearapplies page-wide), and an unread count renders as an em-dash, never 0.Two decisions the contract did not make
The counts are asymmetric. Application events count error-or-worse, because
admin_eventsis graded and that grading is its value. Sentry incidents count ungraded, because the contract forbids silencing an issue on Helm's opinion of a fault Helm never handled. The Sentry number is correlated incidents — it will not match the Sentry dashboard, and is not meant to.The block renders even when the surfaces agree, unlike
BlindnessBeaconwhich returns null when quiet. Silence is right for a warning and wrong for a reconciliation: without a visible row, "the two agree" and "nobody compared them" look identical again — the exact failure being closed.Nothing was mirrored between surfaces; no Sentry issue was silenced. Both are explicitly forbidden by the contract.
Sandbox: designed, deliberately not enabled
ADR-2026-08-30-sandbox-filesystem-allowwrite.md. Three blockers, the third decisive:allowWriteis user-global — making it safe for Helm means enumerating every other project on the machine.git checkoutandgit pullstop working.The protection wanted is "an agent may not edit source in canonical"; the mechanism available is "no process may write under this path". Not the same rule. Gap stays open with its measurement rather than closed by flipping a switch.
Verification
preflight 0 · typecheck 0 · next build 0 ·
helm-os:check0 ·control-plane:verifyVERIFIED · 257 tests across the touched trees · ratchet 30514 unchanged.Injections: rendering disagreement as healthy turns 4 tests red; treating a blind surface as zero turns 2 red. Module restored byte-identical (sha256).
🤖 Generated with Claude Code
https://claude.ai/code/session_01NbDxyygyXRUEERuGocpZZH