diff --git a/.changes/pr-pipeline/feat-hold-path-q26c-3b7e19d2.md b/.changes/pr-pipeline/feat-hold-path-q26c-3b7e19d2.md new file mode 100644 index 0000000000..d2266c1f9e --- /dev/null +++ b/.changes/pr-pipeline/feat-hold-path-q26c-3b7e19d2.md @@ -0,0 +1,8 @@ +--- +bump: minor +--- + +### Changed + +- The pr-merge hold-path floor now gives each glob a class (owner decision Q26 option B). `trust` (check-trusted-trigger, check-kill-switch, request-lane-token, CODEOWNERS, .gitmodules) and `gate` (every pr-merge script, the pr-merge and ci-status workflows, `babysit_merge*.py`, `babysit_risk.py` and `babysit_review_evidence.py` (they compute the review's clear condition), the rate guard, resolve-config and the pipeline config) still hold for a person. `enforcement` (the other modules the floored scripts import and the Python that decides the holds) no longer needs a person: the merge gate holds such a path as `review-path:` until the claude-security-review evidence binds to the head with no open finding, then arms it. ci-status leaves those paths to the gate. One classed list on each side, `HOLD_PATH_CLASSES` (`.github/scripts/pr-merge/merge-holds.mjs`) and `babysit_risk.FLOOR_CLASSES`, pinned equal by `test_merge_holds_sync.py`. +- The rest of `.github/scripts/pr-merge/` (gate-check, rotation, who, lane-records and the other scripts pr-merge.yml runs) joins the floor as `gate`, and both review callers' `hold-paths` and ci-status's exempt-author `hold_patterns` name them. diff --git a/.changes/source-control/feat-hold-path-q26c-3b7e19d2.md b/.changes/source-control/feat-hold-path-q26c-3b7e19d2.md new file mode 100644 index 0000000000..3b6386661c --- /dev/null +++ b/.changes/source-control/feat-hold-path-q26c-3b7e19d2.md @@ -0,0 +1,8 @@ +--- +bump: minor +--- + +### Changed + +- `babysit_risk` splits its path floor by class (`FLOOR_CLASSES`): `HIGH_FLOOR` is now the `trust` and `gate` globs, and an `enforcement` glob (`REVIEW_FLOOR`) scores HIGH as `review-path:` unless the security lane's evidence is bound to the head with no open finding (`security_review_clean`). A clean code-review lane alone clears nothing. `NEVER_FLOOR_EXEMPT` keeps every floor glob. +- An enforcement path in a pull request that also changes a lane source (a workflow, `CLAUDE.md`, `AGENTS.md`, `.claude/`, `docs/standards/`, `.gitattributes`) scores `review-path-unbindable:` instead: its security review can never bind at that head, so `person_reasons` names it and it goes to a person. An unbind a later review or read can clear stays the lane's `review-path:`. diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index a6830d6a19..2a1ad59afd 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -313,10 +313,23 @@ test("a refusal a person must clear, a cost path or a configured denied path, is ]); }); +// Owner decision Q26 option B: the gate holds an enforcement floor path as +// `review-path:` until the security review binds clean to the head; the lanes +// run that review, so the refusal gets the comment and no label. +test("an enforcement floor path the gate holds for its security review is a lane's refusal", async () => { + const reviews = "review-path:.github/scripts/work-resolve-issue/limits.mjs: .github/scripts/work-resolve-issue/limits.mjs; a clean claude-security-review bound to this head clears it"; + const verdict = { refusal: null, needsHuman: false, reviewsPerson: false, head: HEAD, ineligible: null, reviews }; + const { writes, notes } = await run([pull(1, { files: [{ filename: ".github/scripts/work-resolve-issue/limits.mjs" }], verdict })]); + assert.deepEqual(writes, []); + assert.deepEqual(notes, [{ method: "POST", path: `${ISSUES}/1/comments`, body: { body: laneNote(`pr-merge's gate holds it on its risk score (${reviews})`) } }]); +}); + // Owner decisions Q-FLOOR (B) and Q-ENFORCE (A): a floor path, the files // that enforce the floor included, holds with no configured glob naming it, // as a cause a person must clear, so it gets the label; a trusted path off -// the floor arms. +// the floor arms. Owner decision Q26 option B: an `enforcement` floor path is +// left to the gate's verifier read, whose risk score holds it until a clean +// security review binds to the head, so it arms on a passing read. test("a floor path no configured glob names is commented on and labeled human-gated; a trusted path off it arms", async () => { const floor = [ ".github/actions/resolve-config/resolve.mjs", @@ -331,23 +344,23 @@ test("a floor path no configured glob names is commented on and labeled human-ga ".github/scripts/pr-merge/enable-auto-merge.mjs", ".github/scripts/pr-merge/superseded.mjs", ".github/scripts/pr-merge/clear-escalations.mjs", - "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", + ".github/scripts/pr-merge/gate-check.mjs", + ".github/scripts/pr-merge/who.mjs", + "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", ".github/workflows/pr-require-checks.yml", + ".github/workflows/pr-merge.yml", "docs/conventions/pr-pipeline.yaml", - ".github/scripts/pr-unblock/address-feedback.mjs", - ".github/scripts/pr-unblock/gate-source.mjs", - ".github/scripts/pr-unblock/thread-scope.mjs", - ".github/scripts/pr-unblock/retry-fetch.mjs", - ".github/scripts/work-resolve-issue/report-outcome.mjs", - ".github/scripts/work-resolve-issue/limits.mjs", - ".github/scripts/work-resolve-issue/retry-fetch.mjs", ]; const { code, writes, notes } = await run([ ...floor.map((filename, index) => pull(index + 1, { files: [{ filename }] })), - pull(99, { files: [{ filename: ".github/scripts/pr-merge/gate-check.mjs" }] }), + pull(98, { files: [{ filename: "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py" }] }), + pull(99, { files: [{ filename: ".github/scripts/pr-unblock/other.mjs" }] }), ]); assert.equal(code, 0); - assert.deepEqual(writes, [{ mutation: "arm", id: "PR_99", head: HEAD }]); + assert.deepEqual(writes, [ + { mutation: "arm", id: "PR_98", head: HEAD }, + { mutation: "arm", id: "PR_99", head: HEAD }, + ]); assert.deepEqual( notes, floor.flatMap((filename, index) => [ @@ -1151,7 +1164,7 @@ test("a pull request whose head now changes a hold path is not armed, or is disa pull(4, { files: [{ filename: "docs/held.md" }] }), pull(5), // The rest of .github merges on the review lanes. - pull(6, { files: [{ filename: ".github/workflows/x.yml" }, { filename: ".github/scripts/pr-merge/gate-check.mjs" }] }), + pull(6, { files: [{ filename: ".github/workflows/x.yml" }, { filename: ".github/scripts/pr-unblock/other.mjs" }] }), ]); assert.deepEqual(writes, [ { mutation: "disarm", id: "PR_2" }, diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index fc24021a7e..16a1d73231 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -16,7 +16,9 @@ // (babysit_merge.HUMAN_MERGE_VETO_RE). A later one by the same person // lifts it; an item with no text lifts nothing. // - A hold path: a changed file, or a renamed file's old path, matching the -// config's merge.diff-check.denied-paths or HOLD_PATH_FLOOR. A person on +// config's merge.diff-check.denied-paths or HOLD_PATH_FLOOR (the floor's +// `trust` and `gate` globs; its `enforcement` globs are the merge gate's, +// which a clean security review clears). A person on // the trusted-actor list lifts it by approving the head: their newest // review of the head that approves, requests changes or was dismissed must // approve. No other path is held for being trusted alone: the review lanes @@ -231,63 +233,80 @@ export const fromRest = (row) => ({ const undot = (text) => String(text).toLowerCase().replaceAll(/(^|\/)\./g, "$1\u0001"); export const matchesHoldPath = (path, glob) => posix.matchesGlob(undot(path), undot(glob)); -// The narrow floor every hold path check adds to the configured globs (owner -// decision Q-FLOOR (B)): the code main runs to mint and scope lane tokens -// and judge triggers (resolve-config, request-lane-token, -// check-trusted-trigger, check-kill-switch, the rate guard), code owners and -// submodules; and the files that enforce the floor (owner decision -// Q-ENFORCE (A)): this module, enable-auto-merge, superseded, -// clear-escalations, babysit_risk.py, the ci-status workflow, the pr-merge -// workflow that wires them, the pipeline config, every module those scripts -// import, and the babysit-prs modules -// babysit_risk.py and the Python those scripts run (babysit_verify_evidence.py) -// import (test_merge_holds_sync.py derives both closures and requires them -// here). babysit_risk.HIGH_FLOOR, which -// test_merge_holds_sync.py checks this equals. A constant of the module -// ci-status and enable-auto-merge run from the base commit, so a pull -// request cannot rewrite it. -export const HOLD_PATH_FLOOR = Object.freeze([ - ".github/actions/check-trusted-trigger/**", - "**/CODEOWNERS", - "**/.gitmodules", - ".github/actions/resolve-config/**", - ".github/actions/check-kill-switch/**", - ".github/actions/request-lane-token/**", - ".github/actions/log-api-budget/rate-guard.mjs", - ".github/actions/log-api-budget/rate-limits.json", - ".github/scripts/pr-merge/merge-holds.mjs", - ".github/scripts/pr-merge/enable-auto-merge.mjs", - ".github/scripts/pr-merge/superseded.mjs", - ".github/scripts/pr-merge/clear-escalations.mjs", - "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_state.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_util.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py", - ".github/workflows/pr-require-checks.yml", - ".github/workflows/pr-merge.yml", - "docs/conventions/pr-pipeline.yaml", - ".github/scripts/pr-unblock/address-feedback.mjs", - ".github/scripts/pr-unblock/gate-source.mjs", - ".github/scripts/pr-unblock/thread-scope.mjs", - ".github/scripts/pr-unblock/retry-fetch.mjs", - ".github/scripts/work-resolve-issue/report-outcome.mjs", - ".github/scripts/work-resolve-issue/limits.mjs", - ".github/scripts/work-resolve-issue/retry-fetch.mjs", +// The narrow floor every hold path check adds to the configured globs, each +// glob with its class (owner decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 +// option B): `trust`, what mints and scopes lane tokens and judges triggers +// (request-lane-token, check-trusted-trigger, check-kill-switch), code owners +// and submodules; `gate`, the merge gate's own code (every pr-merge script, +// the pr-merge and ci-status workflows, babysit_merge*, babysit_risk.py and +// babysit_review_evidence.py, which compute the review's clear condition, +// the rate guard) and +// the config the token broker loads (resolve-config, effect-grants.json +// included, and the pipeline config); `enforcement`, the other modules the +// floored scripts import and the Python that decides the holds +// (test_merge_holds_sync.py derives both closures and requires them here). +// babysit_risk.FLOOR_CLASSES, which test_merge_holds_sync.py checks this +// equals. A constant of the module ci-status and enable-auto-merge run from +// the base commit, so a pull request cannot rewrite it. +export const HOLD_PATH_CLASSES = Object.freeze([ + [".github/actions/check-trusted-trigger/**", "trust"], + ["**/CODEOWNERS", "trust"], + ["**/.gitmodules", "trust"], + [".github/actions/resolve-config/**", "gate"], + [".github/actions/check-kill-switch/**", "trust"], + [".github/actions/request-lane-token/**", "trust"], + [".github/actions/log-api-budget/rate-guard.mjs", "gate"], + [".github/actions/log-api-budget/rate-limits.json", "gate"], + [".github/scripts/pr-merge/merge-holds.mjs", "gate"], + [".github/scripts/pr-merge/enable-auto-merge.mjs", "gate"], + [".github/scripts/pr-merge/superseded.mjs", "gate"], + [".github/scripts/pr-merge/clear-escalations.mjs", "gate"], + [".github/scripts/pr-merge/carry-run-cache.mjs", "gate"], + [".github/scripts/pr-merge/gate-check.mjs", "gate"], + [".github/scripts/pr-merge/head-ref-map.mjs", "gate"], + [".github/scripts/pr-merge/lane-records.mjs", "gate"], + [".github/scripts/pr-merge/rate-stop.bash", "gate"], + [".github/scripts/pr-merge/refire-refused.mjs", "gate"], + [".github/scripts/pr-merge/rotation.mjs", "gate"], + [".github/scripts/pr-merge/sweep-summary.mjs", "gate"], + [".github/scripts/pr-merge/who.mjs", "gate"], + [".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"], + [".github/scripts/pr-merge/verify-answers.mjs", "gate"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "gate"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", "gate"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", "gate"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", "gate"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_state.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_util.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", "enforcement"], + ["plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py", "enforcement"], + [".github/workflows/pr-require-checks.yml", "gate"], + [".github/workflows/pr-merge.yml", "gate"], + ["docs/conventions/pr-pipeline.yaml", "gate"], + [".github/scripts/pr-unblock/address-feedback.mjs", "enforcement"], + [".github/scripts/pr-unblock/gate-source.mjs", "enforcement"], + [".github/scripts/pr-unblock/thread-scope.mjs", "enforcement"], + [".github/scripts/pr-unblock/retry-fetch.mjs", "enforcement"], + [".github/scripts/work-resolve-issue/report-outcome.mjs", "enforcement"], + [".github/scripts/work-resolve-issue/limits.mjs", "enforcement"], + [".github/scripts/work-resolve-issue/retry-fetch.mjs", "enforcement"], ]); +// The `trust` and `gate` globs (babysit_risk.HIGH_FLOOR), which hold for a +// person. An `enforcement` glob (REVIEW_FLOOR) holds nothing here: the merge +// gate holds it (babysit_risk `review-path:`) until a clean security review +// binds to the head, and enable-auto-merge arms only a head the gate passed. +export const HOLD_PATH_FLOOR = Object.freeze(HOLD_PATH_CLASSES.filter(([, kind]) => kind !== "enforcement").map(([glob]) => glob)); +export const REVIEW_FLOOR = Object.freeze(HOLD_PATH_CLASSES.filter(([, kind]) => kind === "enforcement").map(([glob]) => glob)); // Why the head's changed `files` hold it, or null: one matching `holdPaths` // or HOLD_PATH_FLOOR. Null `holdPaths` or `files` (unreadable) holds it. diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 86d060185b..3e08fc72c0 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -18,7 +18,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; -import { BODY_WRITERS_QUERY, COMMENT_DELETED_QUERY, COMMENT_EDITED_QUERY, PULL_OPENED_QUERY, changeOf, costDecision, costHold, costMarker, deletedSince, evidenceHold, linkHold, main, sameNetChange } from "./merge-holds.mjs"; +import { BODY_WRITERS_QUERY, COMMENT_DELETED_QUERY, COMMENT_EDITED_QUERY, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, PULL_OPENED_QUERY, REVIEW_FLOOR, changeOf, costDecision, costHold, costMarker, deletedSince, evidenceHold, linkHold, main, pathHold, sameNetChange } from "./merge-holds.mjs"; const REPO = "owner/repo"; const LINK = "https://github.com/owner/repo/issues/40#issuecomment-7"; @@ -308,7 +308,9 @@ const approve = { user: { login: "owner-person", id: 5, type: "User" }, state: " // check-kill-switch, the rate guard, CODEOWNERS and .gitmodules hold with no // configured glob, and so do the files that enforce it (owner decision // Q-ENFORCE (A)); a trusted person's approval of the head lifts them as it -// lifts a configured one. +// lifts a configured one. Owner decision Q26 option B: only the `trust` and +// `gate` entries hold here; an `enforcement` one is the merge gate's, which a +// clean security review bound to the head clears (ENFORCEMENT_FILES). const FLOOR_FILES = [ ".github/actions/resolve-config/resolve.mjs", ".github/actions/Resolve-Config/lane-rules.json", @@ -325,9 +327,20 @@ const FLOOR_FILES = [ ".github/scripts/pr-merge/enable-auto-merge.mjs", ".github/scripts/pr-merge/superseded.mjs", ".github/scripts/pr-merge/clear-escalations.mjs", + ".github/scripts/pr-merge/gate-check.mjs", + ".github/scripts/pr-merge/who.mjs", + ".github/scripts/pr-merge/rate-stop.bash", + "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", ".github/workflows/pr-require-checks.yml", + ".github/workflows/pr-merge.yml", "docs/conventions/pr-pipeline.yaml", +]; +const ENFORCEMENT_FILES = [ + "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", ".github/scripts/pr-unblock/address-feedback.mjs", ".github/scripts/pr-unblock/gate-source.mjs", ".github/scripts/pr-unblock/thread-scope.mjs", @@ -336,6 +349,25 @@ const FLOOR_FILES = [ ".github/scripts/work-resolve-issue/limits.mjs", ".github/scripts/work-resolve-issue/retry-fetch.mjs", ]; +test("the floor's enforcement globs are the merge gate's, every other one holds here", () => { + assert.deepEqual(REVIEW_FLOOR, HOLD_PATH_CLASSES.filter(([, kind]) => kind === "enforcement").map(([glob]) => glob)); + assert.deepEqual(HOLD_PATH_FLOOR, HOLD_PATH_CLASSES.filter(([, kind]) => kind !== "enforcement").map(([glob]) => glob)); + assert.deepEqual([...new Set(HOLD_PATH_CLASSES.map(([, kind]) => kind))].sort(), ["enforcement", "gate", "trust"]); + for (const path of ENFORCEMENT_FILES) { + assert.ok(REVIEW_FLOOR.includes(path), path); + assert.equal(pathHold([path], []), null, path); + } +}); +test("ci-status leaves an enforcement path to the merge gate's security review", async () => { + for (const path of ENFORCEMENT_FILES) { + const passed = await ci([{ filename: path, status: "modified" }]); + assert.equal(passed.code, 0, `${path}: ${passed.out}`); + } + // Beside a gate path it still holds on that one. + const both = await ci([{ filename: ENFORCEMENT_FILES[0], status: "modified" }, { filename: ".github/scripts/pr-merge/who.mjs", status: "modified" }]); + assert.equal(both.code, 1, both.out); + assert.ok(both.out.includes("it changes .github/scripts/pr-merge/who.mjs, a hold path"), both.out); +}); test("ci-status holds each floor path with no configured glob, until a trusted person approves the head", async () => { for (const path of FLOOR_FILES) { const held = await ci([{ filename: path, status: "modified" }]); @@ -359,7 +391,7 @@ test("ci-status holds no other trusted path or manifest change, only a configure ".github/actions/log-api-budget/action.yml", ".github/actions/log-api-budget/rate-guard.test.mjs", ".github/actions/check-trusted-trigger-x/a.mjs", - ".github/scripts/pr-merge/gate-check.mjs", + ".github/scripts/pr-unblock/other.mjs", "docs/CODEOWNERS.md", ]) { const passed = await ci([{ filename: path, status: "modified" }]); diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 8d89f0e157..40e0b92eff 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4402,7 +4402,7 @@ jobs: } if [[ $AUTHOR_ID =~ ^[0-9]+$ && " $exempt_author_ids " == *" $AUTHOR_ID "* ]]; then # The trust-gate paths, as bash patterns (`*` crosses `/`). - hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' '.github/actions/resolve-config/*' '.github/actions/request-lane-token/*' '.github/actions/check-kill-switch/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-merge.yml' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules') + hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' '.github/actions/resolve-config/*' '.github/actions/request-lane-token/*' '.github/actions/check-kill-switch/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-merge/carry-run-cache.mjs' '.github/scripts/pr-merge/gate-check.mjs' '.github/scripts/pr-merge/head-ref-map.mjs' '.github/scripts/pr-merge/lane-records.mjs' '.github/scripts/pr-merge/rate-stop.bash' '.github/scripts/pr-merge/refire-refused.mjs' '.github/scripts/pr-merge/rotation.mjs' '.github/scripts/pr-merge/sweep-summary.mjs' '.github/scripts/pr-merge/who.mjs' '.github/scripts/pr-merge/post-unarmed-status.mjs' '.github/scripts/pr-merge/verify-answers.mjs' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-merge.yml' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules') if [[ ! $PR_NUMBER =~ ^[1-9][0-9]*$ ]]; then echo "::error::the pull request number '$PR_NUMBER' is not a number." exit 1 diff --git a/.github/workflows/pr-review-hosted.yml b/.github/workflows/pr-review-hosted.yml index 0bae107e47..8674a1c918 100644 --- a/.github/workflows/pr-review-hosted.yml +++ b/.github/workflows/pr-review-hosted.yml @@ -211,6 +211,17 @@ jobs: .github/scripts/pr-merge/enable-auto-merge.mjs .github/scripts/pr-merge/superseded.mjs .github/scripts/pr-merge/clear-escalations.mjs + .github/scripts/pr-merge/carry-run-cache.mjs + .github/scripts/pr-merge/gate-check.mjs + .github/scripts/pr-merge/head-ref-map.mjs + .github/scripts/pr-merge/lane-records.mjs + .github/scripts/pr-merge/rate-stop.bash + .github/scripts/pr-merge/refire-refused.mjs + .github/scripts/pr-merge/rotation.mjs + .github/scripts/pr-merge/sweep-summary.mjs + .github/scripts/pr-merge/who.mjs + .github/scripts/pr-merge/post-unarmed-status.mjs + .github/scripts/pr-merge/verify-answers.mjs .github/scripts/pr-unblock/address-feedback.mjs .github/scripts/pr-unblock/gate-source.mjs .github/scripts/pr-unblock/thread-scope.mjs diff --git a/.github/workflows/pr-review-security-hosted.yml b/.github/workflows/pr-review-security-hosted.yml index 2811b41cac..47fb86da95 100644 --- a/.github/workflows/pr-review-security-hosted.yml +++ b/.github/workflows/pr-review-security-hosted.yml @@ -211,6 +211,17 @@ jobs: .github/scripts/pr-merge/enable-auto-merge.mjs .github/scripts/pr-merge/superseded.mjs .github/scripts/pr-merge/clear-escalations.mjs + .github/scripts/pr-merge/carry-run-cache.mjs + .github/scripts/pr-merge/gate-check.mjs + .github/scripts/pr-merge/head-ref-map.mjs + .github/scripts/pr-merge/lane-records.mjs + .github/scripts/pr-merge/rate-stop.bash + .github/scripts/pr-merge/refire-refused.mjs + .github/scripts/pr-merge/rotation.mjs + .github/scripts/pr-merge/sweep-summary.mjs + .github/scripts/pr-merge/who.mjs + .github/scripts/pr-merge/post-unarmed-status.mjs + .github/scripts/pr-merge/verify-answers.mjs .github/scripts/pr-unblock/address-feedback.mjs .github/scripts/pr-unblock/gate-source.mjs .github/scripts/pr-unblock/thread-scope.mjs diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index 71651f6c22..5c53feecf6 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -981,18 +981,15 @@ merge: mode: enforce majors: review # No path is held for being trusted alone (owner direction 2026-10-24) - # except the narrow HIGH path floor (babysit_risk.py HIGH_FLOOR, owner - # decision Q-FLOOR (B)): resolve-config, request-lane-token, - # check-trusted-trigger, check-kill-switch, the rate guard, CODEOWNERS, - # .gitmodules and the files that enforce the floor (owner decision - # Q-ENFORCE (A)): merge-holds, enable-auto-merge, superseded, - # clear-escalations, babysit_risk.py, pr-require-checks.yml, this file, - # every module those scripts import (pr-unblock's address-feedback, - # gate-source and thread-scope, work-resolve-issue's report-outcome and - # limits), and the babysit-prs Python that decides the holds - # (babysit_risk.py, babysit_verify_evidence.py, which enable-auto-merge - # runs, and every module they import), which test_merge_holds_sync.py - # derives and requires on the floor. + # except the narrow path floor (babysit_risk.py FLOOR_CLASSES, owner + # decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B). Its `trust` + # globs (request-lane-token, check-trusted-trigger, check-kill-switch, + # CODEOWNERS, .gitmodules) and `gate` globs (resolve-config, the rate + # guard, every pr-merge script, pr-merge.yml, pr-require-checks.yml, this + # file, babysit_merge*, babysit_risk.py, babysit_review_evidence.py) wait + # for a person. Its `enforcement` globs (the other modules the floored + # scripts import and the rest of the Python that decides the holds) wait + # for a clean security review bound to the head instead. # It holds whatever `high-path-floor` says. `high-path-floor: # false` turns off the manifest review, which the security lane flags # instead. The trusted-actor widening and cost holds stay on either way. The diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 452a2b3195..89bcafae0c 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -594,7 +594,8 @@ The rest of `.github/`, `.claude/`, `.mcp.json`, plugin hooks, `.claude-plugin/` `scripts/`, `CLAUDE.md`, `AGENTS.md` and an executable bit reach review, and merge on it: only the narrow [path floor](#merge-authority) (`resolve-config`, whose code the token broker runs from main, `request-lane-token`, the trusted-trigger check, `check-kill-switch`, the rate guard, -`CODEOWNERS`, `.gitmodules` and the files that enforce the floor) waits for a person, and the +`CODEOWNERS`, `.gitmodules` and the merge gate's own code; the other files that enforce the floor +wait for a clean security review instead) waits for a person, and the security lane judges the rest, the trusted-actor list included. The `docs` row also refuses, in any case and through any `..` segment, a path under a `skills/` directory at any depth (`skills`) and a `SKILL.md` anywhere (`skill-md`), and allows only `markdown`, so a non-markdown path escalates @@ -803,29 +804,36 @@ the rules that only tighten in - **Opt-in label.** `merge: agent-allowed` makes a pull request eligible; it does not make it safe, and the diff risk score still decides. It counts only when a human added it last and the pull request body was not edited afterwards. -- **Narrow path floor.** A fixed set of paths in the wrapper's code (`babysit_risk.HIGH_FLOOR`, - owner decisions Q-FLOOR (B) and - [Q-ENFORCE (A)](https://github.com/melodic-software/claude-code-plugins/pull/9826#issuecomment-6401599691)), - joined with `merge.diff-check.denied-paths` (empty here) and - matched case-insensitively, scores HIGH whatever `high-path-floor` says and is never merged - unattended: `.github/actions/resolve-config/**`, whose code the token broker runs from main in - its signer process, `.github/actions/request-lane-token/**`, which mints the lanes App token, - `.github/actions/check-trusted-trigger/**` and the rate guard it sends every call through - (`.github/actions/log-api-budget/rate-guard.mjs`) with its rate-limit defaults - (`.github/actions/log-api-budget/rate-limits.json`), `.github/actions/check-kill-switch/**`, - whose output writer both actions use, `**/CODEOWNERS`, `**/.gitmodules`, and the files that - enforce the floor: `.github/scripts/pr-merge/merge-holds.mjs`, `enable-auto-merge.mjs`, - `superseded.mjs` and `clear-escalations.mjs` there, - `plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py`, - `.github/workflows/pr-require-checks.yml`, `.github/workflows/pr-merge.yml`, which runs those - scripts and sets their environment, `docs/conventions/pr-pipeline.yaml`, and every module - those scripts import (`.github/scripts/pr-unblock/address-feedback.mjs`, `gate-source.mjs` and - `thread-scope.mjs`, `.github/scripts/work-resolve-issue/report-outcome.mjs` and `limits.mjs`, and - the `retry-fetch.mjs` copy in each of those two directories), - and the Python that decides the holds: `babysit_risk.py` and `babysit_verify_evidence.py`, which - `enable-auto-merge.mjs` runs, with every `babysit-prs/scripts/` module they import - (`babysit_merge.py`, `babysit_merge_policy.py` and 14 more). `test_merge_holds_sync.py` derives - both closures from their imports and requires them on the floor. No other path +- **Narrow path floor.** A fixed set of globs in the wrapper's code, each with a class + (`babysit_risk.FLOOR_CLASSES`, equal to `merge-holds.mjs` `HOLD_PATH_CLASSES`; owner decisions + Q-FLOOR (B), + [Q-ENFORCE (A)](https://github.com/melodic-software/claude-code-plugins/pull/9826#issuecomment-6401599691) + and Q26 option B), matched case-insensitively whatever `high-path-floor` says: + - `trust`, a person merges it: `.github/actions/request-lane-token/**`, which mints the lanes + App token, `.github/actions/check-trusted-trigger/**`, `.github/actions/check-kill-switch/**`, + whose output writer both actions use, `**/CODEOWNERS` and `**/.gitmodules`. + - `gate`, a person merges it: the merge gate's own code and the config the token broker loads. + `.github/actions/resolve-config/**`, whose code the broker runs from main in its signer + process, the rate guard (`.github/actions/log-api-budget/rate-guard.mjs`) with its rate-limit + defaults (`.github/actions/log-api-budget/rate-limits.json`), every script of + `.github/scripts/pr-merge/`, `.github/workflows/pr-merge.yml` and + `.github/workflows/pr-require-checks.yml`, `docs/conventions/pr-pipeline.yaml`, + `babysit_merge.py`, `babysit_merge_policy.py`, and `babysit_risk.py` and + `babysit_review_evidence.py`, which compute the security review's clear condition. + - `enforcement`, a clean security review clears it: every other module the floored scripts + import (`.github/scripts/pr-unblock/address-feedback.mjs`, `gate-source.mjs` and + `thread-scope.mjs`, `.github/scripts/work-resolve-issue/report-outcome.mjs` and `limits.mjs`, + and the `retry-fetch.mjs` copy in each of those two directories) + and the rest of the Python that decides the holds (`babysit_verify_evidence.py`, which + `enable-auto-merge.mjs` runs, and the `babysit-prs/scripts/` modules it and `babysit_risk.py` + import). It scores HIGH as `review-path:` until the `claude-security-review` evidence + binds to the head with no open finding, then merges with no person; ci-status leaves it to + the gate. When the pull request also changes a lane source, the review can never bind at that + head, so it scores `review-path-unbindable:`, which a person clears. + + `merge.diff-check.denied-paths` (empty here) joins the person classes. + `test_merge_holds_sync.py` derives the JS and Python import closures and requires them on the + floor, with every pr-merge script as `gate`. No other path is held for being trusted (owner direction 2026-10-24): the trusted-actor list and every other path merge on the review lanes, which judge them. A `plugins//.claude-plugin/plugin.json` or `.claude-plugin/marketplace.json` change that @@ -1165,7 +1173,7 @@ and carries no hold: no `Hold:` line or "do not merge" in its body collaborator) whose newest comment, review or inline comment with text says "do not merge" (the gate's veto rule; a later comment lifts it), and whose head changes no path in the lane's `merge.diff-check.denied-paths` or on the [path floor](#merge-authority) (`merge-holds.mjs` -`HOLD_PATH_FLOOR`, equal to `babysit_risk.HIGH_FLOOR`), read on the files the head changes at each +`HOLD_PATH_FLOOR`, the floor's `trust` and `gate` globs, equal to `babysit_risk.HIGH_FLOOR`), read on the files the head changes at each run, and whose issue link and verifier verdict at that head pr-merge's gate would take: `babysit_verify_evidence.py evidence --lane-link` refuses nothing, so the link is one the gate's @@ -1893,7 +1901,7 @@ defines it) or `critical-security`. `hold-paths` only flags the files it names f scrutiny; a match never holds by itself. The security lane reads such a file's whole change and what it gates, and only a CRITICAL finding there holds. The callers name `.github/standards/trusted-actors/**`, every entry of the narrow path floor above -(`babysit_risk.HIGH_FLOOR`), and the security review's own criteria and eval cases +(`babysit_risk.FLOOR`, every class), and the security review's own criteria and eval cases (`plugins/review/skills/security-review/**`, `plugins/review/evals/security-review-*/**`). All but those last two are also the trust-gate paths ci-status's exempt-author check below fails on. `test_merge_holds_sync.py` checks that both callers' `hold-paths` and that check's diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py index 46e8bcd530..38102c451d 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py @@ -58,7 +58,7 @@ record_pull_request_total, ) from babysit_risk import ( - HIGH_FLOOR, + FLOOR, ChangedFile, glob_match, is_trusted_actors_path, @@ -4203,8 +4203,9 @@ def foreign_paths( # Paths never a trusted author's own, whoever wrote them: the HIGH path floor -# itself, so an adopted pull request's own lines hold there too. -NEVER_FLOOR_EXEMPT = HIGH_FLOOR +# itself, the entries a security review clears included, so an adopted pull +# request's own lines hold there too. +NEVER_FLOOR_EXEMPT = FLOOR # A model-driven App's own lines skip the denied paths and the manifest review diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py index b34e8a4c52..985ef2e3b2 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -17,7 +17,10 @@ The HIGH path floor and the lane table are fixed here, never read from configuration. The floor is narrow (owner decision Q-FLOOR (B)): the code main runs to mint and scope lane tokens and judge triggers, code owners and -submodules, and it holds whatever the floor switch says. No other path rates +submodules, and it holds whatever the floor switch says. Its +`enforcement` entries (`REVIEW_FLOOR`) rate HIGH as `review-path:` +instead, which a clean security review bound to the head clears +(`security_review_clean`, owner decision Q26 option B). No other path rates HIGH for being trusted alone, and the two review lanes, which are the gate, judge it, except the cost and trusted-actor holds below. The configured `merge.diff-check.denied-paths` are joined to the floor. Every glob matches @@ -185,61 +188,90 @@ class RiskResult: reasons: tuple[Reason, ...] # sorted by severity desc, then code -# What a lane pull request must not change unattended (owner decision -# Q-FLOOR (B)): resolve-config, whose code the token broker runs from main in -# its signer process, request-lane-token, which mints and hands out the lanes -# App token, check-trusted-trigger and the rate guard it sends every call -# through, check-kill-switch, whose writeOutputs both of those actions write -# their verdicts through, code owners and submodules; and the files that -# enforce the floor (owner decision Q-ENFORCE (A)): the merge-holds, -# enable-auto-merge, superseded and clear-escalations scripts, this module, -# the ci-status workflow, the pr-merge workflow that wires those scripts, -# the pipeline config, every module those scripts import, and the babysit-prs modules this module and the Python those -# scripts run (babysit_verify_evidence.py) import (test_merge_holds_sync.py -# derives both closures and requires them here). merge-holds.mjs -# HOLD_PATH_FLOOR is the same list (test_merge_holds_sync.py). -HIGH_FLOOR: tuple[str, ...] = ( - ".github/actions/check-trusted-trigger/**", - "**/CODEOWNERS", - "**/.gitmodules", - ".github/actions/resolve-config/**", - ".github/actions/check-kill-switch/**", - ".github/actions/request-lane-token/**", - ".github/actions/log-api-budget/rate-guard.mjs", - ".github/actions/log-api-budget/rate-limits.json", - ".github/scripts/pr-merge/merge-holds.mjs", - ".github/scripts/pr-merge/enable-auto-merge.mjs", - ".github/scripts/pr-merge/superseded.mjs", - ".github/scripts/pr-merge/clear-escalations.mjs", - "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_state.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_util.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py", - ".github/workflows/pr-require-checks.yml", - ".github/workflows/pr-merge.yml", - "docs/conventions/pr-pipeline.yaml", - ".github/scripts/pr-unblock/address-feedback.mjs", - ".github/scripts/pr-unblock/gate-source.mjs", - ".github/scripts/pr-unblock/thread-scope.mjs", - ".github/scripts/pr-unblock/retry-fetch.mjs", - ".github/scripts/work-resolve-issue/report-outcome.mjs", - ".github/scripts/work-resolve-issue/limits.mjs", - ".github/scripts/work-resolve-issue/retry-fetch.mjs", -) +# What a lane pull request must not change unattended (owner decisions +# Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B), each glob with its class: +# - `trust`: what mints and scopes lane tokens and judges triggers +# (request-lane-token, check-trusted-trigger, check-kill-switch, whose +# writeOutputs both of those write their verdicts through), code owners +# and submodules. +# - `gate`: the merge gate's own code and the config the token broker loads: +# resolve-config (which the broker runs from main in its signer process, +# effect-grants.json included), the rate guard, every pr-merge script and +# the workflow that wires them, babysit_merge*, babysit_risk.py and +# babysit_review_evidence.py (which compute the review's clear condition), +# the ci-status workflow and +# the pipeline config. +# - `enforcement`: the rest of the modules the floored scripts import and +# the Python that decides the holds (test_merge_holds_sync.py derives both +# closures and requires them here). +# A `trust` or `gate` path holds for a person (HIGH_FLOOR). An `enforcement` +# path holds until the security lane's review binds to the head with no +# open finding (REVIEW_FLOOR, `review-path:`), so no person is needed. +# merge-holds.mjs HOLD_PATH_CLASSES is the same list (test_merge_holds_sync.py). +FLOOR_CLASSES: tuple[tuple[str, str], ...] = ( + (".github/actions/check-trusted-trigger/**", "trust"), + ("**/CODEOWNERS", "trust"), + ("**/.gitmodules", "trust"), + (".github/actions/resolve-config/**", "gate"), + (".github/actions/check-kill-switch/**", "trust"), + (".github/actions/request-lane-token/**", "trust"), + (".github/actions/log-api-budget/rate-guard.mjs", "gate"), + (".github/actions/log-api-budget/rate-limits.json", "gate"), + (".github/scripts/pr-merge/merge-holds.mjs", "gate"), + (".github/scripts/pr-merge/enable-auto-merge.mjs", "gate"), + (".github/scripts/pr-merge/superseded.mjs", "gate"), + (".github/scripts/pr-merge/clear-escalations.mjs", "gate"), + (".github/scripts/pr-merge/carry-run-cache.mjs", "gate"), + (".github/scripts/pr-merge/gate-check.mjs", "gate"), + (".github/scripts/pr-merge/head-ref-map.mjs", "gate"), + (".github/scripts/pr-merge/lane-records.mjs", "gate"), + (".github/scripts/pr-merge/rate-stop.bash", "gate"), + (".github/scripts/pr-merge/refire-refused.mjs", "gate"), + (".github/scripts/pr-merge/rotation.mjs", "gate"), + (".github/scripts/pr-merge/sweep-summary.mjs", "gate"), + (".github/scripts/pr-merge/who.mjs", "gate"), + (".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"), + (".github/scripts/pr-merge/verify-answers.mjs", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_state.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_util.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py", "enforcement"), + (".github/workflows/pr-require-checks.yml", "gate"), + (".github/workflows/pr-merge.yml", "gate"), + ("docs/conventions/pr-pipeline.yaml", "gate"), + (".github/scripts/pr-unblock/address-feedback.mjs", "enforcement"), + (".github/scripts/pr-unblock/gate-source.mjs", "enforcement"), + (".github/scripts/pr-unblock/thread-scope.mjs", "enforcement"), + (".github/scripts/pr-unblock/retry-fetch.mjs", "enforcement"), + (".github/scripts/work-resolve-issue/report-outcome.mjs", "enforcement"), + (".github/scripts/work-resolve-issue/limits.mjs", "enforcement"), + (".github/scripts/work-resolve-issue/retry-fetch.mjs", "enforcement"), +) # fmt: skip +PERSON_CLASSES = frozenset({"trust", "gate"}) +HIGH_FLOOR: tuple[str, ...] = tuple(g for g, c in FLOOR_CLASSES if c in PERSON_CLASSES) +REVIEW_FLOOR: tuple[str, ...] = tuple(g for g, c in FLOOR_CLASSES if c == "enforcement") +# Every floor glob, whichever clears it. +FLOOR: tuple[str, ...] = tuple(g for g, _ in FLOOR_CLASSES) +REVIEW_PATH_PREFIX = "review-path:" +# An enforcement path whose security review can never bind at this head: the +# pull request also changes a lane source (babysit_review_evidence +# `lane-source-changed`), so no lane can clear it and a person does. +UNBINDABLE_PATH_PREFIX = "review-path-unbindable:" +_UNBINDABLE_REASON = "lane-source-changed" # plugin.json keys whose values Claude Code runs or reads as run config. _EXECUTABLE_KEYS = ("hooks", "mcpServers", "lspServers", "monitors") @@ -593,6 +625,20 @@ def _high_path_reasons( hits = [path for path in paths if glob_match(pattern, path)] if hits: reasons.append(Reason(f"high-path:{pattern}", Risk.HIGH, ", ".join(hits))) + if not security_review_clean(inp.lanes): + unbindable = _security_review_unbindable(inp.lanes) + for pattern in REVIEW_FLOOR: + hits = [path for path in paths if glob_match(pattern, path)] + if hits and unbindable: + detail = f"{', '.join(hits)}; it also changes a lane source, so no security review binds to this head" + reasons.append( + Reason(f"{UNBINDABLE_PATH_PREFIX}{pattern}", Risk.HIGH, detail) + ) + elif hits: + detail = f"{', '.join(hits)}; a clean claude-security-review bound to this head clears it" + reasons.append( + Reason(f"{REVIEW_PATH_PREFIX}{pattern}", Risk.HIGH, detail) + ) review = [ f"{f.path}: {', '.join(changes)}" for f, raw in zip(files, raw_files) @@ -977,6 +1023,41 @@ def blocking(lane: Any) -> Any: return getattr(lane, "open_findings", None) if count is None else count +def _security_review_unbindable(raw_lanes: Any) -> bool: + """Whether the security lane's evidence is unbound for good at this head: + its one entry says `lane-source-changed`. Any other unbind (no run yet, a + read that failed) a later review or read can clear.""" + lanes = _sequence(raw_lanes) or () + entries = [lane for lane in lanes if getattr(lane, "lane", None) == _HOLDING_LANE] + return ( + len(entries) == 1 + and getattr(entries[0], "unbound_reason", None) == _UNBINDABLE_REASON + ) + + +def security_review_clean(raw_lanes: Any) -> bool: + """Whether the security lane's review clears a `REVIEW_FLOOR` path: its + one evidence entry is bound to the head, with no unbound reason, and + counts no open finding at all, CRITICAL or below (the evidence does not + tell IMPORTANT from SUGGESTION, so any open finding holds). Anything + else, a missing or doubled entry included, holds.""" + lanes = _sequence(raw_lanes) or () + entries = [lane for lane in lanes if getattr(lane, "lane", None) == _HOLDING_LANE] + if len(entries) != 1: + return False + lane = entries[0] + total = getattr(lane, "open_findings", None) + count = blocking(lane) + return ( + getattr(lane, "bound", None) is True + and getattr(lane, "unbound_reason", None) is None + and _is_count(total) + and total == 0 + and _is_count(count) + and count == 0 + ) + + def _lane_reasons(raw_lanes: Any) -> list[Reason]: lanes = _sequence(raw_lanes) or () reasons = [] @@ -1082,7 +1163,7 @@ def holding(result: RiskResult) -> list[Reason]: # both the merge gate and enable-auto-merge.mjs read. FILES_CAPPED_CODE = "files-capped" PERSON_CODES = frozenset({LANES[_HOLDING_LANE][0], COST_CODE, FILES_CAPPED_CODE}) -PERSON_PREFIXES = (TRUST_GATE_PREFIX, "high-path:") +PERSON_PREFIXES = (TRUST_GATE_PREFIX, "high-path:", UNBINDABLE_PATH_PREFIX) def person_reasons(reasons: Iterable[Reason]) -> list[Reason]: diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py index 3aa5f4f6ee..b08074a324 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk.py @@ -275,50 +275,71 @@ def test_security_finding_is_high(self): def test_floor_paths_are_high_whatever_the_switch(self): # Owner decisions Q-FLOOR (B) and Q-ENFORCE (A): the narrow floor, # with the files that enforce it, holds with high-path-floor off too. + # Owner decision Q26 option B: each glob's class; a person clears a + # `trust` or `gate` path, a clean security review an `enforcement` one. self.assertEqual( - babysit_risk.HIGH_FLOOR, + babysit_risk.FLOOR_CLASSES, ( - ".github/actions/check-trusted-trigger/**", - "**/CODEOWNERS", - "**/.gitmodules", - ".github/actions/resolve-config/**", - ".github/actions/check-kill-switch/**", - ".github/actions/request-lane-token/**", - ".github/actions/log-api-budget/rate-guard.mjs", - ".github/actions/log-api-budget/rate-limits.json", - ".github/scripts/pr-merge/merge-holds.mjs", - ".github/scripts/pr-merge/enable-auto-merge.mjs", - ".github/scripts/pr-merge/superseded.mjs", - ".github/scripts/pr-merge/clear-escalations.mjs", - "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_state.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_util.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", - "plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py", - ".github/workflows/pr-require-checks.yml", - ".github/workflows/pr-merge.yml", - "docs/conventions/pr-pipeline.yaml", - ".github/scripts/pr-unblock/address-feedback.mjs", - ".github/scripts/pr-unblock/gate-source.mjs", - ".github/scripts/pr-unblock/thread-scope.mjs", - ".github/scripts/pr-unblock/retry-fetch.mjs", - ".github/scripts/work-resolve-issue/report-outcome.mjs", - ".github/scripts/work-resolve-issue/limits.mjs", - ".github/scripts/work-resolve-issue/retry-fetch.mjs", + (".github/actions/check-trusted-trigger/**", "trust"), + ("**/CODEOWNERS", "trust"), + ("**/.gitmodules", "trust"), + (".github/actions/resolve-config/**", "gate"), + (".github/actions/check-kill-switch/**", "trust"), + (".github/actions/request-lane-token/**", "trust"), + (".github/actions/log-api-budget/rate-guard.mjs", "gate"), + (".github/actions/log-api-budget/rate-limits.json", "gate"), + (".github/scripts/pr-merge/merge-holds.mjs", "gate"), + (".github/scripts/pr-merge/enable-auto-merge.mjs", "gate"), + (".github/scripts/pr-merge/superseded.mjs", "gate"), + (".github/scripts/pr-merge/clear-escalations.mjs", "gate"), + (".github/scripts/pr-merge/carry-run-cache.mjs", "gate"), + (".github/scripts/pr-merge/gate-check.mjs", "gate"), + (".github/scripts/pr-merge/head-ref-map.mjs", "gate"), + (".github/scripts/pr-merge/lane-records.mjs", "gate"), + (".github/scripts/pr-merge/rate-stop.bash", "gate"), + (".github/scripts/pr-merge/refire-refused.mjs", "gate"), + (".github/scripts/pr-merge/rotation.mjs", "gate"), + (".github/scripts/pr-merge/sweep-summary.mjs", "gate"), + (".github/scripts/pr-merge/who.mjs", "gate"), + (".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"), + (".github/scripts/pr-merge/verify-answers.mjs", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", "gate"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_state.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_util.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", "enforcement"), + ("plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py", "enforcement"), + (".github/workflows/pr-require-checks.yml", "gate"), + (".github/workflows/pr-merge.yml", "gate"), + ("docs/conventions/pr-pipeline.yaml", "gate"), + (".github/scripts/pr-unblock/address-feedback.mjs", "enforcement"), + (".github/scripts/pr-unblock/gate-source.mjs", "enforcement"), + (".github/scripts/pr-unblock/thread-scope.mjs", "enforcement"), + (".github/scripts/pr-unblock/retry-fetch.mjs", "enforcement"), + (".github/scripts/work-resolve-issue/report-outcome.mjs", "enforcement"), + (".github/scripts/work-resolve-issue/limits.mjs", "enforcement"), + (".github/scripts/work-resolve-issue/retry-fetch.mjs", "enforcement"), ), + ) # fmt: skip + self.assertEqual( + babysit_risk.HIGH_FLOOR, + tuple(g for g, c in babysit_risk.FLOOR_CLASSES if c in {"trust", "gate"}), + ) + self.assertEqual( + babysit_risk.REVIEW_FLOOR, + tuple(g for g, c in babysit_risk.FLOOR_CLASSES if c == "enforcement"), ) cases = [ (".gitmodules", "**/.gitmodules"), @@ -350,19 +371,30 @@ def test_floor_paths_are_high_whatever_the_switch(self): (".github/scripts/work-resolve-issue/limits.mjs", ".github/scripts/work-resolve-issue/limits.mjs"), (".github/scripts/work-resolve-issue/retry-fetch.mjs", ".github/scripts/work-resolve-issue/retry-fetch.mjs"), ] # fmt: skip + unreviewed = ( + _lane("claude-review"), + _lane("claude-security-review", bound=False), + ) for path, glob in cases: for on in (True, False): with self.subTest(path, high_floor=on): - result = score(_input(files=(_file(path),), high_floor=on)) + result = score( + _input(files=(_file(path),), high_floor=on, lanes=unreviewed) + ) self.assertEqual(result.score, Risk.HIGH) - self.assertEqual(_codes(result), (f"high-path:{glob}",)) + code = ( + "review-path" + if glob in babysit_risk.REVIEW_FLOOR + else "high-path" + ) + self.assertIn(f"{code}:{glob}", _codes(result)) def test_trusted_paths_off_the_floor_hold_only_when_configured(self): # Owner direction 2026-10-24: a trusted-path match alone holds nothing. for path in ( ".github/actions/log-api-budget/action.yml", ".github/actions/check-trusted-trigger-x/a.mjs", - ".github/scripts/pr-merge/gate-check.mjs", + ".github/scripts/pr-unblock/other.mjs", "docs/CODEOWNERS.md", ): for on in (True, False): @@ -1192,6 +1224,8 @@ def test_the_floor_holds_with_the_switch_off(self): ".github/scripts/pr-merge/enable-auto-merge.mjs", ".github/scripts/pr-merge/superseded.mjs", ".github/scripts/pr-merge/clear-escalations.mjs", + ".github/scripts/pr-merge/gate-check.mjs", + ".github/scripts/pr-merge/who.mjs", "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", @@ -1207,12 +1241,18 @@ def test_the_floor_holds_with_the_switch_off(self): ".github/scripts/work-resolve-issue/retry-fetch.mjs", ): with self.subTest(path): - result = score(_lane_input((_file(path),))) + unreviewed = ( + _lane("claude-review"), + _lane("claude-security-review", open_findings=1), + ) + result = score( + dataclasses.replace(_lane_input((_file(path),)), lanes=unreviewed) + ) self.assertEqual(result.score, Risk.HIGH, result.reasons) def test_trusted_paths_off_the_floor_merge(self): for path in ( - ".github/scripts/pr-merge/gate-check.mjs", + ".github/scripts/pr-unblock/other.mjs", "plugins/source-control/scripts/source-control-babysit-merge", ".github/actions/log-api-budget/action.yml", ): @@ -1742,3 +1782,165 @@ def test_a_reported_flag_beside_a_hold_is_still_listed(self): ) self.assertIn("findings:claude-review", _codes(result)) self.assertTrue(holds(result)) + + +def _floor_sample(glob): + """A path the floor glob `glob` matches.""" + return glob.removeprefix("**/").replace("**", "x.mjs") + + +class ReviewClearedFloorTests(unittest.TestCase): + """Owner decision Q26 option B: an `enforcement` floor path holds as + `review-path:` until the security lane's review is bound to the head with + no open finding; a `trust` or `gate` path holds for a person whatever + the review says.""" + + def _score(self, path, security): + lanes = (_lane("claude-review"), *security) + return score(_input(files=(_file(path),), lanes=lanes)) + + def test_a_clean_bound_security_review_clears_each_enforcement_path(self): + self.assertEqual(len(babysit_risk.REVIEW_FLOOR), 21) + for glob in babysit_risk.REVIEW_FLOOR: + with self.subTest(glob): + result = self._score(glob, (_lane("claude-security-review"),)) + self.assertEqual(result.score, Risk.LOW, _codes(result)) + self.assertFalse(holds(result)) + + def test_an_enforcement_path_holds_without_a_clean_bound_review(self): + security = "claude-security-review" + cases = { + "unbound": (_lane(security, bound=False, unbound_reason="no-run"),), + "bound with an unbound reason": (_lane(security, unbound_reason="x"),), + "a CRITICAL finding": ( + LaneEvidence(security, True, None, 1, blocking_findings=1), + ), + "an IMPORTANT finding open": ( + LaneEvidence(security, True, None, 1, blocking_findings=0), + ), + "an unread count": (_lane(security, open_findings=None),), + "a negative count": (_lane(security, open_findings=-1),), + "a bool count": (_lane(security, open_findings=False),), + "a float count": ( + LaneEvidence(security, True, None, 0.0, blocking_findings=0), + ), + "a CRITICAL count the open count misses": ( + LaneEvidence(security, True, None, 0, blocking_findings=1), + ), + "an unread CRITICAL count": ( + LaneEvidence(security, True, None, 0, blocking_findings=-1), + ), + "no security lane": (), + "two security entries": (_lane(security), _lane(security)), + } + path = B + "babysit_verify_evidence.py" + for name, lanes in cases.items(): + with self.subTest(name): + result = self._score(path, lanes) + self.assertIn(f"review-path:{path}", _codes(result)) + self.assertTrue(holds(result)) + + def test_a_clean_code_review_alone_clears_nothing(self): + path = ".github/scripts/work-resolve-issue/report-outcome.mjs" + lanes = (_lane("claude-security-review", bound=False), _lane("claude-review")) + result = score(_input(files=(_file(path),), lanes=lanes)) + self.assertIn(f"review-path:{path}", _codes(result)) + + def test_the_review_hold_is_not_a_person_code(self): + # enable-auto-merge labels a `high-path:` reason for a person; this + # one the lanes clear by reviewing again. + result = self._score(B + "babysit_gh.py", ()) + codes = [c for c in _codes(result) if c.startswith("review-path:")] + self.assertEqual(codes, [f"review-path:{B}babysit_gh.py"]) + self.assertFalse(any(c.startswith("high-") for c in _codes(result))) + self.assertEqual(babysit_risk.person_reasons(result.reasons), []) + + def test_each_gate_and_trust_path_holds_even_with_a_clean_review(self): + for glob in babysit_risk.HIGH_FLOOR: + with self.subTest(glob): + result = self._score( + _floor_sample(glob), (_lane("claude-security-review"),) + ) + self.assertEqual(_codes(result), (f"high-path:{glob}",)) + self.assertTrue(holds(result)) + + def test_a_review_caller_workflow_change_unbinds_the_review(self): + # Any .github/workflows/ change is a lane source + # (babysit_review_evidence.LANE_SOURCE_PREFIXES), so no lane binds and + # the enforcement path stays held. + import babysit_review_evidence + + path = B + "babysit_verify_evidence.py" + changed = [path, ".github/workflows/pr-review-security-hosted.yml"] + lanes = babysit_review_evidence.bind( + "a" * 40, [], [], [], is_human=lambda _: False, changed_paths=changed + ) + self.assertEqual( + {lane.unbound_reason for lane in lanes}, {"lane-source-changed"} + ) + files = tuple(_file(each) for each in changed) + result = score(_input(files=files, lanes=lanes)) + self.assertIn(f"{babysit_risk.UNBINDABLE_PATH_PREFIX}{path}", _codes(result)) + self.assertTrue(holds(result)) + + +class UnbindableReviewPathTests(unittest.TestCase): + """Owner decision Q26 option B, follow-up: an enforcement path whose + security review can never bind at this head, because the pull request + also changes a lane source (`lane-source-changed`), is a person's + reason; a review not yet run or not read stays a lane's.""" + + PATH = B + "babysit_verify_evidence.py" + + def _score(self, security): + lanes = (_lane("claude-review"), security) + return score(_input(files=(_file(self.PATH),), lanes=lanes)) + + def test_a_lane_source_change_makes_the_hold_a_persons(self): + security = _lane( + "claude-security-review", bound=False, unbound_reason="lane-source-changed" + ) + result = self._score(security) + code = f"{babysit_risk.UNBINDABLE_PATH_PREFIX}{self.PATH}" + self.assertIn(code, _codes(result)) + self.assertNotIn(f"review-path:{self.PATH}", _codes(result)) + self.assertTrue(holds(result)) + self.assertIn( + code, [r.code for r in babysit_risk.person_reasons(result.reasons)] + ) + + def test_a_workflow_change_through_bind_is_a_persons(self): + import babysit_review_evidence + + changed = [self.PATH, ".github/workflows/pr-review-security-hosted.yml"] + lanes = babysit_review_evidence.bind( + "a" * 40, [], [], [], is_human=lambda _: False, changed_paths=changed + ) + result = score(_input(files=tuple(_file(p) for p in changed), lanes=lanes)) + persons = [r.code for r in babysit_risk.person_reasons(result.reasons)] + self.assertIn(f"{babysit_risk.UNBINDABLE_PATH_PREFIX}{self.PATH}", persons) + + def test_a_transient_unbind_stays_a_lanes(self): + for reason in ("no-run", "evidence-error", "changed-files-unread", None): + with self.subTest(reason): + security = _lane( + "claude-security-review", bound=False, unbound_reason=reason + ) + result = self._score(security) + self.assertIn(f"review-path:{self.PATH}", _codes(result)) + self.assertEqual( + [ + r.code + for r in babysit_risk.person_reasons(result.reasons) + if self.PATH in r.code + ], + [], + ) + + def test_a_lane_source_change_on_the_code_lane_alone_is_not_enough(self): + lanes = ( + _lane("claude-review", bound=False, unbound_reason="lane-source-changed"), + _lane("claude-security-review", bound=False, unbound_reason="no-run"), + ) + result = score(_input(files=(_file(self.PATH),), lanes=lanes)) + self.assertIn(f"review-path:{self.PATH}", _codes(result)) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index 661ed313d0..6dc6dd0052 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -9,13 +9,17 @@ lanes App pushes only: another identity with write access can still push a workflow change after auto-merge is armed. -- The floor (owner decisions Q-FLOOR (B) and Q-ENFORCE (A)): `HOLD_PATH_FLOOR` is - `babysit_risk.HIGH_FLOOR`, and `pathHold` holds, with no configured glob, - exactly the paths `glob_match` matches over it, case variants included; - a trusted path off the floor holds only for a configured glob. Every module - the floor actions load is on it, and of babysit-prs/scripts/ exactly the - Python that decides the holds. The review callers' `hold-paths` and - ci-status's exempt-author `hold_patterns` each match every floor entry. +- The floor (owner decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B): + `HOLD_PATH_CLASSES` is `babysit_risk.FLOOR_CLASSES`, glob and class alike, + `HOLD_PATH_FLOOR` is `babysit_risk.HIGH_FLOOR` (its `trust` and `gate` + globs), and `pathHold` holds, with no configured glob, exactly the paths + `glob_match` matches over that, case variants included; an `enforcement` + path is the gate's (`review-path:`), and a trusted path off the floor holds + only for a configured glob. Every module the floor actions load is on the + floor, every pr-merge script as `gate`, and of babysit-prs/scripts/ exactly + the Python that decides the holds, `babysit_merge*` as `gate`. The review + callers' `hold-paths` and ci-status's exempt-author `hold_patterns` each + match every floor entry, whatever its class. - The cost paths: the same globs as `babysit_risk.COST_PATHS`, and the same paths matched (`glob_match` folds case and covers what sits under a glob). - The cost decision: `costDecision` accepts the same comments as @@ -90,7 +94,8 @@ def import_closure(entries: tuple[str, ...]) -> list[str]: def python_closure(js_files: list[str]) -> list[str]: todo = [SCRIPTS / "babysit_risk.py"] - floored = {g for g in risk.HIGH_FLOOR if g.endswith(".mjs")} + # A floor entry for a script not in this tree yet runs nothing here. + floored = {g for g in risk.FLOOR if g.endswith(".mjs") and (ROOT / g).is_file()} for name in sorted(floored | set(js_files)): for path in PY_RUN.findall((ROOT / name).read_text(encoding="utf-8")): todo.append(ROOT / path) @@ -115,11 +120,13 @@ def python_closure(js_files: list[str]) -> list[str]: NODE = """ -import { COST_PATHS, HOLD_PATH_FLOOR, costHold, pathHold } from %s; +import { COST_PATHS, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, REVIEW_FLOOR, costHold, pathHold } from %s; const [paths, globs] = JSON.parse(process.argv[1]); console.log(JSON.stringify({ costPaths: COST_PATHS, + classes: HOLD_PATH_CLASSES, floor: HOLD_PATH_FLOOR, + review: REVIEW_FLOOR, cost: paths.map((path) => costHold([path], null) !== null), held: paths.map((path) => pathHold([path], []) !== null), configured: paths.map((path) => pathHold([path], globs) !== null), @@ -174,6 +181,11 @@ def python_closure(js_files: list[str]) -> list[str]: ".github/Standards/trusted-actors/trusted-actors.json", ".github/actions/check-trusted-trigger/action.yml", ".github/actions/Check-Trusted-Trigger/action.yml", + ".github/scripts/pr-merge/rotation.mjs", + ".github/scripts/pr-merge/rotation.test.mjs", + "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", + ".github/scripts/work-resolve-issue/report-outcome.mjs", "Docs/Conventions/PR-Pipeline.yaml", ".github/actions/other/action.yml", ".github/workflows/x.yml", @@ -208,8 +220,14 @@ def test_the_cost_paths_are_the_same(self) -> None: def test_the_floor_is_the_same_on_both_sides(self) -> None: answer = module(FLOOR_PATHS) + self.assertEqual(answer["classes"], [list(row) for row in risk.FLOOR_CLASSES]) self.assertEqual(answer["floor"], list(risk.HIGH_FLOOR)) - self.assertEqual(len(risk.HIGH_FLOOR), 40) + self.assertEqual(answer["review"], list(risk.REVIEW_FLOOR)) + self.assertEqual(len(risk.FLOOR_CLASSES), 51) + self.assertEqual(len(risk.REVIEW_FLOOR), 21) + self.assertEqual( + {kind for _, kind in risk.FLOOR_CLASSES}, {"trust", "gate", "enforcement"} + ) for path, held, configured in zip( FLOOR_PATHS, answer["held"], answer["configured"], strict=True ): @@ -235,8 +253,28 @@ def test_the_floor_holds_what_its_actions_load(self) -> None: answer = module(files) for path, held in zip(files, answer["held"], strict=True): with self.subTest(path): - self.assertTrue(held) - self.assertTrue(any(risk.glob_match(g, path) for g in risk.HIGH_FLOOR)) + self.assertTrue(any(risk.glob_match(g, path) for g in risk.FLOOR)) + self.assertEqual( + held, any(risk.glob_match(g, path) for g in risk.HIGH_FLOOR) + ) + + def test_every_pr_merge_script_is_gate_code(self) -> None: + # Owner decision Q26 option B: the merge gate's own code stays a + # person's, every script pr-merge.yml runs from that directory. + folder = ROOT / ".github/scripts/pr-merge" + scripts = sorted( + f".github/scripts/pr-merge/{f.name}" + for f in folder.iterdir() + if f.is_file() + and f.suffix in {".mjs", ".bash"} + and ".test." not in f.name + and ".fixture." not in f.name + ) + self.assertIn(".github/scripts/pr-merge/who.mjs", scripts) + gate = {g for g, kind in risk.FLOOR_CLASSES if kind == "gate"} + for path in scripts: + with self.subTest(path): + self.assertIn(path, gate) def test_the_floor_holds_the_python_that_decides_the_holds(self) -> None: files = python_closure(import_closure(FLOOR_ENTRIES)) @@ -246,11 +284,21 @@ def test_the_floor_holds_the_python_that_decides_the_holds(self) -> None: answer = module(files) for path, held in zip(files, answer["held"], strict=True): with self.subTest(path): - self.assertTrue(held) - self.assertTrue(any(risk.glob_match(g, path) for g in risk.HIGH_FLOOR)) + self.assertTrue(any(risk.glob_match(g, path) for g in risk.FLOOR)) + # babysit_merge*, and the scorer and binder that compute the + # security review's clear condition, are the gate's own code, + # a person's; the rest is enforcement, which a clean security + # review clears. + name = pathlib.PurePosixPath(path).name + gate = name.startswith("babysit_merge") or name in { + "babysit_risk.py", + "babysit_review_evidence.py", + } + self.assertEqual(held, gate) + self.assertEqual(path in risk.REVIEW_FLOOR, not gate) # Nothing else of that directory: the floor tracks the closure. self.assertEqual( - sorted(g for g in risk.HIGH_FLOOR if g.startswith(f"{scripts}/")), files + sorted(g for g in risk.FLOOR if g.startswith(f"{scripts}/")), files ) @@ -286,7 +334,7 @@ def test_each_list_covers_every_floor_entry(self) -> None: ) for name, (patterns, match) in lists.items(): self.assertTrue(patterns, name) - for entry in risk.HIGH_FLOOR: + for entry in risk.FLOOR: with self.subTest(list=name, entry=entry): self.assertTrue(any(match(p, entry) for p in patterns)) diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index ff3c6697c2..5c9eded94a 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -436,8 +436,8 @@ STUB_FILES='[["docs/held/x\n::stop-commands::t"]]' STUB_PULL="$(pull 'Closes #1' expect "a newline in a held file name cannot start a workflow command" 1 3 "it changes docs/held/x%0A::stop-commands::t, a hold path" STUB_FILES='[["docs/guide.md"],["docs/held/.x.mjs"]]' STUB_PULL="$(STUB_CHANGED=2 pull 'Closes #1')" expect "a dot-prefixed file under a denied tree fails" 1 3 "it changes docs/held/.x.mjs, a hold path" -# Owner decisions Q-FLOOR (B) and Q-ENFORCE (A): the narrow floor, with the -# files that enforce it, holds with no denied path configured, a dot-prefixed +# Owner decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B: the floor's +# trust and gate entries hold with no denied path configured, a dot-prefixed # file and a renamed-from path included. for path in CODEOWNERS docs/CODEOWNERS .gitmodules .github/actions/resolve-config/resolve.mjs \ .github/actions/check-trusted-trigger/.x.mjs .github/actions/request-lane-token/action.yml \ @@ -445,15 +445,23 @@ for path in CODEOWNERS docs/CODEOWNERS .gitmodules .github/actions/resolve-confi .github/actions/log-api-budget/rate-limits.json \ .github/scripts/pr-merge/merge-holds.mjs .github/scripts/pr-merge/enable-auto-merge.mjs \ .github/scripts/pr-merge/superseded.mjs .github/scripts/pr-merge/clear-escalations.mjs \ - plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py .github/workflows/pr-require-checks.yml \ - .github/workflows/pr-merge.yml \ - docs/conventions/pr-pipeline.yaml .github/scripts/pr-unblock/address-feedback.mjs \ - .github/scripts/pr-unblock/gate-source.mjs .github/scripts/pr-unblock/thread-scope.mjs \ - .github/scripts/pr-unblock/retry-fetch.mjs .github/scripts/work-resolve-issue/report-outcome.mjs \ - .github/scripts/work-resolve-issue/limits.mjs .github/scripts/work-resolve-issue/retry-fetch.mjs; do + .github/scripts/pr-merge/gate-check.mjs .github/scripts/pr-merge/who.mjs \ + plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py \ + plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py .github/workflows/pr-require-checks.yml \ + .github/workflows/pr-merge.yml docs/conventions/pr-pipeline.yaml; do STUB_FILES="[[\"$path\"]]" STUB_PULL="$(pull 'Closes #1')" expect "the floor holds $path" 1 3 "it changes $path, a hold path" done +# Its enforcement entries are the merge gate's, which a clean security review +# bound to the head clears; ci-status leaves them to it. +for path in plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py \ + .github/scripts/pr-unblock/address-feedback.mjs .github/scripts/pr-unblock/gate-source.mjs \ + .github/scripts/pr-unblock/thread-scope.mjs .github/scripts/pr-unblock/retry-fetch.mjs \ + .github/scripts/work-resolve-issue/report-outcome.mjs .github/scripts/work-resolve-issue/limits.mjs \ + .github/scripts/work-resolve-issue/retry-fetch.mjs; do + STUB_FILES="[[\"$path\"]]" STUB_PULL="$(pull 'Closes #1')" + expect "the gate, not ci-status, holds $path" 0 3 "no hold path" +done STUB_FILES='[["docs/guide.md"],["docs/moved.md",".github/actions/resolve-config/x.md"]]' STUB_PULL="$(STUB_CHANGED=2 pull 'Closes #1')" expect "a rename away from a floor path fails on its old path" 1 3 "it changes .github/actions/resolve-config/x.md, a hold path" # Owner direction 2026-10-24: no other path holds for being trusted alone, and @@ -461,7 +469,7 @@ expect "a rename away from a floor path fails on its old path" 1 3 "it changes . # never carries the workflows permission, so no lanes App push changes a # workflow. for path in .github/standards/trusted-actors/README.md docs/conventions/pr-pipeline/README.md \ - plugins/p/.claude-plugin/plugin.json .github/workflows/x.yml .github/scripts/pr-merge/gate-check.mjs .github/actions/other/action.yml \ + plugins/p/.claude-plugin/plugin.json .github/workflows/x.yml .github/scripts/pr-unblock/other.mjs .github/actions/other/action.yml \ .github/actions/log-api-budget/action.yml docs/CODEOWNERS.md; do STUB_FILES="[[\"$path\"]]" STUB_PULL="$(pull 'Closes #1')" expect "$path is no hold path" 0 3 "no hold path"