Skip to content

fix(ledger,guard): survive parallel reconciles and rewritten history - #1978

Merged
BigSimmo merged 14 commits into
mainfrom
claude/db-remediation-phase-0-wfaiyl
Aug 15, 2026
Merged

fix(ledger,guard): survive parallel reconciles and rewritten history#1978
BigSimmo merged 14 commits into
mainfrom
claude/db-remediation-phase-0-wfaiyl

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

Two defects in this repo's own tooling, both found the hard way while landing #1951, both caused by code that assumes a single serialized writer. Parallel reconciles are no longer rare — four ran on 2026-08-14 alone.

  • planRequestBatch no longer wedges on a cancellation that lost a race (scripts/ledger-inbox.mjs). A cancel whose target was already applied by another branch's reconciliation is now a recorded no-op instead of a throw.
  • guardBaseForRange no longer compares against abandoned history (scripts/guard-push.mjs). After a force-push it falls back to the merge base with origin/main — the base CI already uses.

Why the ledger one matters more than it looks

It is not a cosmetic error path. planRequestBatch is reached by check-docs-links.mjs, check-ledger-write-discipline.mjsandledger-inbox.mjs reconcile, so a single dangling cancel red-lines docs:check-links (in verify:cheap, verify:pr-local and CI static-pr), the write-discipline gate, and reconcile — simultaneously. And it could not be cleared by deleting the queued file, because write discipline forbids removing a request. That is a deadlock, and it happened: Static PR checks on #1951 failed with

Error: cancel request e265c3b4… targets missing pending request bbf21714…
at planRequestBatch (scripts/ledger-inbox.mjs:122)
at markdownForTarget (scripts/check-docs-links.mjs:128)

after a parallel reconcile applied bbf21714 while #1951 was open.

The new behaviour is deliberately loud, not lenient. An already-applied target warns by name and says the cancellation did not take effect and the row must be corrected with a fresh update — because the cancel's intent did fail, and silence would hide that. A genuinely unknown target still throws; so does cancelling a cancellation, or cancelling the same request twice; and an ineffective cancellation does not resolve a competing-mutation conflict.

Why the guard-push one matters

guardBaseForRange returned range.remoteSha unconditionally. After a force-push that tip is a discarded line of history, so every request it carried reads as "removed without an audit record" and the ledger transaction guard can never pass — no matter how clean the rebuild is. That is exactly why landing #1951 required SKIP_LEDGER_WRITE_GUARD=1.

The repo already disagreed with itself here: .github/workflows/ci.yml passes the pull request's base sha as LEDGER_WRITE_BASE_SHA, so the local hook and CI were asking different questions of the same diff. The fix keeps the remote tip when it is an ancestor of the pushed commit (ordinary push, unchanged) and otherwise uses the merge base. changedFilesForRange follows the same rule so a discarded commit's files are not reported as deletions this push introduced.

This PR was pushed without any override — the first real exercise of the fix.

Verification

Both fixes are mutation-verified, because a passing test proves nothing on its own:

MutationResult
revert the cancel tolerance (throw again)3 ledger tests red
silence the already-applied warning1 ledger test red
revert guardBaseForRange to the unconditional remote tipforce-push case red
revert the changedFilesForRange ancestry checkforce-push case red
  • npm run verify:pr-local
PR-local verification summary:
- completed: check:runtime, check:installed-lock-parity, format:changed, lint, typecheck, test,
check:rag:fixtures, check:medication-interactions, check:medication-lexicon-report
- failed: (none)
- not reached: (none)
  • npx vitest run tests/guard-push.test.ts tests/ledger-inbox-cancellation.test.ts tests/guard-push-no-merge-base.test.tsTest Files 3 passed (3) / Tests 39 passed (39).
  • node scripts/guard-push.mjs --self-test[guard-push] self-test passed.
  • Ledger consumers on this branch: Ledger write discipline passed for bf486de406ed..HEAD. and docs link check passed: 1772 repo path references resolve.
  • Typecheck initially failed on the new test helper (unknown[] vs object[]) and was fixed before commit — noting it because the gate catching it is the point.

