audit fixes (P8b): collect the orphaned browser tests and make the local gates match CI by BigSimmo · Pull Request #2621 · BigSimmo/Database

BigSimmo · 2026-09-04T14:13:41Z

Summary

Audit remediation package P8b — Local gates and orphaned tests, from docs/audit/full-repository-audit-2026-09-02.md (PR #2573). Findings fixed: M24, M30, M33, L22, L47, M31, M32.

The theme is tests and checks that existed but were never actually running.

M32 — ui-tools-show-all.spec.ts was written and committed but matched no Playwright project, so it never ran anywhere. It is now collected and assigned to a shard. M31 — the 320px AccessibleTable journey was restored, and its fixture route confirmed to still exist on main after the mockup retirement in #2543.

M24 — verify:pr-local did not know about several checks that CI's static-pr job runs, so the local gate could pass while CI failed. A staticHeavyGuards list now carries them, and the assertPlan self-test spreads that same list, so the two cannot drift apart. M30 adds check:medication-interactions to static-pr under the same condition as its siblings, with a unit test comparing the interaction index to the snapshot drug by drug.

M33 — check:clinical-hazard-controls now runs in CI with a full-history fetch, and prints a clear message on a shallow clone instead of failing. Proved on a real --depth 1 clone: the new script prints CLINICAL_HAZARD_CONTROLS_SHALLOW_CLONE and exits 0, where main's script emits 37 errors.

L22 — a metacharacter-escaping regex escaped nothing. The old character class let \bfoo.bar\b match fooXbar and threw outright on (; the new one fixes both, with a case that fails against the old pattern.

L47 — the widening was correctly NOT done. The package permitted extending the owner-scope check to src/lib/** files importing createAdminClient only if a complete allowlist of deliberate public-corpus reads kept check:owner-scope green. It could not be assembled with confidence, so phase 1 still covers src/app/api only. Reporting a conditional as unmet rather than forcing it is the correct outcome here; the 26 candidate sites remain for an owner decision.

The merge with main needed care, and one thing the clean part broke

Four of this package's conflicts with main were lists of which browser tests run in CI — where both sides had added entries. The resolution is the union, never one side, because dropping an entry silently stops a test running. That was proved entry by entry across all 55 spec basenames: productionSpecPattern 66 ours + 68 theirs → 68 merged; mockupSpecPattern 16 + 18 → 19; testMatch 98 + 106 → 108; the shard pattern 33 + 34 → 34. Every entry from either side survives, and none was invented.

The cleanly auto-merged part had also broken two couplings that produced no conflict at all: a duplicated shard row for ui-tools-show-all, and check:design-drift-ratchet — a new static-heavy step that landed on main — missing from staticHeavyGuards. Both repaired in 365bef87e.

RAG impact: none — test collection, gate wiring and a regex fix; no retrieval, ranking, ordering or selection surface touched.

Verification

npm run verify:pr-local — 46 steps completed, - failed: (none), - not reached: (none) · Test Files 1128 passed | 1 skipped (1129) · Tests 14973 passed | 2 expected fail | 3 skipped (14978)
npm run check:gate-manifest — Gate-manifest OK: all 41 verify:cheap gates are enforced in CI (static-pr + mapped jobs), and the 38 static gates are documented consistently. The counts derive from verify:cheap:internal, which this package does not change, so no count bump is owed here.
npm run check:verification-plan — PR-local verification plan self-test passed.
npm run check:ci-scope — CI change scope self-test passed. · Mockup spec parity: 19 advisory specs all match mockupPatterns.
npm run check:playwright-pr-shards — PR UI shard parity OK: 36 production specs across 3 groups.
npm run check:browser-test-plan — browser-test-plan self-test passed.
npm run check:owner-scope — ✓ owner-scope phase 1: 60 src/app/api files clean against 26 owner-scoped tables.
npm run check:clinical-hazard-controls — CLINICAL_HAZARD_CONTROLS_PASS hazards=6 decisions=2
npm run check:diff-integrity — [diff-integrity] PASS — 5 changed test file(s), 68 -> 80 test case(s), against base 51ddfcd83. No test deleted, skipped or weakened.

Verification not run: npm run verify:ui — browser proof left to CI; the pinned Chromium is not installed in this container. The two newly collected specs will run there for the first time.
Verification not run: npm run verify:release — no release or handoff confidence is claimed.
Verification not run: provider-backed gates — nothing here reaches OpenAI, Supabase, Railway or Sentry at run time.

Note: origin/main was merged in (not rebased) after the gate ran, to pick up #2610 and #2612; that merge was clean with no conflicts. This branch touches no path under supabase/, so it creates no preview branch.

Risk and rollout

Risk: medium, and front-loaded. Two browser specs that have never run in CI will run for the first time on this pull request; if either was written against stale expectations it will fail here rather than silently continue to not exist. That is the intended outcome — please read a failure in those two as information, not as this pull request breaking something.
Rollback: revert this pull request's commits; each is one finding and reverts independently.
Provider or production effects: None.
RAG impact: none.

Notes

M33 deviation worth an owner decision. The control-reference assertion is per-hazard ("at least one listed test references the control"), not per-test. The strict per-test form goes red on this tree — tests/ward-output.test.ts references formatAnswerRenderCopyText zero times, and tests/rag-trust.test.ts references assessAndEnforceClaimSupport zero times — and the fix for that lives in files this package does not own. Tightening it is a follow-up.
Owner follow-ups recorded: schedule the hazard-register re-review before 2026-11-23; decide on the per-test tightening above; and the mockups/README.md row that another package owns.
One owner per file across the remediation programme: no other open remediation pull request edits these files.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DHSyfuC6mS98ystWFiitAR

Generated by Claude Code

Note

Medium Risk
Tightens API owner-scope static analysis and adds clinical/medication safety gates; two Playwright specs will run in CI for the first time and may surface stale expectations.

Overview
Closes audit gaps where checks and browser journeys existed on disk but never ran in CI or in verify:pr-local.

Local vs CI parity (M24) — verify-pr-local.mjs now runs a staticHeavyGuards list that matches static-pr’s static-heavy steps (migration role, function grants, owner-scope, design drift, etc.) before lint/typecheck/test; ci-cache-safety.test.ts asserts every gated CI step is in the heavy plan. Docs-scope scripts (including check:clinical-hazard-controls and check:mockups) also run when static_heavy_changed, matching CI.

CI gates — static-pr adds check:medication-interactions (M30) and check:clinical-hazard-controls on docs or static-heavy changes (M33, full-history checkout).

Orphaned Playwright (M31, M32) — Restores the 320px AccessibleTable journey in ui-accessible-table-mockup.spec.ts, wires it through mockupSpecPattern / testMatch, and pins routing in playwright-pr-shards.test.ts. Adds two-way parity so every .spec.ts is collected by testMatch and a project matcher (catching ui-tools-show-all).

Validator hardening — check-clinical-hazard-controls: fixed control-symbol regex escaping (L22), requires listed tests to reference controls (M33), warns on shallow clones. check-owner-scope-api: owner scoping uses filter-shaped regexes, not bare owner_id substrings (L47). New unit tests for interaction index vs snapshot freshness and hazard-control rules.

Docs — test-deletion-guard.md documents merge-commit blind spots and name-pinned journeys.

^{Reviewed by Cursor Bugbot for commit 7268c4f. Configure here.}

…y plan (M24) Defect: the heavy-scope plan of scripts/verify-pr-local.mjs selected only lint, typecheck and test, while CI's static-pr job additionally runs the migration-role, function-grant and owner-scope guards plus the design-system, generated-artifact, docs and ledger gates for the same scope. A migration using a non-postgres role, a SECURITY DEFINER function left open to PUBLIC, or an API handler reading an owner-scoped table without an owner filter passed the documented "PR mirror" green locally and reddened only after push. Trigger: `npm run verify:pr-local` on any PR touching supabase/migrations/** or src/app/api/** (both classify static_heavy). Fix: a `staticHeavyGuards` list in CI step order, selected for static_heavy scope ahead of lint/typecheck/test; the docs/ledger steps CI runs for heavy scope now ride docsScripts for docs_changed || static_heavy_changed, and check:mockups joins that list to match static-pr. The assertPlan self-test (check:verification-plan) is updated in the same change and pins the three tenancy/database guards by name. Test: tests/ci-cache-safety.test.ts "mirrors every static-heavy static-pr step in the verify:pr-local heavy plan (M24)" parses the static-pr job and fails on any static_heavy-gated `npm run` step missing from the local heavy plan (red on the previous script with 25 missing gates), and checks the guards stay out of the docs-only plan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

Defect: `npm run check:medication-interactions` rebuilt the index from the snapshot and diffed it, but ran only inside verify:pr-local — no CI job invoked it — and the only unit test compared data/medication-interaction- index.json against its own sourceRowCount, never against data/medications-snapshot.json. A snapshot or lexicon edit merged without re-running `npm run medications:interactions` (the bare-PR route permits exactly that) shipped a stale index: a renamed drug missing from names/bySlug, an added row that never alerts, or a severity token rendered with the old tone on the reverse path, with every required check green. Trigger: any PR editing data/medications-snapshot.json or the interaction lexicon without regenerating the index, pushed from an environment that skipped verify:pr-local. Fix: a "Medication interaction index drift" step in the static-pr job, under the same static_heavy_changed condition as its lexicon-report sibling. Tests: tests/check-medication-interactions-index.test.ts compares the committed index to the committed snapshot drug for drug and row for row (names, row counts, row keys, verbatim note text, sourceRowCount) and proves the failure shape against a mutated record (renamed drug, edited row, added row). tests/ci-cache-safety.test.ts pins the new static-pr step (red before the workflow change). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

…er gate on PRs (M33) Defect: scripts/check-clinical-hazard-controls.mjs proved only that each listed control symbol appeared somewhere in the control paths and that each listed test file existed with a tests/*.test.ts name. It never checked that any cited test references the control, so a proof test emptied of its case, or a symbol renamed and re-added as a comment, kept CLINICAL_HAZARD_CONTROLS_PASS printing. The gate also ran only inside the provider-backed governance:release chain, so a control rename or the register's reviewExpiresAt lapsing produced no pull-request signal. Trigger: any PR renaming one of the 13 control symbols or deleting the relevant it(...) case; or the calendar reaching the manifest's reviewExpiresAt. Fix: for every controlled/partial hazard, at least one listed test must name a control symbol or import a control path module, reported as "<id>: no listed test references a control symbol or imports a control path". The gate joins the static-pr job (docs or static-heavy scope, which already checks out with fetch-depth 0) and verify:pr-local's docs/heavy plan. On a shallow clone the script prints CLINICAL_HAZARD_CONTROLS_SHALLOW_CLONE and skips only the reviewedCommit ancestry checks instead of reporting every reviewed commit as missing; every file, symbol, test-reference and date check still runs (verified against a depth-one clone of this branch). The existing date, commit and role checks are unchanged. Tests: tests/clinical-hazard-controls.test.ts "requires at least one listed test to reference a control symbol or control path" (red before the change) and tests/ci-cache-safety.test.ts pins the static-pr step and its full-history checkout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

Defect: the symbol-presence check in scripts/check-clinical-hazard-controls.mjs escaped symbols with /[.*+?^${}()|[\\]\\]/g, which JavaScript parses as the class [.*+?^${}()|[\\] followed by a literal "\]", so no metacharacter was ever escaped. A dotted symbol matched as a wildcard (fail-open: reported present when absent) and a symbol carrying "(" or "[" threw or never matched. Trigger: any control symbol that is not a plain identifier. Today's 13 are plain identifiers, so the defect was latent. Fix: the repository's standard escape class /[.*+?^${}()|[\]\\]/g, in the one symbolPattern helper both the control-path and test-reference checks use. Test: tests/clinical-hazard-controls.test.ts "escapes regex metacharacters in control symbols instead of treating them as wildcards" — a runtime-assembled "foo.bar" must not match the file's "fooxbar" sentinel (red before the fix), and a "sentinel(" symbol must neither throw nor be reported missing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

…g (L47) Defect: scripts/check-owner-scope-api.mjs treated a handler as owner-scoped if the literal substring "owner_id" appeared anywhere in its body. A projection such as .select("id,owner_id,title"), a comment, or an unrelated object key satisfied it, so the two most likely tenancy regressions — a handler that returns owner_id without filtering on it, and one that only mentions it in prose — merged green while the gate reported "60 files clean". Trigger: a future src/app/api handler that reads an owner-scoped table and names owner_id without filtering on it. Fix: SCOPE_TOKENS are now filter shapes — .eq/.neq/.is/.in/.not/.match/ .filter("owner_id" or "documents.owner_id"), PostgREST owner_id.eq./.is./.in. strings, an owner_id: write payload key, the documents!inner(owner_id) join projection — plus the existing word-bounded helper names. The guard stays green on the current tree (60 files, 26 tables; the setup-status and clinical-quality reads remain covered by the documented allowlist). Child tables without an owner_id column and src/lib callers of createAdminClient are unchanged and reported for an owner decision. Test: the script's --self-test now fails a select-list-only handler with an owner_id comment (red before the change) and passes every filter shape the codebase uses (.is, .in, .or("owner_id.eq."), documents.owner_id, the inner join, an insert payload, single-quoted .eq). tests/owner-scope-guard.test.ts still passes unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

@mockup

…y (M31) Defect: PR #2006 (54585e9, 2026-08-16) added the /mockups/accessible- table-browser-fixture route and a Playwright journey proving that a low-confidence clinical table keeps "Not recorded" and its verify-against- source warning fully legible at 320px; it closed /issues #237. Merge commit ab7c513 (PR #2025, one day later) dropped the journey from tests/ui-tools.spec.ts while no non-merge commit ever removed it, so no PR diff showed the loss and nothing has run it since. Trigger: any change to AccessibleTable's low-confidence / missing-value rendering at 320px. Fix: the journey is restored verbatim from 54585e9 into its own file, tests/ui-accessible-table-mockup.spec.ts (tagged @mockup, so it runs in the advisory chromium-mockups project against the surviving fixture route), and `accessible-table-mockup` joins mockupSpecPattern and the top-level testMatch in playwright.config.ts. check:ci-scope's mockup-spec parity, the shard parity gate and the browser-test-plan self-test all pass with the new arm. docs/agents/test-deletion-guard.md records the merge-commit blind spot of check:diff-integrity. mockups/README.md is outside this change's scope; its 2026-09-02 row still says no committed test navigates to the fixture. The pre-commit hook regenerated docs/design-system/adoption-manifest.json (the new pin test now references AccessibleTable); that output is included. Test: tests/playwright-pr-shards.test.ts "collects the 320px AccessibleTable mockup journey in the advisory mockup project (M31)" pins the spec file, its @mockup tag, the fixture route it navigates to, and its collection by testMatch and mockupSpecPattern but not the production matcher (red before this change: the spec was missing). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

…h directions (M32) Defect: tests/ui-tools-show-all.spec.ts landed with PR #2008 (0b95d06, 2026-08-16) but its basename was in neither the top-level testMatch nor productionSpecPattern, mockupSpecPattern or seededSpecPattern in playwright.config.ts, so no project ever collected it — not test:e2e, not verify:ui, not the PR shards, not the release matrix. Both existing parity guards checked one direction only (what the matcher selects is sharded; named specs are collected), so a never-selected file was invisible to them. Trigger: any Playwright run; the phone launcher journey (/?mode=tools "Show all" link, 48px tap target, keyboard navigation to /tools) had zero browser evidence for 17 days while appearing to be tested. Fix: `tools-show-all` joins testMatch and productionSpecPattern in playwright.config.ts and the mirrored productionSpecFilePattern in scripts/playwright-pr-shards.mjs, and the spec is assigned to PR shard 2 (3.0s estimate on the shard with the smallest full total; the balance guards still hold at 1.5s/3.0s spread). No spec was deleted, moved or skipped. Tests: tests/playwright-pr-shards.test.ts "collects every on-disk spec in the top-level testMatch and at least one project (M32)" asserts the missing direction against the files on disk (red before the change, naming exactly tests/ui-tools-show-all.spec.ts), and "collects the phone launcher Show-all journey as a sharded production spec (M32)" pins the file by name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FSPY4VSqg7WVukCvmHQP9t

…M32) All four conflicted hunks are lists where both sides added entries, so the resolution is the union in every case — dropping either side's entry would silently stop a browser spec from running in CI. - playwright.config.ts productionSpecPattern: + token-layer-resolution (main). - playwright.config.ts mockupSpecPattern: + accessible-table-mockup (this branch, M31) + ward-morning, ward-referrals, ward-forced-colors (main). - playwright.config.ts testMatch: + accessible-table-mockup (M31) on top of main's caring-contacts-populated, tools-show-all, ward-morning, ward-referrals, ward-forced-colors and token-layer-resolution. - scripts/playwright-pr-shards.mjs productionSpecFilePattern: + tools-show-all (M32, also added on main) + token-layer-resolution (main). - scripts/check-owner-scope-api.mjs: keeps this branch's L47 self-test fixtures (select-list-only rejection, the filter-shape sweep) and main's expanded phase-1 comment on the non-owner-table fixture. Proved by evaluating each side's pre-merge regex and the merged regex against every spec basename in both trees: no filename matched by either side fails to match the merged pattern, and no new filename is matched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DHSyfuC6mS98ystWFiitAR

The merge itself was clean on these two files, which is exactly why they broke: each side added something the other side's guard counts. - scripts/playwright-pr-shards.mjs: main and this branch both added a shard row for tests/ui-tools-show-all.spec.ts, so the file listed it twice and `check:playwright-pr-shards` reported "duplicated". Kept main's shard-1 row (zero timing, the lightest measured shard) and folded this branch's M32 rationale into its comment. The spec is still sharded exactly once; the by-name pin in tests/playwright-pr-shards.test.ts asserts membership, not which shard. - scripts/verify-pr-local.mjs: main added a `Design drift ratchet` step to static-pr under static_heavy_changed, which the M24 mirror test in tests/ci-cache-safety.test.ts requires the local heavy plan to carry. Added check:design-drift-ratchet to staticHeavyGuards in CI step order (after check:icon-scale). The assertPlan expectations spread that list, so they move with it. check:playwright-pr-shards: "PR UI shard parity OK: 36 production specs across 3 groups." check:verification-plan: "PR-local verification plan self-test passed." Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DHSyfuC6mS98ystWFiitAR

coderabbitai · 2026-09-04T14:13:49Z

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 5c3df718-fbaa-4ede-aa7b-1e43b6d607ef

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.

❤️ Share

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

supabase · 2026-09-04T14:13:49Z

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 ↗︎.

chatgpt-codex-connector · 2026-09-04T18:31:42Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

cursor · 2026-09-04T18:31:47Z

Bugbot couldn't run - usage limit reached

Bugbot 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_a66dd747-ec7a-4f76-bb9e-d8dce8d14099)

cursor · 2026-09-05T07:42:13Z

Bugbot couldn't run - usage limit reached

Bugbot 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_cf02c540-46f4-478f-bfcc-91ce7c7af6a7)

claude added 10 commits September 2, 2026 22:43

Merge remote-tracking branch 'origin/main' into wt/p8b

0c8f7ca

BigSimmo marked this pull request as ready for review September 4, 2026 18:31

Merge branch 'main' into claude/audit-fix-p8b

13779d8

BigSimmo enabled auto-merge (squash) September 4, 2026 18:31

Merge branch 'main' into claude/audit-fix-p8b

7268c4f

Merge branch 'main' into claude/audit-fix-p8b

379eb9b

BigSimmo merged commit 6cfaa94 into main Sep 5, 2026
29 checks passed

BigSimmo deleted the claude/audit-fix-p8b branch September 5, 2026 08:21

Navigation Menu

audit fixes (P8b): collect the orphaned browser tests and make the local gates match CI - #2621

audit fixes (P8b): collect the orphaned browser tests and make the local gates match CI#2621
BigSimmo merged 13 commits into
mainfrom
claude/audit-fix-p8b

BigSimmo commented Sep 4, 2026 •

edited by cursor Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 4, 2026 •

edited

Loading

Uh oh!

supabase Bot commented Sep 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 4, 2026

Uh oh!

cursor Bot commented Sep 4, 2026

Uh oh!

cursor Bot commented Sep 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

BigSimmo commented Sep 4, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The merge with main needed care, and one thing the clean part broke

Verification

Risk and rollout

Notes

Uh oh!

coderabbitai Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

supabase Bot commented Sep 4, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 4, 2026

Uh oh!

cursor Bot commented Sep 4, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

cursor Bot commented Sep 5, 2026

Bugbot couldn't run - usage limit reached

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BigSimmo commented Sep 4, 2026 •

edited by cursor Bot

Loading

coderabbitai Bot commented Sep 4, 2026 •

edited

Loading