From 129882511a440e9ac82748f9d14b5b2509582a51 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Jul 2026 08:04:26 +0000 Subject: [PATCH] fix(pr-policy): only clinical-governance gaps block; rest are advisory Follow-up to the classification narrowing: make the PR Policy check stop blocking PRs for metadata nits. evaluatePullRequestPolicy now returns a warnings[] array alongside errors[]: - errors[] (blocking): only a clinical-risk PR with a missing or incomplete Clinical Governance Preflight fails the check. - warnings[] (advisory): title, Summary, Verification, UI verify:ui, and Risk and rollout expectations are surfaced as warnings that never fail the check or block a merge. The workflow emits warnings via core.warning and only calls setFailed on blocking clinical-governance errors. Self-tests updated to assert the block-vs-warn split. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014MG846R4YM51NojUf9XthF --- .github/workflows/pr-policy.yml | 12 ++++- scripts/pr-policy.mjs | 83 ++++++++++++++++++++++----------- 2 files changed, 68 insertions(+), 27 deletions(-) diff --git a/.github/workflows/pr-policy.yml b/.github/workflows/pr-policy.yml index 1322bdcc7..74e113acb 100644 --- a/.github/workflows/pr-policy.yml +++ b/.github/workflows/pr-policy.yml @@ -64,14 +64,24 @@ jobs: files: changedFiles.map((file) => file.filename), }); + const warnings = result.warnings || []; 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"}
`) + .addRaw(`Blocking issues: ${result.errors.length}
`) + .addRaw(`Advisory warnings: ${warnings.length}
`) .write(); + // Advisory metadata nudges (title/summary/verification/UI/risk) are + // surfaced but never fail the check. Only a clinical-risk PR with an + // incomplete Clinical Governance Preflight blocks the merge. + for (const warning of warnings) core.warning(warning); + 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.`); + core.setFailed( + `PR policy found ${result.errors.length} blocking clinical-governance issue(s). Complete the Clinical Governance Preflight and rerun.`, + ); } diff --git a/scripts/pr-policy.mjs b/scripts/pr-policy.mjs index 01cd65cfd..3e6a1b6a6 100644 --- a/scripts/pr-policy.mjs +++ b/scripts/pr-policy.mjs @@ -146,7 +146,12 @@ export function classifyPullRequestFiles(files) { } export function evaluatePullRequestPolicy({ title, body, headRef, files }) { + // Only genuine clinical-risk governance gaps block the PR (hard failure). + // Every other metadata expectation (title, summary, verification, UI, risk + // and rollout) is advisory: it is surfaced as a warning so authors still get + // the nudge, but it never fails the check or blocks a merge. const errors = []; + const warnings = []; const classification = classifyPullRequestFiles(files); const summary = section(body, "Summary"); // The summary must be its own prose: content nested under a sub-heading @@ -158,14 +163,17 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { 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."); + warnings.push("Use a specific, outcome-focused PR title (at least 12 characters)."); + if (branchLikeTitle(title, headRef)) + warnings.push("Replace the branch-style PR title with an outcome-focused title."); if (!meaningfulText(summaryDirect)) - errors.push("Complete the `## Summary` section with the outcome and affected area."); + warnings.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."); + warnings.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."); + warnings.push( + "Verification should contain a checked result or an explicit `Verification not run: ` entry.", + ); } if ( @@ -173,9 +181,13 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { !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: `."); + warnings.push( + "UI changes should include checked `npm run verify:ui` evidence or `UI verification not run: `.", + ); } + // Blocking gate: a clinical-risk PR must carry a complete Clinical Governance + // Preflight. This is the only condition that fails the check. if (classification.clinicalRisk) { if (!meaningfulText(governance)) { errors.push("Clinical-risk paths require the `## Clinical Governance Preflight` section."); @@ -195,18 +207,18 @@ export function evaluatePullRequestPolicy({ title, body, headRef, files }) { if (classification.clinicalRisk || classification.operationalRisk) { if (!meaningfulText(riskAndRollout)) { - errors.push("High-risk changes require the `## Risk and rollout` section."); + warnings.push("High-risk changes should include the `## Risk and rollout` section."); } else { if (!substantiveRisk(fieldValue(riskAndRollout, "Risk"))) { - errors.push("Risk and rollout must include `Risk: `."); + warnings.push("Risk and rollout should include `Risk: `."); } if (!substantiveRollback(fieldValue(riskAndRollout, "Rollback"))) { - errors.push("Risk and rollout must include a concrete `Rollback: `."); + warnings.push("Risk and rollout should include a concrete `Rollback: `."); } } } - return { classification, errors, ok: errors.length === 0 }; + return { classification, errors, warnings, ok: errors.length === 0 }; } function selfTest() { @@ -221,22 +233,22 @@ function selfTest() { }).ok, true, ); - assert.match( - evaluatePullRequestPolicy({ - title: "Codex/pr-policy", - body: "", - headRef: "codex/pr-policy", - files: [], - }).errors.join(" "), - /branch-style/, - ); + // Advisory metadata issues are surfaced as warnings and NEVER block the check. + const branchStyle = evaluatePullRequestPolicy({ + title: "Codex/pr-policy", + body: "", + headRef: "codex/pr-policy", + files: [], + }); + assert.match(branchStyle.warnings.join(" "), /branch-style/); + assert.equal(branchStyle.ok, true, "advisory-only issues must not fail the check"); 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(" "), + }).warnings.join(" "), /verify:ui/, ); // Outline semantics: a ### sub-heading inside a required section must not @@ -258,7 +270,7 @@ function selfTest() { body: "## Summary\n\n### Verification\n\n- [x] `npm run verify:pr-local`\n", headRef: "codex/x", files: ["docs/a.md"], - }).errors.join(" "), + }).warnings.join(" "), /## Summary/, ); // ...but a level-1 heading is shallower than ## and DOES end the section, so @@ -269,7 +281,7 @@ function selfTest() { body: completeBody.replace("- [x] `npm run verify:ui`\n", "# Appendix\n\n- [x] `npm run verify:ui`\n"), headRef: "codex/search-fix", files: ["src/components/search.tsx"], - }).errors.join(" "), + }).warnings.join(" "), /verify:ui/, ); // Tolerant governance: a lightly reworded but still-checked item passes — the @@ -324,8 +336,8 @@ function selfTest() { 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/, + }).warnings.join(" "), + /Risk and rollout should include/, ); assert.match( evaluatePullRequestPolicy({ @@ -333,7 +345,7 @@ function selfTest() { 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(" "), + }).warnings.join(" "), /concrete `Rollback/, ); assert.match( @@ -342,7 +354,7 @@ function selfTest() { 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(" "), + }).warnings.join(" "), /checked result/, ); assert.deepEqual(classifyPullRequestFiles(["src/app/api/search/route.ts"]), { @@ -376,6 +388,25 @@ function selfTest() { }).ok, true, ); + // Block-only contract: a completely bare body on non-clinical files raises + // several advisory warnings but never blocks the check. + const bare = evaluatePullRequestPolicy({ + title: "x", + body: "", + headRef: "codex/whatever", + files: ["src/components/ui/button.tsx"], + }); + assert.equal(bare.ok, true, "non-clinical PRs must never be blocked by advisory metadata gaps"); + assert.ok(bare.warnings.length > 0, "advisory gaps should still be surfaced as warnings"); + // ...but a clinical-risk PR with no governance section is the one hard block. + const clinicalBlocked = evaluatePullRequestPolicy({ + title: "fix: adjust answer synthesis grounding", + body: "## Summary\n\n- Tweak synthesis.", + headRef: "codex/answer-fix", + files: ["src/lib/answer-synthesis.ts"], + }); + assert.equal(clinicalBlocked.ok, false, "clinical-risk PRs missing governance must still block"); + assert.match(clinicalBlocked.errors.join(" "), /Clinical Governance Preflight/); assert.equal( section("### Summary ###\n\n- concise summary\n\n### Verification\n\n- [x] `npm run verify:pr-local`\n", "Summary"), "- concise summary",