Not run:verify:ui (no UI change); provider-backed gates (nothing here touches a provider).

RAG impact

Not required: no file under src/lib/rag/**, no match_* RPC, no ranking, selection, eval-harness or fixture surface. check:rag:fixtures ran green anyway.

Risk and rollout

  • Risk: Low-to-moderate, and concentrated in the ledger change since it relaxes a guard. The relaxation is narrow — it applies only when the target is present in applied/, i.e. provably a lost race rather than a bad reference — and every other rejection path is unchanged and still covered by tests. The guard-push change only alters behaviour on a non-fast-forward push, which previously could not pass at all.
  • Rollback:git revert the single commit; the two fixes are independent in effect, and reverting restores exactly the prior throw/base behaviour.
  • Provider or production effects: None.

Notes

  • Found while closing out Phase 0 (test(live-drift): prove the drift routing, and close out Phase 0 #1951). The dangling cancel that triggered it never reached main and was removed from that branch before merge, so main is not currently broken — this is a recurrence fix, not an outage fix.
  • A follow-up PR covers the other two items from the same review: wiring check:medication-lexicon-report into CI (it currently runs in verify:pr-local but no CI job — #333's open half), and correcting the remediation plan's stale drift figures.

Generated by Claude Code

Two defects that both surfaced during the Phase 0 handoff, both caused by
tooling that assumes a single serialized writer.
planRequestBatch threw when a cancellation's target had already been applied by
another branch's reconciliation. That is a lost race, not corruption, but the
throw wedged every consumer at once — check:docs-links, ledger write discipline
and reconcile itself — and write discipline forbids deleting the queued request,
so there was no legal way out. An already-applied target is now a recorded
no-op. It warns loudly, naming the cancellation and its target and saying the
correction did not take effect, because silence here would be worse than the
crash. A genuinely unknown target still throws, as does cancelling a
cancellation or cancelling twice.
guardBaseForRange returned the branch's remote tip unconditionally. After a
force-push that tip is abandoned history, so every request it carried reads as
deleted and the ledger transaction guard can never pass however clean the
rebuild is — which is why landing the Phase 0 branch needed
SKIP_LEDGER_WRITE_GUARD=1. It now keeps the remote tip only when that tip is an
ancestor of the pushed commit, and otherwise falls back to the merge base with
origin/main. That is the base CI already uses: ci.yml passes the pull request's
base sha as LEDGER_WRITE_BASE_SHA, so the hook and CI were asking different
questions. changedFilesForRange follows the same rule, so a discarded commit's
files are not reported as deletions introduced by the push.
Both fixes are mutation-verified: reverting the cancel tolerance turns 3 ledger
tests red and silencing its warning turns 1 red; reverting either half of the
guard-push change turns the force-push case red.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BKNFogaYfCQBvFVqFQnfRt
@coderabbitai

coderabbitaiBot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:48 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: deade03f-d8d6-4e7c-96ba-189aa4d083d5

📥 Commits

Reviewing files that changed from the base of the PR and between 8f5ffeb and f0da51a.

📒 Files selected for processing (9)
  • docs/branch-review-records/44c2668e0a184a3856e8f614a4358bb33d033ffc179a15ef4b69248c4df83669.record.md
  • docs/branch-review-records/7bd50bd9c4093cf764692231945db6e88a27e21ebb7545745fa66facf618d208.record.md
  • docs/branch-review-records/7ce860ee652bd3f4e91c56cab26f002c8fd5e5b18eee189d01baaaa18ffd432c.record.md
  • docs/branch-review-records/b2482dceb2ee840589af0dfbe5f66b5f406c49c01ccd942a2c3f16bca8ae8ade.record.md
  • docs/branch-review-records/fb53209f910afc80ba5fb207e1a2f36d30bc14b98d5c069c3e7fa8b7f89edee4.record.md
  • scripts/guard-push.mjs
  • scripts/ledger-inbox.mjs
  • tests/guard-push.test.ts
  • tests/ledger-inbox-cancellation.test.ts

Comment @coderabbitai help to get the list of available commands.

@supabase

supabaseBot commented Aug 15, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@BigSimmoChatGPT Codex Connector

Copy link
Copy Markdown
OwnerAuthor

Final Codex review snapshot

Head: 316d705fe5e03828a472ca7ccd4cbf2eefe8a8f5
Base: 6f7b7deefaf7e0cd062b748f18fc6ca8988093f6

  • Branch is current with main; merge tree is clean (0a4a7df5e394b81e398727cd80da5f00be3038fa).
  • Independent manual adversarial review found no additional PR-introduced defect. No review threads were actionable.
  • Decisive local checks: git diff --check, ledger inbox, outstanding-issues, branch-ledger/write-discipline, and CI-scope self-tests passed.
  • Exact-head required CI is green: CI, SAST, and Secret Scan. Advisory UI was skipped/not required.
  • No residual code blocker identified; provider-backed/live checks were not run locally.

The PR was not merged. Final merge is left to the user.

@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

Final review snapshot — PR #1978

  • Final head: 7944574804c028b74be605dee5f6f12c0daa6c79; latest base: 32afb0874f0f2596a7b03a3629238a0cd7f644d1.
  • Branch update: merged latest main normally (no rebase or history rewrite). GitHub reports MERGEABLE/CLEAN; local merge-tree is conflict-free and the final head contains the latest base.
  • Issues fixed: restored the executable mode that the PR accidentally removed from scripts/guard-push.mjs; preserved the cancellation invariant for already-applied cancellation targets instead of incorrectly recording cancel-of-cancel as an ineffective no-op; added the focused regression case. The original parallel-reconciliation and rewritten-history fixes remain intact.
  • Adversarial review: the CodeRabbit CLI was unavailable in this Windows environment, so no CodeRabbit result is claimed. A distinct manual adversarial pass reproduced both fixes and re-read the final diff. CodeRabbit/GitHub review threads: 0 total, 0 actionable.
  • Local verification: focused Vitest 39 passed / 1 Windows-only skip; guard:push:self-test, format, docs links/inventory/index, outstanding-issues guard, branch-review-ledger guard, ledger-write-discipline, lint, and typecheck passed. verify:pr-local was partial because its full unit stage reproduced 6 unrelated Windows/Cloud-hook failures; the same 6 failures reproduced on untouched base 32afb087…. Later fixture/medication checks were not reached and were not rerun because this PR does not touch those domains.
  • Exact-head CI: required PR required, Unit coverage, Static PR checks, Safety and config checks, Change scope, PR policy, PR mergeability, Semgrep, Gitleaks, and GitGuardian are green. Build, container, ingestion, product/UI, Lighthouse, migration, visual/advisory UI, Supabase Preview, and release-browser-matrix jobs were scope-skipped, not green.
  • Blockers: none. Residual risk: local Windows cannot provide a fully green repository-wide unit run for the documented baseline reasons; exact-head hosted unit coverage is green. Auto-merge is not armed and was not changed.

The PR was not merged. Final merge is left to the user.

@BigSimmo
BigSimmo enabled auto-merge (squash) August 15, 2026 12:54
@BigSimmo
BigSimmo merged commit 7e91545 into mainAug 15, 2026
24 checks passed
@BigSimmo
BigSimmo deleted the claude/db-remediation-phase-0-wfaiyl branch August 15, 2026 13:02
BigSimmo added a commit that referenced this pull request Aug 17, 2026
…) (#2044)
* docs(db): add coordination handover for multi-chat remediation oversight
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L7umYGe6hbCfq3NmpZ4Gz4
* docs(db): re-baseline the remediation coordination board to main (#316)
Carries the coordination handover onto the coordination-chat branch and corrects it
against the repository record as of main f5b0932: the tracking anchor is #316 (not
#312), Phase 0 is complete (#1938/#1939/#1951/#1978), Phase 1 is partial with 1.2 the
only executable next step, Phase 3 is blocked on ten UNCLASSIFIED RPCs, and Phase 4/5
have incident-scope partial evidence. The originating "never executed" verdict is
marked superseded by the Phase 1.1 fingerprint.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore(ledger): record review of the coordination board PR (#2044)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@BigSimmo@claude