docs(issues): reconcile 17 queued inbox requests into the canonical ledger - #2560
docs(issues): reconcile 17 queued inbox requests into the canonical ledger#2560BigSimmo wants to merge 3 commits into
Conversation
…edger The immutable inbox had accumulated 17 unapplied requests across several sessions, six of them from the snapshot-conflict work in #2530. They are applied here in the single fresh-base reconciliation operation the ledger discipline requires, rather than by editing table rows directly. Composition: eight `done` closures, five `add` findings, two `update` amendments, one `cancel` decision, and one request whose closure another session had already queued independently. Open rows go 70 -> 67. Each request moves to `docs/outstanding-issues-inbox/applied/` as its own immutable audit record, so the canonical diff can be checked against the recorded transaction — which is what `check:ledger-write-discipline` compares, and why the requests are moved rather than deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 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 |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a221ac18-040a-44bf-9ae4-40359d9ad628) |
Blocked: this duplicates #2559, and the concurrency guard is doing its job
This is a real failure, not an environment artefact — and it is not fixable from inside this PR. What happened#2559 was opened at 09:55:44 and this PR at 09:56:38, 54 seconds apart, by two sessions that could not see each other. Both branched from The red check is a second-order consequence. Why re-reconciling here does not fix itThe obvious repair is to re-run the reconciler over the merged base. It refuses, correctly:
No re-run has been spent: a re-run cannot change this verdict, because the failure is a property of the tree rather than of the runner. RecommendationClose this PR in favour of #2559. It was opened first, it is not blocked ( Not closing it unilaterally: auto-merge was armed on this PR by its owner, and reversing that is the owner's call. This PR cannot merge while the check above is red, so there is no race in the meantime. Left for whichever PR landsThe sixteen requests that arrived with Generated by Claude Code |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e7908c12b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | #707F09 <!-- issue-ulid:01M1GC7018707F0970T8GT50SM --> | P3 | issue | The developer hub's pending-requests panel reads empty during local dev, because no lifecycle regenerates the snapshot for the dev server | Raised by Codex review on PR #2530 (scripts/generate-outstanding-issues-snapshot.mjs) and verified rather than accepted on trust. THE FINDING IS CORRECT: npm run dev and npm run ensure invoke no prebuild, so ledger-snapshot.ts imports the committed JSON and the ledger page's 'requests not yet applied' count and list render empty while requests exist in docs/outstanding-issues-inbox/. That is the #338 class of failure the developer hub exists to prevent - a panel silently under-reporting. IT IS NOT A REGRESSION FROM PR #2530, and this matters for how it gets fixed: measured on main at 45a3dcacb before that branch existed, the committed snapshot already carried counts.pending 0 while 8 inbox requests were tracked in the same commit. The panel was already blank in dev. PR #2530 changed it from accidentally blank to blank by design, and added a gate that keeps it that way, so the fix now has to be deliberate. THE OBVIOUS FIX IS WRONG: adding a predev that generates the pending-inclusive form would write a populated pending into the tracked working tree on every dev-server start, which is precisely the churn #Y090R5 records and PR #2530 removed - and check:outstanding-issues-snapshot now fails on a committed non-empty pending, so a developer would be left with a permanently dirty tracked file that the gate refuses. Two shapes are worth weighing instead. (a) A development-only reader: the ledger page merges docs/outstanding-issues-inbox/*.json from disk when NODE_ENV is not production, leaving the committed artefact untouched. Cheap, but it adds an environment-conditional code path to a module whose docstring records that reading docs/ live was deliberately rejected because the production image never copies docs/ - the divergence would be benign here (production gets the true list from prebuild) but the reasoning must be written down rather than quietly reversed. (b) Generate the pending-inclusive form to a SEPARATE gitignored sidecar that the page reads when present, so nothing tracked is ever dirtied; costs a second artefact and a tolerant reader. Neither is urgent: the panel is administrator-gated, 404s for non-admins in production, and is correct in the deployed image because prebuild regenerates it there. | Codex review on PR #2530, 2026-09-02 | 2026-09-02 | | ||
| | #5ECZQA <!-- issue-ulid:01M1G5EHVK5ECZQA6630XACRSP --> | P3 | issue | Batch image signed-url route swallows per-item createSignedUrls errors and still returns 200 | src/app/api/images/signed-urls/route.ts:105 checks only the top-level signed.error returned by createSignedUrls. supabase-js returns a per-path result array of { error, path, signedUrl }, so a single path that fails to sign yields an entry with no usable signedUrl while the top-level error stays null. The loop at :112-122 then skips that image because of the if (signedUrl) guard, and the route returns HTTP 200 with the image silently absent from the urls map. The client (src/lib/batch-signed-urls.ts) treats a missing key as "not returned" rather than "failed", so a figure disappears from the document view with nothing logged and no error surfaced anywhere. This is a milder instance of exactly the silent-failure class that #Z61JRT was about, and it survived that fix because the fix replaced getPublicUrl with createSignedUrls without adding per-item error handling. It is not a privacy or tenancy defect: the owner-scope gate at :70-88 has already run, so only images the caller is entitled to reach ever get to the signing step. The impact is diagnosability and a confusing partial render, not exposure. FIX: inspect each per-path result, and either surface a per-image error field in the response so the client can distinguish "failed" from "not found", or log the failing paths through the existing observability layer so a systematic storage problem is visible rather than presenting as scattered missing figures. Prefer the first: the response shape is already a per-id record, so an error discriminant fits without a breaking change. Note the singular route src/app/api/images/[id]/signed-url/route.ts:71 has the mirror-image gap, dereferencing signed.data.signedUrl without a null guard where the batch route has an explicit !signed.data check. Worth aligning both in the same change. | Found while verifying #Z61JRT on main 45a3dca, 2026-09-02; supabase-schema-guardian review of src/app/api/images/signed-urls/route.ts | 2026-09-02 | | ||
| | #9ZGNW7 <!-- issue-ulid:01M1G8RXVY9ZGNW7CR1YQ4SE5J --> | P3 | issue | Developer-hub CODE still flips perf_changed, so an admin-only mockup route pulls a full Lighthouse run | Found while fixing #EFETZT on 2026-09-02 and deliberately NOT fixed in that PR, because it means editing a fail-closed CI classification surface. PR #2530 added data/repo-awareness-snapshot.json to perfExclusionPatterns in scripts/ci-change-scope.mjs, mirroring the carve-out data/outstanding-issues-snapshot.json already had for the same reason (PR #2302). That closes the common case - a handoff PR that only regenerates the snapshot no longer pays a ~7-minute Lighthouse budget run against a budget the change cannot move. It does NOT close the code case. src/components/developer-area/hub/** and src/lib/developer-area/** match the generic 'src' entry in perfPatterns (ci-change-scope.mjs:226) and are not excluded, because only the ROUTE WRAPPER lives under the excluded src/app/mockups prefix - the panel components live one directory hop away under src/components. So a PR touching the developer hub's own code still triggers lighthouse-budget for /mockups/development/**, which 404s for non-admins in production (src/app/mockups/layout.tsx and src/proxy.ts gate it behind DEVELOPER_AREA_HEADER) and cannot appear in either budgeted journey. WHY IT WAS LEFT: the exclusion list is a fail-closed safety surface, and widening it by directory prefix risks exempting a future component that IS reachable from a budgeted route. The safe shape is probably an explicit list of the developer-hub component and lib paths rather than a prefix, pinned by an assertScope self-test beside the two that already exist (ci-change-scope.mjs:1022), plus a test proving a non-hub file under src/components still flips perf_changed. Cost of leaving it is bounded and only paid by developer-hub PRs, which are rare. | session 2026-09-02, PR #2530; verification-router review | 2026-09-02 | | ||
| | #1M0J6D <!-- issue-ulid:01M1G8RF0Q1M0J6DDX9C5G79V9 --> | P2 | issue | Web-container sessions start on a shallow clone, so two committed specs fail with messages that read as content errors | Measured 2026-09-02 in a Claude Code web container on origin/main 45a3dcacb. The session's checkout is a SHALLOW CLONE (depth 102). Two specs in the committed suite then fail, stably across two full runs, and neither message says anything about history depth: tests/clinical-hazard-controls.test.ts fails with 'CLINICAL-TRUTH-AUTHORITY: reviewedCommit does not exist 883f1007a85cd4e02198f39c12c4a4e467d4b89e' plus 'path is absent from reviewedCommit', and tests/rag-plan-package-parity.test.ts fails with 'manifest reconciledBase is unavailable locally: f3d1a3cce2c943ad3083425ed9c7c46dbef23087' from scripts/build-rag-plan-packages.mjs --check --require-origin-main. Both read as governance-manifest corruption, which is exactly the wrong conclusion: the commits are real and simply absent from a truncated history. PROOF: git fetch --deepen=2000 takes the clone from 102 to 5462 commits, both commits then resolve under git cat-file -e, and the two specs pass 27/27 with no code change. AGENTS.md already prescribes this remedy under 'Deleting code you believe is dead' for check:dead-code-candidate ('assessed on a shallow clone, where nothing can be dated - run git fetch --deepen=2000 first'), so the fix is known; it is simply not applied at session start and these two specs are not covered by that note. COST: a full npm run test is about six minutes, so a session can burn two of them before recognising the pattern, and the plausible-looking failure text invites someone to 'fix' a hazard manifest that is not broken. OPTIONS, cheapest first: have .claude/hooks/session-start.sh deepen the clone when git rev-parse --is-shallow-repository reports true (it already provisions Node and runs npm ci, so it is the natural place and the cost is one fetch); or make the two checks detect a shallow repository and fail with that as the stated reason rather than naming the commit; or document it in docs/testing.md. The first is preferred because it fixes the dead-code gate's known problem at the same time. | session 2026-09-02, PR #2530 verification | 2026-09-02 | |
There was a problem hiding this comment.
When checked against its actual parent 969cc7f, this commit moves only 17 requests while leaving 16 parent inbox requests pending, so node scripts/check-ledger-write-discipline.mjs --base 969cc7f... --head 0567793... fails with “reconciliation moved only part of the base inbox.” This row also duplicates the shallow-clone defect already queued by 4478f605-1e05-4a8e-addd-32cf70e5d155.json, which a later reconciliation would add again because add requests do not conflict. Re-run the reconciliation from the current base and cancel any superseded duplicate requests.
AGENTS.md reference: AGENTS.md:L325-L326
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Both halves verified, and the second half is a finding I had missed. Thank you — it survives this PR being closed, so it is recorded here rather than waved off.
The partial-batch half is confirmed and already established. check:ledger-write-discipline fails identically in CI and locally, naming the same 16 requests. Cause is in the PR comment above: Update branch merged 969cc7f8 in, and that merge carried sixteen newly queued requests, so a reconciliation that was complete when made is now partial against its own merged base.
The duplicate half is correct, and I had not spotted it. Checked rather than accepted:
4478f605-…json— pending onmain,action: add, P3, "Two tests fail on a shallow clone as ordinary assertion failures rather than refusing…"b11bdfdd-…json— applied by this PR,action: add, P2, "Web-container sessions start on a shallow clone, so two committed specs fail with messages that read as content errors"
Same defect, same two specs (clinical-hazard-controls, rag-plan-package-parity), same two commit shas, filed at different priorities by two sessions that each hit it independently. Your mechanism is right: add requests carry no baseRowFingerprint to conflict on, so nothing stops the second one landing as a separate row.
Where the fix belongs is not this PR. The remedy you name — re-reconcile from the current base and cancel the superseded duplicate — is exactly right, and the reconciler refuses to perform it here:
refusing to reconcile: detected unmerged branch(es) on origin carrying pending inbox reconciliations:
- claude/issues-reconcile-2526 (b5a059f5)
Concurrent reconciliations cause canonical-ledger conflicts or corrupted reconciliation journals (#EH9VA6).
This PR duplicates #2559, opened 54 seconds earlier from the same base over the same seventeen requests. The recommendation is to close this one, so re-reconciling here would mean forcing past #EH9VA6's guard to repair a PR that should not land.
One correction to the scope of your finding, in the direction of it mattering more. It is not specific to this PR. #2559 applies its own shallow-clone add (#1M0J6D), and 4478f605 stays pending either way — so the duplicate row appears on the next reconciliation whichever of the two lands. Closing this PR does not close the finding.
So the concrete action, carried to whichever reconciliation runs next: queue an immutable cancel for 4478f605 before reconciling, per the "use an immutable cancel request for each rejected mutation" clause the gate itself cites. Leaving this thread open, because that work is still outstanding.
Generated by Claude Code
There was a problem hiding this comment.
Closing this PR now as the duplicate of #2559 it is — same 17 requests, same base, opened 54 seconds apart. #2559 has been synced to current main and merged; this one is being closed unmerged so the two reconciliations never land side by side (which is exactly what check:ledger-write-discipline's concurrent-reconciliation guard exists to prevent). The 4478f605 duplicate-row follow-up you raised is still real and still open — it isn't specific to this PR, so it'll be handled as its own immutable cancel request in a later reconciliation rather than lost with this PR's closure.
_Generated by Claude Code
Generated by Claude Code
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #15081 (success). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7bf3667f-7d0a-4b23-ba81-016311bec3a0) |
Pull request was closed
…comes a second row Two sessions independently hit the same defect and each filed an `add` request for it, and both are still pending: - `b11bdfdd` (P2) "Web-container sessions start on a shallow clone, so two committed specs fail with messages that read as content errors" - `4478f605` (P3) "Two tests fail on a shallow clone as ordinary assertion failures rather than refusing…" They are the same defect, not merely adjacent: both name `tests/clinical-hazard-controls.test.ts` and `tests/rag-plan-package-parity.test.ts`, and both name commits `883f1007` and `f3d1a3cc` as the truncated-history cause. `add` requests carry no `baseRowFingerprint`, so nothing makes them conflict. Left alone, the next reconciliation files two ledger rows for one defect — and there is no row yet, so neither would look like a duplicate of anything already recorded. This cancels the P3 and keeps the P2, which carries the fuller reproduction and the more accurate severity. Queued as an immutable cancel request rather than by deleting the P3, per the clause `check:ledger-write-discipline` cites when it rejects a partial batch: "use an immutable cancel request for each rejected mutation". The request is additive and reconciles nothing, so it does not trip the concurrent-reconciliation guard (`#EH9VA6`) while `claude/issues-reconcile-2526` is still unmerged. Raised by Codex review on PR #2530's successor #2560 and verified there. That PR was closed as a duplicate of #2559; the finding was not specific to it, so it is carried here rather than lost with the closure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
2 participants
Summary
docs/outstanding-issues-inbox/, six of them queued by the snapshot-conflict work in fix(snapshots): stop the two generated snapshots conflicting on every PR #2530, the rest by earlier sessions. This is the single fresh-basenpm run issues:reconcileoperation the ledger discipline requires — the canonicaldocs/outstanding-issues.mdis never edited row-by-row, because a direct table edit is exactly whatcheck:ledger-write-disciplinerejects.doneclosures, fiveaddfindings, twoupdateamendments, and onecanceldecision. Open rows go 70 → 67. The reconciler reportedReconciling 17 request(s) … with 1 cancellation decision(s)and thenApplied 17 request(s).docs/outstanding-issues-inbox/applied/rather than being deleted, keeping its immutable audit record. That is what makes the canonical diff checkable against the recorded transaction, which is the comparisoncheck:ledger-write-disciplineperforms — and why the diff shows 17 renames rather than 17 deletions.data/outstanding-issues-snapshot.jsonis regenerated alongside the ledger, as the reconciler instructs. It lands withpending: 0, which is the#Y090R5fix from fix(snapshots): stop the two generated snapshots conflicting on every PR #2530 working as intended on its first real use: the committed artefact carries the reconciled state and none of any other branch's queued requests.No behaviour, code or configuration changes — this PR is entirely repository bookkeeping.
Verification
npm run verify:pr-local— the routed handoff gate for this scope: 19 gates completed,failed: (none),not reached: (none), includinglint,typecheck, the full offline unit suite, a productionbuild,check:outstanding-issues,check:repo-awareness-snapshotandcheck:ledger-write-disciplinenpm run check:ledger-write-discipline—Ledger write discipline passed for f2c8d6c9d1a7..HEAD.This is the decisive gate for this change: it compares two committed refs, so it was re-run after committing, an uncommitted ledger edit being invisible to it.npm run check:outstanding-issues—[snapshot] in step with data/outstanding-issues-snapshot.json (67 open, 0 pending)npm run check:repo-awareness-snapshot—in step with data/repo-awareness-snapshot.json (204 pages, 577 documents, 2663 reviews)node scripts/ledger-inbox.mjs reconcile --dry-runbefore applying —Would reconcile 17 request(s) into docs/outstanding-issues.md with 1 cancellation decision(s), matching the real run exactlynpm run format(whole tree), committedeval:*,verify:release) — nothing here reaches a provider.eval:retrieval:qualityis not applicable: no retrieval, ranking, selection, chunking or scoring behaviour changed.Risk and rollout
npm run snapshot:issuesrestores whatever the reverted ledger produces, and every applied request keeps its immutable record underdocs/outstanding-issues-inbox/applied/so the transaction stays auditable either way.Notes
origin/main(f2c8d6c9d) rather than continuing the merged fix(snapshots): stop the two generated snapshots conflicting on every PR #2530 branch, per the rule that a merged pull request cannot track follow-up work.canceldecision is the reconciler correctly refusing a second pending mutation on an issue another session had already queued a closure for — described in fix(snapshots): stop the two generated snapshots conflicting on every PR #2530 and resolved here rather than left in the inbox.🤖 Generated with Claude Code
https://claude.ai/code/session_014Mn8yfo1bQzk4kDEiG1TEn
Generated by Claude Code
Note
Low Risk
Documentation and generated ledger metadata only; no runtime, security, or data-path changes.
Overview
Applies 17 queued
docs/outstanding-issues-inbox/requests via the canonical reconcile flow, updatingdocs/outstanding-issues.mdand regeneratingdata/outstanding-issues-snapshot.json(67 open, 0 pending). No application code, schema, or config changes.Closes eight tracked items in the resolved archive, including ledger snapshot merge/conflict and clinical-risk misclassification (#Y090R5), repo-awareness snapshot staleness/conflicts (#EFETZT), git-less behaviour for check:repo-awareness-snapshot (#JFRCZ4), batch image getPublicUrl on a private bucket (#Z61JRT), isPublicDocument metadata ordering (#N8B176), docs:check-links coverage (#ZM8902), stale D4 forensics text (#CTA8CR), and Ward Flow roadmap assignments (#875H6T).
Adds five open rows (mostly P2/P3): empty developer-hub pending panel in local dev (#707F09), per-item errors swallowed on batch image signed URLs (#5ECZQA), developer-hub code still triggering Lighthouse (#9ZGNW7), shallow-clone false failures in hazard/RAG parity tests (#1M0J6D), and live verification needed for corpus health / hub document count (#6APN03).
Amends two open rows: #ZBAC9D with a 2026-09-02 live re-measurement and corpus access-mode context; #4STSM1 updated to Phases 1–2 merged and Phase 3 demonstrable plan drafted in PR #2520.
Reviewed by Cursor Bugbot for commit 59326b5. Configure here.