From 4983a7f3123c3f5c5defacd409cec22674193d1a Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:28:48 +0800 Subject: [PATCH 1/2] ci: harden pull request policy Add trusted PR evidence validation and exact default-branch CI failure attribution. Document and self-test the safer review workflow. --- .github/pull_request_template.md | 10 ++ .github/workflows/ci-triage.yml | 60 ++++----- .github/workflows/ci.yml | 6 + .github/workflows/pr-policy.yml | 74 +++++++++++ docs/branch-review-ledger.md | 2 + docs/codex-review-protocol.md | 1 + docs/process-hardening.md | 20 +++ package.json | 4 +- scripts/ci-change-scope.mjs | 2 +- scripts/ci-triage.mjs | 115 ++++++++++++++++++ scripts/pr-policy.mjs | 202 +++++++++++++++++++++++++++++++ 11 files changed, 465 insertions(+), 31 deletions(-) create mode 100644 .github/workflows/pr-policy.yml create mode 100644 scripts/ci-triage.mjs create mode 100644 scripts/pr-policy.mjs diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 8dda12c6e..c18ca110d 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -18,6 +18,16 @@ For retrieval, ranking, selection, chunking, source/citation rendering, or answe - [ ] `npm run check:production-readiness` when clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changed - [ ] `npm run check:deployment-readiness` when deployment startup, hosting, or rollout behavior changed + + +## Risk and rollout + +Complete this section for clinical, data, API, auth/privacy, workflow, dependency, build, or deployment changes. + +- Risk: +- Rollback: +- Provider or production effects: None / describe the explicitly authorized effect + ## Clinical Governance Preflight Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output. diff --git a/.github/workflows/ci-triage.yml b/.github/workflows/ci-triage.yml index f9b3ab35b..6366a0acc 100644 --- a/.github/workflows/ci-triage.yml +++ b/.github/workflows/ci-triage.yml @@ -1,7 +1,8 @@ # CI failure triage. When CI fails on a pull request, post ONE comment that # classifies the failure so attention is not spent on failures that are not the # author's fault: -# - main-side: the same job is also failing on the latest default-branch run +# - main-side: the same job is also failing on the latest completed run of the +# same workflow on the default branch # (CI merges the PR branch with current main, so a main regression surfaces on # every open PR — this has cost debugging time before). # - needs investigation: everything else. @@ -35,15 +36,24 @@ jobs: github.event.workflow_run.conclusion == 'failure' && github.event.workflow_run.event == 'pull_request' steps: - - name: Checkout default branch (trusted) for the flake ledger + - name: Checkout default branch (trusted) for triage policy uses: actions/checkout@9f698171ed81b15d1823a05fc7211befd50c8ae0 # v6.0.3 with: + ref: ${{ github.event.repository.default_branch }} persist-credentials: false - name: Post triage comment uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: script: | + const { pathToFileURL } = require("node:url"); + const moduleUrl = pathToFileURL(`${process.env.GITHUB_WORKSPACE}/scripts/ci-triage.mjs`).href; + const { + buildTriageBody, + classifyFailedJobs, + failedJobNames, + selectLatestDefaultBranchRun, + } = await import(moduleUrl); const run = context.payload.workflow_run; // Resolve the PR for this run (empty for fork PRs → skip quietly). @@ -60,55 +70,47 @@ jobs: run_id: run.id, per_page: 100, }); - const failed = jobs.filter((j) => j.conclusion === "failure").map((j) => j.name); + const failed = failedJobNames(jobs); if (failed.length === 0) { core.info("No failed jobs found; skipping."); return; } - // Is the same job failing on the latest default-branch CI run? → main-side. - let mainFailingJobs = new Set(); + // Compare only with the latest completed run of this exact workflow + // on the default branch. A repository-wide per_page=1 lookup can + // accidentally select a different workflow and misattribute a PR failure. + let mainRun; + let mainFailingJobs = []; try { - const { data: mainRuns } = await github.rest.actions.listWorkflowRunsForRepo({ + const { data: mainRuns } = await github.rest.actions.listWorkflowRuns({ owner: context.repo.owner, repo: context.repo.repo, + workflow_id: run.workflow_id, event: "push", branch: context.payload.repository.default_branch, - per_page: 1, + status: "completed", + per_page: 10, }); - // reuse: find the CI workflow's latest main run - const latestMain = (mainRuns.workflow_runs || []).find((r) => r.name === run.name); - if (latestMain && latestMain.conclusion === "failure") { + mainRun = selectLatestDefaultBranchRun(mainRuns.workflow_runs, { + currentRunId: run.id, + defaultBranch: context.payload.repository.default_branch, + }); + if (mainRun?.conclusion === "failure") { const mainJobs = await github.paginate(github.rest.actions.listJobsForWorkflowRun, { owner: context.repo.owner, repo: context.repo.repo, - run_id: latestMain.id, + run_id: mainRun.id, per_page: 100, }); - mainFailingJobs = new Set(mainJobs.filter((j) => j.conclusion === "failure").map((j) => j.name)); + mainFailingJobs = failedJobNames(mainJobs); } } catch (e) { core.info(`main-side check skipped: ${e.message}`); } - const looksUi = (name) => /ui|playwright|browser|e2e/i.test(name); - - const lines = failed.map((name) => { - if (mainFailingJobs.has(name)) return `- \`${name}\` — **main-side**: also failing on the latest \`main\` run, likely not your change.`; - if (looksUi(name)) return `- \`${name}\` — **needs investigation**: use the uploaded JUnit classification and trace; job type alone is not evidence of a known flake.`; - return `- \`${name}\` — **needs investigation**.`; - }); - + const classifications = classifyFailedJobs(failed, mainRun, mainFailingJobs); + const body = buildTriageBody(classifications, mainRun); const marker = ""; - const body = [ - marker, - `### CI triage`, - `CI failed on this PR. Automated classification of the ${failed.length} failed job(s):`, - "", - ...lines, - "", - "_Heuristic only — a main-side or flake label is a starting point, not a verdict._", - ].join("\n"); // Replace a prior triage comment instead of stacking. const { data: comments } = await github.rest.issues.listComments({ diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f2e21311a..d02ec6290 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -82,6 +82,12 @@ jobs: - name: CI scope self-test run: npm run check:ci-scope + - name: CI triage self-test + run: npm run check:ci-triage + + - name: PR policy self-test + run: npm run check:pr-policy + - name: Format check run: npm run format:check diff --git a/.github/workflows/pr-policy.yml b/.github/workflows/pr-policy.yml new file mode 100644 index 000000000..1c2532d4f --- /dev/null +++ b/.github/workflows/pr-policy.yml @@ -0,0 +1,74 @@ +name: PR Policy + +on: + pull_request_target: + branches: [main] + types: [opened, edited, synchronize, reopened, ready_for_review, labeled, unlabeled] + merge_group: + +concurrency: + group: pr-policy-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: read + +jobs: + policy: + name: PR policy + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + - name: Accept merge queue validation + if: github.event_name == 'merge_group' + run: echo "PR metadata was validated before merge-queue entry." + + # pull_request_target runs trusted base-branch code. Pin the checkout to + # the PR's exact base SHA and never execute the PR head or persist credentials. + - name: Checkout trusted policy + if: github.event_name == 'pull_request_target' + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + + - name: Validate pull request evidence + if: github.event_name == 'pull_request_target' + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const { pathToFileURL } = require("node:url"); + const moduleUrl = pathToFileURL(`${process.env.GITHUB_WORKSPACE}/scripts/pr-policy.mjs`).href; + const { evaluatePullRequestPolicy } = await import(moduleUrl); + const pr = context.payload.pull_request; + + if (pr.draft) { + core.notice("Draft PR: metadata policy will be enforced when the PR is marked ready for review."); + return; + } + + const changedFiles = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number, + per_page: 100, + }); + const result = evaluatePullRequestPolicy({ + title: pr.title, + body: pr.body || "", + headRef: pr.head.ref, + files: changedFiles.map((file) => file.filename), + }); + + await core.summary + .addHeading("PR policy") + .addRaw(`Clinical risk: ${result.classification.clinicalRisk ? "yes" : "no"}
`) + .addRaw(`Operational risk: ${result.classification.operationalRisk ? "yes" : "no"}
`) + .addRaw(`UI change: ${result.classification.ui ? "yes" : "no"}
`) + .write(); + + if (!result.ok) { + for (const error of result.errors) core.error(error); + core.setFailed(`PR policy found ${result.errors.length} issue(s). Edit the PR title/body and rerun.`); + } diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index afd92b10d..96310305a 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -572,3 +572,5 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-17 | codex/chat-forms-import-6914 | e5caaa46cad5fb9a937f1dc43312723799b98abb + working-tree diff | shared Forms/Services catalogue access and LOCAL_NO_AUTH_OWNER_ID review | Fixed one P1 availability/design defect: authenticated reads materialized a private copy of the shared catalogue on first access, creating drift, unnecessary writes, and possible registry-corpus side effects. Forms/Services now merge the reviewed shared baseline with private owner overrides for list, detail, and universal search; older partial overrides retain missing shared metadata; no registry GET seeds or embeds. Private rows and linked documents remain owner-scoped. The ignored local owner setting was corrected to the verified sole live-owner UUID and source validation now requires a UUID. No remaining high-confidence defect in scope. | Focused registry/universal Vitest initially exposed four local expectation/count mismatches; corrected registry suite passed 17/17 and registry/logging suite passed 23/23. Full TypeScript passed. `verify:cheap` passed runtime, action pins, sitemap, brand, type/icon/function guards, full lint, and TypeScript; full Vitest reached 2,588 passing with one stale logging-guard failure, which was fixed and focused-verified. The final full-suite rerun was terminated by the 5-minute host timeout without a reported assertion failure. `git diff --check` passed. No Supabase/OpenAI/provider call or schema/RLS mutation was run for this review. | | 2026-07-17 | codex/scrolling-cleanup-20260717 | ff77cd06c + latest origin/main sync | cross-page scrolling and interaction stability review | Fixed two confirmed P2 defects: desktop action-popup placement performed synchronous geometry work for every captured scroll event, and submitted differential searches with zero document matches fell back to the home state. Placement is now coalesced per animation frame with passive scroll listeners, and submitted empty-evidence results remain visible. Hardened three popup/navigation browser helpers that reproduced hydration timing failures. No other high-confidence defect remains in the scoped diff. | Scroll-focused Chromium 28/28; final affected Chromium 5/5; source regressions 2/2; scoped ESLint; Prettier; TypeScript and production build passed before the final upstream-only sync; `git diff --check`. The aggregate local Vitest/UI runs were affected by concurrent-worktree resource contention, so exact-head hosted CI remains required before merge. No Supabase/OpenAI/live-provider checks run. | | 2026-07-17 | PR #713 / codex/chat-workflow-ideas-0916 | b52112df6aa36311d7420189064acd79dcf2c3f5 + reviewed follow-up diff | workflow toolkit review follow-up | Fixed all 14 actionable Codex and CodeRabbit threads: cross-platform path fixtures, installation-managed preflight guidance, complete Supabase-backed API database scoping, per-command approval boundaries, plugin-ignore narrowing, isolated CI-scope proof, remote-Git command guarding, repository-skill verification classification, `TypeError` diagnosis, strict CLI option values, machine-parseable JSON evidence output, and preservation of baseline database/clinical approval gates in the RAG lab. No unresolved actionable finding remains in the reviewed scope. | `npm run verify:cheap` passed with 273 files and 2,599 tests; focused toolkit Vitest 20/20; CI-scope self-test; plugin-ignore proof; `git diff --check`. Exact-head hosted CI remains required after the follow-up push. No Supabase, OpenAI, or other live product-provider command was run. | +| 2026-07-17 | PR #699 / codex/test-reliability-hardening | 6202835ab1cf3703af311b9afdf372f74c63e040 | branch-cleanup-superseded | Closed as fully superseded by merged PR #705 (`e5caaa46c`). Range-diff maps the original implementation commit to #705's first integration commit; #705 then adds seven focused reliability fixes, while #699's remaining commit is merge-only and contributes no unique relevant patch. The exact-SHA remote ref and the unregistered local predecessor ref (`b518c1de9`) were deleted after final rechecks. | Fresh GitHub PR/head/status inventory; exact `ls-remote` and local-ref checks; cherry-pick-aware log; range-diff against PR #705's merged head; merge-parent verification; exact leased remote deletion and exact-old-value local `update-ref` deletion. No Supabase/OpenAI/product-provider checks run. | +| 2026-07-17 | codex/pr-process-hardening-20260717 | 220de891f10f82df11eb2e8137367514c139a206 + reviewed working diff | PR metadata, CI triage, review routing, Actions permissions, secret scanning, and branch-hygiene hardening | No remaining high-confidence P0-P2 defect in the reviewed process diff. Added a trusted base-SHA PR metadata/risk policy with draft and merge-queue handling; corrected CI triage to compare only the same workflow's latest completed `main` run; wired self-tests into local/hosted gates; documented the policy; created the missing durable Codex routing labels; removed four unused per-head routing labels; and applied read-only default Actions tokens, immutable Action pinning, no Actions-authored approvals, push protection, and automatic merged-branch deletion. One review defect was fixed before handoff: API-only `src/app/api/**` changes no longer incorrectly require UI verification. | Offline `check:pr-policy`, `check:ci-triage`, `check:ci-scope`, action-pin check, scoped Prettier, and `git diff --check` passed. Broader local gates were deferred because other registered worktrees repeatedly held the heavyweight lock; exact-head hosted CI remains required. GitHub provider inspection/settings changes were explicitly authorized. No Supabase/OpenAI/product-provider command ran. | diff --git a/docs/codex-review-protocol.md b/docs/codex-review-protocol.md index eecff9612..fda9de1db 100644 --- a/docs/codex-review-protocol.md +++ b/docs/codex-review-protocol.md @@ -11,6 +11,7 @@ Use this protocol for every Codex review, audit, bug hunt, PR review, release-re - Before branch or PR review, check `docs/branch-review-ledger.md`: resolve the target with `git rev-parse`, compare the HEAD and scope, and skip unchanged completed reviews unless the user asks for a fresh pass. - Treat GitHub automatic review as one pass per pull request. A repair commit or later head does not authorize another automatic pass; require an explicit human request before re-reviewing. - Route automatic repair only for high-risk paths, at least 10 changed non-test source files, at least 300 changed non-test source lines, or an explicit `codex-review` label. `skip-codex-review` always opts out, including when both labels are present. Small low-risk, docs-only, test-only, and generated-only changes should not receive the automatic repair request. +- Ready PRs must pass the trusted `PR policy` metadata check. It reads only the base-branch policy implementation, never executes PR code, and requires concrete verification plus risk/rollback evidence for high-risk changes. ## Review Output diff --git a/docs/process-hardening.md b/docs/process-hardening.md index 5ef94eb9f..5a996d03d 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -351,6 +351,26 @@ the durable index for the tooling; `docs/operator-backlog.md` tracks the human-o as main-side or needs-investigation. Inert until repo var `CI_TRIAGE_ENABLED=true` (now set). UI jobs use their uploaded JUnit classification and trace; job names alone never produce a known-flake verdict. The workflow reads only trusted default-branch job metadata and never runs PR code. +- **PR metadata policy** (`.github/workflows/pr-policy.yml`, `scripts/pr-policy.mjs`): ready PRs to `main` + must use an outcome-focused title, complete Summary and Verification evidence, and provide risk/rollback + evidence for clinical or operationally sensitive paths. UI changes require `verify:ui` evidence (or an + explicit reason it could not run), while clinical-risk changes must fully disposition the governance + checklist. The `pull_request_target` job checks out the exact base SHA, has read-only permissions, and + never executes PR-head code. Drafts remain non-blocking until marked ready; merge-queue runs emit the + same stable `PR policy` check name. +- **Default-branch failure attribution** (`scripts/ci-triage.mjs`): triage now compares a failed PR only + with the latest completed run of the same workflow on `main`. It no longer samples the latest arbitrary + repository workflow, which could incorrectly label a PR failure as main-side. A main-side label remains + routing evidence only; it never suppresses the required failure. +- **Repository permission baseline (applied 2026-07-17):** Actions receive read-only tokens by default, + cannot approve pull requests, and must reference external actions by immutable SHA. Workflows that post + issues/comments retain narrow explicit permissions. Secret-scanning push protection is enabled and + merged branches are deleted automatically. Non-provider pattern scanning and credential-validity checks + remain unavailable for this user-owned repository/plan, so the ordinary secret scan, push protection, + Gitleaks check, and local secret-surface guards remain the active layers. +- **Review-routing labels (applied 2026-07-17):** `codex-review` is the explicit opt-in for a normally + low-risk PR, and `skip-codex-review` is the unconditional opt-out. Four obsolete per-head `codex-ar-*` + labels from the retired routing mechanism were removed after confirming no open PR used them. - **Repo hygiene:** `check:env-parity` (env-var NAME reconciliation across `env.ts`, `check-ci-env.mjs`, and — opt-in, names-only — `gh secret list` / Railway) and `sweep:branch-ledger` (report-only branch inventory, cherry-pick-aware). diff --git a/package.json b/package.json index df1c29c3d..a285a076e 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "test:e2e:visual": "node scripts/run-playwright.mjs --config=playwright.visual.config.ts", "test:cross-tenant:staging": "node scripts/run-tsx.mjs scripts/test-cross-tenant-staging.ts", "verify:cheap": "node scripts/run-heavy.mjs --npm-script verify:cheap:internal", - "verify:cheap:internal": "npm run check:runtime && npm run check:github-actions && npm run sitemap:check && npm run brand:check && npm run check:type-scale && npm run check:icon-scale && npm run check:function-grants && npm run lint && npm run typecheck && npm run test", + "verify:cheap:internal": "npm run check:runtime && npm run check:github-actions && npm run check:ci-scope && npm run check:ci-triage && npm run check:pr-policy && npm run sitemap:check && npm run brand:check && npm run check:type-scale && npm run check:icon-scale && npm run check:function-grants && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:ui": "npm run check:runtime && npm run test:e2e:pr", "verify:release": "npm run check:runtime && npm run lint && npm run typecheck && npm run test && npm run build && npm run test:e2e && npm run check:production-readiness && npm run governance:release && npm run eval:quality:release", @@ -53,6 +53,8 @@ "check:client-bundle-secrets": "node scripts/check-client-bundle-secrets.mjs", "check:knip": "knip", "check:ci-scope": "node scripts/ci-change-scope.mjs --self-test", + "check:ci-triage": "node scripts/ci-triage.mjs --self-test", + "check:pr-policy": "node scripts/pr-policy.mjs --self-test", "check:env-parity": "node scripts/check-env-parity.mjs", "sweep:branch-ledger": "node scripts/sweep-branch-ledger.mjs", "docs:check-links": "node scripts/check-docs-links.mjs", diff --git a/scripts/ci-change-scope.mjs b/scripts/ci-change-scope.mjs index 10cee0418..2014a8e10 100644 --- a/scripts/ci-change-scope.mjs +++ b/scripts/ci-change-scope.mjs @@ -73,7 +73,7 @@ const workflowPatterns = [ "AGENTS.md", "docs/codex-review-protocol.md", "docs/process-hardening.md", - /^scripts\/(?:ci-change-scope|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|check-github-action-pins|check-codex-autofix-workflow|productivity-core|productivity-workflow|external-workflow)\.mjs$/, ]; const codexAutofixPatterns = [ diff --git a/scripts/ci-triage.mjs b/scripts/ci-triage.mjs new file mode 100644 index 000000000..228b31c52 --- /dev/null +++ b/scripts/ci-triage.mjs @@ -0,0 +1,115 @@ +#!/usr/bin/env node +import assert from "node:assert/strict"; +import { pathToFileURL } from "node:url"; + +export function failedJobNames(jobs) { + return [ + ...new Set( + (jobs ?? []) + .filter((job) => job.conclusion === "failure") + .map((job) => job.name) + .filter(Boolean), + ), + ]; +} + +export function selectLatestDefaultBranchRun(runs, { currentRunId, defaultBranch }) { + return (runs ?? []) + .filter( + (run) => + run.id !== currentRunId && + run.event === "push" && + run.head_branch === defaultBranch && + run.status === "completed" && + Boolean(run.conclusion), + ) + .sort( + (left, right) => + Date.parse(right.run_started_at ?? right.created_at) - Date.parse(left.run_started_at ?? left.created_at), + )[0]; +} + +export function classifyFailedJobs(failedNames, mainRun, mainFailedNames) { + const mainFailures = new Set(mainRun?.conclusion === "failure" ? mainFailedNames : []); + return failedNames.map((name) => ({ + name, + classification: mainFailures.has(name) ? "main-side" : "needs-investigation", + })); +} + +export function buildTriageBody(classifications, mainRun) { + const marker = ""; + const lines = classifications.map(({ name, classification }) => + classification === "main-side" + ? `- \`${name}\` — **main-side**: the same job also failed on the latest completed \`main\` CI run.` + : `- \`${name}\` — **needs investigation**: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.`, + ); + const baseline = mainRun + ? `Compared with main CI run [#${mainRun.run_number}](${mainRun.html_url}) (${mainRun.conclusion}).` + : "No completed main CI baseline was available; no failure was labeled main-side."; + return [ + marker, + "### CI triage", + `CI failed on this PR. Automated classification of the ${classifications.length} failed job(s):`, + "", + ...lines, + "", + baseline, + "", + "_Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger._", + ].join("\n"); +} + +function selfTest() { + const runs = [ + { + id: 2, + event: "push", + head_branch: "main", + status: "completed", + conclusion: "failure", + run_started_at: "2026-07-17T02:00:00Z", + }, + { + id: 1, + event: "push", + head_branch: "main", + status: "completed", + conclusion: "success", + run_started_at: "2026-07-17T01:00:00Z", + }, + { + id: 3, + event: "pull_request", + head_branch: "main", + status: "completed", + conclusion: "failure", + run_started_at: "2026-07-17T03:00:00Z", + }, + ]; + assert.equal(selectLatestDefaultBranchRun(runs, { currentRunId: 99, defaultBranch: "main" })?.id, 2); + assert.deepEqual( + failedJobNames([ + { name: "Build", conclusion: "failure" }, + { name: "Lint", conclusion: "success" }, + ]), + ["Build"], + ); + assert.deepEqual(classifyFailedJobs(["Build", "Lint"], runs[0], ["Build"]), [ + { name: "Build", classification: "main-side" }, + { name: "Lint", classification: "needs-investigation" }, + ]); + assert.match( + buildTriageBody([{ name: "Build", classification: "main-side" }], null), + /No completed main CI baseline/, + ); + console.error("[ci-triage] self-test passed"); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + if (process.argv.includes("--self-test")) selfTest(); + else { + console.error("usage: ci-triage.mjs --self-test"); + process.exitCode = 1; + } +} diff --git a/scripts/pr-policy.mjs b/scripts/pr-policy.mjs new file mode 100644 index 000000000..5054c2246 --- /dev/null +++ b/scripts/pr-policy.mjs @@ -0,0 +1,202 @@ +#!/usr/bin/env node +import assert from "node:assert/strict"; +import { pathToFileURL } from "node:url"; + +const clinicalRiskPatterns = [ + /^supabase\//, + /^src\/app\/api\//, + /^src\/(?:lib|app|components)\/.*(?:auth|permission|privacy|security|rag|retriev|rank|search|answer|clinical|citation|source|document|upload|download)/i, + /^scripts\/.*(?:ingest|reindex|migration|governance|production|drift|supabase)/i, +]; + +const operationalRiskPatterns = [ + /^\.github\/(?:actions|workflows)\//, + /^(?:package|package-lock)\.json$/, + /^(?:next|playwright|vitest)(?:\..+)?\.config\.[cm]?[jt]s$/, + /^(?:Dockerfile|railway(?:\.[^.]+)?\.json|nixpacks\.toml)$/, +]; + +const uiPatterns = [ + /^src\/app\/(?!api\/)/, + /^src\/(?:components|styles)\//, + /^public\//, + /^tests\/ui-.*\.spec\.ts$/, + /^playwright(?:\..*)?\.config\.ts$/, +]; + +function normalizePath(filePath) { + return String(filePath ?? "") + .trim() + .replaceAll("\\", "/") + .replace(/^\.\/+/, ""); +} + +function section(body, heading) { + const source = String(body ?? ""); + const headings = [...source.matchAll(/^##\s+(.+?)\s*$/gim)]; + const matchIndex = headings.findIndex((match) => match[1]?.trim().toLowerCase() === heading.toLowerCase()); + if (matchIndex < 0) return ""; + const start = (headings[matchIndex]?.index ?? 0) + (headings[matchIndex]?.[0].length ?? 0); + const end = headings[matchIndex + 1]?.index ?? source.length; + return source.slice(start, end).trim(); +} + +function meaningfulText(value) { + const normalized = String(value ?? "") + .replace(//g, "") + .replace(/^\s*[-*]\s*/gm, "") + .trim(); + return Boolean(normalized && !/^(?:-|n\/?a|none|todo|tbd)$/i.test(normalized)); +} + +function checkedCommand(value, command) { + const escaped = command.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`-\\s*\\[[xX]\\]\\s*[^\\n]*${escaped}`, "i").test(value); +} + +function explicitNotRun(value, scope = "verification") { + const escaped = scope.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const withoutComments = String(value ?? "").replace(//g, ""); + return new RegExp(`${escaped}[^\\n]{0,40}not run\\s*:\\s*\\S.{5,}`, "i").test(withoutComments); +} + +function branchLikeTitle(title, headRef) { + const value = String(title ?? "").trim(); + const normalized = (input) => + String(input ?? "") + .toLowerCase() + .replace(/[^a-z0-9]+/g, " ") + .trim(); + return ( + value.includes("/") || + /^(?:codex|claude|copilot)(?:\b|[-_:])/i.test(value) || + (normalized(value) && normalized(value) === normalized(headRef)) + ); +} + +export function classifyPullRequestFiles(files) { + const normalized = [...new Set((files ?? []).map(normalizePath).filter(Boolean))]; + return { + files: normalized, + clinicalRisk: normalized.some((file) => clinicalRiskPatterns.some((pattern) => pattern.test(file))), + operationalRisk: normalized.some((file) => operationalRiskPatterns.some((pattern) => pattern.test(file))), + ui: normalized.some((file) => uiPatterns.some((pattern) => pattern.test(file))), + }; +} + +export function evaluatePullRequestPolicy({ title, body, headRef, files }) { + const errors = []; + const classification = classifyPullRequestFiles(files); + const summary = section(body, "Summary"); + const verification = section(body, "Verification"); + const riskAndRollout = section(body, "Risk and rollout"); + const governance = section(body, "Clinical Governance Preflight"); + + if (String(title ?? "").trim().length < 12) + errors.push("Use a specific, outcome-focused PR title (at least 12 characters)."); + if (branchLikeTitle(title, headRef)) errors.push("Replace the branch-style PR title with an outcome-focused title."); + if (!meaningfulText(summary)) errors.push("Complete the `## Summary` section with the outcome and affected area."); + if (!meaningfulText(verification)) { + errors.push("Complete the `## Verification` section with exact results or a reason checks were not run."); + } else if (!/-\s*\[[xX]\]/.test(verification) && !explicitNotRun(verification)) { + errors.push("Verification must contain a checked result or an explicit `Verification not run: ` entry."); + } + + if ( + classification.ui && + !checkedCommand(verification, "npm run verify:ui") && + !explicitNotRun(verification, "UI verification") + ) { + errors.push("UI changes require checked `npm run verify:ui` evidence or `UI verification not run: `."); + } + + if (classification.clinicalRisk) { + if (!meaningfulText(governance)) { + errors.push("Clinical-risk paths require the `## Clinical Governance Preflight` section."); + } else if (/-\s*\[\s\]/.test(governance)) { + errors.push("Resolve every Clinical Governance Preflight checkbox before marking the PR ready."); + } + } + + if (classification.clinicalRisk || classification.operationalRisk) { + if (!meaningfulText(riskAndRollout)) { + errors.push("High-risk changes require the `## Risk and rollout` section."); + } else { + if (!/^\s*-?\s*Risk\s*:\s*\S.{2,}$/im.test(riskAndRollout)) { + errors.push("Risk and rollout must include `Risk: `."); + } + if (!/^\s*-?\s*Rollback\s*:\s*\S.{5,}$/im.test(riskAndRollout)) { + errors.push("Risk and rollout must include a concrete `Rollback: `."); + } + } + } + + return { classification, errors, ok: errors.length === 0 }; +} + +function selfTest() { + const completeBody = `## Summary\n\n- Add a trusted PR policy.\n\n## Verification\n\n- [x] \`npm run verify:pr-local\`\n- [x] \`npm run verify:ui\`\n\n## Risk and rollout\n\n- Risk: low; metadata-only validation.\n- Rollback: revert the workflow commit.\n\n## Clinical Governance Preflight\n\n- [x] Source behavior remains conservative.`; + assert.equal( + evaluatePullRequestPolicy({ + title: "ci: enforce pull request evidence", + body: completeBody, + headRef: "codex/pr-policy", + files: [".github/workflows/pr-policy.yml"], + }).ok, + true, + ); + assert.match( + evaluatePullRequestPolicy({ + title: "Codex/pr-policy", + body: "", + headRef: "codex/pr-policy", + files: [], + }).errors.join(" "), + /branch-style/, + ); + assert.match( + evaluatePullRequestPolicy({ + title: "fix: update search behavior", + body: completeBody.replace("- [x] `npm run verify:ui`\n", ""), + headRef: "codex/search-fix", + files: ["src/components/search.tsx"], + }).errors.join(" "), + /verify:ui/, + ); + assert.match( + evaluatePullRequestPolicy({ + title: "fix: update clinical search", + body: completeBody.replace( + "- [x] Source behavior remains conservative.", + "- [ ] Source behavior remains conservative.", + ), + headRef: "codex/search-fix", + files: ["src/lib/clinical-search.ts"], + }).errors.join(" "), + /every Clinical Governance/, + ); + assert.match( + evaluatePullRequestPolicy({ + title: "docs: explain the review process", + body: "## Summary\n\n- Useful documentation.\n\n## Verification\n\n- [ ] `npm run verify:pr-local`\n", + headRef: "codex/review-docs", + files: ["docs/process-hardening.md"], + }).errors.join(" "), + /checked result/, + ); + assert.deepEqual(classifyPullRequestFiles(["src/app/api/search/route.ts"]), { + files: ["src/app/api/search/route.ts"], + clinicalRisk: true, + operationalRisk: false, + ui: false, + }); + console.error("[pr-policy] self-test passed"); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + if (process.argv.includes("--self-test")) selfTest(); + else { + console.error("usage: pr-policy.mjs --self-test"); + process.exitCode = 1; + } +} From 980aed3b4fac73d642546a66610dedbeb4b544fb Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:37:57 +0800 Subject: [PATCH 2/2] fix: close PR policy bypasses Require the full governance checklist, substantive risk and rollback evidence, and preserve valid slash-bearing outcome titles. Cover CI triage fallback paths. --- docs/branch-review-ledger.md | 2 +- scripts/ci-triage.mjs | 2 + scripts/pr-policy.mjs | 89 +++++++++++++++++++++++++++++++----- 3 files changed, 81 insertions(+), 12 deletions(-) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 96310305a..fff042348 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -573,4 +573,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-17 | codex/scrolling-cleanup-20260717 | ff77cd06c + latest origin/main sync | cross-page scrolling and interaction stability review | Fixed two confirmed P2 defects: desktop action-popup placement performed synchronous geometry work for every captured scroll event, and submitted differential searches with zero document matches fell back to the home state. Placement is now coalesced per animation frame with passive scroll listeners, and submitted empty-evidence results remain visible. Hardened three popup/navigation browser helpers that reproduced hydration timing failures. No other high-confidence defect remains in the scoped diff. | Scroll-focused Chromium 28/28; final affected Chromium 5/5; source regressions 2/2; scoped ESLint; Prettier; TypeScript and production build passed before the final upstream-only sync; `git diff --check`. The aggregate local Vitest/UI runs were affected by concurrent-worktree resource contention, so exact-head hosted CI remains required before merge. No Supabase/OpenAI/live-provider checks run. | | 2026-07-17 | PR #713 / codex/chat-workflow-ideas-0916 | b52112df6aa36311d7420189064acd79dcf2c3f5 + reviewed follow-up diff | workflow toolkit review follow-up | Fixed all 14 actionable Codex and CodeRabbit threads: cross-platform path fixtures, installation-managed preflight guidance, complete Supabase-backed API database scoping, per-command approval boundaries, plugin-ignore narrowing, isolated CI-scope proof, remote-Git command guarding, repository-skill verification classification, `TypeError` diagnosis, strict CLI option values, machine-parseable JSON evidence output, and preservation of baseline database/clinical approval gates in the RAG lab. No unresolved actionable finding remains in the reviewed scope. | `npm run verify:cheap` passed with 273 files and 2,599 tests; focused toolkit Vitest 20/20; CI-scope self-test; plugin-ignore proof; `git diff --check`. Exact-head hosted CI remains required after the follow-up push. No Supabase, OpenAI, or other live product-provider command was run. | | 2026-07-17 | PR #699 / codex/test-reliability-hardening | 6202835ab1cf3703af311b9afdf372f74c63e040 | branch-cleanup-superseded | Closed as fully superseded by merged PR #705 (`e5caaa46c`). Range-diff maps the original implementation commit to #705's first integration commit; #705 then adds seven focused reliability fixes, while #699's remaining commit is merge-only and contributes no unique relevant patch. The exact-SHA remote ref and the unregistered local predecessor ref (`b518c1de9`) were deleted after final rechecks. | Fresh GitHub PR/head/status inventory; exact `ls-remote` and local-ref checks; cherry-pick-aware log; range-diff against PR #705's merged head; merge-parent verification; exact leased remote deletion and exact-old-value local `update-ref` deletion. No Supabase/OpenAI/product-provider checks run. | -| 2026-07-17 | codex/pr-process-hardening-20260717 | 220de891f10f82df11eb2e8137367514c139a206 + reviewed working diff | PR metadata, CI triage, review routing, Actions permissions, secret scanning, and branch-hygiene hardening | No remaining high-confidence P0-P2 defect in the reviewed process diff. Added a trusted base-SHA PR metadata/risk policy with draft and merge-queue handling; corrected CI triage to compare only the same workflow's latest completed `main` run; wired self-tests into local/hosted gates; documented the policy; created the missing durable Codex routing labels; removed four unused per-head routing labels; and applied read-only default Actions tokens, immutable Action pinning, no Actions-authored approvals, push protection, and automatic merged-branch deletion. One review defect was fixed before handoff: API-only `src/app/api/**` changes no longer incorrectly require UI verification. | Offline `check:pr-policy`, `check:ci-triage`, `check:ci-scope`, action-pin check, scoped Prettier, and `git diff --check` passed. Broader local gates were deferred because other registered worktrees repeatedly held the heavyweight lock; exact-head hosted CI remains required. GitHub provider inspection/settings changes were explicitly authorized. No Supabase/OpenAI/product-provider command ran. | +| 2026-07-17 | PR #716 / codex/pr-process-hardening-20260717 | 220de891f10f82df11eb2e8137367514c139a206 + reviewed implementation/follow-up diff | PR metadata, CI triage, review routing, Actions permissions, secret scanning, and branch-hygiene hardening | No remaining high-confidence P0-P2 defect in the reviewed process diff. Added a trusted base-SHA PR metadata/risk policy with draft and merge-queue handling; corrected CI triage to compare only the same workflow's latest completed `main` run; wired self-tests into local/hosted gates; documented the policy; created the missing durable Codex routing labels; removed four unused per-head routing labels; and applied read-only default Actions tokens, immutable Action pinning, no Actions-authored approvals, push protection, and automatic merged-branch deletion. Four policy defects were fixed before merge: API-only paths no longer trigger UI evidence, slash-bearing outcome titles are accepted, all seven governance items are required exactly, and placeholder risk/rollback text is rejected. | Offline `check:pr-policy`, `check:ci-triage`, `check:ci-scope`, action-pin check, scoped Prettier, and `git diff --check` passed. Initial hosted CI passed static, safety, coverage, build, images, Semgrep, Gitleaks, GitGuardian, and the aggregate gate; exact-head CI is required after the review fix. Broader local gates were deferred because other registered worktrees repeatedly held the heavyweight lock. GitHub provider inspection/settings changes were explicitly authorized. No Supabase/OpenAI/product-provider command ran. | diff --git a/scripts/ci-triage.mjs b/scripts/ci-triage.mjs index 228b31c52..34c94952e 100644 --- a/scripts/ci-triage.mjs +++ b/scripts/ci-triage.mjs @@ -88,6 +88,7 @@ function selfTest() { }, ]; assert.equal(selectLatestDefaultBranchRun(runs, { currentRunId: 99, defaultBranch: "main" })?.id, 2); + assert.equal(selectLatestDefaultBranchRun([], { currentRunId: 99, defaultBranch: "main" }), undefined); assert.deepEqual( failedJobNames([ { name: "Build", conclusion: "failure" }, @@ -99,6 +100,7 @@ function selfTest() { { name: "Build", classification: "main-side" }, { name: "Lint", classification: "needs-investigation" }, ]); + assert.deepEqual(classifyFailedJobs(["Build"], null, []), [{ name: "Build", classification: "needs-investigation" }]); assert.match( buildTriageBody([{ name: "Build", classification: "main-side" }], null), /No completed main CI baseline/, diff --git a/scripts/pr-policy.mjs b/scripts/pr-policy.mjs index 5054c2246..c26e0e8ea 100644 --- a/scripts/pr-policy.mjs +++ b/scripts/pr-policy.mjs @@ -1,7 +1,18 @@ #!/usr/bin/env node import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +export const requiredClinicalGovernanceItems = [ + "Source-backed claims still require linked source verification before clinical use", + "No patient-identifiable document workflow was introduced or expanded without explicit governance approval", + "Supabase target remains `Clinical KB Database` (`sjrfecxgysukkwxsowpy`)", + "Service-role keys and private document access remain server-only", + "Demo/synthetic content remains clearly separated from real clinical sources", + "Source metadata, review status, and outdated/unknown-source behavior remain conservative", + "Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed", +]; + const clinicalRiskPatterns = [ /^supabase\//, /^src\/app\/api\//, @@ -68,12 +79,33 @@ function branchLikeTitle(title, headRef) { .replace(/[^a-z0-9]+/g, " ") .trim(); return ( - value.includes("/") || /^(?:codex|claude|copilot)(?:\b|[-_:])/i.test(value) || (normalized(value) && normalized(value) === normalized(headRef)) ); } +function checkedChecklistItem(value, item) { + const escaped = item.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`^\\s*-\\s*\\[[xX]\\]\\s*${escaped}\\s*$`, "m").test(value); +} + +function fieldValue(value, field) { + const escaped = field.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return ( + String(value ?? "") + .match(new RegExp(`^\\s*-?\\s*${escaped}\\s*:\\s*(.+?)\\s*$`, "im"))?.[1] + ?.trim() ?? "" + ); +} + +function substantiveRisk(value) { + return value.length >= 12 && !/^(?:low|medium|high)[.!]?$/i.test(value); +} + +function substantiveRollback(value) { + return value.length >= 12 && !/^(?:none|n\/?a|not applicable|no rollback|no-?op)\b/i.test(value); +} + export function classifyPullRequestFiles(files) { const normalized = [...new Set((files ?? []).map(normalizePath).filter(Boolean))]; return { @@ -113,8 +145,15 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { if (classification.clinicalRisk) { if (!meaningfulText(governance)) { errors.push("Clinical-risk paths require the `## Clinical Governance Preflight` section."); - } else if (/-\s*\[\s\]/.test(governance)) { - errors.push("Resolve every Clinical Governance Preflight checkbox before marking the PR ready."); + } else { + const missingGovernance = requiredClinicalGovernanceItems.filter( + (item) => !checkedChecklistItem(governance, item), + ); + if (missingGovernance.length > 0) { + errors.push( + `Resolve every required Clinical Governance Preflight item before marking the PR ready (missing: ${missingGovernance.join("; ")}).`, + ); + } } } @@ -122,10 +161,10 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { if (!meaningfulText(riskAndRollout)) { errors.push("High-risk changes require the `## Risk and rollout` section."); } else { - if (!/^\s*-?\s*Risk\s*:\s*\S.{2,}$/im.test(riskAndRollout)) { + if (!substantiveRisk(fieldValue(riskAndRollout, "Risk"))) { errors.push("Risk and rollout must include `Risk: `."); } - if (!/^\s*-?\s*Rollback\s*:\s*\S.{5,}$/im.test(riskAndRollout)) { + if (!substantiveRollback(fieldValue(riskAndRollout, "Rollback"))) { errors.push("Risk and rollout must include a concrete `Rollback: `."); } } @@ -135,7 +174,8 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { } function selfTest() { - const completeBody = `## Summary\n\n- Add a trusted PR policy.\n\n## Verification\n\n- [x] \`npm run verify:pr-local\`\n- [x] \`npm run verify:ui\`\n\n## Risk and rollout\n\n- Risk: low; metadata-only validation.\n- Rollback: revert the workflow commit.\n\n## Clinical Governance Preflight\n\n- [x] Source behavior remains conservative.`; + const completeGovernance = requiredClinicalGovernanceItems.map((item) => `- [x] ${item}`).join("\n"); + const completeBody = `## Summary\n\n- Add a trusted PR policy.\n\n## Verification\n\n- [x] \`npm run verify:pr-local\`\n- [x] \`npm run verify:ui\`\n\n## Risk and rollout\n\n- Risk: low; metadata-only validation.\n- Rollback: revert the workflow commit.\n\n## Clinical Governance Preflight\n\n${completeGovernance}`; assert.equal( evaluatePullRequestPolicy({ title: "ci: enforce pull request evidence", @@ -166,14 +206,38 @@ function selfTest() { assert.match( evaluatePullRequestPolicy({ title: "fix: update clinical search", - body: completeBody.replace( - "- [x] Source behavior remains conservative.", - "- [ ] Source behavior remains conservative.", - ), + body: completeBody.replace(`- [x] ${requiredClinicalGovernanceItems[0]}`, "- [x] Safe"), headRef: "codex/search-fix", files: ["src/lib/clinical-search.ts"], }).errors.join(" "), - /every Clinical Governance/, + /every required Clinical Governance/, + ); + assert.equal( + evaluatePullRequestPolicy({ + title: "fix: handle /api/search failures", + body: completeBody, + headRef: "codex/api-search-failures", + files: ["docs/process-hardening.md"], + }).ok, + true, + ); + assert.match( + evaluatePullRequestPolicy({ + title: "ci: enforce pull request evidence", + body: completeBody.replace("Risk: low; metadata-only validation.", "Risk: low"), + headRef: "codex/pr-policy", + files: [".github/workflows/pr-policy.yml"], + }).errors.join(" "), + /Risk and rollout must include/, + ); + assert.match( + evaluatePullRequestPolicy({ + title: "ci: enforce pull request evidence", + body: completeBody.replace("Rollback: revert the workflow commit.", "Rollback: none because this is small"), + headRef: "codex/pr-policy", + files: [".github/workflows/pr-policy.yml"], + }).errors.join(" "), + /concrete `Rollback/, ); assert.match( evaluatePullRequestPolicy({ @@ -190,6 +254,9 @@ function selfTest() { operationalRisk: false, ui: false, }); + const template = readFileSync(new URL("../.github/pull_request_template.md", import.meta.url), "utf8"); + for (const item of requiredClinicalGovernanceItems) + assert.match(template, new RegExp(`- \\[ \\] ${item.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`)); console.error("[pr-policy] self-test passed"); }