diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0aa4a717a..9bbc50e0a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -196,6 +196,9 @@ jobs: - name: CI scope self-test run: npm run check:ci-scope + - name: Pinned Gitleaks range self-test + run: npm run check:gitleaks-pinned + - name: CI triage self-test run: npm run check:ci-triage @@ -330,9 +333,16 @@ jobs: if: needs.changes.outputs.codex_autofix_changed == 'true' run: npm run check:codex-autofix-workflow + # Fixtures for ordinary non-docs PRs; full offline contracts (includes + # fixtures) when retrieval/answer surfaces change. - name: Offline RAG fixture and manifest validation + if: needs.changes.outputs.rag_eval_changed != 'true' run: npm run check:rag:fixtures + - name: Offline RAG production contracts + if: needs.changes.outputs.rag_eval_changed == 'true' + run: npm run eval:rag:offline + coverage: name: Unit coverage needs: changes @@ -406,10 +416,53 @@ jobs: contents: read uses: ./.github/workflows/docker-image.yml + # Fail-fast @critical Chromium smoke on PRs/merge_group before the full + # production suite. Skipped on main/schedule — those run the full job only. + # Keeps merge safety: pr-required still demands the full Production UI job. + ui-critical-fast: + name: Production UI critical + needs: changes + if: > + needs.changes.outputs.ui_changed == 'true' && + (github.event_name == 'pull_request' || github.event_name == 'merge_group') + runs-on: ubuntu-24.04 + timeout-minutes: 25 + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Setup UI e2e environment + uses: ./.github/actions/setup-ui-e2e + + - name: Chromium @critical journeys + run: npm run test:e2e:critical + + - name: Classify exact failed test identities + if: failure() + run: node scripts/classify-playwright-failures.mjs + + - name: Upload critical UI diagnostics + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: production-ui-critical-diagnostics-${{ github.run_id }} + path: | + test-results/ + playwright-report/ + if-no-files-found: ignore + ui-critical: name: Production UI - needs: changes - if: needs.changes.outputs.ui_changed == 'true' + needs: [changes, ui-critical-fast] + # Run when UI scope applies and the fail-fast job succeeded or was skipped + # (skipped on main/schedule/dispatch where critical-first is not used). + if: > + always() && + needs.changes.result == 'success' && + needs.changes.outputs.ui_changed == 'true' && + (needs.ui-critical-fast.result == 'success' || needs.ui-critical-fast.result == 'skipped') runs-on: ubuntu-24.04 timeout-minutes: 45 steps: @@ -542,7 +595,11 @@ jobs: pr-required: name: PR required - needs: [changes, static-pr, safety, coverage, build, container-images, ui-critical, db-reset-verify] + needs: + [changes, static-pr, safety, coverage, build, container-images, ui-critical-fast, ui-critical, db-reset-verify] + # #095: keep `if: always()` — a skipped required check counts as PASSING + # on GitHub, so `!cancelled()` would cancel-to-green a hand-cancelled tip. + # Cancelled vs failure is distinguished in the script below (PR #1409). if: always() runs-on: ubuntu-24.04 timeout-minutes: 5 @@ -562,6 +619,7 @@ jobs: COVERAGE_RESULT: ${{ needs.coverage.result }} BUILD_RESULT: ${{ needs.build.result }} CONTAINER_RESULT: ${{ needs.container-images.result }} + UI_FAST_RESULT: ${{ needs.ui-critical-fast.result }} UI_RESULT: ${{ needs.ui-critical.result }} DB_RESULT: ${{ needs.db-reset-verify.result }} run: | @@ -650,8 +708,14 @@ jobs: fi if [ "$UI_CHANGED" = "true" ]; then + if [ "$EVENT_NAME" = "pull_request" ] || [ "$EVENT_NAME" = "merge_group" ]; then + require_success "production-ui-critical" "$UI_FAST_RESULT" + else + require_skipped_or_success "production-ui-critical" "$UI_FAST_RESULT" + fi require_success "production-ui" "$UI_RESULT" else + require_skipped_or_success "production-ui-critical" "$UI_FAST_RESULT" require_skipped_or_success "production-ui" "$UI_RESULT" fi @@ -689,9 +753,18 @@ jobs: echo "Required in-scope PR checks passed." + # Firefox/WebKit matrix must not wait on pr-required: a blocking weekly + # dependency audit (full-run sentinel sets lockfile_changed) previously + # skipped the matrix entirely while Chromium UI was already green (#023). release-browser-matrix: - if: github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/') - needs: [pr-required] + if: > + always() && + (github.event_name == 'workflow_dispatch' || github.event_name == 'schedule' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/')) && + needs.changes.result == 'success' && + needs.static-pr.result == 'success' && + (needs.build.result == 'success' || needs.build.result == 'skipped') && + (needs.ui-critical.result == 'success' || needs.ui-critical.result == 'skipped') + needs: [changes, static-pr, build, ui-critical] runs-on: ubuntu-24.04 timeout-minutes: 70 diff --git a/.github/workflows/secret-scan.yml b/.github/workflows/secret-scan.yml index a403d8920f..bcee018523 100644 --- a/.github/workflows/secret-scan.yml +++ b/.github/workflows/secret-scan.yml @@ -17,22 +17,50 @@ permissions: pull-requests: read security-events: write +env: + # Match the version gitleaks-action@v3 installs by default. + GITLEAKS_VERSION: "8.24.3" + # SHA-256 of gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz from the release + # checksums.txt (https://github.com/gitleaks/gitleaks/releases/tag/v8.24.3). + GITLEAKS_LINUX_X64_SHA256: "9991e0b2903da4c8f6122b5c3186448b927a5da4deef1fe45271c3793f4ee29c" + jobs: gitleaks: name: Gitleaks runs-on: ubuntu-24.04 timeout-minutes: 20 steps: - - name: Checkout + - name: Checkout pinned head + # gitleaks/gitleaks-action@v3 does not support the merge_group event; + # the scan already ran on pull_request so skipping here is safe. + if: github.event_name != 'merge_group' uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 + # Pin the workspace to the triggering SHA so a later push cannot move + # HEAD under the scanner (#097). + ref: ${{ github.event.pull_request.head.sha || github.sha }} persist-credentials: false - - name: Scan for secrets - # gitleaks/gitleaks-action@v3 does not support the merge_group event; - # the scan already ran on pull_request so skipping here is safe. + - name: Install Gitleaks + if: github.event_name != 'merge_group' + run: | + set -euo pipefail + archive="gitleaks_${GITLEAKS_VERSION}_linux_x64.tar.gz" + curl -fsSL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/${archive}" -o /tmp/gitleaks.tgz + echo "${GITLEAKS_LINUX_X64_SHA256} /tmp/gitleaks.tgz" | sha256sum -c - + tar -xzf /tmp/gitleaks.tgz -C /tmp gitleaks + sudo install -m 0755 /tmp/gitleaks /usr/local/bin/gitleaks + gitleaks version + + - name: Scan for secrets (pinned event SHAs) + # Do not use gitleaks-action's PR path: it re-queries the commits API and + # can build a range against a newer tip that is absent from this checkout + # (#097). Event payload SHAs are immutable for the run. if: github.event_name != 'merge_group' - uses: gitleaks/gitleaks-action@e0c47f4f8be36e29cdc102c57e68cb5cbf0e8d1e # v3 env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GITLEAKS_BIN: gitleaks + GITHUB_EVENT_NAME: ${{ github.event_name }} + GITLEAKS_PINNED_BASE: ${{ github.event.pull_request.base.sha || github.event.before }} + GITLEAKS_PINNED_HEAD: ${{ github.event.pull_request.head.sha || github.sha }} + run: node scripts/run-gitleaks-pinned.mjs diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index f12dc77aef..036c7dc9b0 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -144,3 +144,7 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | PR #1396 / claude/latency-findings-impl-s8g01v | 70e810b66881e17aa9f58126fdad970986bda911 | User ask: resolve comments + Production UI phone-scroll + main sync | FIXED: synced main (DIRTY was staleness); removed union ledger dup; adapted phone-scroll asserts for Answer strategy-overlay + overlay/reserve-only calculator budget + focus pre-scroll inside 8px reveal band. Codex P1s already on tip; 0 unresolved threads. Focused Chromium phone-scroll 9/9 green (system Chrome). | phone-scroll focused 9/9; check:branch-review-ledger PASS; merge-tree clean; prior Codex P1s retained | | 2026-07-30 | HEAD | 13c16cf07c854b50daa35a2ef2a2ea76d5e059e1 | ci-testing-approach | findings: UI-load flake #093 dominates PR reds; schedule full-sentinel blocks release-browser via audit; UI scope overfires on src/app/api; ~40% PR runs cancelled wasting ~12 UI-hrs; CI_TRIAGE inert; eval:rag:offline claimed-in-CI but only fixtures run | gh-ci-500-runs,ci.yml,ci-change-scope,testing.md,process-hardening,outstanding-issues-093-095-097-023,flake-ledger-empty | | 2026-07-30 | cursor/ci-testing-review-1bf5 | 13c16cf07c854b50daa35a2ef2a2ea76d5e059e1 | ci-testing-approach | Corrects the ref cell from the unresolved placeholder "HEAD" to the actual branch name, so ledger:lookup can match this review by branch (Codex P2 finding on PR #1406). | node scripts/branch-review-ledger.mjs lookup cursor/ci-testing-review-1bf5 --scope ci-testing-approach | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | ad9da6a6f8ba3884b389fa78e678bb88ee72d9d1 | ci-hygiene-gates | implemented matrix unblock, scope narrow, cancelled≠failure, pinned gitleaks, critical-first UI, eval:rag:offline; skipped #093; verify:cheap 4471 pass | verify:cheap,check:ci-scope,check:gitleaks-pinned,check:gate-manifest,eval:rag:offline | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | b660dbc5a10d7ca3da03541028017f0abc6b5bd3 | ci-hygiene-gates merge-readiness | findings | check:ci-scope;check:gitleaks-pinned;scope-classify PR files ui_changed=false;sim cancelled-as-neutral | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 8f3283d00da274dee507a1b8e9b611321d1f35be | pr-1413-merge-readiness | READY after main sync + cancel-to-green fix; draft until tip CI green; deferred #093 + CI_TRIAGE_ENABLED confirm | verify:cheap:4481-pass;format:outstanding-issues;merge-tree:clean;cancelled:!cancelled();hosted:awaiting-tip | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 0d70de480f370fec3e7f3774f13d906318a09b3c | pr-1413-merge-readiness | READY: synced with main/#1409; tip CI success incl PR required; draft; deferred #093 + CI_TRIAGE_ENABLED | merge-tree:clean;ci-cache-safety:13/13;hosted:30520195863:success;PR-required:pass | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index f04025257e..32e6ca535a 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -54,7 +54,7 @@ removed after current-main verification; it is not missing recommended work. | 2 | `#053` | A1 | Operator — legal/privacy | Start now; finish before real patient use/privacy-approved release | 4–8 hours internal; 1–6 weeks elapsed | Execute DPAs; decide ZDR/residency; obtain cache behavior in writing; review subprocessors; obtain APP 8 and APP 5/1 counsel sign-off. Do not change public copy before approval. | | 5 | `#024` | A2 | High — browser/Next diagnostics | Provider-free macOS Safari host available | 1–2 hours | Reproduce document-source fallbacks in Safari/STP without Playwright interception; capture `_rsc` response evidence. Treat as an app defect only if native Safari reproduces; otherwise return to the harness. Never suppress `pageerror` or change CORS without proof. | | 6 | `#022` | A2 | Operator — clinical governance + Specialist | Policy implemented locally; hosted apply and human review pending | 1–2 hours apply; 0.5–1 day first ten | The auditable BMJ `third_party_reference_attested` policy, migration and top-ten evidence manifest are prepared without changing `clinical_validation_status=unverified`. A qualified operator must review evidence, apply the migration deliberately, attest eligible records, review the ten visible local documents, then remeasure warnings. | -| 7 | `#023` | A2 | Specialist — RAG/browser diagnostics | After hosted dependency audit is green | 1–2 hours | Capture the skipped Firefox/WebKit scheduled datapoint and disposition the human irrelevant-at-10 labels. Retrieval and answer artifacts are already compared under resolved #051; do not spend on another RAG run. | +| 7 | `#023` | A2 | Specialist — RAG/browser diagnostics | After next weekly/manual matrix green (audit no longer blocks it) | 1–2 hours | Capture one Firefox/WebKit scheduled/manual datapoint and disposition the human irrelevant-at-10 labels. Matrix is structurally unblocked from blocking audit; do not spend on another RAG run. | | 8 | `#018` | A2 | Specialist — clinical RAG/retrieval | Lithium closed; ADHD/metabolic evidence debt remains | Corpus/operator follow-up | Lithium's bounded subject/row-aware fix passed its targeted answer plus the full 36-case retrieval and 44-case answer canaries. ADHD's expected CAMHS document remains absent and the surfaced chart has no accessible table; metabolic schedule evidence remains unavailable and its standalone classifier candidate was reverted. | | 10 | `#001` | A2 | Specialist — retrieval/ranking | After rollout approval | 0.5–1 day plus canary | Keep semantic reranking off unless an approved ambiguity comparison preserves 36/36, recall 1.0, zero per-case regressions, and shows measured gain; otherwise record keep-off and stop. | | 11 | `#025` | A2 | Operator — Railway/GitHub/chat/Supabase | Next approved observability window | 1–3 hours/channel | Choose owned deployment, CI, ingestion, and SLO alerts; mock first, then one approved controlled provider event/channel. The merged Supabase trigger remains inert until its verified inputs are configured. Stop without an accountable responder. | @@ -113,7 +113,7 @@ removed after current-main verification; it is not missing recommended work. | #018 | P2 | task | Split the lithium, ADHD and metabolic residuals by mechanism | Current evidence keeps the mechanisms separate. **Lithium — closed within this item:** the row/atom-aware subject guard, foreign-parameter rejection and query-specific range promotion returned `0.5–1.0 mmol/L` with correct targeting/citation; the full retrieval canary remained 36/36 with recall 1.0 and zero per-case RR regressions, and the full answer canary passed every blocking gate. **ADHD — open corpus debt:** `CG.MHSP.ADHD.pdf` is absent from the hosted corpus and the retrieved chart exposes `accessible_table_count=0`; repair corpus/fixture or ingestion evidence rather than weakening extractive budgets. **Metabolic — open structured-evidence debt:** the standalone plural classifier worsened the live answer and was reverted; obtain auditable schedule text/table evidence before another candidate. | targeted live lithium/ADHD/metabolic evidence 2026-07-27; `docs/evidence/rag-reliability-evidence-2026-07-27.md`; refuted approaches | 2026-07-21 | | #021 | P3 | rec | E-3d H2 residual: strong/comparison generation discards | approx. 6 generation attempts per full 44-case run still fail the final quality gate and fall to extractive on strong-route comparison/complex shapes (the designed-conservative outcome). PARKED: weakest cost/benefit on the queue — a wave (approx. $2-4 pair + reviewer cycle) to shave seconds off a few hard cases. Revisit only if latency/waste complaints or a cheaper lever appears. | E-3c design record; runs #59-#61 diagnostics | 2026-07-21 | | #022 | P2 | task | Source-governance metadata refresh (operator) | The selected policy is now encoded locally as auditable `third_party_reference_attested` evidence with policy version, reviewer qualification, evidence references and append-only review history. It deliberately preserves `clinical_validation_status=unverified`; malformed, stale or non-BMJ evidence remains review debt. Migration `20260727010000_bmj_third_party_source_attestation.sql` is prepared but was **not applied**. The ten most visible local-document candidates are captured in `docs/evidence/rag-top-local-review-manifest-2026-07-26.json` with `attestation_applied=false`; qualified human review, deliberate hosted apply/attestation, and warning-rate remeasurement remain operator work. | governance worklist; local policy/migration tests; top-ten evidence manifest | 2026-07-21 | -| #023 | P2 | task | Complete scheduled browser and labeling disposition | The 2026-07-26 retrieval and answer artifacts are read and compared under resolved #051. Scheduled CI run `30216361999` failed its existing production dependency audit before Firefox/WebKit, while production Chromium passed. After that audit is green, capture one scheduled/manual browser-matrix datapoint; separately record the human decision for the stable irrelevant-at-10 set. #084 now makes each top-10 grade and matched signal reproducible, but it does not substitute for the human disposition. Do not rerun or spend on RAG for this item. | runs `30216191889`/`30216361999`; per-rank diagnostics #084; session 2026-07-27 | 2026-07-21 | +| #023 | P2 | task | Complete scheduled browser and labeling disposition | **Partial 2026-07-30:** `release-browser-matrix` no longer depends on `pr-required`, so a blocking scheduled dependency audit cannot skip Firefox/WebKit. Still need one green matrix datapoint + human irrelevant-at-10 disposition. The 2026-07-26 retrieval and answer artifacts are read and compared under resolved #051. Scheduled CI run `30216361999` failed its existing production dependency audit before Firefox/WebKit, while production Chromium passed. After that audit is green, capture one scheduled/manual browser-matrix datapoint; separately record the human decision for the stable irrelevant-at-10 set. #084 now makes each top-10 grade and matched signal reproducible, but it does not substitute for the human disposition. Do not rerun or spend on RAG for this item. | runs `30216191889`/`30216361999`; per-rank diagnostics #084; session 2026-07-27 | 2026-07-21 | | #024 | P2 | issue | WebKit e2e `_rsc` prefetch access-control errors | PR #1205 narrowed catch-all interception and duplicate navigation, but Next 16.2.11 still raises `_rsc` access-control `pageerror`s after document-source fallbacks: `/documents/source?id=&page=2&chunk=safety%20plan` → `/documents/?page=2&chunk=safety+plan`; `/documents/source/evidence?id=not-a-uuid&page=2` → `/documents/search`. The invalid-id failure survived removing every Playwright route; Chromium passed both. **Next:** on a provider-free macOS host, run both URLs in stable Safari and Safari Technology Preview without interception, capture console text plus `_rsc` status/access-control headers, and compare Playwright WebKit with routing on/off. Treat as an app defect only if native Safari reproduces; otherwise return to the harness. Never suppress `pageerror` or change CORS without native evidence. | PRs #1179/#1205; current-main local WebKit evidence; session 2026-07-28 | 2026-07-28 | | #025 | P2 | task | Activate the three webhooks (operator secrets) | Merged (#968/#1100) + deployed but inert — verified live: `POST /api/webhooks/railway` returns `503 webhook_not_configured`; the Supabase document-change trigger exists but lacks both activation inputs. To turn on: (1) Railway ΓåÆ set `RAILWAY_WEBHOOK_SECRET` + add the `?token=…` webhook URL; (2) set `SLACK_WEBHOOK_URL`/`DISCORD_WEBHOOK_URL` in BOTH the Railway **app/server env** and **GitHub repo secrets**; (3) set one matching document-change secret in the Railway app env as `SUPABASE_INGESTION_WEBHOOK_SECRET` and in Supabase Vault as `ingestion_webhook_secret`, then set the per-environment database GUC `app.ingestion_webhook_base_url` to the deployed app origin. Each path fails closed until fully configured, so this is pure ops. See `docs/webhooks.md` for verification and rotation. | sessions 2026-07-22/24; PRs #968/#1100; docs/webhooks.md | 2026-07-22 | | #027 | P3 | rec | External uptime monitor independent of GitHub/Railway | `live-domain-monitor.yml` runs on GitHub's cron, so it won't run in exactly the outage it should catch (Actions or the deploy itself down). Add an off-platform synthetic monitor (UptimeRobot / Better Stack / Checkly) hitting `/api/health` with a webhook alert. Provider setup, not code. | session 2026-07-22 webhook review | 2026-07-22 | @@ -135,7 +135,6 @@ removed after current-main verification; it is not missing recommended work. | #094 | P2 | rec | Design-system gates assert structure, not rendered effect | **Outcome:** a style contract cannot pass while the style is inert. **Detail:** PR #1316's accent rail shipped inert because `.search-band` sat in `@layer components`, which loses to Tailwind's utilities layer regardless of specificity — and the test asserted `toHaveClass("search-band")`, i.e. class presence, not effect. Computed style showed `1px rgb(229,231,235)` where `2px rgb(11,111,134)` was intended. The same shape of gap let a rail-colour assertion compare a colour against a width and pass unconditionally. **Next:** for contracts where the visual IS the requirement (rails, forced-colors thickness, tap targets), assert `getComputedStyle` in a Playwright case rather than class names in a DOM test, and add the unlayered-component convention to the design-system contract check. **Stop:** do not convert existing passing DOM tests wholesale; add computed-style proof only where the effect carries the meaning. | PR #1316 Codex P2 finding; session 2026-07-28 | 2026-07-28 | | #095 | P3 | issue | `PR required` reports failure for concurrency-cancelled jobs | **Outcome:** a red `PR required` means a real failure. **Detail:** the aggregate calls `require_success` on `coverage`/`production-ui`, so any push that supersedes an in-flight run reports `coverage result was cancelled` → exit 1, indistinguishable at a glance from a genuine failure. Eleven such reds were produced on PR #1316 in one session, and `Production UI` never once ran to completion. **FIXED 2026-07-30:** the aggregate now distinguishes the two. `require_success` / `require_skipped_or_success` route a `cancelled` result through a shared `cancelled_error` helper instead of reporting it as a plain failure. **First attempt was invalid and hosted CI was the only thing that caught it:** it passed the workflow-level cancelled status function through an `env:` value, but GitHub allows those functions (success/failure/cancelled/always) ONLY in `if:` conditions, so `ci.yml` failed to parse — the run was named `.github/workflows/ci.yml` rather than `CI` and created **zero jobs**. Valid YAML and invalid Actions schema, so prettier, lint, typecheck, `check:github-actions` and the full unit suite all passed the broken file. Reading each job's own `cancelled` result needs no such expression and loses nothing, since a supersession cancels the upstream jobs anyway. A new guard now fails locally on any status-check function outside an `if:` across `.github/workflows/**` — and it immediately caught a second instance in the explanatory comment written to warn about the first, because expressions are interpolated inside `run:` blocks too. **Second review finding, also real:** the first working version exited on the first non-success, so a run that was cancelled AND broken — `safety` cancelled while `build` had already failed — announced "not a real failure" and hid the break entirely, which is worse than the ambiguity the change set out to remove. It also asserted supersession as fact, wrong for a hand-cancelled run. Now two-pass: every requirement is recorded before anything is reported, **genuine failures win and are all listed**, a concurrent cancellation is demoted to a `::warning::` context line, and the cancelled-only headline states both possible causes instead of asserting one. Mutation-proven: four cases fail against the first-exit version. The message is actionable rather than merely accurate: it names the supersession, points at the newest run for the current head, and tells a reader who finds NO newer run that the run was hand-cancelled, verified nothing, and must be re-run rather than merged past. Measured cost of the ambiguity before the fix: one 2026-07-30 session spent four separate investigations on `::error::changes result was cancelled` / `static-pr result was cancelled`, while PR #1401 merged straight through an unrelated red the repo had learned to ignore. **The tempting fix was rejected as unsafe.** Treating `cancelled` as neutral, or skipping the aggregate via `if: !cancelled()`, would make the red disappear — but GitHub counts a SKIPPED required check as PASSING, so a hand-cancelled run on the current head would become mergeable with nothing verified. The result therefore stays RED; only the diagnosis cost was removed. Guarded by seven cases in `tests/ci-cache-safety.test.ts` that EXECUTE the extracted aggregate script under synthetic job results rather than grepping the YAML (the defect was behavioural, and a structural assertion passed against it — see `#094`), including one that pins `if: always()` plus the wiring, and one that asserts no cancelled required job can ever exit 0. Mutation-proven: three of them fail against the pre-fix aggregate. **Stop:** do not relax `require_success` for genuine failures, and do not make this aggregate skippable — a skipped required check reads as green. | PR #1316 runs 30340972329 / 30341225585; session 2026-07-28 | 2026-07-28 | | #096 | P2 | task | PR #1316 review follow-ups — adoption-gate coverage closed | **Outcome:** the two remaining PR #1316 review findings are fixed on `main` with tests. **Do not chase the commits.** The seven Codex follow-up SHAs (`ff5b682`, `77cfe12`, `9840ed9`, `81ffb86`, `a5d6561`, `967e16c`, `e544d0d`) are **unreachable** — `git fetch origin ` fails for all seven, no open PR or branch carries them, and none was in the squash merge `4bcfeb90`. They were authored in a sandbox on a branch named `work` and never pushed, so the "follow-up PR metadata" each reported does not exist. **Durable source:** the [PR #1316 review threads](https://github.com/BigSimmo/Database/pull/1316/files) persist and describe every fix with file and line detail; re-derive from those, not from the hashes. **Was live on `main` through 2026-07-28:** the band adoption gate skipped query-backed root modes — `modeHrefToPagePath` returned null for `pathOnly === "/"`, so `/?mode=prescribing` and Documents never entered the route inventory and the root dashboard page was unchecked. Closed on PR #1394 (see Adoption-gate gap closed below). **Already fixed independently, no action:** favourites hub counts (`libraryCountsTrusted`), the document-search status derivation, the 401 session-expiry path, and the record-path duplicate notice. **Corrected 2026-07-28 — the Therapy Compass retry-waiter finding is NOT a live defect.** `use-therapy-data.ts:68` `retryWaitersRef` is genuinely unscoped, so a newer request can settle an older retry's promise, but no caller observes it: `useTherapyData` lives in the long-lived `TcProvider` (`bindings.tsx:206`) and `requestKey` derives only from `screen`, so it cannot change without the screen changing; the sole awaiting caller is the band's `AsyncButton` inside `search-screen.tsx:33`, which unmounts on that transition, and `workspace.tsx:36` uses `onClick={b.retryData}` which discards the promise. An earlier note here claimed a visible "Retry stops being busy" symptom — that was wrong and is retained only as the correction. It becomes real if a future caller ever awaits `retry()` from a control that survives a `requestKey` change. **Adoption-gate gap closed 2026-07-29.** Root-path and href-less modes now resolve to `src/app/(search-app)/page.tsx`. Closing it surfaced two further defects in the same gate that the original finding did not name: the hand-rolled walk was capped at two import hops while the root route's real chain is four (`layout -> shared-search-app-shell -> global-search-shell -> ClinicalDashboard -> document-search-results`), and it followed neither `layout.tsx` — which is where that route's band actually comes from, since the page renders only a pass-through — nor `dynamic(() => import(...))`, which is how the dashboard code-splits its mode workspaces. All three are fixed together with a bounded BFS; each was verified load-bearing by reverting it and watching the gate fail. **Stop:** not user-facing; do not let it block a release, and do not add waiter keying without a reproducer showing a still-mounted control whose busy state clears early. | PR #1316 review sweep; session 2026-07-28 | 2026-07-28 | -| #097 | P3 | issue | Gitleaks reports a false red when the PR head moves mid-run | **Outcome:** a red `Gitleaks` means a secret was found, not that someone pushed. **Detail:** on 2026-07-28 the job triggered for head `9bace1d1` checked out that merge ref, then queried the API and built its range against head `40278453` — pushed seconds later and absent from the checkout. Git rejected the range (`fatal: Invalid revision range`), so it scanned `~0 bytes`, logged `no leaks found in partial scan`, and exited 1. The scan did not run at all, which is worse than a normal failure because the natural reading is "noise, ignore it". It cleared on its own once the head stopped moving (`23 commits scanned`, `~198 KB`, `no leaks found`). Both range endpoints resolve in any complete checkout — verified locally against the branch and the PR merge ref — so this is not a `fetch-depth` problem. **Next:** pin the scan to a range the job controls (`base.sha`..the checked-out head) instead of re-querying the API mid-run, so a concurrent push cannot invalidate it. **Related:** same push-churn family as #095. **Stop:** do not weaken the gate to a soft-pass; the fix is a stable range, not a tolerated failure. | PR #1316 runs 30344938800 / 30346797225; session 2026-07-28 | 2026-07-28 | | #098 | P2 | task | Offline round-trip budget harness for the hot routes | **Outcome:** per-scenario Supabase round-trip counts are pinned by a test, so an extra round trip on a hot path is a red gate rather than an inference. **Done 2026-07-29:** the measurement gap is closed — `Server-Timing` now covers `auth`/`ratelimit`/`scope` on `/api/answer`, `auth`/`ratelimit`/`search`/`total` on `/api/search`, and `auth`/`ratelimit` on `/api/answer/stream` (previously the route the UI actually calls emitted no header at all). Headers flush before the first SSE frame, so in-stream stages cannot reach a header and must NOT be routed through the governed `progress`/`final` contract. `tests/answer-route-preamble.test.ts` pins admission-before-scope (no scope call while the limiter is pending or after a deny) and the client-disconnect abort signal. **Next:** generalise it — wrap the Supabase client in a counting proxy and assert per-scenario query budgets over the existing offline suites — `scripts/eval-rag-offline.mjs`, `scripts/test-rag-offline.mjs`, `scripts/rag-offline-contract.mjs` and the contract fixture `scripts/fixtures/rag-offline-contract-tests.json`. **An earlier version of this row named `test-cache-path.mjs` and `check-rag-fixtures.mjs`** (corrected 2026-07-29, PR #1377 review, matching the audit's own retraction): neither exercises a RAG request — the first computes Vitest/TypeScript cache paths, the second only validates fixture manifests — so building the harness on them would have counted nothing. Sequence before #099 and #101: it is the enabler and the standing guard. No providers, no DB. | `docs/audit/latency-audit-2026-07-28.md` measurement plan; `src/lib/server-timing.ts`; `src/lib/answer-stream-contract.ts:18-21` | 2026-07-29 | | #099 | P2 | task | Remove the remaining fixed per-request round trips | **Outcome:** the answer path stops paying avoidable per-request Supabase round trips. **Done 2026-07-29:** shared-cache-hit promotion deferred off the response path with its mid-request staleness guard intact and documented (`rag.ts:3234`, `rag-cache.ts`); scope resolution overlapped with the rate-limit RPC, signal threaded so a client disconnect finally cancels its paginated queries (`answer/route.ts`). **REFUTED on PR #1377 review — do not retry:** the same pass also overlapped scope with the rate-limit RPC and aborted it on deny, claiming the limiter could "deny for free". It cannot. With caller-supplied `filters` or explicit ids, scope passes its zero-query early returns (`search-scope.ts:242,253`) into the paginated `documents` loop at `:269`, and an `AbortSignal` cancels the client request without un-executing a statement Postgres already began — so throttled traffic kept burning database capacity while collecting 429s, against `capacity-review.md:106-113`'s first-soft-failure warning. Scope is behind admission again, pinned by `tests/answer-route-preamble.test.ts`. Re-attempting the overlap requires a non-database admission gate ahead of the durable limiter first. **Remaining:** (a) the 8 `setCachedSearch` awaits — deferring changes `throwIfAborted` semantics and widens a real mutation window because the clone happens after an `await`, so each branch needs discharging individually; (b) batch the anonymous subject+global rate-limit pair, which needs a NEW atomic RPC modelled on `consume_summary_rate_limits_atomic` and cannot be called until the operator applies it — `Promise.all` is the WRONG fix because it consumes the global bucket even when the subject bucket already denied; (c) stop the proxy and route handler resolving identity twice per authenticated request — no in-process memo can do this (different `Request` objects), so the proxy must forward unspoofable verified claims via a header it controls. Cross-references #011: halving auth resolutions eases the ~10-connection Auth cap that `capacity-review.md:106-113` calls the first hard failure. | `docs/audit/latency-audit-2026-07-28.md` L1-1/L1-3/L1-4; `src/lib/api-rate-limit.ts:276-282`; `src/proxy.ts:125` | 2026-07-29 | | #100 | P2 | rec | Buffered answer generation has no incremental verified delivery | **Outcome:** a clinician sees verified answer content before the whole generation completes. Highest-leverage latency finding in the 2026-07-28 audit: generation is buffered (`openai.ts:465`) and delivered in ONE `final` SSE frame, so time-to-first-content equals total latency — a strong answer inside its 25 s SLO still shows a blank panel for 25 s. The 15 s `sse-heartbeat` exists because that silence routinely exceeds 15 s; it instruments the defect rather than fixing it. **Naive token streaming is REFUTED, not merely unbuilt:** `answer-stream-contract.ts:18-21` removed `token`/`revising` deliberately because a rolling deployment would "re-expose unvalidated clinical prose", and raw tokens bypass the numeric-faithfulness gate the 2026-07-01 audit filed as H1. **Only admissible shape:** progressive disclosure of already-verified units (evidence/sources at retrieval-complete, then per-section after that section clears verification) over the existing whitelisted `progress` event. Needs a clinical-governance decision plus a canary pair. Also add the refutation to `docs/rag-behaviour/refuted-approaches.md`. Cross-references #021. **Stop:** do not re-land `token` streaming. | `docs/audit/latency-audit-2026-07-28.md` L0-1; `src/lib/answer-stream-contract.ts:18-21`; `src/lib/sse-heartbeat.ts` | 2026-07-29 | @@ -166,6 +165,7 @@ Move resolved rows here with the resolution date and a one-line outcome. Keep th | #088 | task | Union-driver ledger duplication watch after repair | CLOSED 2026-07-30. Post-repair merges take main's repaired lines; residual exact-dupe babysit twins from stock `merge=union` are addressed by the custom `merge=ledger` driver (union + exact-row dedupe), `npm run ledger:dedupe`, and the Run PR anti-churn ledger policy. Success criterion (three consecutive post-repair guard passes) met on ordinary main syncs; ongoing exact-dupe class is now gated rather than watched. | 2026-07-30 | | #087 | issue | `npm run check:knip` reported pre-existing dependency findings | NOT A DEFECT — false positive. The findings (unused `rimraf`/`tsx`/`@testing-library/dom`, unlisted `playwright-core`) only appeared because the worktree had no `node_modules` of its own and tooling resolved from the parent checkout. After `npm ci` in the same worktree, `npm run check:knip` exits 0 with no ignore-list change. Durable lesson: never act on a knip finding from a worktree that has not been installed. | 2026-07-28 | | #089 | issue | Branch-review-ledger hygiene deferred from PR #1275 | Closed by the 2026-07-28 hygiene pass. The MD056 cell-count and duplicate-row findings dispositioned as "belongs in a dedicated main ledger hygiene pass" on PR #1275 are fixed: 140 mojibake lines restored byte-exact from git history, 6 residual separators repaired, 46 exact duplicates removed, 21 wrong-width rows normalised, and 4 heading+bullet records converted to table rows — 1067 records, all six cells. Root cause closed too: `npm run ledger:lookup` / `ledger:append` (`scripts/branch-review-ledger.mjs`) replace hand-written rows, and `check:branch-review-ledger` now fails on mojibake, cell width, heading records, impossible dates, table gaps, and (from 2026-07-29) unresolvable HEADs and near-duplicates. | 2026-07-28 | +| #097 | issue | Gitleaks reports a false red when the PR head moves mid-run | RESOLVED 2026-07-30: Secret Scan checks out the event head SHA and runs `scripts/run-gitleaks-pinned.mjs` against the immutable event base..head range (no mid-run PR commits API re-query). | 2026-07-30 | | #111 | issue | `ui-overlap` phone-inset case still flakes under load | RESOLVED 2026-07-29 (PR #1391). The inset measurement now retries inside `expect(async () => …).toPass({ timeout: 15_000 })` (`ui-overlap.spec.ts:159`) — the same retry-the-measurement shape PR #1375 applied to the eight overlap cases. The 2px symmetry tolerance and the assertions themselves are byte-for-byte unchanged, so a genuinely asymmetric header still fails once the retry budget is spent; only a transient mid-remount sample settles. Full spec green across 4 consecutive runs (14 passed each). Do not relax the tolerance if this recurs — the assertion is the point; make the measurement robust instead. Source: PR #1375 flake triage; session 2026-07-29 | 2026-07-29 | | #112 | issue | `issues:next-id` has no concurrency protection | RESOLVED 2026-07-30. `npm run check:outstanding-issues` now gates this file, in `verify:cheap` and in the `static-pr` CI job (the gate-manifest check refuses a local gate that CI does not run). It fails on a duplicate id, an id present in both tables, a marker at or below the highest id, a malformed row, and a missing heading or marker — so every shape the 2026-07-29 triple collision took is now a red gate rather than a silent row loss. Verified by replaying that collision against the real file: two rows claiming `#110` produced "#110 appears 2 times (lines 151, 164)", and a lost marker bump produced "issues:next-id=113 is not above the highest id #114". The checker honours `\|` escapes — its first run against the live file flagged row #042, which is correctly escaped, and a gate with false positives is a gate people switch off. NOT fixed: the underlying race. Ids are still allocated by read-modify-write with no lock, and this file still has no `merge=union` driver; what changed is that a collision can no longer land silently. Source: session 2026-07-29 PR sweep; PR #1391 conflict resolution; `.gitattributes` | 2026-07-30 | | #113 | issue | `ModeNav` clips its labels at every phone width on `main` | RESOLVED 2026-07-30 (PR #1405, landed on `main` as `020c1260`). The two lower density bands used `grid-auto-columns: 1fr`, so the widest slot set every slot's width and the label's `truncate` hid the shortfall — nothing overflowed and nothing failed, the word was simply gone. `display: flex` at every band removes the mode rather than retuning it; thresholds went `16/26/34rem` -> `22/33/42rem` against remeasured intrinsic widths (471px for four labels, not the 394px originally budgeted). `tests/ui-mode-nav-density.spec.ts` asserts at both band boundaries and one pixel either side, registered in BOTH Playwright allowlists. **Carry-forward:** the ~8% threshold headroom is not padding — a measured-plus-one-pixel attempt (21/31rem) passed locally and failed CI by exactly 1px, because the variable face rasterises wider on `ubuntu-24.04`. A threshold calibrated to one machine's font metrics is calibrated to nothing. | 2026-07-30 | diff --git a/docs/process-hardening.md b/docs/process-hardening.md index c80096d8dd..b6c4fb0709 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -209,10 +209,11 @@ passes `p_worker_id`. Ordered apply steps, R17 manual `CONCURRENTLY` index, and ## PR merge gate: risk-scoped CI + required aggregate (2026-07-10) -- CI now has one always-reporting required aggregate: `CI / PR required`. The aggregate depends on `changes`, `static-pr`, `safety`, `coverage`, `build`, `ui-critical`, and `db-reset-verify`, then enforces only the jobs whose scopes apply. -- `static-pr` is the deterministic baseline for every PR: runtime, action pin check, CI scope self-test, format, lint, and typecheck. Coverage is the one required full unit run. Build, safety/config (the `safety` job includes RAG fixture validation), production UI, and migration replay are independent jobs so reruns stay focused. Coverage includes source, tests, package/test-runner configuration, while process-only documentation does not trigger builds. -- `db-reset-verify` is path-scoped to Supabase migrations/schema/config and database-access code. Do not also require an external Supabase Preview replay unless the repo owner intentionally wants duplicate migration replay. -- `ui-critical` retains its job ID for branch-protection compatibility but runs one required production Chromium invocation covering all non-quarantined critical and regression journeys (`test:e2e:pr`). `ui-advisory` runs quarantined and mockup journeys together when UI scope applies. A JUnit failure is considered a known flake only when its exact spec/title matches the validated ledger. The full browser matrix remains main/release/manual/scheduled. +- CI now has one always-reporting required aggregate: `CI / PR required`. The aggregate depends on `changes`, `static-pr`, `safety`, `coverage`, `build`, `ui-critical-fast`, `ui-critical`, and `db-reset-verify`, then enforces only the jobs whose scopes apply. The aggregate keeps `if: always()` and labels concurrency `cancelled` distinctly from real failures (#095 / PR #1409); it stays red because a skipped required check would count as passing. +- `static-pr` is the deterministic baseline for every PR: runtime, action pin check, CI scope self-test, format, lint, and typecheck. Coverage is the one required full unit run. Build, safety/config (fixtures always; `eval:rag:offline` when `rag_eval_changed`), production UI, and migration replay are independent jobs so reruns stay focused. Coverage includes source, tests, package/test-runner configuration, while process-only documentation does not trigger builds. +- `db-reset-verify` is path-scoped to Supabase migrations/schema/`src/lib/supabase` and drift tooling — not every API route. Do not also require an external Supabase Preview replay unless the repo owner intentionally wants duplicate migration replay. +- `ui-critical` retains its job ID for branch-protection compatibility and still runs the full required production Chromium suite (`test:e2e:pr`). On pull requests / merge_group, `ui-critical-fast` runs `@critical` first for fail-fast signal. `src/app/api/**` does not set `ui_changed`. `ui-advisory` runs quarantined and mockup journeys together when UI scope applies. A JUnit failure is considered a known flake only when its exact spec/title matches the validated ledger. The full browser matrix remains main/release/manual/scheduled and depends on static/build/UI success — not on `pr-required` — so a blocking scheduled dependency audit cannot skip Firefox/WebKit (#023 structural half). +- Secret Scan pins Gitleaks to the workflow event base/head SHAs and the checked-out commit (`scripts/run-gitleaks-pinned.mjs`) so a concurrent push cannot invalidate the range (#097). - The 2026-07-13 cold-server and historical ledger candidates are tracked through the reproduction policy in `docs/testing.md`: run each three times on the same SHA, fix fail/pass races, treat deterministic failures as regressions, and remove entries that do not reproduce. On `0c56f27a3`, the historical composer/tap/answer-fallback entries and three cold-route candidates did not reproduce in three runs; the narrow differential viewport reproduced once in three cold runs, was fixed with route-specific readiness before its single submit, then passed three of three. The ledger is intentionally empty. - Branch protection for `main` should require `CI / PR required` and `Secret Scan / Gitleaks`. Keep `SAST / Semgrep` required only if the repository owner accepts its external-rule/network dependency as part of the normal merge gate. Container-affecting PRs are enforced through `CI / PR required`; do not separately require `Docker image build / app-image` or `Docker image build / worker-image`. Also do not require other path-filtered or scheduled/manual contexts such as `CI / Unit coverage`, `CI / Critical UI smoke`, `CI / Migration replay`, `CI / release-browser-matrix`, `Eval Canary`, or `Live drift check`; they can be skipped on ordinary PRs and would leave branches stuck at "Expected - Waiting for status to be reported." diff --git a/docs/testing.md b/docs/testing.md index 8bdc744440..56b8a601b2 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -60,7 +60,7 @@ Phone-chrome work uses `npm run verify:phone-chrome`. Inspect its classification ## CI topology -PR CI keeps static checks separate from one required full unit run with coverage. UI scope uses one required production Chromium invocation for non-quarantined critical, regression, and dashboard/document visual-artifact journeys, plus one advisory invocation for quarantined and mockup journeys. Container scope calls the reusable Docker workflow and requires both app and worker image builds through the `pr-required` aggregate. Build, migration, security, and release behavior remain independently scoped. +PR CI keeps static checks separate from one required full unit run with coverage. UI scope runs a fail-fast `@critical` Chromium job on pull requests, then one required full production Chromium invocation (`test:e2e:pr`) for non-quarantined journeys, plus one advisory invocation for quarantined and mockup journeys. `src/app/api/**` does not set `ui_changed` or `db_changed` — API handlers stay on unit/coverage (and offline RAG when retrieval-scoped). The `PR required` aggregate keeps `if: always()` and distinguishes `cancelled` from `failure` in its messages (stays red; a skipped required check would count as passing). Secret Scan pins Gitleaks to the workflow event base/head SHAs and the checked-out commit, and verifies the linux_x64 release tarball against a pinned SHA-256 before install. The weekly `release-browser-matrix` depends on static/build/UI success, not on the full aggregate, so a blocking scheduled dependency audit cannot skip Firefox/WebKit. Container scope calls the reusable Docker workflow and requires both app and worker image builds through the `pr-required` aggregate. Build, migration, safety/RAG, and release behavior remain independently scoped. ## Contribution checklist (UI changes) diff --git a/package.json b/package.json index c45728f0e6..29259d4f64 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "clean:worktree": "node scripts/clean-worktree.mjs", "verify:preflight": "npm run check:installed-lock-parity && npm run typecheck && npm run verify:cheap && npm run clean:worktree", "verify:cheap": "npm run verify:cheap:internal", - "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run sitemap:check && npm run docs:check-index && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", + "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:gitleaks-pinned && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run sitemap:check && npm run docs:check-index && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:phone-chrome": "node scripts/verify-phone-chrome.mjs", "audit:final-merge": "node scripts/final-merge-audit.mjs", @@ -62,6 +62,7 @@ "check:knip:exports": "knip --no-progress --exports --no-exit-code", "check:maintainability-budgets": "node scripts/check-maintainability-budgets.mjs", "check:ci-scope": "node scripts/ci-change-scope.mjs --self-test", + "check:gitleaks-pinned": "node scripts/run-gitleaks-pinned.mjs --self-test", "check:ci-triage": "node scripts/ci-triage.mjs --self-test", "check:gate-manifest": "node scripts/check-gate-manifest.mjs", "check:branch-review-ledger": "node scripts/check-branch-review-ledger.mjs --self-test && node scripts/branch-review-ledger.mjs --self-test && node scripts/merge-branch-review-ledger.mjs --self-test && node scripts/check-branch-review-ledger.mjs", diff --git a/scripts/ci-change-scope.mjs b/scripts/ci-change-scope.mjs index 24a20ba3cb..9a466e3cff 100644 --- a/scripts/ci-change-scope.mjs +++ b/scripts/ci-change-scope.mjs @@ -6,6 +6,9 @@ const zeroSha = /^0{40}$/; const fullRunSentinelFiles = [ "src/app/api/answer/__ci_full_run__.ts", + // UI sentinel must stay outside src/app/api/** once API routes are excluded + // from ui_changed (otherwise schedule/full-run would skip Production UI). + "src/components/__ci_full_run__.tsx", "supabase/__ci_full_run__.sql", "Dockerfile", ".github/workflows/codex-autofix-review-comments.yml", @@ -55,6 +58,12 @@ function pathMatches(filePath, patterns) { }); } +/** App Router API handlers are not browser journeys — keep them out of ui_changed. */ +function isUiChangedPath(filePath) { + if (filePath === "src/app/api" || filePath.startsWith("src/app/api/")) return false; + return pathMatches(filePath, uiPatterns); +} + const docPatterns = [ "docs", "mockups", @@ -73,7 +82,7 @@ const workflowPatterns = [ "AGENTS.md", "docs/codex-review-protocol.md", "docs/process-hardening.md", - /^scripts\/(?:ci-change-scope|ci-triage|pr-policy|verify-pr-local|eval-rag-offline|check-github-action-pins|check-codex-autofix-workflow|productivity-core|productivity-workflow|external-workflow)\.mjs$/, + /^scripts\/(?:ci-change-scope|ci-triage|pr-policy|verify-pr-local|eval-rag-offline|run-gitleaks-pinned|check-github-action-pins|check-codex-autofix-workflow|productivity-core|productivity-workflow|external-workflow)\.mjs$/, ]; const codexAutofixPatterns = [ @@ -100,32 +109,21 @@ const uiPatterns = [ /^scripts\/(run-playwright|playwright-base-url)\.(?:mjs|ts)$/, ]; +// Migration replay validates schema/SQL tooling, not every API handler. API +// route edits stay covered by unit/coverage (+ RAG offline when rag-scoped). const dbPatterns = [ "supabase", "src/lib/supabase", - "src/app/api/answer", - "src/app/api/differentials", - "src/app/api/documents", - "src/app/api/eval-cases", - "src/app/api/health", - "src/app/api/images", - "src/app/api/ingestion", - "src/app/api/jobs", - "src/app/api/medications", - "src/app/api/registry", - "src/app/api/search", - "src/app/api/setup-status", - "src/app/api/upload", "docs/database-drift-detection.md", "docs/supabase-migration-reconciliation.md", /^scripts\/(check-drift|generate-drift-manifest|check-m13-migration|check-retrieval-owner-migration|check-supabase-project|audit-tables|reindex|reindex-health|cleanup-abandoned-reindex-generations)\.ts$/, /^tests\/(supabase|drift|private-rag|private-access|retrieval-owner).*\.test\.ts$/, ]; -// NOTE: rag_eval_changed is an ADVISORY narrowing signal only. The clinical -// offline-grounding gate (eval:rag:offline) runs for every non-docs change in -// both CI (.github/workflows/ci.yml) and local verify:pr-local, so a new -// retrieval file that falls outside these patterns can never silently skip it. +// rag_eval_changed selects the heavier offline RAG contract (eval:rag:offline). +// Fixture validation (check:rag:fixtures) still runs for every non-docs change +// in CI safety + verify:pr-local so a retrieval file outside these patterns +// cannot silently skip fixture checks. const ragEvalPatterns = [ "scripts/fixtures", "src/app/api/answer", @@ -194,7 +192,7 @@ function classify(files) { // change. Narrower signals still scope build/UI/database work, but must not // leave runtime, worker, or configuration changes without unit coverage. const coverageChanged = normalized.some((file) => !pathMatches(file, docPatterns)); - const uiChanged = normalized.some((file) => pathMatches(file, uiPatterns)); + const uiChanged = normalized.some((file) => isUiChangedPath(file)); const dbChanged = normalized.some((file) => pathMatches(file, dbPatterns)); const containerChanged = normalized.some((file) => pathMatches(file, containerPatterns)); const ragEvalChanged = normalized.some((file) => pathMatches(file, ragEvalPatterns)); @@ -456,8 +454,25 @@ function selfTest() { { rag_eval_changed: true, source_changed: true, + // API handlers alone must not pull Chromium or migration replay. + ui_changed: true, // answer-content.tsx is UI + db_changed: false, }, ); + assertScope("api-only-skips-ui-and-db", ["src/app/api/answer/route.ts"], { + source_changed: true, + coverage_changed: true, + rag_eval_changed: true, + build_changed: true, + ui_changed: false, + db_changed: false, + }); + assertScope("app-page-keeps-ui", ["src/app/(search-app)/page.tsx"], { + source_changed: true, + ui_changed: true, + build_changed: true, + db_changed: false, + }); assertScope("rag-fixture", ["src/lib/retrieval-selection.ts", "scripts/fixtures/rag-retrieval-golden.json"], { rag_eval_changed: true, source_changed: true, @@ -475,15 +490,31 @@ function selfTest() { coverage_changed: true, docs_only: false, }); - assertScope("database-access", ["src/app/api/documents/route.ts"], { - db_changed: true, + assertScope("database-access-api-no-longer-trips-migration", ["src/app/api/documents/route.ts"], { + db_changed: false, source_changed: true, + coverage_changed: true, + build_changed: true, }); + assertScope( + "database-schema-trips-migration", + ["supabase/migrations/20260710000000_example.sql", "src/lib/supabase/server.ts"], + { + db_changed: true, + source_changed: true, + }, + ); assertScope("workflow", [".github/workflows/ci.yml", "docs/process-hardening.md"], { workflow_changed: true, docs_only: false, build_changed: false, }); + assertScope("gitleaks-pin-script", ["scripts/run-gitleaks-pinned.mjs"], { + workflow_changed: true, + source_changed: true, + docs_only: false, + build_changed: false, + }); assertScope("repo-skill", [".agents/skills/database-flightplan/SKILL.md"], { workflow_changed: true, source_changed: false, diff --git a/scripts/run-gitleaks-pinned.mjs b/scripts/run-gitleaks-pinned.mjs new file mode 100644 index 0000000000..b47ae33ff1 --- /dev/null +++ b/scripts/run-gitleaks-pinned.mjs @@ -0,0 +1,145 @@ +#!/usr/bin/env node +/** + * Pin Gitleaks to the workflow event's base/head SHAs and the checked-out commit. + * + * The stock gitleaks-action re-queries the PR commits API mid-run; a concurrent + * push can move the tip so the scan range is not in the workspace (#097). + * Event payload SHAs are immutable for the run — use those, then verify HEAD. + */ +import { readFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { childProcessExitCode } from "./child-process-result.mjs"; + +const zeroSha = /^0{40}$/; + +/** Keep in lockstep with `.github/workflows/secret-scan.yml` env pins. */ +export const PINNED_GITLEAKS_LINUX_X64 = { + version: "8.24.3", + sha256: "9991e0b2903da4c8f6122b5c3186448b927a5da4deef1fe45271c3793f4ee29c", +}; + +export function resolveGitleaksScanRange({ eventName, pinnedBase, pinnedHead, checkedOutHead }) { + if (!pinnedHead || !checkedOutHead) { + throw new Error("pinnedHead and checkedOutHead are required for a pinned Gitleaks scan."); + } + if (pinnedHead !== checkedOutHead) { + throw new Error( + `Checked-out HEAD ${checkedOutHead} does not match pinned event head ${pinnedHead}. ` + + "Refuse to scan an unstable tip (issue #097).", + ); + } + + const base = typeof pinnedBase === "string" ? pinnedBase.trim() : ""; + const useRange = + (eventName === "pull_request" || eventName === "pull_request_target" || eventName === "push") && + base.length > 0 && + !zeroSha.test(base); + + if (useRange) { + return { mode: "range", base, head: pinnedHead, logOpts: `${base}..${pinnedHead}` }; + } + + // schedule / workflow_dispatch / unreachable before-sha: scan the tip commit only. + return { mode: "tip", base: null, head: pinnedHead, logOpts: "-1" }; +} + +function selfTest() { + const head = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const base = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + + const pr = resolveGitleaksScanRange({ + eventName: "pull_request", + pinnedBase: base, + pinnedHead: head, + checkedOutHead: head, + }); + if (pr.mode !== "range" || pr.logOpts !== `${base}..${head}`) { + throw new Error(`expected PR range scan, got ${JSON.stringify(pr)}`); + } + + let failed = false; + try { + resolveGitleaksScanRange({ + eventName: "pull_request", + pinnedBase: base, + pinnedHead: head, + checkedOutHead: "cccccccccccccccccccccccccccccccccccccccc", + }); + } catch { + failed = true; + } + if (!failed) throw new Error("expected mismatch between pinned head and checkout to throw"); + + const schedule = resolveGitleaksScanRange({ + eventName: "schedule", + pinnedBase: "", + pinnedHead: head, + checkedOutHead: head, + }); + if (schedule.mode !== "tip" || schedule.logOpts !== "-1") { + throw new Error(`expected tip scan for schedule, got ${JSON.stringify(schedule)}`); + } + + const zeroBefore = resolveGitleaksScanRange({ + eventName: "push", + pinnedBase: "0000000000000000000000000000000000000000", + pinnedHead: head, + checkedOutHead: head, + }); + if (zeroBefore.mode !== "tip") { + throw new Error(`expected tip scan for zero before-sha, got ${JSON.stringify(zeroBefore)}`); + } + + const workflowPath = join(dirname(fileURLToPath(import.meta.url)), "..", ".github", "workflows", "secret-scan.yml"); + const workflow = readFileSync(workflowPath, "utf8"); + if (!workflow.includes(`GITLEAKS_VERSION: "${PINNED_GITLEAKS_LINUX_X64.version}"`)) { + throw new Error(`secret-scan.yml must pin GITLEAKS_VERSION to ${PINNED_GITLEAKS_LINUX_X64.version}`); + } + if (!workflow.includes(`GITLEAKS_LINUX_X64_SHA256: "${PINNED_GITLEAKS_LINUX_X64.sha256}"`)) { + throw new Error("secret-scan.yml must pin GITLEAKS_LINUX_X64_SHA256 to the release checksum"); + } + if (!workflow.includes("sha256sum -c -")) { + throw new Error("secret-scan.yml must verify the Gitleaks archive with sha256sum before install"); + } + + console.log("Pinned Gitleaks range self-test passed."); +} + +function runGit(args) { + const result = spawnSync("git", args, { encoding: "utf8" }); + if (childProcessExitCode(result) !== 0) { + throw new Error(`git ${args.join(" ")} failed: ${result.stderr || result.stdout}`); + } + return result.stdout.trim(); +} + +function main(argv) { + if (argv.includes("--self-test")) { + selfTest(); + return; + } + + const eventName = process.env.GITHUB_EVENT_NAME || ""; + const pinnedBase = process.env.GITLEAKS_PINNED_BASE || ""; + const pinnedHead = process.env.GITLEAKS_PINNED_HEAD || ""; + const gitleaksBin = process.env.GITLEAKS_BIN || "gitleaks"; + const checkedOutHead = runGit(["rev-parse", "HEAD"]); + const range = resolveGitleaksScanRange({ + eventName, + pinnedBase, + pinnedHead, + checkedOutHead, + }); + + console.log(`Pinned Gitleaks scan mode=${range.mode} log-opts=${range.logOpts}`); + const args = ["detect", "--source=.", `--log-opts=${range.logOpts}`, "--redact", "--verbose", "--exit-code=1"]; + const result = spawnSync(gitleaksBin, args, { stdio: "inherit" }); + process.exit(childProcessExitCode(result)); +} + +const invokedDirectly = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href; +if (invokedDirectly) { + main(process.argv.slice(2)); +} diff --git a/scripts/verify-pr-local.mjs b/scripts/verify-pr-local.mjs index a9970244f7..3a255a9733 100644 --- a/scripts/verify-pr-local.mjs +++ b/scripts/verify-pr-local.mjs @@ -67,8 +67,10 @@ function readScope(files) { function selectedScripts(scope, extended) { const scripts = [...baseScripts]; if (scope.build_changed) scripts.push("build"); - // Full unit testing already includes every offline RAG contract suite. - if (!scope.docs_only) scripts.push("check:rag:fixtures"); + // Fixtures for every non-docs change; full offline RAG contracts when + // retrieval/answer surfaces are in scope (eval:rag:offline includes fixtures). + if (scope.rag_eval_changed) scripts.push("eval:rag:offline"); + else if (!scope.docs_only) scripts.push("check:rag:fixtures"); if (extended && scope.ui_changed) scripts.push("verify:ui"); return scripts; } @@ -82,7 +84,9 @@ if (options.dryRun) { console.log("\nPR-local verification plan (dry run):"); for (const script of scripts) console.log(`- npm run ${script}`); if (!scope.build_changed) console.log("- build skipped: no build-affecting changes detected"); - if (scope.docs_only) console.log("- offline RAG fixture validation skipped: docs-only change"); + if (scope.docs_only) console.log("- offline RAG checks skipped: docs-only change"); + else if (!scope.rag_eval_changed) + console.log("- offline RAG production contracts skipped: no RAG-scoped changes (fixtures still selected)"); if (options.extended && !scope.ui_changed) console.log("- Chromium UI gate skipped: no UI-affecting changes detected"); process.exit(0); diff --git a/tests/ci-cache-safety.test.ts b/tests/ci-cache-safety.test.ts index 4765ffebe9..3a97913956 100644 --- a/tests/ci-cache-safety.test.ts +++ b/tests/ci-cache-safety.test.ts @@ -75,6 +75,8 @@ describe("PR required aggregate — cancelled vs failed (#095)", () => { COVERAGE_RESULT: "skipped", BUILD_RESULT: "skipped", CONTAINER_RESULT: "skipped", + // Critical-first UI job (this PR); skipped when ui_changed is false. + UI_FAST_RESULT: "skipped", UI_RESULT: "skipped", DB_RESULT: "skipped", }; @@ -82,7 +84,7 @@ describe("PR required aggregate — cancelled vs failed (#095)", () => { function runAggregate(overrides: Record = {}) { const result = spawnSync("bash", ["-c", script], { // process.env is spread because this repo augments ProcessEnv with required keys, so a - // bare object does not typecheck. All fifteen variables the script reads are overridden + // bare object does not typecheck. All sixteen variables the script reads are overridden // below, and it runs under `set -u`, so the ambient environment cannot change the outcome. env: { ...process.env, ...allGreen, ...overrides }, encoding: "utf8", @@ -173,7 +175,20 @@ describe("PR required aggregate — cancelled vs failed (#095)", () => { } expect(runAggregate({ DOCS_ONLY: "false", SAFETY_RESULT: "cancelled" }).status).not.toBe(0); expect(runAggregate({ COVERAGE_CHANGED: "true", COVERAGE_RESULT: "cancelled" }).status).not.toBe(0); - expect(runAggregate({ UI_CHANGED: "true", UI_RESULT: "cancelled" }).status).not.toBe(0); + expect( + runAggregate({ + UI_CHANGED: "true", + UI_FAST_RESULT: "success", + UI_RESULT: "cancelled", + }).status, + ).not.toBe(0); + expect( + runAggregate({ + UI_CHANGED: "true", + UI_FAST_RESULT: "cancelled", + UI_RESULT: "success", + }).status, + ).not.toBe(0); }); it("keeps `if: always()`, since a skipped required check counts as passing", () => { diff --git a/tests/verify-pr-local.test.ts b/tests/verify-pr-local.test.ts index a52c8017f1..7c056fa21c 100644 --- a/tests/verify-pr-local.test.ts +++ b/tests/verify-pr-local.test.ts @@ -34,12 +34,21 @@ describe("verify-pr-local CLI", () => { expect(output).not.toContain("\n> npm run "); }); - it("selects build, offline RAG, and extended UI checks for affected source", () => { + it("selects build and offline RAG contracts for API answer routes without UI", () => { const output = dryRun("src/app/api/answer/stream/route.ts", "--extended"); expect(output).toContain("- npm run build"); - expect(output).toContain("- npm run check:rag:fixtures"); - expect(output).not.toContain("- npm run eval:rag:offline"); + expect(output).toContain("- npm run eval:rag:offline"); + expect(output).not.toContain("- npm run check:rag:fixtures"); + expect(output).toContain("- Chromium UI gate skipped: no UI-affecting changes detected"); + expect(output).not.toContain("- npm run verify:ui"); + }); + + it("selects extended UI checks for component changes", () => { + const output = dryRun("src/components/clinical-dashboard/answer-content.tsx", "--extended"); + + expect(output).toContain("- npm run build"); + expect(output).toContain("- npm run eval:rag:offline"); expect(output).toContain("- npm run verify:ui"); });