fix: audit P2/P3 hardening (M13 guard, upload rate limit, P0 carry-over) - #296
Merged
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
BigSimmo
marked this pull request as ready for review
July 5, 2026 18:00
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Inflight answers repopulate stale cache
- setCachedAnswer now skips writes when the indexing version captured at retrieval start no longer matches, preventing stale inflight answers from repopulating the cache after invalidation.
- ✅ Fixed: Synopsis omitted from numeric exemption
- hasNumericOrTableEvidence now uses clinicalResultEvidenceHaystack so synopsis-only numeric dosing qualifies for numericEvidenceExempt penalties.
- ✅ Fixed: Late delete abort wipes cache
- The late active-job TOCTOU check now runs before deleteDocumentIndexTraceRows so a 409 abort no longer leaves the owner-wide shared cache cleared while the document remains.
- ✅ Fixed: Index unit delete partial wipe
- document_index_units are deleted first so a failed delete throws before memory cards and sections are removed, avoiding inconsistent partial state.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 1864748. Configure here.
…elete ordering - Skip setCachedAnswer when indexing version changed during inflight retrieval - Include retrieval_synopsis in hasNumericOrTableEvidence haystack - Run late active-job check before destructive trace/cache cleanup on delete - Delete document_index_units before memory cards/sections to avoid partial wipe
BigSimmo
enabled auto-merge
July 5, 2026 18:36
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
BigSimmo
added a commit
that referenced
this pull request
Aug 9, 2026
…tes, and pin the factsheet heading census (#1779) * test(factsheets): census every h1 in the factsheet detail document The hero <h1> and the portaled print sheet's <h1> are correct and mutually exclusive by construction: on screen `.factsheet-print-sheet { display: none }` removes the print subtree, and in print `html.factsheets-printing body > *:not(.factsheet-print-portal)` removes the shell that owns the hero. Neither state exposes two headings to the accessibility tree, and the printed PDF is a separate document whose section headings are already <h2>, so demoting its title would leave it with no top-level heading. The real gap was that the existing assertion was scoped to the page testid, so the document-level invariant was asserted nowhere and a stray third <h1> would not have been caught. Pin the census instead: exactly two, one per container, both carrying the title, plus a non-empty <h2> outline in the print sheet. jsdom applies no stylesheet, so a census is the right guard rather than a visibility assertion. FactsheetPrintSheet stays in factsheet-detail-page.tsx — design-system-contract-utils.mjs scopes its raw-colour exemption to the literal factsheet-print-sheet marker and fails closed if it moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * docs(testing): record the verified recipe for restoring local browser gates The remote/Cloud drift note said to delegate browser proof to CI and left the impression that local gates were unrecoverable. They are recoverable; the blocker was two separate image faults, and the second is why the obvious fix looks impossible. The baked node_modules is stale or incomplete — containers have shipped none at all, and earlier ones reported playwright 1.62.0 against a locked 1.62.1 with tailwind-merge absent entirely, which is an incomplete install rather than a version skew, so the lockfile pin was never wrong. And npm ci cannot repair it because jsdom@30.0.1 requires node ^22.22.2 || ^24.15.0 || >=26.0.0 while images have shipped v24.13.0, so the install dies on EBADENGINE under engine-strict. Installing Node 24.19.0 clears that, npm ci then exits 0 and parity reports all seven pinned packages, and `npx playwright install` supplies Chromium 1234 (images ship only 1194). Verified end to end this session, launch included. Keeps the existing Stop intact and makes it cheap to honour: install the matching revision rather than forcing a run against 1194. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * fix(bundle-budget): split production weight from mockup scratch One number could not honestly answer two questions. totalGzipBytes summed every built client chunk, including src/app/mockups/** design scratch that 404s in production, against a ceiling named as though it were production weight. #13 held that mockup chunks are not a production bundle; this gate charged them anyway, which is how PR #1580 blocked at +10.1% for chunks no user can load. #252 recorded the contradiction and left the metric undecided. Measured on a clean build of main at af85cbc, the blur had become the whole signal: 1546.5 KiB total was +9.96% of the 1406.4 KiB baseline — 576 bytes from failing Build — while production-only was 1279.1 KiB, 9.06% BELOW that same baseline. Every byte of the apparent regression was design scratch (267.5 KiB across 76 chunks over 66 mockup routes) and production had actually shrunk. latency-audit-2026-07-28 corroborates: 1,309,274 bytes then against 1,309,772 production-only now, flat to +0.04%, so the 2026-08-04 bump to 1,440,201 had absorbed mockup growth as production growth. Raising the ceiling again would have hidden that permanently, so this splits rather than ratchets. production (10%) covers every chunk a non-mockup route reaches plus chunks no manifest claims — framework, polyfills, runtime. mockups (25%) covers chunks reachable only from /mockups/**, as a runaway detector rather than a per-mockup gate; a ceiling tight enough to fire on the next mockup would just be --update'd reflexively. A chunk shared by both counts as production because it would be built either way. Attribution reads the per-route *_client-reference-manifest.js files under .next/server/app, since Next 16 webpack emits no app-build-manifest.json, and fails closed when that tree is missing or resolves no routes so the buckets can never silently collapse. Both fail paths proven against the real build. Also captures #296: pr-handoff-stop.test.ts fails in any root container because it chmods a fixture dir to force a write failure and root ignores permission bits — pre-existing, reproduced on clean af85cbc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YEFowCVUVrybKvKvReQ924 * fix(bundle-budget): fail closed on bad manifests and zero baselines Review feedback on PR #1779: treat unparseable route manifests as fatal attribution errors instead of counting them as resolved empty routes, and handle a zero mockup/production baseline without NaN percentage math. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo
added a commit
that referenced
this pull request
Aug 9, 2026
Four rows, all measured against origin/main 199b303 in this session: - #295 (P2 task) the ErrorState enforcement check. GATES.md:106 still reads "planned" and grep over scripts/ and eslint-rules/ returns zero references to ErrorState, so the component exists but nothing requires it. Deliberately not flipped to implemented. - #296 (P3 task) adopting ErrorState at the three surfaces that genuinely hand-roll the guard. Live-look change, downstream of the redesign. - #297 (P3 issue) the safe-area exemption in the new raw-value ratchet, which is by form rather than by absence of a literal. - #298 (P2 issue) the three sites carried into M4 as guards that are not guards, so the next reader does not convert them. Written with npm run issues:add, never by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo
pushed a commit
that referenced
this pull request
Aug 10, 2026
…a duplicate of #296 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XDuNkNHU4zFn1cDquUXv5
15 tasks
BigSimmo
pushed a commit
that referenced
this pull request
Aug 12, 2026
…system rows Archived as verified-delivered or duplicate: #303, #284 (dup of #296), #149, #159, #286, #287, #288, #256, #261, #291. Re-scoped with re-measured evidence: #210 (gate was never missing; the defect is the .next/dev types include), #262 (aliases 228 -> 193), #266 (counts moved, 23 unadopted holds), #275 (premise 'only one implementation' is false — the values leaked to five files). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
BigSimmo
pushed a commit
that referenced
this pull request
Aug 12, 2026
docs/outstanding-issues.md has no merge driver by design, so this conflicted in seven hunks. Resolved by taking main's file as the base — it carries main's own archives (#207, #226, #230, #261, #284, #296) — then re-applying all 23 of this branch's changes through scripts/outstanding-issues.mjs: 15 archives and 8 detail rewrites. Verified afterwards that every one landed and that the guard reports no ids deleted from base. Two rows are deliberately NOT resurrected. This branch's #302 (style-contract line-break artifacts) and #303 (issues:next-id sync) no longer exist anywhere on main under any id — main has since reused both numbers for entirely different live rows. Both of this branch's rows were verified-delivered trivia, so nothing actionable is lost, but the disappearance is itself evidence for #156/#168: a main-side merge dropped rows, which is the hazard those rows describe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
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
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.

Summary
Continues the repo audit remediation after P0/P1 work, on top of latest
main.P0 carry-over (commit 1): RAG cache owner/indexing-version guards, synopsis parity in ranking/detectors, DELETE TOCTOU re-check, worker cache invalidation on job completion, and
RAG_QUERY_HASH_SECRETrequired in production-like readiness checks.P2 (commit 2):
search_schema_health()now reportscommit_document_index_generation.preserve_legacy_artifacts_migrationwhen the live DB is still running the pre-20260702 commit body. New migration20260706010000_search_schema_health_m13_guard.sqlandnpm run check:m13-migrationhelper./api/uploadconsumes thedocument_uploadrate-limit bucket (12/min owner, 3/min anonymous).P3 disposition (documented in
docs/process-hardening.md):image_countcomment inworker/main.tsreadFilealready documented at ingestion siteaudit_logspolicy — do not edit applied migrationsunsafe-inline— accepted risk, deferredVerification
tests/rag-cache-utils.test.ts,tests/supabase-schema.test.ts(60/60 in targeted run; schema M13 guard assertions included)npm run verify:cheap— blocked in the audit worktree by a slow/incompletenpm ci(next/serverresolution); CI should run the full gate on the PRnpm run eval:retrieval:quality— required before merge because P0 touches RAG cache/selection paths; needs live Supabase + OpenAI keysnpm run check:m13-migration— run after applying20260702000000_commit_generation_preserve_legacy_artifacts.sqland20260706010000_search_schema_health_m13_guard.sqlto live SupabaseClinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Live migration follow-up
Apply via normal Supabase migration workflow (if not already live):
20260702000000_commit_generation_preserve_legacy_artifacts.sql20260706010000_search_schema_health_m13_guard.sqlThen:
npm run check:m13-migration,npm run reindex:health,npm run check:indexing