diff --git a/.github/scripts/notify-pr-merge-review.mjs b/.github/scripts/notify-pr-merge-review.mjs index 714a43c5a05..63ca5cc4895 100644 --- a/.github/scripts/notify-pr-merge-review.mjs +++ b/.github/scripts/notify-pr-merge-review.mjs @@ -68,9 +68,10 @@ export function green(pr, requiredNames = minimumChecks) { const passed = (check) => check.__typename === "StatusContext" ? check.state === "SUCCESS" : check.__typename === "CheckRun" && check.status === "COMPLETED" && check.conclusion === "SUCCESS"; - // A skipped routing lane is normal. A skipped required aggregate is not a pass. + // Optional skipped/neutral checks can mean no work or unavailable coverage. + // They permit assessment, not approval. Required checks must really succeed. return relevant.every((check) => passed(check) || (check.__typename === "CheckRun" && - check.status === "COMPLETED" && check.conclusion === "SKIPPED")) && + check.status === "COMPLETED" && ["SKIPPED", "NEUTRAL"].includes(check.conclusion))) && [...new Set([...minimumChecks, ...requiredNames])].every((name) => relevant.some((check) => (check.name ?? check.context) === name && passed(check))); } diff --git a/.github/scripts/tests/notify-pr-merge-review.test.mjs b/.github/scripts/tests/notify-pr-merge-review.test.mjs index 0f128a86735..baff48c312b 100644 --- a/.github/scripts/tests/notify-pr-merge-review.test.mjs +++ b/.github/scripts/tests/notify-pr-merge-review.test.mjs @@ -66,6 +66,8 @@ test("waits for pending checks and statuses, permits skipped optional routing la } assert.equal(green(candidate({ statusCheckRollup: [...candidate().statusCheckRollup, check("not selected", "SKIPPED"), { __typename: "StatusContext", context: "security", state: "SUCCESS" }] })), true); + assert.equal(green(candidate({ statusCheckRollup: [...candidate().statusCheckRollup, + check("optional scanner coverage", "NEUTRAL")] })), true); }); test("excludes its own notification and profiling exporter, but not other workflows", () => { diff --git a/doc/PR-MERGE-REVIEW-WEBHOOK.md b/doc/PR-MERGE-REVIEW-WEBHOOK.md index 9406f756164..a1eab6bd0eb 100644 --- a/doc/PR-MERGE-REVIEW-WEBHOOK.md +++ b/doc/PR-MERGE-REVIEW-WEBHOOK.md @@ -36,9 +36,10 @@ When adding another PR verification workflow, add its name to `workflow_run`. The notifier reads the current PR's status rollup and branch rules. Both `ci / verify` and `ci / e2e`, plus any additional required checks, must succeed. -Other checks must succeed or be explicitly skipped. A neutral result is not a -pass. Skipped optional routing lanes do not imply that their tests ran. The -advisory routine evaluates actual coverage and reviews before recommending merge. +Other checks must succeed or finish as skipped or neutral. Optional skipped or +neutral checks permit an assessment; they do not prove successful verification. +For example, a neutral scanner can mean coverage is unavailable. The advisory +routine evaluates these evidence gaps and reviews before recommending merge. The notification workflow itself and the optional CI profile exporter are excluded from verification checks to avoid feedback loops.