fix(doctor): notice receipts destroyed after run.finished was audited

[ophiocus]

Closes #226 — credit to @Julian-Genuario for spotting the missing reverse question. Claimed in-thread with right of way offered; the courtesy window has passed, so here is the fix in the shape the claim promised.

The gap, closed

verifyStoredReceipts() proved every surviving receipt against the audit chain but never asked whether an audited run.finished digest still has a receipt behind it — so UPDATE runs SET receipt = null (or deleting the row) just shrank checked while ok stayed true, exactly as the issue demonstrated.

• The reverse pass now runs over the audited digests: any id with an audited digest and no stored receipt lands in a new, distinct missingReceiptRunIds class — kept separate from invalidRunIds and unauditedRunIds so "receipt destroyed" and "never audited" remain distinguishable stories, per the issue's point that in current code this state can only be an anomaly.
• The runIds scope is respected (asking about other runs stays clean), and the doctor's failure line now reports all three counts.

The regression bites

The test finishes a real run through /internal/runs/:id/result, confirms ok: true, then nulls the receipt — doctor goes red with missingReceiptRunIds: [run.id] — and finally deletes the run row entirely, which trips the same wire because the audit trail outlives it. The "absence must fail loudly" shape, same family as the truncation disclosure above it in the suite.

sandbox.test.ts: 40 tests, 0 failures; tsc clean.

[javiertoledo]

Reviewed this end to end and pushed nothing — I could not find anything that warranted a commit.

Verified on main rather than taken from the description. The query filters receipt IS NOT NULL and the loop walks only survivors, while auditedDigests is built but read solely through .get(row.id) — never traversed in reverse. So nulling a receipt shrinks checked and leaves ok true, exactly as #226 said.

I also went looking for the obvious way this fix could be wrong — a legitimate deletion path that would make missingReceiptRunIds fire as a false positive. There is none: nothing in services/api or packages/db deletes runs or auditEvents, and there is no retention or purge job. Your comment that this "can only be an anomaly" is precise, not optimistic.

Worth adding that this restores a written decision rather than inventing a rule. architecture.md records that "the receipt SHA-256 detects accidental or post-production changes" (decision 19). Destruction is a post-production change the check could not see, so the reverse pass closes the gap between the doc and the code.

The test earns its place: it drives a real run through /internal/runs/:id/result, confirms green first, then nulls the receipt and deletes the row, so it covers both shapes of the anomaly rather than restating the implementation.

Three things I looked at and deliberately did not act on:

The test leaves an audited-but-missing run in the shared org, and checkReceiptIntegrity queries org-wide with no scope. Nothing breaks today — no test asserts receipt_integrity, and test-critical.mjs runs api before sandbox with --fileParallelism=false. It is a landmine for whoever first writes a doctor-level test for this check, though, so it seemed worth naming even without a trigger.

The failure message reads N audited-but-missing receipts found across ${checked} runs, but checked counts only surviving receipts, so the missing ones are not in their own denominator. Null the only receipt in an org and an operator sees "1 audited-but-missing receipts found across 0 runs." Cosmetic, and I would rather not churn a string on your branch.

receipt-integrity.ts has no unit-test home, and sandbox.test.ts skips silently when Postgres is unreachable. I raised the equivalent on #262 and added the cases there — but that file already existed and covered the sibling provider, so it was three cheap cases. Here it would mean a new file plus drizzle fakes for a function whose whole job is two queries, which is more machinery than the gap justifies. test-critical.mjs also refuses to let a skip pass as green, so CI is not exposed.

The unbounded row loads are real but pre-existing and unchanged by this branch, so they are tracked separately in #295 rather than held against you here.

CI is green across all six checks, verify included. From my side this is good to merge as it stands.

[ophiocus]

Thank you — a review that verifies the bug on main, hunts for the false-positive path and names why there isn't one, and then ties the fix back to architecture.md decision 19 is a review that teaches. "Restores a written decision rather than inventing a rule" is exactly the framing I was reaching for and didn't find.

On your three deliberate non-actions:

• The denominator — you were right, and since the branch is still open I folded it in rather than leave a known-wrong string for an operator's worst morning: 4ac067e (on top of your main merge) divides by checked + missing, so it reads "found across N audited runs". One line, no other churn.
• The org-scope landmine for a future doctor-level test: agreed, and worth naming now. Whoever writes that test will want a scoped fixture org — I'll carry that thought into the test work I have open elsewhere if it lands on me.
• The unit-test-home cost/benefit — I read it the same way: two queries don't justify drizzle fakes, and test-critical.mjs refusing to count a skip as green is what actually protects CI here.

And thank you for routing the pre-existing unbounded loads to #295 instead of holding them against this branch — and for #262 this morning. From my side this stays good to merge.

[javiertoledo]

Closing this one — #289 just landed and it deleted the code this PR fixes.

It was a big restructure, 600 files. These are all gone from main now:

services/api/src/receipt-integrity.ts
services/api/src/doctor.ts
services/api/test/sandbox.test.ts

services/api went from 155 files down to 91, doctor.ts included. There's no rebase that makes this branch apply.

The bug was real, though — I checked it on main before the merge. Forward-only verification can't see deletion, so nulling a receipt kept ok true.

If you want to try it again on the new code, the closest equivalents look like stories/evidence.ts and turns/git-evidence.ts. auditEvents is still in the schema. One thing to check first: I couldn't find any integrity verification at all in the new tree, so it may have been dropped on purpose.

Up to you. Thanks for the original catch, and to @Julian-Genuario for spotting it in #226.