diff --git a/.changes/pr-pipeline/feat-lane-arm-gate-paths-7d41c2e9.md b/.changes/pr-pipeline/feat-lane-arm-gate-paths-7d41c2e9.md new file mode 100644 index 0000000000..f508735180 --- /dev/null +++ b/.changes/pr-pipeline/feat-lane-arm-gate-paths-7d41c2e9.md @@ -0,0 +1,11 @@ +--- +bump: minor +--- + +### Added + +- `merge.lane-arm-gate` (on) and `merge.person-held` (owner decisions Q7 and Q5): a pull request held only by the merge gate's own code, `babysit_risk.py` and the `.github/scripts/pr-merge/` scripts, merges with no person once both review lanes are bound to the main-run review of its head with no finding, no review thread is open, and pr-merge has attested it. Every other `trust` and `gate` row stays with a person, and `scripts/sync-hold-floor.py` refuses a `gate` row outside the merge gate's code that `merge.person-held` does not list. +- pr-merge's `lane-armed` job (`lane-armed.mjs`) posts the check run `pr-merge/lane-armed` at a head whose gate passed its lane-armed paths and whose hold floor does not shrink against the base, then re-runs its red ci-status job. Each attestation is one `lane-armed {...}` record line, in the check run and as a notice; each refusal logs the failed condition. +- ci-status (`merge-holds.mjs` `laneArmedHold`) holds a lane-armed path until that check run is at the head and names a run of `pr-merge.yml` on the default branch, started by schedule or workflow_dispatch, whose commit is on that branch and which uploaded the run-scoped attestation artifact; a trusted person's approval still lifts it. +- `pr-review-main.yml` also reviews a pull request after a red ci-status when it changes main's lane-armed code, so neither the red status nor the human-gated label blocks the review the attestation needs. clear-escalations lifts that label at the same head once the gate's reader refuses nothing there. +- The review criteria `pr-review-main.yml` copies from main (`plugins/review/skills/**`, `plugins/review/reference/**`) are `enforcement` floor rows: a change to them waits for a clean bound security review. diff --git a/.changes/source-control/feat-lane-arm-gate-paths-a95e03b4.md b/.changes/source-control/feat-lane-arm-gate-paths-a95e03b4.md new file mode 100644 index 0000000000..1e96f8b389 --- /dev/null +++ b/.changes/source-control/feat-lane-arm-gate-paths-a95e03b4.md @@ -0,0 +1,18 @@ +--- +bump: minor +--- + +### Added + +- lane-arm the merge gate's own code on a clean main-run review + + With `merge.lane-arm-gate` on, the merge gate's risk score drops the + `high-path:` hold on `babysit_risk.py` and the pr-merge scripts + (`LANE_ARMED_FLOOR`) when both review lanes are bound to a main-run review + of the head (`LaneEvidence.main_run`) with no open finding and the pull + request has no unresolved review thread (`RiskInput.open_threads`). Those + paths are lane sources, so only a main-run review binds them. The gate + records what pr-merge's attestation needs as `laneArmed` + (`babysit_risk.lane_arm_record`), with the first failed condition. Every + other floor path, a configured denied path and any unread input keep the + person hold. diff --git a/.github/artifact-retention.json b/.github/artifact-retention.json index cd2ac89caa..578d4ae89a 100644 --- a/.github/artifact-retention.json +++ b/.github/artifact-retention.json @@ -44,6 +44,16 @@ "reason": "The post-merge check reads the dispatch record of every merge it judges, back to the merges it has not yet judged.", "pending-cut": "Under owner review (poteto): about 5,660 artifacts at 90 days." }, + "pr-merge-lane-armed-*": { + "retention-days": 7, + "readers": [ + ".github/scripts/pr-merge/merge-holds.mjs laneArmedHold: ci-status passes the merge gate's own code only on this artifact of the pr-merge.yml run its pr-merge/lane-armed check run names" + ], + "reason": "Read by every ci-status run of the head, the merge queue's included, until it merges; past 7 days the next sweep attests the head again.", + "uploads": [ + ".github/workflows/pr-merge.yml#lane-armed" + ] + }, "pr-merge-rerun-*": { "retention-days": "max", "readers": [ diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 978dda84b0..28cf989827 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -105,7 +105,17 @@ // that head: a person removes the label, or a push clears it as head-moved // above and enable-auto-merge labels the new head again if the cause holds. // An unread answer and a failed read keep it; so does a hold or cost path, -// which no reader answer clears. At most NATIVE_READS_PER_RUN reads a run, +// which no reader answer clears, except under merge.lane-arm-gate (owner +// decisions Q7 and Q5): a refusal recorded only on the merge gate's own code +// the switch lane-arms (merge-holds LANE_ARMED_FLOOR; `armedCause`) is read +// again at that head and comes off as `refusal-cleared` only when the reader +// refuses nothing there, which needs both review lanes bound to a main-run +// review of the head with no finding and no open review thread; pr-review-main +// runs that review while the label keeps ci-status red. A person-held +// (merge.person-held) path in it keeps the label. ci-status still holds the +// head until pr-merge's `pr-merge/lane-armed` attestation, and reads the removal as no +// approval: merge-holds counts only a trusted person's approving review, and +// its review-lane hold check fails a review lane's hold an App lifted. At most NATIVE_READS_PER_RUN reads a run, // starting at an offset that moves with the run number (rotated), so labels // past the cap are read on a later run. // @@ -162,8 +172,8 @@ import { spoken, } from "../pr-unblock/gate-source.mjs"; import { isProtectedPath } from "../pr-unblock/thread-scope.mjs"; -import { LANES_APP_LOGIN as DEFAULT_BOT } from "./merge-holds.mjs"; -import { pythonVerify, READER_HOLDS, REFUSAL_MARKER, refusalBody, rotated, verifyHold } from "./enable-auto-merge.mjs"; +import { LANES_APP_LOGIN as DEFAULT_BOT, HOLD_PATH_CLASSES, LANE_ARMED_FLOOR, matchesHoldPath } from "./merge-holds.mjs"; +import { holdPathOnly, pythonVerify, READER_HOLDS, REFUSAL_MARKER, refusalBody, rotated, verifyHold } from "./enable-auto-merge.mjs"; const REASON = /^- Reason: `([a-z-]+)`$/m; const clearedMarker = (reason, head) => ``; @@ -230,10 +240,44 @@ export const LANE_CAUSES = Object.freeze({ // Whether the `kind` reason (verifyHold's) names only causes the lanes clear: // each "; "-separated code on LANE_CAUSES[kind]. function laneCause(kind, reason) { - const codes = reason.slice(READER_HOLDS[kind].length, -1).split("; ").map((part) => part.split(": ")[0]); - return codes.every((code) => LANE_CAUSES[kind].some((entry) => (entry.endsWith(":") ? code.startsWith(entry) : code === entry))); + return reasonCodes(kind, reason).every((code) => onLaneCauses(kind, code)); } +const reasonCodes = (kind, reason) => reason.slice(READER_HOLDS[kind].length, -1).split("; ").map((part) => part.split(": ")[0]); +const onLaneCauses = (kind, code) => LANE_CAUSES[kind].some((entry) => (entry.endsWith(":") ? code.startsWith(entry) : code === entry)); + +// Owner decisions Q7 and Q5 (#10743): with merge.lane-arm-gate on, a hold the +// switch lane-arms (merge-holds LANE_ARMED_FLOOR) was a person's when it was +// recorded and is the lanes' now. A refusal recorded on such a hold alone is +// judged again at its head, and lifted only when the gate's reader refuses +// nothing there (`armedLift`): a person-held path, any other +// person code, or a reader answer short of a pass keeps the label. +// - A risk refusal: each code is on LANE_CAUSES.risk or `high-path:` +// with the glob on LANE_ARMED_FLOOR, and at least one is the latter. +// - A hold-path refusal (merge-holds pathHoldKind's "it changes , a +// hold path", every reason line one): each path matches a LANE_ARMED_FLOOR +// glob and no other `trust` or `gate` floor glob. +// `armed` is LANE_ARMED_FLOOR, a parameter so tests can give it rows. +export function armedCause(refusal, armed = LANE_ARMED_FLOOR) { + if (armed.length === 0 || refusal === null) { + return false; + } + const armedCode = (code) => code.startsWith("high-path:") && armed.includes(code.slice("high-path:".length)); + if (refusal.kind === "path") { + const held = HOLD_PATH_CLASSES.filter(([glob, kind]) => kind !== "enforcement" && !armed.includes(glob)).map(([glob]) => glob); + return refusal.paths.length > 0 && refusal.paths.every((path) => armed.some((glob) => matchesHoldPath(path, glob)) && !held.some((glob) => matchesHoldPath(path, glob))); + } + if (refusal.kind !== "risk") { + return false; + } + const codes = reasonCodes(refusal.kind, refusal.reason); + return codes.some(armedCode) && codes.every((code) => armedCode(code) || onLaneCauses("risk", code)); +} + +// Whether the reader's `evidence` at `head` lifts a refusal `armedCause` +// accepted: the gate refuses nothing at that head. +export const armedLift = (evidence, head) => verifyHold(evidence, head) === null; + // The workflow whose lanes ci-status aggregates. An advisory lane's failure // (pr-test-windows) leaves ci-status green once the label is off, so only @@ -462,10 +506,17 @@ function nativeRefusal(appComments, merged, label, head) { if (!(Date.parse(newest.updated_at) <= Date.parse(newest.created_at))) { return null; } + // A refusal of hold paths alone (enable-auto-merge holdPathOnly), which + // only `armedCause` ever lifts here. + if (holdPathOnly(newest.body, head, label)) { + const paths = lines(newest.body).flatMap((line) => HOLD_PATH_REASON.exec(line)?.slice(1, 2) ?? []); + return { kind: "path", reason: paths.map((path) => `it changes ${path}, a hold path`).join("; "), paths }; + } const reason = lines(newest.body).slice(5, -1).join("\n"); const kind = Object.keys(READER_HOLDS).find((key) => reason.startsWith(READER_HOLDS[key])); return kind !== undefined && refusalBody(true, reason, head, label) === newest.body ? { kind, reason } : null; } +const HOLD_PATH_REASON = /^it changes (.+), a hold path$/; // Whether the reader's `evidence` at `head` lifts the `kind` refusal, whose // recorded cause the caller has found the lanes' (laneCause): nothing @@ -508,7 +559,7 @@ function pairedWithPrMerge(appComments, since) { } // What to clear on `number`, or { reason } saying why nothing is. -async function verdict({ github, checks, repo, number, label, bot, mainSha, lanePrs, verify, reads, reader, trusted }) { +async function verdict({ github, checks, repo, number, label, bot, mainSha, lanePrs, verify, reads, reader, trusted, laneArmed }) { const timeline = await paginate(github, `/repos/${repo}/issues/${number}/timeline`); const { labeled, since, lifted } = labelHistory(timeline, label); if (!isApp(labeled?.actor, bot)) { @@ -572,7 +623,8 @@ async function verdict({ github, checks, repo, number, label, bot, mainSha, lane const native = lanePrs === "native" && isApp(pull.user, bot) ? nativeRefusal(appComments, merged, label, pull.head.sha) : null; // A person's recorded cause stays until a person removes the label or the // head moves (owner decision Q-CLEAR B): no reader answer lifts it. - if (native !== null && !laneCause(native.kind, native.reason)) { + const armed = armedCause(native, laneArmed); + if (native !== null && !armed && (native.kind === "path" || !laneCause(native.kind, native.reason))) { return { reason: `enable-auto-merge's refusal at ${pull.head.sha} is one a person clears (${native.reason})` }; } if (native !== null) { @@ -593,13 +645,17 @@ async function verdict({ github, checks, repo, number, label, bot, mainSha, lane return { reason: `the gate's reader did not answer: ${escapeCommand(error?.message ?? error)}` }; } const held = verifyHold(evidence, pull.head.sha); - if (!liftsRefusal(native.kind, evidence, pull.head.sha)) { + if (armed ? !armedLift(evidence, pull.head.sha) : !liftsRefusal(native.kind, evidence, pull.head.sha)) { return { reason: `enable-auto-merge's refusal still needs a person at ${pull.head.sha}: ${held}` }; } return { ...cleared(timeline, bot, since, [REFUSAL_CLEARED], pull.head.sha, [ `it refused this head (${native.reason})`, - held === null ? "the gate now takes its issue link, verifier verdict and risk score" : `what refuses now is one the lanes clear: ${held}`, + held === null + ? armed + ? "merge.lane-arm-gate lane-arms the merge gate's own code it changes, and the gate now takes its issue link, verifier verdict and risk score: both review lanes are bound to a main-run review of this head with no finding and no review thread is open" + : "the gate now takes its issue link, verifier verdict and risk score" + : `what refuses now is one the lanes clear: ${held}`, ]), threadsRead: true, }; @@ -679,7 +735,7 @@ function trustedIds(path) { } } -export async function main({ env = process.env, github, checks, verify, reader } = {}) { +export async function main({ env = process.env, github, checks, verify, reader, laneArmed = LANE_ARMED_FLOOR } = {}) { try { if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) { throw new Error("GITHUB_REPOSITORY is not owner/repo"); @@ -716,7 +772,7 @@ export async function main({ env = process.env, github, checks, verify, reader } continue; } try { - const result = await verdict({ github, checks, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS, verify, reads, reader, trusted }); + const result = await verdict({ github, checks, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS, verify, reads, reader, trusted, laneArmed }); if (result.reason !== null) { console.log(`clear-escalations: #${number} kept: ${result.reason}`); continue; diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 5e48969b28..a3efa6548c 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -96,12 +96,12 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = return { github, checks, verify, reader, reads, calls, writes: () => calls.filter((call) => call.method !== "GET") }; } -const run = async (w, env = {}) => { +const run = async (w, env = {}, laneArmed = undefined) => { const logs = []; const original = console.log; console.log = (line) => logs.push(line); try { - return { code: await main({ env: { ...ENV, ...env }, github: w.github, checks: w.checks, verify: w.verify, reader: w.reader }), logs }; + return { code: await main({ env: { ...ENV, ...env }, github: w.github, checks: w.checks, verify: w.verify, reader: w.reader, ...(laneArmed ? { laneArmed } : {}) }), logs }; } finally { console.log = original; } @@ -789,6 +789,83 @@ test("a lane-class label comes off at its head once nothing refuses", async () = } }); +// Owner decisions Q7 and Q5 (#10743): with merge.lane-arm-gate on, a refusal +// recorded only on the merge gate's own code ("it changes , a hold +// path" from before it was lane-armed, or the risk score's `high-path:` on +// one) is read again at its head and comes off only when the reader refuses +// nothing there. ci-status still holds the head until pr-merge's +// attestation, which the label no longer blocks: pr-review-main reviews a +// lane-armed head after a red ci-status too. +const WHO = ".github/scripts/pr-merge/who.mjs"; +const RISK_PY = "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py"; +const AGENTS_PATH = `it changes ${WHO}, a hold path`; +const AGENTS_RISK = risk(`high-path:${WHO}: ${WHO}`); +test("a refusal on lane-armed gate paths alone comes off at its head once the gate's reader refuses nothing", async () => { + for (const reason of [ + AGENTS_PATH, + `${AGENTS_PATH}\nit changes ${RISK_PY}, a hold path`, + AGENTS_RISK, + risk(`high-path:${RISK_PY}: ${RISK_PY}`), + risk(`high-path:${WHO}: ${WHO}; evidence-unbound:claude-review: not bound: rerun-allowed`), + ]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence: clean }); + const { code, logs } = await run(w, NATIVE); + assert.equal(code, 0, logs.join("\n")); + assert.equal(w.reads.length, 1, reason); + assert.equal(w.writes().length, 2, reason); + assert.equal(w.writes()[0].body.body.split("\n")[0], ``, reason); + assert.match(w.writes()[0].body.body, /merge\.lane-arm-gate lane-arms the merge gate's own code it changes/, reason); + assert.deepEqual(w.writes()[1], DELETE, reason); + } +}); + +test("a refusal on lane-armed gate paths stays while the gate's reader refuses anything at its head", async () => { + for (const evidence of [ + // Still the person's high-path hold: a lane unbound or not main-run, a finding, an open thread. + reviewsNow(`high-path:${WHO}: ${WHO}`), + // A lane cause a lane-class label would tolerate is not enough here. + reviewsNow("evidence-unbound:claude-review: not bound: rerun-allowed"), + reviewsNow("unreadable: no agent identity"), + refusing("verify:fail"), + refusing("verify:missing"), + (pull) => ({ ...clean(pull), head: "c".repeat(40) }), + ]) { + for (const reason of [AGENTS_PATH, AGENTS_RISK]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence }); + const { code, logs } = await run(w, NATIVE); + assert.equal(code, 0, logs.join("\n")); + assert.equal(w.reads.length, 1, reason); + assert.deepEqual(w.writes(), [], `${reason} ${evidence}`); + assert.match(logs.join("\n"), /still needs a person/, reason); + } + } +}); + +test("a refusal naming a person-held path is never read or lifted at its head", async () => { + for (const reason of [ + "it changes .github/workflows/pr-require-checks.yml, a hold path", + "it changes AGENTS.md, a hold path", + "it changes .claude/settings.json, a hold path", + "it changes plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py, a hold path", + "it changes .github/workflows/pr-review-main.yml, a hold path", + `${AGENTS_PATH}\nit changes .github/workflows/pr-merge.yml, a hold path`, + risk("high-path:.github/workflows/pr-require-checks.yml: .github/workflows/pr-require-checks.yml"), + risk("high-path:.github/actions/request-lane-token/**: .github/actions/request-lane-token/action.yml"), + risk(`high-path:${WHO}: ${WHO}; high-path:docs/conventions/pr-pipeline.yaml: docs/conventions/pr-pipeline.yaml`), + risk(`high-path:${WHO}: ${WHO}; security-finding: 1 open`), + // A path off the floor is a configured denied path: a person's. + "it changes docs/held.md, a hold path", + risk("high-path:**/AGENTS.md: AGENTS.md"), + ]) { + const w = world({ timeline: nativeEscalated(reason), author: BOT, evidence: clean }); + const { code, logs } = await run(w, NATIVE); + assert.equal(code, 0, logs.join("\n")); + assert.deepEqual(w.reads, [], reason); + assert.deepEqual(w.writes(), [], reason); + assert.match(logs.join("\n"), /one a person clears/, reason); + } +}); + // A risk score is read only when the verdict passes (babysit_verify_evidence.py // `evidence`), so a refusal or an unread answer hides a lane's recorded cause: // the label stays. diff --git a/.github/scripts/pr-merge/enable-auto-merge.test.mjs b/.github/scripts/pr-merge/enable-auto-merge.test.mjs index 36465d3106..82a0a65b53 100644 --- a/.github/scripts/pr-merge/enable-auto-merge.test.mjs +++ b/.github/scripts/pr-merge/enable-auto-merge.test.mjs @@ -400,13 +400,11 @@ test("a floor path no configured glob names is commented on and labeled human-ga ".github/actions/log-api-budget/rate-limits.json", ".github/CODEOWNERS", "vendor/.gitmodules", - ".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/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_review_evidence.py", + "scripts/sync-hold-floor.py", + ".github/scripts/pr-review-main/write-verdict.mjs", + "AGENTS.md", "docs/conventions/pr-pipeline.yaml", // A workflow that mints a lane token is a gate path. ".github/workflows/pr-merge.yml", @@ -417,11 +415,18 @@ test("a floor path no configured glob names is commented on and labeled human-ga ...floor.map((filename, index) => pull(index + 1, { files: [{ filename }] })), 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" }] }), + // Owner decision Q5: the merge gate's own code is the gate's to judge on + // a passing read, never a hold path here; ci-status then holds it until + // pr-merge's attestation is at the head. + pull(96, { files: [{ filename: ".github/scripts/pr-merge/clear-escalations.mjs" }] }), + pull(97, { files: [{ filename: "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py" }] }), ]); assert.equal(code, 0); assert.deepEqual(writes, [ { mutation: "arm", id: "PR_98", head: HEAD }, { mutation: "arm", id: "PR_99", head: HEAD }, + { mutation: "arm", id: "PR_96", head: HEAD }, + { mutation: "arm", id: "PR_97", head: HEAD }, ]); assert.deepEqual( notes, diff --git a/.github/scripts/pr-merge/floor-ast.py b/.github/scripts/pr-merge/floor-ast.py new file mode 100755 index 0000000000..35edb47962 --- /dev/null +++ b/.github/scripts/pr-merge/floor-ast.py @@ -0,0 +1,233 @@ +#!/usr/bin/env python3 +"""Judge a pull request's babysit_risk.py against the base copy, for +lane-armed.mjs, by parsing both and never importing or running either. + +Usage: python3 -I floor-ast.py +Exit 0 and no output: the head binds the floor as the base does. Exit 1 +with one line: why the head could rebind it. Any other exit refuses too. + +A line test cannot see a rebinding spread over lines, aliased +(`g = globals`) or reached by reflection, so the trees are compared: + +- The head may reference no reflective or frame name (REFLECTIVE, FRAMES) + more often than the base does, counted over every Name, every attribute + and every imported name, so an alias or a split still counts. Nor may it + hold more dunder names (`__globals__`, `__dict__` and any other) or more + imports than the base. +- Each floor table (TABLES) is bound exactly once in the head, at module + level, by a plain or annotated assignment to that one name, with a value + `ast.literal_eval` accepts. Each set derived from them (DERIVED) is bound + the same way, to the same expression as in the base. Any other binding of + either (augmented, deleted, a loop, with, except or match target, an + import alias, global or nonlocal, a walrus, an argument, a def or class) + refuses. +""" + +from __future__ import annotations + +import ast +import sys +from collections import Counter + +REFLECTIVE = frozenset( + { + "globals", + "locals", + "vars", + "setattr", + "delattr", + "getattr", + "exec", + "eval", + "compile", + "__import__", + "importlib", + "__builtins__", + "builtins", + "__dict__", + "__setattr__", + "__delattr__", + "modules", + "__class__", + "__getattribute__", + } +) +# `re.compile` is a pattern, not the builtin; the builtin is a Name, and +# `builtins.compile` counts through `builtins`. +NOT_AS_ATTRIBUTE = frozenset({"compile"}) +# Frame and introspection names: a frame's globals are the module's, so +# `sys._getframe().f_globals[...]` rebinds a table as `globals()` would. +FRAMES = frozenset( + { + "f_globals", + "f_locals", + "f_back", + "_getframe", + "currentframe", + "gi_frame", + "cr_frame", + "ctypes", + "gc", + "inspect", + } +) +# Kinds counted whatever the name: any dunder Name or attribute (a +# function's `__globals__` is the module's namespace), and any import. +DUNDER = "a dunder name" +IMPORT = "an import" +TABLES = ("FLOOR_CLASSES", "LANE_ARM_GATE", "LANE_ARMED", "PERSON_HELD", "COST_PATHS") +DERIVED = ( + "PERSON_CLASSES", + "LANE_ARMED_FLOOR", + "HIGH_FLOOR", + "REVIEW_FLOOR", + "AUTHOR_REVIEW_CLASSES", + "_NO_AUTHOR_LIFT", + "AUTHOR_REVIEW_FLOOR", + "FLOOR", +) +NAMES = frozenset((*TABLES, *DERIVED)) + + +def reflective(tree: ast.AST) -> Counter[str]: + """How often `tree` references each reflective or frame name, any dunder + name (DUNDER) and any import (IMPORT).""" + found: Counter[str] = Counter() + names = REFLECTIVE | FRAMES + for node in ast.walk(tree): + if isinstance(node, (ast.Import, ast.ImportFrom)): + found[IMPORT] += 1 + name = ( + node.id + if isinstance(node, ast.Name) + else node.attr + if isinstance(node, ast.Attribute) + else None + ) + if name is not None: + if len(name) > 4 and name.startswith("__") and name.endswith("__"): + found[DUNDER] += 1 + if name in names and not ( + isinstance(node, ast.Attribute) and name in NOT_AS_ATTRIBUTE + ): + found[name] += 1 + elif isinstance(node, ast.alias): + for part in (*node.name.split("."), node.asname): + if part in names: + found[part] += 1 + return found + + +def bound(node: ast.AST) -> list[str]: + """Every floor name `node` binds, however.""" + names: list[str] = [] + + def targets(target: ast.AST) -> None: + for each in ast.walk(target): + if isinstance(each, ast.Name): + names.append(each.id) + + if isinstance(node, ast.Assign): + for target in node.targets: + targets(target) + elif isinstance(node, (ast.AugAssign, ast.AnnAssign, ast.NamedExpr)): + targets(node.target) + elif isinstance(node, ast.Delete): + for target in node.targets: + targets(target) + elif isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)): + targets(node.target) + elif isinstance(node, ast.withitem) and node.optional_vars is not None: + targets(node.optional_vars) + elif isinstance(node, ast.alias): + names.append(node.asname or node.name.split(".")[0]) + elif isinstance(node, (ast.Global, ast.Nonlocal)): + names.extend(node.names) + elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + names.append(node.name) + elif isinstance(node, ast.arg): + names.append(node.arg) + elif isinstance(node, ast.ExceptHandler) and node.name: + names.append(node.name) + elif isinstance(node, (ast.MatchAs, ast.MatchStar)) and node.name: + names.append(node.name) + elif isinstance(node, ast.MatchMapping) and node.rest: + names.append(node.rest) + return [name for name in names if name in NAMES] + + +def bindings(tree: ast.Module) -> dict[str, list[ast.AST]]: + found: dict[str, list[ast.AST]] = {name: [] for name in NAMES} + for node in ast.walk(tree): + for name in bound(node): + found[name].append(node) + return found + + +def value(tree: ast.Module, name: str) -> tuple[ast.expr | None, str | None]: + """The value of `name`'s one module-level plain binding, or why not.""" + nodes = bindings(tree)[name] + if len(nodes) != 1: + return None, f"{name} is bound {len(nodes)} times" + node = nodes[0] + plain = ( + isinstance(node, ast.Assign) + and len(node.targets) == 1 + and isinstance(node.targets[0], ast.Name) + ) or ( + isinstance(node, ast.AnnAssign) + and isinstance(node.target, ast.Name) + and node.value is not None + ) + if not plain or node not in tree.body: + return None, f"{name} is not bound by one module-level assignment" + return node.value, None + + +def judge(base_text: str, head_text: str) -> str | None: + try: + base, head = ast.parse(base_text), ast.parse(head_text) + except (SyntaxError, ValueError) as error: + return f"a copy does not parse: {error.__class__.__name__}" + before, after = reflective(base), reflective(head) + for name in sorted(after): + if after[name] > before[name]: + return ( + f"it adds {name}" + if name in (DUNDER, IMPORT) + else f"it adds a reference to {name}" + ) + for name in NAMES: + old, why = value(base, name) + if why: + return f"the base copy: {why}" + new, why = value(head, name) + if why: + return why + if name in TABLES: + try: + ast.literal_eval(new) + except ValueError: + return f"{name} is not a literal" + elif ast.dump(new) != ast.dump(old): + return f"it changes how {name} is derived" + return None + + +def main(argv: list[str]) -> int: + if len(argv) != 2: + print("usage: floor-ast.py ") + return 2 + with ( + open(argv[0], encoding="utf-8") as base, + open(argv[1], encoding="utf-8") as head, + ): + why = judge(base.read(), head.read()) + if why: + print(why) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/.github/scripts/pr-merge/lane-armed.mjs b/.github/scripts/pr-merge/lane-armed.mjs new file mode 100755 index 0000000000..81daadd6d5 --- /dev/null +++ b/.github/scripts/pr-merge/lane-armed.mjs @@ -0,0 +1,423 @@ +#!/usr/bin/env node +// lane-armed.mjs: posts pr-merge's `pr-merge/lane-armed` attestation (owner +// decision Q5) on the first candidate whose gate passed its lane-armed paths, +// from the sweep's pr-merge-summary.json, and names the artifact the next +// step uploads; `rerun` then re-runs that head's red ci-status job so +// ci-status reads both (merge-holds.mjs laneArmedHold). No model. +// +// The check run alone proves nothing: any workflow with checks: write posts +// as the same app and can copy this run's id. The artifact +// laneArmedArtifact(pr, head), which only this run's own steps can upload, is +// the proof, so one head is attested a run (an upload step names one +// artifact); a later passing head waits for the next run. +// +// A row attests only when the gate recorded `laneArmed` with no refusal (both +// review lanes bound to a clean main-run review of the head, no open review +// thread, a risk score and verdict that pass: babysit_risk.lane_arm_record), +// the row's head is its pull request's head, and its change leaves the hold +// floor whole (floorChange). The head's text is never parsed: a decoy marker +// pair in a docstring or block comment, or a row inside an expression that +// drops it, would fool any text parse short of the language's own. The +// change is judged instead, against the base's marked regions (main's copy, +// so trusted): in each floor-definition file a patch line may name a floor +// marker or a hold identifier only as an added row of the exact generated +// shape inside a base region, and an added line holds no non-ASCII +// character. babysit_risk.py's head and base copies are also parsed, never +// run, by floor-ast.py, which refuses a new reflective reference and any +// rebinding of a floor table or a set derived from one. The base copy is read at the merge base the patch is +// measured from. A removed row is a shrink. Any read that fails, a missing +// patch, or a cut-short file list refuses. +// +// Each attestation is one countable record: the line `lane-armed {...}` in +// the check run's summary, printed again as a `::notice::`. Each refusal +// prints a `::notice::lane-armed refused` line naming the failed condition. +// +// Environment: +// GH_TOKEN the job's own token: checks: write, actions: write, +// pull-requests: read, contents: read +// GITHUB_REPOSITORY owner/repo +// SUMMARY_PATH pr-merge-summary.json (attest) +// LANE_ARMED_OUT the directory the record file is written to (attest) +// GITHUB_OUTPUT gets `artifact` and `head` for the upload (attest) +// LANE_ARMED_HEAD the attested head (rerun) +// GITHUB_RUN_ID, GITHUB_SERVER_URL, GITHUB_API_URL +import { Buffer } from "node:buffer"; +import { execFileSync } from "node:child_process"; +import { appendFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import process from "node:process"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +import { createGitHub } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; +import { LANE_ARMED_CHECK, laneArmedArtifact } from "./merge-holds.mjs"; + +const REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; +const SHA = /^[0-9a-f]{40}$/; +const RUN_ID = /^[1-9][0-9]*$/; +const RISK = "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py"; +const HOLDS = ".github/scripts/pr-merge/merge-holds.mjs"; +const LANE_ARMED_MJS = ".github/scripts/pr-merge/lane-armed.mjs"; +// The files that define the floor or how it is read; every other file holding +// a generated floor block (the review callers, pr-require-checks.yml, +// scripts/sync-hold-floor.py) is person-held, so the gate never attests it. +const FLOOR_AST = ".github/scripts/pr-merge/floor-ast.py"; +export const FLOOR_FILES = Object.freeze([RISK, HOLDS, LANE_ARMED_MJS, FLOOR_AST]); +const MARKER = /\b(?:BEGIN|END) (?:floor-rows|hold-floor|js-dynamic)\b/; +// A non-ASCII character in an added line: Python folds a fullwidth or other +// NFKC-equivalent identifier to ASCII, so a fullwidth `FLOOR_CLASSES = ()` +// rebinds it past IDENTIFIER. Reflection and aliasing in babysit_risk.py are +// floor-ast.py's to judge, on the parsed trees. +const NON_ASCII = /[^\x00-\x7f]/; +// The names that hold the floor tables and the sets derived from them. +const IDENTIFIER = + /\b(?:FLOOR|FLOOR_CLASSES|HOLD_PATH_CLASSES|HOLD_PATH_FLOOR|HIGH_FLOOR|REVIEW_FLOOR|AUTHOR_REVIEW_FLOOR|AUTHOR_REVIEW_CLASSES|PERSON_CLASSES|PERSON_HELD|_NO_AUTHOR_LIFT|GATE_WORKFLOWS|COST_PATHS|LANE_ARM\w*)\b/; +const LANES = ["claude-review", "claude-security-review"]; +const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); + +// The marker pairs that bound each file's floor rows and its generated +// LANE_ARMED list, and the one shape each line between them may take: a +// comment, or exactly what the generator writes. Any other line (a spread, a +// slice, a block comment, a string spanning lines) could evaluate to a list +// that differs from its text, so it refuses. +const GLOB = String.raw`"([^"\\]+)"`; +const KIND = String.raw`"(trust|gate|enforcement)"`; +const SHAPES = { + [RISK]: { + rows: "floor-rows FLOOR_CLASSES", + comment: /^\s*#(?: .*)?$/, + row: new RegExp(String.raw`^ \(${GLOB}, ${KIND}\),$`), + armed: [/^LANE_ARM_GATE = (?:True|False)$/, /^LANE_ARMED: tuple\[str, \.\.\.\] = \($/, /^PERSON_HELD: tuple\[str, \.\.\.\] = \($/, /^\)$/], + }, + [HOLDS]: { + rows: "hold-floor HOLD_PATH_CLASSES", + comment: /^\s*\/\/(?: .*)?$/, + row: new RegExp(String.raw`^ \[${GLOB}, ${KIND}\],$`), + armed: [/^export const LANE_ARM_GATE = (?:true|false);$/, /^export const LANE_ARMED = Object\.freeze\(\[$/, /^export const PERSON_HELD = Object\.freeze\(\[$/, /^\]\);$/], + }, +}; +const ARMED_MARKER = "hold-floor LANE_ARM"; +const ENTRY = { [RISK]: new RegExp(String.raw`^ ${GLOB},$`), [HOLDS]: new RegExp(String.raw`^ ${GLOB},$`) }; + +// The 1-based line numbers of the one `BEGIN ` and the one +// `END ` comment line, or null when either is missing or repeated, or +// END comes first. +export function bounds(lines, name) { + const at = (word) => lines.flatMap((line, i) => (new RegExp(`^\\s*(?:#|//) ${word} ${name}(?::.*)?$`).test(line) ? [i + 1] : [])); + const [begin, end] = [at("BEGIN"), at("END")]; + return begin.length === 1 && end.length === 1 && begin[0] < end[0] ? { begin: begin[0], end: end[0] } : null; +} + +// The base copy of `path` (a floor table file) read exactly: its rows region +// and LANE_ARM region, each line between the markers a comment or the shape +// the generator writes, and which LANE_ARM list each base line sits in. Null +// on any other line, a marker missing, repeated or out of order, or no row. +export function floorOf(text, path) { + const shape = SHAPES[path]; + const lines = String(text).split("\n"); + const rows = shape && bounds(lines, shape.rows); + const armed = shape && bounds(lines, ARMED_MARKER); + if (!rows || !armed) { + return null; + } + const kinds = new Map(); + for (const line of lines.slice(rows.begin, rows.end - 1)) { + const row = shape.row.exec(line); + if (row) kinds.set(row[1], row[2]); + else if (!shape.comment.test(line)) return null; + } + const lists = new Map([[armed.begin, null]]); + let list = null; + for (let n = armed.begin + 1; n < armed.end; n += 1) { + const line = lines[n - 1]; + if (ENTRY[path].test(line)) { + if (list === null) return null; + } else if (shape.armed[1].test(line)) list = "LANE_ARMED"; + else if (shape.armed[2].test(line)) list = "PERSON_HELD"; + else if (shape.armed[3].test(line)) list = null; + else if (!shape.armed[0].test(line) && !shape.comment.test(line)) return null; + lists.set(n, list); + } + return kinds.size === 0 ? null : { shape, rows, armed, kinds, lists }; +} + +// Each changed line of a unified-diff `patch`: { sign, text, at }, where `at` +// is the base line a removed line was, or the base line an added line +// follows. Null for a patch that is not a well-formed diff. +export function patchLines(patch) { + if (typeof patch !== "string" || !patch.startsWith("@@")) return null; + const out = []; + let old = null; + for (const line of patch.split("\n")) { + const hunk = /^@@ -(\d+)(,\d+)? \+\d+(?:,\d+)? @@/.exec(line); + if (hunk) { + // A pure insertion's header names the line it follows. + old = Number(hunk[1]) + (hunk[2] === ",0" ? 1 : 0); + continue; + } + if (old === null) return null; + const [sign, text] = [line[0], line.slice(1)]; + if (sign === " ") old += 1; + else if (sign === "-") out.push({ sign, text, at: old++ }); + else if (sign === "+") out.push({ sign, text, at: old - 1 }); + else if (sign !== "\\" && line !== "") return null; + } + return out; +} + +// Why the change `patch` to floor-definition file `path` could weaken the +// floor, or null. `base` is floorOf of the base copy, required for a floor +// table file. +export function floorChange(path, patch, base, locked = null) { + const changes = patchLines(patch); + if (changes === null) return `${path} has no readable patch`; + // Only babysit_risk.py and merge-holds.mjs hold floor tables; the judge + // code (lane-armed.mjs, floor-ast.py) is checked for markers and names. + const table = Object.hasOwn(SHAPES, path); + if (table && !base) return `the base copy of ${path} cannot be read exactly`; + for (const { sign, text, at } of changes) { + const what = `${sign === "+" ? "an added" : "a removed"} line of ${path}`; + if (MARKER.test(text)) return `${what} names a floor marker`; + if (locked && (sign === "-" ? locked.begin < at && at < locked.end : locked.begin <= at && at < locked.end)) return `${what} is inside the JS_DYNAMIC list`; + if (sign === "+" && NON_ASCII.test(text)) return `${what} holds a non-ASCII character`; + // A removed line sits strictly between the markers; an added one follows + // BEGIN or a line before END. + const inside = (r) => (sign === "-" ? r.begin < at && at < r.end : r.begin <= at && at < r.end); + if (table && inside(base.rows)) { + const row = base.shape.row.exec(text); + if (sign === "-" && row) return `it removes the floor row ${row[1]}`; + if (sign === "-" ? !base.shape.comment.test(text) : !row) return `${what} inside the floor rows is not a row of the generated shape`; + continue; + } + if (table && inside(base.armed)) { + if (sign === "+" && ENTRY[path].test(text) && base.lists.get(at) === "PERSON_HELD") continue; + return `${what} changes the generated LANE_ARM block other than by a person-held entry`; + } + const named = IDENTIFIER.exec(text); + if (named) return `${what} names ${named[0]} outside the floor rows`; + } + return null; +} + +// The JS ways to run or reach code by a name no line spells: a head copy of +// merge-holds.mjs or lane-armed.mjs may hold none of these more often than +// its base copy. Counted over the whole text but this list, so splitting a +// string around a table name does not help: the token itself must appear. +// Each is call-shaped or otherwise not a path segment, so a floor row such +// as `.github/workflows/eval-browser-dispatch.yml` counts as nothing. The +// list sits in its own marker block: its literals are not counted, and a +// patch line inside the block refuses, so deleting one literal cannot make +// room for a real call. +const JS_DYNAMIC = Object.freeze([ + // BEGIN js-dynamic JS_DYNAMIC + ["eval(", /\beval\s*\(/g], + ["Function(", /\bFunction\s*\(/g], + ["new Function", /\bnew\s+Function\b/g], + ["globalThis", /\bglobalThis\b/g], + ["Reflect.", /\bReflect\s*\./g], + ["Object.defineProperty", /\bObject\s*\.\s*defineProperty\b/g], + ["__proto__", /__proto__/g], + ["import(", /\bimport\s*\(/g], + ["require(", /\brequire\s*\(/g], + ["vm.", /\bvm\s*\./g], + ["process.binding", /\bprocess\s*\.\s*binding\b/g], + // END js-dynamic JS_DYNAMIC +]); +const DYNAMIC_BLOCK = "js-dynamic JS_DYNAMIC"; + +// Why the head copy of a JS floor file reaches code more dynamically than +// its base copy, or null. +export function jsRefusal(baseText, headText) { + // The text less the one JS_DYNAMIC block, when a copy has exactly one. + const outside = (text) => { + const lines = String(text).split("\n"); + const block = bounds(lines, DYNAMIC_BLOCK); + return block ? [...lines.slice(0, block.begin - 1), ...lines.slice(block.end)].join("\n") : lines.join("\n"); + }; + const [base, head] = [outside(baseText), outside(headText)]; + for (const [name, pattern] of JS_DYNAMIC) { + const count = (text) => (text.match(pattern) ?? []).length; + if (count(head) > count(base)) return `it adds ${name}`; + } + return null; +} + +// floor-ast.py from this checkout (the base's, never the head's) on the base +// and head copies of babysit_risk.py: its one line of output, "" when it +// accepts the head, or why it refused. +export function astRefusal(baseText, headText, run = execFileSync) { + const dir = mkdtempSync(join(tmpdir(), "lane-armed-ast-")); + try { + writeFileSync(join(dir, "base.py"), baseText); + writeFileSync(join(dir, "head.py"), headText); + try { + run("python3", ["-I", fileURLToPath(new URL("./floor-ast.py", import.meta.url)), join(dir, "base.py"), join(dir, "head.py")], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); + return ""; + } catch (error) { + return String(error?.stdout ?? "").trim() || "floor-ast.py could not judge it"; + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +// Why the pull request's change could weaken the floor, or null. The base +// copy and the patches come from one compare of the base branch with the +// head: GitHub measures a pull request's patch from the merge base, not the +// base branch's current tip, so the base copy is read at that merge base. +// The compare lists at most 300 files, fewer than the pull request's +// `changed_files` refuses. +async function floorRefusal(github, repo, pull) { + const compare = await github("GET", `/repos/${repo}/compare/${pull.base.sha}...${pull.head.sha}`); + const mergeBase = compare?.merge_base_commit?.sha; + if (!SHA.test(String(mergeBase))) return "its merge base cannot be read"; + const files = compare.files; + if (!Array.isArray(files) || !Number.isSafeInteger(pull.changed_files) || files.length < pull.changed_files) return "its file list is cut short"; + for (const file of files) { + const touched = [file.filename, file.previous_filename].filter((name) => FLOOR_FILES.includes(name)); + if (touched.length === 0) continue; + if (file.status !== "modified" || file.previous_filename) return `it ${file.status} ${touched[0]}`; + const baseText = await text(github, repo, file.filename, mergeBase); + const base = Object.hasOwn(SHAPES, file.filename) ? floorOf(baseText, file.filename) : null; + let locked = null; + if (file.filename === LANE_ARMED_MJS) { + locked = bounds(baseText.split("\n"), DYNAMIC_BLOCK); + if (!locked) return `the base copy of ${LANE_ARMED_MJS} has no single JS_DYNAMIC block`; + } + const why = floorChange(file.filename, file.patch, base, locked); + if (why) return why; + if (file.filename === RISK) { + const parsed = astRefusal(await text(github, repo, RISK, mergeBase), await text(github, repo, RISK, pull.head.sha)); + if (parsed) return `its ${RISK}: ${parsed}`; + } + if (file.filename.endsWith(".mjs")) { + const dynamic = jsRefusal(await text(github, repo, file.filename, mergeBase), await text(github, repo, file.filename, pull.head.sha)); + if (dynamic) return `its ${file.filename}: ${dynamic}`; + } + } + return null; +} + +// The record an attestation carries, from a summary row and this run. +export const recordOf = (row, run) => ({ + v: 1, + pr: row.pr, + head: row.head, + paths: row.laneArmed.paths, + lanes: Object.fromEntries(LANES.map((lane) => [lane, row.laneArmed.lanes?.[lane] ?? null])), + threads: row.laneArmed.threads, + run: Number(run), +}); + +// Why `row` does not attest before any read, or null. +export function rowRefusal(row) { + const armed = row?.laneArmed; + if (!armed || typeof armed !== "object") return "the gate recorded no lane-armed paths"; + if (armed.refused) return String(armed.refused); + if (!Array.isArray(armed.paths) || armed.paths.length === 0 || !armed.paths.every((path) => typeof path === "string")) return "the gate recorded no lane-armed paths"; + if (armed.threads !== 0) return "the open review threads are not zero"; + if (!LANES.every((lane) => armed.lanes?.[lane] === 0)) return "a review lane has an open finding"; + return null; +} + +async function text(github, repo, path, ref) { + const file = await github("GET", `/repos/${repo}/contents/${path}?ref=${ref}`); + if (file?.encoding !== "base64" || typeof file.content !== "string") throw new Error(`${path} at ${ref} is not a file`); + return Buffer.from(file.content, "base64").toString("utf8"); +} + +// Re-run the ci-status job of the head's newest pull_request run of +// pr-require-checks.yml when it finished red; a running one reads the +// attestation itself. +async function rerunCiStatus(github, repo, head) { + const runs = (await github("GET", `/repos/${repo}/actions/workflows/pr-require-checks.yml/runs?head_sha=${head}&event=pull_request&per_page=20`))?.workflow_runs; + const run = (Array.isArray(runs) ? runs : []).reduce((a, b) => (a === null || b.id > a.id ? b : a), null); + if (!run || run.status !== "completed") return "no finished ci-status run to re-run"; + const jobs = (await github("GET", `/repos/${repo}/actions/runs/${run.id}/jobs?per_page=100`))?.jobs; + const job = (Array.isArray(jobs) ? jobs : []).find((each) => each?.name === "ci-status"); + if (!job || job.conclusion === "success") return "ci-status is not red"; + await github("POST", `/repos/${repo}/actions/jobs/${job.id}/rerun`); + return `re-ran ci-status job ${job.id}`; +} + +export async function main({ env = process.env, github, fetchImpl, mode = process.argv[2] ?? "attest" } = {}) { + try { + if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) throw new Error("GITHUB_REPOSITORY is not owner/repo"); + if (!RUN_ID.test(env.GITHUB_RUN_ID ?? "")) throw new Error("GITHUB_RUN_ID is not a run id"); + const repo = env.GITHUB_REPOSITORY; + github ??= createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined, fetchImpl }); + if (mode === "rerun") { + if (!SHA.test(env.LANE_ARMED_HEAD ?? "")) throw new Error("LANE_ARMED_HEAD is not a commit SHA"); + console.log(`lane-armed: ${await rerunCiStatus(github, repo, env.LANE_ARMED_HEAD)}`); + return 0; + } + let rows; + try { + rows = JSON.parse(readFileSync(env.SUMMARY_PATH, "utf8")); + } catch { + rows = []; + } + const details = `${env.GITHUB_SERVER_URL || "https://github.com"}/${repo}/actions/runs/${env.GITHUB_RUN_ID}`; + let failed = 0; + let attested = null; + for (const row of Array.isArray(rows) ? rows : []) { + if (!row?.laneArmed || !Number.isSafeInteger(row.pr) || !SHA.test(String(row.head)) || Object.hasOwn(row, "carried")) continue; + const refuse = (why) => console.log(`::notice::lane-armed refused ${JSON.stringify({ v: 1, pr: row.pr, head: row.head, reason: why })}`); + const early = rowRefusal(row) ?? (attested ? `this run attested #${attested.pr}; the next run attests this head` : null); + if (early) { + refuse(early); + continue; + } + try { + const pull = await github("GET", `/repos/${repo}/pulls/${row.pr}`); + if (pull?.head?.sha !== row.head || !SHA.test(String(pull?.base?.sha))) { + refuse("the head the gate judged is not the pull request's head"); + continue; + } + const shrunk = await floorRefusal(github, repo, pull); + if (shrunk) { + refuse(`its change could weaken the hold floor: ${shrunk}`); + continue; + } + const record = recordOf(row, env.GITHUB_RUN_ID); + const line = `lane-armed ${JSON.stringify(record)}`; + await github("POST", `/repos/${repo}/check-runs`, { + name: LANE_ARMED_CHECK, + head_sha: row.head, + external_id: env.GITHUB_RUN_ID, + details_url: details, + status: "completed", + conclusion: "success", + output: { + title: `pr-merge's gate passed ${record.paths.length} lane-armed path(s) on clean main-run reviews`, + summary: [ + `Both review lanes are bound to a main-run review of \`${row.head}\` with no open finding, no review thread is open, the risk score and verdict pass, and the hold floor is not shrunk against the base. ci-status trusts this only with this run's artifact \`${laneArmedArtifact(row.pr, row.head)}\`.`, + "", + "```text", + line, + "```", + ].join("\n"), + }, + }); + console.log(`::notice::${line}`); + attested = record; + } catch (error) { + failed += 1; + refuse(`a read or write failed: ${escapeCommand(error?.message ?? error)}`); + } + } + if (attested) { + mkdirSync(env.LANE_ARMED_OUT, { recursive: true }); + writeFileSync(join(env.LANE_ARMED_OUT, "lane-armed.json"), `${JSON.stringify(attested)}\n`); + appendFileSync(env.GITHUB_OUTPUT, `artifact=${laneArmedArtifact(attested.pr, attested.head)}\nhead=${attested.head}\n`); + } + return failed === 0 ? 0 : 1; + } catch (error) { + console.log(`::error title=lane-armed::${escapeCommand(error?.message ?? error)}`); + return 1; + } +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { + process.exitCode = await main(); +} diff --git a/.github/scripts/pr-merge/lane-armed.test.mjs b/.github/scripts/pr-merge/lane-armed.test.mjs new file mode 100644 index 0000000000..524fa97ada --- /dev/null +++ b/.github/scripts/pr-merge/lane-armed.test.mjs @@ -0,0 +1,434 @@ +// Owner decision Q5 (#10743): pr-merge attests a head whose lane-armed paths +// its gate passed, and never one whose change could shrink the hold floor. +// Expected values come from the decision's conditions (both bound main-run +// lanes clean, no open thread, no hold-floor shrink against the base) and +// the security review's attack shapes, not from the code. Patches come from +// `git diff`, not from the parser under test. +import assert from "node:assert/strict"; +import { Buffer } from "node:buffer"; +import { spawnSync } from "node:child_process"; +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { test } from "node:test"; + +import { astRefusal, floorChange, floorOf, jsRefusal, main, patchLines, recordOf, rowRefusal } from "./lane-armed.mjs"; +import { laneArmedRecord } from "./merge-holds.mjs"; +import { tempDir } from "../../../lib/test-tmp.mjs"; + +const REPO = "owner/repo"; +const HEAD = "a".repeat(40); +const BASE = "b".repeat(40); +const MERGE_BASE = "c".repeat(40); +const RISK = "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py"; +const HOLDS = ".github/scripts/pr-merge/merge-holds.mjs"; +const LANE_ARMED_MJS = ".github/scripts/pr-merge/lane-armed.mjs"; +// Each file's shape as the generator writes it: logic before, its floor rows +// and its LANE_ARMED and PERSON_HELD lists between the markers. +const HELD = [".github/workflows/pr-merge.yml"]; +const PY = (rows, armed, held = HELD) => + `import re\n\n\ndef held(path):\n return path in FLOOR\n\n\nFLOOR_CLASSES: tuple[tuple[str, str], ...] = (\n # BEGIN floor-rows FLOOR_CLASSES\n${rows.map(([g, c]) => ` ("${g}", "${c}"),`).join("\n")}\n # END floor-rows FLOOR_CLASSES\n) # fmt: skip\n# BEGIN hold-floor LANE_ARM: generated by scripts/sync-hold-floor.py\nLANE_ARM_GATE = True\nLANE_ARMED: tuple[str, ...] = (\n${armed.map((g) => ` "${g}",`).join("\n")}\n)\nPERSON_HELD: tuple[str, ...] = (\n${held.map((g) => ` "${g}",`).join("\n")}\n)\n# END hold-floor LANE_ARM\nFLOOR = tuple(g for g, _ in FLOOR_CLASSES)\n\n\ndef score(x):\n return x + 1\n`; +const MJS = (rows, armed, held = HELD) => + `import { posix } from "node:path";\n\nexport const HOLD_PATH_CLASSES = Object.freeze([\n // BEGIN hold-floor HOLD_PATH_CLASSES: generated by scripts/sync-hold-floor.py\n${rows.map(([g, c]) => ` ["${g}", "${c}"],`).join("\n")}\n // END hold-floor HOLD_PATH_CLASSES\n]);\n// BEGIN hold-floor LANE_ARM: generated by scripts/sync-hold-floor.py\nexport const LANE_ARM_GATE = true;\nexport const LANE_ARMED = Object.freeze([\n${armed.map((g) => ` "${g}",`).join("\n")}\n]);\nexport const PERSON_HELD = Object.freeze([\n${held.map((g) => ` "${g}",`).join("\n")}\n]);\n// END hold-floor LANE_ARM\nexport const HOLD_PATH_FLOOR = HOLD_PATH_CLASSES.map(([glob]) => glob);\n\nexport const score = (x) => x + 1;\n`; +// The unified diff git writes from `before` to `after`, hunks only. +function diff(before, after) { + const dir = tempDir("lane-armed-diff-"); + writeFileSync(join(dir, "a"), before); + writeFileSync(join(dir, "b"), after); + const out = spawnSync("git", ["diff", "--no-index", "--no-color", "-U3", "a", "b"], { cwd: dir, encoding: "utf8" }).stdout; + return out.slice(out.indexOf("@@")).replace(/\n$/, ""); +} +const ROWS = [ + [".github/actions/request-lane-token/**", "trust"], + [".github/scripts/pr-merge/who.mjs", "gate"], + [".github/workflows/pr-merge.yml", "gate"], + [".github/scripts/pr-unblock/gate-source.mjs", "enforcement"], +]; +const ARMED = [".github/scripts/pr-merge/who.mjs"]; +const TOKEN = ".github/actions/request-lane-token/**"; + +test("floorOf reads the base's rows, LANE_ARM lists and their bounds", () => { + for (const [text, path] of [[PY(ROWS, ARMED), RISK], [MJS(ROWS, ARMED), HOLDS]]) { + const floor = floorOf(text, path); + assert.deepEqual([...floor.kinds], ROWS); + assert.equal(floor.rows.end - floor.rows.begin, ROWS.length + 1, path); + } + assert.equal(floorOf("nothing here", RISK), null); + // The other file's markers are not this file's. + assert.equal(floorOf(PY(ROWS, ARMED), HOLDS), null); +}); + +test("floorOf refuses a base whose markers are missing, repeated or out of order, or whose region holds another line", () => { + const good = PY(ROWS, ARMED); + for (const [name, text] of [ + ["no BEGIN", good.replace(" # BEGIN floor-rows FLOOR_CLASSES\n", "")], + ["no END", good.replace(" # END floor-rows FLOOR_CLASSES\n", "")], + ["a repeated BEGIN", good.replace(" # BEGIN floor-rows FLOOR_CLASSES\n", " # BEGIN floor-rows FLOOR_CLASSES\n # BEGIN floor-rows FLOOR_CLASSES\n")], + ["a repeated END", `${good}# END floor-rows FLOOR_CLASSES\n`], + ["END before BEGIN", good.replace("# BEGIN floor-rows FLOOR_CLASSES", "# TEMP").replace("# END floor-rows FLOOR_CLASSES", "# BEGIN floor-rows FLOOR_CLASSES").replace("# TEMP", "# END floor-rows FLOOR_CLASSES")], + ["no LANE_ARM END", good.replace("# END hold-floor LANE_ARM\n", "")], + ["a row in an expression", good.replace(` ("${TOKEN}", "trust"),`, ` *(("${TOKEN}", "trust"),)[:0],`)], + ]) { + assert.equal(floorOf(text, RISK), null, name); + } +}); + +test("patchLines places each changed line against the base", () => { + assert.deepEqual(patchLines(diff("a\nb\nc\n", "a\nx\nc\nd\n")), [ + { sign: "-", text: "b", at: 2 }, + { sign: "+", text: "x", at: 2 }, + { sign: "+", text: "d", at: 3 }, + ]); + assert.deepEqual(patchLines("@@ -2,0 +3,1 @@\n+new"), [{ sign: "+", text: "new", at: 2 }]); + for (const bad of [undefined, "", "diff --git a b", "@@ -1 +1 @@\n?odd"]) assert.equal(patchLines(bad), null, String(bad)); +}); + +// Each attack the security review named, as the patch from the base's text. +const judge = (path, before, after) => floorChange(path, diff(before, after), floorOf(before, path)); +test("floorChange refuses every change that could hide or drop a floor row", () => { + const py = PY(ROWS, ARMED); + const mjs = MJS(ROWS, ARMED); + const kept = ROWS.filter(([glob]) => glob !== TOKEN); + const cases = [ + // The real rows move into a docstring with a decoy marker pair; the live + // tuple loses its markers and a trust row. + ["a Python docstring decoy", RISK, py, `"""\n # BEGIN floor-rows FLOOR_CLASSES\n${ROWS.map(([g, c]) => ` ("${g}", "${c}"),`).join("\n")}\n # END floor-rows FLOOR_CLASSES\n"""\n${py.replace(" # BEGIN floor-rows FLOOR_CLASSES\n", "").replace(" # END floor-rows FLOOR_CLASSES\n", "").replace(` ("${TOKEN}", "trust"),\n`, "")}`, /names a floor marker/], + ["a JS block-comment decoy", HOLDS, mjs, `/*\n // BEGIN hold-floor HOLD_PATH_CLASSES\n ["${TOKEN}", "trust"],\n // END hold-floor HOLD_PATH_CLASSES\n*/\n${MJS(kept, ARMED).replace(" // BEGIN hold-floor HOLD_PATH_CLASSES: generated by scripts/sync-hold-floor.py\n", "").replace(" // END hold-floor HOLD_PATH_CLASSES\n", "")}`, /names a floor marker/], + ["a LANE_ARM decoy", HOLDS, mjs, `/*\n// BEGIN hold-floor LANE_ARM\nexport const LANE_ARMED = Object.freeze([\n]);\n// END hold-floor LANE_ARM\n*/\n${mjs.replace(` "${ARMED[0]}",\n]);\nexport const PERSON_HELD`, ` "${ARMED[0]}",\n ".github/workflows/pr-refine.yml",\n]);\nexport const PERSON_HELD`)}`, /names a floor marker/], + [".slice(0, 0)", HOLDS, mjs, mjs.replace(` [".github/workflows/pr-merge.yml", "gate"],\n`, ` ...[\n [".github/workflows/pr-merge.yml", "gate"],\n ].slice(0, 0),\n`), /inside the floor rows is not a row of the generated shape/], + ["[:0]", RISK, py, py.replace(` (".github/workflows/pr-merge.yml", "gate"),\n`, ` *((".github/workflows/pr-merge.yml", "gate"),)[:0],\n`), /removes the floor row \.github\/workflows\/pr-merge\.yml/], + ["a removed row", RISK, py, PY(kept, ARMED), /^it removes the floor row \.github\/actions\/request-lane-token\/\*\*$/], + ["a weakened row", HOLDS, mjs, mjs.replace(`["${TOKEN}", "trust"]`, `["${TOKEN}", "enforcement"]`), /removes the floor row/], + ["a moved marker", RISK, py, py.replace(` # BEGIN floor-rows FLOOR_CLASSES\n ("${TOKEN}", "trust"),\n`, ` ("${TOKEN}", "trust"),\n # BEGIN floor-rows FLOOR_CLASSES\n`), /names a floor marker/], + ["a re-definition", HOLDS, mjs, `${mjs}HOLD_PATH_CLASSES = [];\n`, /^an added line of .* names HOLD_PATH_CLASSES outside the floor rows$/], + ["a rebound derived set", RISK, py, py.replace("FLOOR = tuple(g for g, _ in FLOOR_CLASSES)", "FLOOR = ()"), /names FLOOR /], + ["a newly lane-armed glob", HOLDS, mjs, MJS(ROWS, [...ARMED, ".github/workflows/pr-merge.yml"]), /changes the generated LANE_ARM block/], + ["a person-held entry dropped", RISK, py, PY(ROWS, ARMED, []), /changes the generated LANE_ARM block/], + ["a comment added inside the rows", HOLDS, mjs, mjs.replace(` ["${TOKEN}", "trust"],\n`, ` ["${TOKEN}", "trust"],\n // fine\n`), /not a row of the generated shape/], + // Python folds a fullwidth identifier to ASCII (NFKC): this rebinds FLOOR_CLASSES. + ["a fullwidth identifier", RISK, py, `${py}\uFF26\uFF2C\uFF2F\uFF2F\uFF32_CLASSES = ()\n`, /holds a non-ASCII character/], + ]; + for (const [name, path, before, after, why] of cases) { + assert.match(String(judge(path, before, after)), why, name); + } +}); + +test("floorChange accepts a pure row addition, a person-held entry and a logic edit that names no hold", () => { + const py = PY(ROWS, ARMED); + const mjs = MJS(ROWS, ARMED); + for (const [name, path, before, after] of [ + ["a row added", RISK, py, PY([...ROWS, ["docs/held/**", "gate"]], ARMED)], + ["a row added first", HOLDS, mjs, MJS([["docs/held/**", "gate"], ...ROWS], ARMED)], + ["a person-held entry added", HOLDS, mjs, MJS(ROWS, ARMED, [...HELD, "docs/held/**"])], + ["a logic edit", RISK, py, py.replace("return x + 1", "return x + 2")], + ["a method named exec", HOLDS, mjs, mjs.replace("x + 1", "/a/.exec(x)")], + ]) { + assert.equal(judge(path, before, after), null, name); + } + // lane-armed.mjs has no rows: only a hold name or marker refuses there. + assert.equal(floorChange(LANE_ARMED_MJS, diff("const a = 1;\n", "const a = 2;\n"), null), null); + assert.match(floorChange(LANE_ARMED_MJS, diff("const a = 1;\n", "const a = LANE_ARMED;\n"), null), /names LANE_ARMED outside/); + // No patch (GitHub omits one for a large or binary change) refuses. + assert.equal(floorChange(HOLDS, undefined, floorOf(mjs, HOLDS)), `${HOLDS} has no readable patch`); + assert.equal(floorChange(HOLDS, diff(mjs, `${mjs}x\n`), null), `the base copy of ${HOLDS} cannot be read exactly`); +}); + +const ROW = { pr: 7, head: HEAD, ready: false, laneArmed: { paths: ARMED, lanes: { "claude-review": 0, "claude-security-review": 0 }, threads: 0, refused: null } }; + +test("rowRefusal passes only a clean record the gate did not refuse", () => { + assert.equal(rowRefusal(ROW), null); + for (const [armed, why] of [ + [{ ...ROW.laneArmed, refused: "2 review thread(s) open" }, "2 review thread(s) open"], + [{ ...ROW.laneArmed, threads: 1 }, "the open review threads are not zero"], + [{ ...ROW.laneArmed, threads: null }, "the open review threads are not zero"], + [{ ...ROW.laneArmed, lanes: { "claude-review": 0, "claude-security-review": 1 } }, "a review lane has an open finding"], + [{ ...ROW.laneArmed, lanes: { "claude-review": 0 } }, "a review lane has an open finding"], + [{ ...ROW.laneArmed, paths: [] }, "the gate recorded no lane-armed paths"], + ]) { + assert.equal(rowRefusal({ ...ROW, laneArmed: armed }), why); + } +}); + +test("recordOf is the countable line ci-status reads back", () => { + const record = recordOf(ROW, "4242"); + assert.deepEqual(record, { v: 1, pr: 7, head: HEAD, paths: ARMED, lanes: { "claude-review": 0, "claude-security-review": 0 }, threads: 0, run: 4242 }); + assert.deepEqual(laneArmedRecord(`x\nlane-armed ${JSON.stringify(record)}\n`), record); +}); + +// A GitHub with `files` at each ref, the pull's head and base, existing check +// runs and a red ci-status job; it records every write. +// `files` maps a changed path to its head text (diffed against the merge +// base), or to a file row as GitHub lists it; `changed` is the count the pull +// reports. `moved` is the base branch tip's copy of each file, when main moved +// past the merge base. +function world({ head = HEAD, files = {}, changed, moved = {}, mergeBase = MERGE_BASE, checks = [], ciRun = { id: 9, status: "completed" }, ciJob = { id: 77, name: "ci-status", conclusion: "failure" }, fail } = {}) { + const writes = []; + const content = (text) => ({ encoding: "base64", content: Buffer.from(text).toString("base64") }); + const at = { [MERGE_BASE]: { [RISK]: PY(ROWS, ARMED), [HOLDS]: MJS(ROWS, ARMED) } }; + at[BASE] = { ...at[MERGE_BASE], ...moved }; + at[head] = { ...at[MERGE_BASE], ...Object.fromEntries(Object.entries(files).filter(([, after]) => typeof after === "string")) }; + const listed = [ + { filename: ".github/scripts/pr-merge/who.mjs", status: "modified", patch: "@@ -1 +1 @@\n-a\n+b" }, + ...Object.entries(files).map(([filename, after]) => (typeof after === "string" ? { filename, status: "modified", patch: diff(at[MERGE_BASE][filename], after) } : { filename, ...after })), + ]; + const github = async (method, path, body) => { + if (fail?.(path)) throw new Error("GET failed with status 502"); + if (method !== "GET") { + writes.push({ method, path, body }); + return {}; + } + if (path === `/repos/${REPO}/pulls/7`) return { head: { sha: head }, base: { sha: BASE }, changed_files: changed ?? listed.length }; + if (path === `/repos/${REPO}/compare/${BASE}...${head}`) return { merge_base_commit: { sha: mergeBase }, files: listed }; + const file = /^\/repos\/owner\/repo\/contents\/(.+)\?ref=([0-9a-f]{40})$/.exec(path); + // A floor file the world does not hold is this checkout's copy. + if (file) return content(at[file[2]]?.[file[1]] ?? readFileSync(new URL(`../../../${file[1]}`, import.meta.url), "utf8")); + if (path.includes("/check-runs?check_name=pr-merge%2Flane-armed")) return { check_runs: checks }; + if (path.includes("/actions/workflows/pr-require-checks.yml/runs?head_sha=")) return { workflow_runs: ciRun ? [{ id: 3, status: "completed" }, ciRun] : [] }; + if (path === `/repos/${REPO}/actions/runs/9/jobs?per_page=100`) return { jobs: [{ id: 76, name: "lint-repo", conclusion: "success" }, ciJob] }; + throw new Error(`unexpected ${path}`); + }; + return { github, writes }; +} + +async function run(rows, w, mode = "attest", extra = {}) { + const dir = tempDir("lane-armed-"); + writeFileSync(join(dir, "summary.json"), JSON.stringify(rows)); + writeFileSync(join(dir, "output"), ""); + const lines = []; + const log = console.log; + console.log = (line) => lines.push(line); + try { + const env = { GITHUB_REPOSITORY: REPO, GITHUB_RUN_ID: "4242", SUMMARY_PATH: join(dir, "summary.json"), LANE_ARMED_OUT: join(dir, "out"), GITHUB_OUTPUT: join(dir, "output"), ...extra }; + const code = await main({ env, github: w.github, mode }); + return { code, out: lines.join("\n"), output: readFileSync(join(dir, "output"), "utf8"), dir }; + } finally { + console.log = log; + } +} + +test("a clean lane-armed head gets the check run, its record notice, and names the artifact to upload", async () => { + const w = world(); + const { code, out, output, dir } = await run([ROW, { pr: 8, head: "c".repeat(40), ready: true }], w); + assert.equal(code, 0, out); + assert.equal(w.writes.length, 1, out); + const [post] = w.writes; + assert.equal(post.path, `/repos/${REPO}/check-runs`); + assert.equal(post.body.name, "pr-merge/lane-armed"); + assert.equal(post.body.head_sha, HEAD); + assert.equal(post.body.external_id, "4242"); + assert.equal(post.body.details_url, `https://github.com/${REPO}/actions/runs/4242`); + assert.equal(post.body.conclusion, "success"); + const record = laneArmedRecord(post.body.output.summary); + assert.deepEqual(record, recordOf(ROW, 4242)); + assert.ok(out.includes(`::notice::lane-armed ${JSON.stringify(record)}`), out); + assert.equal(output, `artifact=pr-merge-lane-armed-7-${HEAD}\nhead=${HEAD}\n`); + assert.deepEqual(JSON.parse(readFileSync(join(dir, "out", "lane-armed.json"), "utf8")), record); +}); + +test("one head is attested a run: a later clean head waits for the next", async () => { + const w = world(); + const second = { ...ROW, pr: 9 }; + const { code, out, output } = await run([ROW, second], w); + assert.equal(code, 0, out); + assert.equal(w.writes.length, 1, out); + assert.ok(out.includes(`::notice::lane-armed refused ${JSON.stringify({ v: 1, pr: 9, head: HEAD, reason: "this run attested #7; the next run attests this head" })}`), out); + assert.equal(output, `artifact=pr-merge-lane-armed-7-${HEAD}\nhead=${HEAD}\n`); +}); + +test("rerun re-runs only a finished red ci-status job of the attested head", async () => { + const rerun = world(); + let result = await run([], rerun, "rerun", { LANE_ARMED_HEAD: HEAD }); + assert.equal(result.code, 0, result.out); + assert.deepEqual(rerun.writes, [{ method: "POST", path: `/repos/${REPO}/actions/jobs/77/rerun`, body: undefined }]); + for (const [w, said] of [ + [world({ ciJob: { id: 77, name: "ci-status", conclusion: "success" } }), "ci-status is not red"], + [world({ ciRun: { id: 9, status: "in_progress" } }), "no finished ci-status run to re-run"], + [world({ ciRun: null }), "no finished ci-status run to re-run"], + ]) { + result = await run([], w, "rerun", { LANE_ARMED_HEAD: HEAD }); + assert.equal(result.code, 0, result.out); + assert.ok(result.out.includes(said), result.out); + assert.deepEqual(w.writes, []); + } + result = await run([], world(), "rerun", { LANE_ARMED_HEAD: "x" }); + assert.equal(result.code, 1, result.out); +}); + +test("each failed condition is logged as a refusal and nothing is written", async () => { + const refused = (pr, reason) => `::notice::lane-armed refused ${JSON.stringify({ v: 1, pr, head: HEAD, reason })}`; + for (const [rows, w, reason] of [ + [[{ ...ROW, laneArmed: { ...ROW.laneArmed, refused: "1 review thread(s) open" } }], world(), "1 review thread(s) open"], + [[ROW], world({ head: "d".repeat(40) }), "the head the gate judged is not the pull request's head"], + [[ROW], world({ files: { [RISK]: PY(ROWS.slice(1), ARMED) } }), "its change could weaken the hold floor: it removes the floor row .github/actions/request-lane-token/**"], + [[ROW], world({ files: { [HOLDS]: { status: "modified" } } }), `its change could weaken the hold floor: ${HOLDS} has no readable patch`], + [[ROW], world({ files: { [HOLDS]: { status: "renamed", previous_filename: "x.mjs", patch: "@@ -1 +1 @@\n-a\n+b" } } }), `its change could weaken the hold floor: it renamed ${HOLDS}`], + [[ROW], world({ changed: 5 }), "its change could weaken the hold floor: its file list is cut short"], + [[ROW], world({ mergeBase: "unknown" }), "its change could weaken the hold floor: its merge base cannot be read"], + ]) { + const { code, out } = await run(rows, w); + assert.equal(code, 0, out); + assert.ok(out.includes(refused(7, reason)), `${reason}\n${out}`); + assert.deepEqual(w.writes, [], reason); + } + const w = world({ fail: (path) => path.includes("/compare/") }); + const { code, out } = await run([ROW], w); + assert.equal(code, 1, out); + assert.match(out, /lane-armed refused .*a read or write failed/); + assert.deepEqual(w.writes, []); +}); + +// GitHub measures the patch from the merge base. When main has since dropped +// lines above the rows, the moved tip's line numbers put a removed last row +// past END, outside the region, where its text names nothing. +test("the base copy is read at the merge base the patch is measured from", async () => { + const mergeBase = PY(ROWS, ARMED); + const moved = mergeBase.replace("import re\n\n\n", ""); + const last = ROWS.at(-1)[0]; + const w = world({ moved: { [RISK]: moved }, files: { [RISK]: PY(ROWS.slice(0, -1), ARMED) } }); + const { code, out } = await run([ROW], w); + assert.equal(code, 0, out); + assert.ok(out.includes(`its change could weaken the hold floor: it removes the floor row ${last}`), out); + assert.deepEqual(w.writes, []); +}); + +// babysit_risk.py as it stands in this checkout: the base copy each +// rebinding below is added to. floor-ast.py parses both and runs neither. +const RISK_TEXT = readFileSync(new URL(`../../../${RISK}`, import.meta.url), "utf8"); +const ADDED = (lines) => `${RISK_TEXT}\n\n${lines}\n`; +test("floor-ast.py refuses each reflective or aliased rebinding of the Python floor", () => { + const cases = [ + ["locals()", 'locals()["FLOOR_" "CLASSES"] = ()', /adds a reference to locals/], + ["an alias of globals", 'g = globals\ng()["FLOOR_" "CLASSES"] = ()', /adds a reference to globals/], + ["globals split over lines", '(globals\n)()["FLOOR_" "CLASSES"] = ()', /adds a reference to globals/], + ["builtins.setattr", 'import builtins\nbuiltins.setattr(object(), "x", 1)', /adds (a reference to builtins|a reference to setattr|an import)/], + // A function's __globals__ is the module's namespace. + ["a lambda's __globals__", '(lambda: 0).__globals__["FLOOR_" "CLASSES"] = ()', /adds a dunder name/], + ["a frame's f_globals", 'sys._getframe().f_globals["FLOOR_" "CLASSES"] = ()', /adds a reference to (_getframe|f_globals)/], + ["inspect.currentframe", 'inspect.currentframe().f_globals["FLOOR_" "CLASSES"] = ()', /adds a reference to (currentframe|f_globals|inspect)/], + ["an added import", "import helper", /adds an import/], + ["object.__setattr__", 'object.__setattr__(object(), "x", 1)', /adds a reference to __setattr__/], + ["sys.modules __setattr__", 'sys.modules[__name__].__setattr__("FLOOR_" "CLASSES", ())', /adds (a reference to (__setattr__|modules)|a dunder name)/], + ["__import__ exec", '__import__("builtins").exec("FLOOR_CLASSES = ()")', /adds a reference to (__import__|exec)/], + ["importlib", 'import importlib\nimportlib.reload', /adds (a reference to importlib|an import)/], + ["getattr", 'getattr(object(), "FLOOR_" "CLASSES")', /adds a reference to getattr/], + ["the compile builtin", 'compile("FLOOR_CLASSES = ()", "x", "exec")', /adds a reference to compile/], + ["__builtins__", '__builtins__["exec"]("FLOOR_CLASSES = ()")', /adds a reference to __builtins__/], + ["a second binding", "FLOOR_CLASSES = ()", /FLOOR_CLASSES is bound 2 times/], + ["an augmented binding", "FLOOR_CLASSES += ()", /FLOOR_CLASSES is bound 2 times/], + ["a loop target", "for FLOOR in []:\n pass", /FLOOR is bound 2 times/], + ["a with target", "with open(\"x\") as HIGH_FLOOR:\n pass", /HIGH_FLOOR is bound 2 times/], + ["an import alias", "import os as REVIEW_FLOOR", /REVIEW_FLOOR is bound 2 times|adds an import/], + ["a walrus", "(PERSON_HELD := ())", /PERSON_HELD is bound 2 times/], + ["a def", "def LANE_ARMED_FLOOR():\n return ()", /LANE_ARMED_FLOOR is bound 2 times/], + ["an argument", "def f(FLOOR=()):\n return FLOOR", /FLOOR is bound 2 times/], + ["a del", "del FLOOR", /FLOOR is bound 2 times/], + ["a global", "def f():\n global HIGH_FLOOR", /HIGH_FLOOR is bound 2 times/], + ]; + for (const [name, lines, why] of cases) { + assert.match(astRefusal(RISK_TEXT, ADDED(lines)), why, name); + } + // A derived set re-derived, a table no longer literal, a copy that does not parse. + assert.match(astRefusal(RISK_TEXT, RISK_TEXT.replace("FLOOR: tuple[str, ...] = tuple(g for g, _ in FLOOR_CLASSES)", "FLOOR: tuple[str, ...] = ()")), /changes how FLOOR is derived/); + assert.match(astRefusal(RISK_TEXT, RISK_TEXT.replace("LANE_ARM_GATE = True", "LANE_ARM_GATE = bool(1)")), /LANE_ARM_GATE is not a literal/); + assert.match(astRefusal(RISK_TEXT, `${RISK_TEXT}\ndef (\n`), /does not parse/); +}); + +test("floor-ast.py accepts a logic edit with no reflective name and a row addition", () => { + assert.equal(astRefusal(RISK_TEXT, RISK_TEXT), ""); + assert.equal(astRefusal(RISK_TEXT, ADDED('def extra(x):\n return re.compile("a").match(x) or x + 1')), ""); + // A pure logic edit inside an existing function: no new dunder, import or reflective name. + const edited = RISK_TEXT.replace("def lane_armed(inp: RiskInput) -> bool:", "def lane_armed(inp: RiskInput) -> bool:\n _unused = len(str(inp)) + 1"); + assert.notEqual(edited, RISK_TEXT); + assert.equal(astRefusal(RISK_TEXT, edited), ""); + const row = ' (".github/scripts/pr-merge/lane-armed.mjs", "gate"),\n'; + assert.ok(RISK_TEXT.includes(row)); + assert.equal(astRefusal(RISK_TEXT, RISK_TEXT.replace(row, `${row} ("docs/held/**", "gate"),\n`)), ""); +}); + +test("a rebinding the patch rules cannot see is refused through floor-ast.py", async () => { + const w = world({ files: { [RISK]: `${PY(ROWS, ARMED)}locals()["FLOOR_" "CLASSES"] = ()\n` } }); + const { code, out } = await run([ROW], w); + assert.equal(code, 0, out); + assert.ok(out.includes(`its change could weaken the hold floor: its ${RISK}: it adds a reference to locals`), out); + assert.deepEqual(w.writes, []); +}); + +test("floor-ast.py is judge code: a logic edit is judged for markers and names, never read as a floor table", async () => { + const FLOOR_AST = ".github/scripts/pr-merge/floor-ast.py"; + const source = readFileSync(new URL("./floor-ast.py", import.meta.url), "utf8"); + const logic = diff(source, source.replace('f"it adds {name}"', 'f"it now adds {name}"')); + assert.equal(floorChange(FLOOR_AST, logic, null), null); + assert.match(floorChange(FLOOR_AST, diff(source, source.replace('TABLES = ("FLOOR_CLASSES",', 'TABLES = (')), null), /names TABLES|names FLOOR_CLASSES/); + const w = world({ files: { [FLOOR_AST]: { status: "modified", patch: logic } } }); + const { code, out } = await run([ROW], w); + assert.equal(code, 0, out); + assert.doesNotMatch(out, /cannot be read exactly/); + assert.ok(out.includes("::notice::lane-armed {"), out); +}); + +test("jsRefusal refuses a JS floor file that reaches code more dynamically than its base", () => { + const base = MJS(ROWS, ARMED); + for (const [name, line] of [ + ["eval(", 'eval("HOLD_" + "PATH_FLOOR.length = 0");'], + ["eval(", "eval (x);"], + ["globalThis", 'globalThis["HOLD_" + "PATH_CLASSES"] = [];'], + ["Function(", 'Function("return 1")();'], + ["Reflect.", 'Reflect.set(x, "a", 1);'], + ["Object.defineProperty", 'Object.defineProperty(x, "a", {});'], + ["__proto__", "x.__proto__ = null;"], + ["import(", 'await import("./x.mjs");'], + ["require(", 'require("vm");'], + ]) { + assert.equal(jsRefusal(base, `${base}${line}\n`), `it adds ${name}`, name); + } + // As many as the base: accepted, and so is a logic edit or a method named eval. + assert.equal(jsRefusal(`${base}eval(1);\n`, `${base}eval(2);\n`), null); + assert.equal(jsRefusal(base, base.replace("x + 1", "x + 2")), null); + assert.equal(jsRefusal(base, `${base}const evaluate = 1;\n`), null); + // A floor row for another eval-* workflow is a tightening, not a call. + assert.equal(jsRefusal(base, MJS([...ROWS, [".github/workflows/eval-foo.yml", "gate"]], ARMED)), null); + assert.equal(judge(HOLDS, base, MJS([...ROWS, [".github/workflows/eval-foo.yml", "gate"]], ARMED)), null); +}); + +// lane-armed.mjs holds the very tokens it counts, as literals in its +// JS_DYNAMIC list; they are not counted, and the list cannot be edited. +test("lane-armed.mjs cannot trade a JS_DYNAMIC literal for a real call", async () => { + const source = readFileSync(new URL("./lane-armed.mjs", import.meta.url), "utf8"); + const literal = ' ["eval(", /\\beval\\s*\\(/g],\n'; + assert.ok(source.includes(literal)); + const traded = source.replace(literal, "").replace("export function jsRefusal(baseText, headText) {", 'export function jsRefusal(baseText, headText) {\n eval("1");'); + // The count alone sees the real call once the literal is not counted. + assert.equal(jsRefusal(source, traded), "it adds eval("); + // The patch rule refuses the edit inside the list before the count runs. + const w = world({ files: { [LANE_ARMED_MJS]: { status: "modified", patch: diff(source, traded) } } }); + const { code, out } = await run([ROW], w); + assert.equal(code, 0, out); + assert.ok(out.includes(`its change could weaken the hold floor: a removed line of ${LANE_ARMED_MJS} is inside the JS_DYNAMIC list`), out); + assert.deepEqual(w.writes, []); + // Moving or dropping the block's markers refuses too. + assert.match(floorChange(LANE_ARMED_MJS, diff(source, source.replace(" // END js-dynamic JS_DYNAMIC\n", "")), null), /names a floor marker/); +}); + +test("an added eval in merge-holds.mjs is refused end to end", async () => { + const w = world({ files: { [HOLDS]: MJS(ROWS, ARMED).replace("x + 1", 'eval("x + 1")') } }); + const { code, out } = await run([ROW], w); + assert.equal(code, 0, out); + assert.ok(out.includes(`its change could weaken the hold floor: its ${HOLDS}: it adds eval(`), out); + assert.deepEqual(w.writes, []); +}); + +// ci-status trusts only a schedule or workflow_dispatch run of pr-merge.yml on +// main (merge-holds laneArmedHold), so the job that attests runs on nothing +// else: a pull_request_target run's head_branch is the pull request's. +test("pr-merge.yml attests only from a scheduled or hand-started run on main", async () => { + const { readFileSync } = await import("node:fs"); + const workflow = readFileSync(new URL("../../workflows/pr-merge.yml", import.meta.url), "utf8"); + const job = /\n lane-armed:\n([\s\S]*?)(?=\n [a-z-]+:\n)/.exec(workflow)[1]; + assert.match(job, /needs: \[merge\]/); + assert.match(job, /&& github\.ref == 'refs\/heads\/main'\n\s+&& \(github\.event_name == 'schedule' \|\| github\.event_name == 'workflow_dispatch'\)/); + // Post, then upload the proof, then re-run ci-status, each its own step. + const attest = job.indexOf("run: node .base/.github/scripts/pr-merge/lane-armed.mjs attest"); + const upload = job.indexOf("name: ${{ steps.attest.outputs.artifact }}"); + const rerun = job.indexOf("run: node .base/.github/scripts/pr-merge/lane-armed.mjs rerun"); + assert.ok(attest > 0 && upload > attest && rerun > upload, job); + assert.match(workflow, /reason: \(\.unattended\.reason \/\/ \.error\),[\s\S]*?\+ \(if \(\.unattended\.laneArmed \| type\) == "object" then \{laneArmed: \.unattended\.laneArmed\} else \{\} end\)/); +}); diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index 75fdfaac6c..046e84e135 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -18,7 +18,9 @@ // - 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 (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 +// which a clean security review clears, and so is the merge gate's own +// code merge.lane-arm-gate lane-arms, LANE_ARMED_FLOOR, which instead holds +// until pr-merge's attestation is at the head: laneArmedHold). 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), or by // approving an earlier head that carries to it (liftersOf): their newest @@ -293,6 +295,8 @@ export const HOLD_PATH_CLASSES = Object.freeze([ [".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"], [".github/scripts/pr-merge/verify-answers.mjs", "gate"], [".github/scripts/pr-merge/listing-carry.mjs", "gate"], + [".github/scripts/pr-merge/lane-armed.mjs", "gate"], + [".github/scripts/pr-merge/floor-ast.py", "gate"], [".github/actions/report-check-run/posting-run.mjs", "gate"], ["scripts/sync-hold-floor.py", "trust"], ["plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "trust"], @@ -340,6 +344,8 @@ export const HOLD_PATH_CLASSES = Object.freeze([ [".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", "enforcement"], [".github/scripts/work-resolve-issue/stop-routes.json", "enforcement"], [".github/scripts/work-resolve-issue/stop-routes.mjs", "enforcement"], + ["plugins/review/skills/**", "enforcement"], + ["plugins/review/reference/**", "enforcement"], [".github/scripts/pr-review-main/**", "gate"], ["**/.claude/**", "gate"], ["**/.claude-lane/**", "gate"], @@ -377,10 +383,98 @@ export const HOLD_PATH_CLASSES = Object.freeze([ // END hold-floor HOLD_PATH_CLASSES ]); // The `trust` and `gate` globs (babysit_risk.HIGH_FLOOR), which hold for a -// person. An `enforcement` glob (REVIEW_FLOOR) holds nothing here: the merge +// person, less LANE_ARMED_FLOOR below. 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)); +// Owner decisions Q7 and Q5 (#10743): with `merge.lane-arm-gate` on, the +// merge gate's own code (LANE_ARMED: babysit_risk.py and the pr-merge +// scripts) is LANE_ARMED_FLOOR and is no hold path. The merge gate holds it +// for a person (babysit_risk `high-path:`) until both review lanes are bound +// to a main-run review of the head with no finding and no review thread is +// open, and ci-status holds it (laneArmedHold) until a pr-merge.yml run on +// the default branch posts `pr-merge/lane-armed` at the head (lane-armed.mjs). +// Every other `trust` and `gate` glob is person-held (PERSON_HELD). +// Copied here by scripts/sync-hold-floor.py. +// BEGIN hold-floor LANE_ARM: generated by scripts/sync-hold-floor.py +export const LANE_ARM_GATE = true; +export const LANE_ARMED = Object.freeze([ + ".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/last-report.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-merge/listing-carry.mjs", + ".github/scripts/pr-merge/lane-armed.mjs", + ".github/scripts/pr-merge/floor-ast.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", +]); +export const PERSON_HELD = 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/actions/report-check-run/posting-run.mjs", + "scripts/sync-hold-floor.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_review_evidence.py", + "docs/conventions/pr-pipeline.yaml", + ".github/scripts/pr-review-main/**", + "**/.claude/**", + "**/.claude-lane/**", + "docs/conventions/instruction-exception-register/**", + "docs/conventions/untrusted-content/**", + "**/.mcp.json", + "**/CLAUDE.md", + "**/CLAUDE.local.md", + "**/AGENTS.md", + "REVIEW.md", + "docs/standards/**", + ".github/workflows/dependabot-fragments.yml", + ".github/workflows/eval-browser-dispatch.yml", + ".github/workflows/intake-reenter.yml", + ".github/workflows/intake-triage.yml", + ".github/workflows/maintenance-audit-lane-runs.yml", + ".github/workflows/maintenance-reconcile-lost-runs.yml", + ".github/workflows/post-merge-demotion-alert.yml", + ".github/workflows/post-merge-test-mutation.yml", + ".github/workflows/pr-feedback-dispatch.yml", + ".github/workflows/pr-merge.yml", + ".github/workflows/pr-refine.yml", + ".github/workflows/pr-require-checks.yml", + ".github/workflows/pr-review-hosted.yml", + ".github/workflows/pr-review-main.yml", + ".github/workflows/pr-review-security-hosted.yml", + ".github/workflows/pr-run-activity-read.yml", + ".github/workflows/pr-run-activity-write.yml", + ".github/workflows/pr-unblock-dispatch.yml", + ".github/workflows/pr-unblock.yml", + ".github/workflows/pr-verify.yml", + ".github/workflows/release-publish-plugins.yml", + ".github/workflows/work-resolve-issue.yml", + ".github/workflows/work-unblock.yml", +]); +// END hold-floor LANE_ARM +// The globs the switch lane-arms: none unless it is exactly true. +export const LANE_ARMED_FLOOR = Object.freeze(LANE_ARM_GATE === true ? [...LANE_ARMED] : []); +export const HOLD_PATH_FLOOR = Object.freeze( + HOLD_PATH_CLASSES.filter(([glob, kind]) => kind !== "enforcement" && !LANE_ARMED_FLOOR.includes(glob)).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` @@ -404,6 +498,127 @@ export function pathHoldKind(files, holdPaths) { return held === undefined ? null : { kind: "hold-path", reason: `it changes ${held}, a hold path` }; } +// The attestation pr-merge posts at a head whose lane-armed paths its gate +// passed (lane-armed.mjs), and the one record line its output carries. +export const LANE_ARMED_CHECK = "pr-merge/lane-armed"; +export const LANE_ARMED_WORKFLOW = ".github/workflows/pr-merge.yml"; +// The artifact only that run's own steps can upload, naming what it attested. +export const laneArmedArtifact = (pr, head) => `pr-merge-lane-armed-${pr}-${head}`; +const LANE_ARMED_EVENTS = Object.freeze(["schedule", "workflow_dispatch"]); +const RECORD = /^lane-armed (\{.*\})$/m; +export const laneArmedRecord = (text) => { + try { + const record = JSON.parse(RECORD.exec(String(text ?? ""))?.[1] ?? "null"); + return record && typeof record === "object" && record.v === 1 ? record : null; + } catch { + return null; + } +}; +// The changed `files` the switch lane-arms. +export const laneArmedPaths = (files) => (Array.isArray(files) ? files : []).filter((path) => LANE_ARMED_FLOOR.some((glob) => matchesHoldPath(path, glob))); + +// Why the lane-armed `files` of pull request `pr` hold `head`, or null: none +// is lane-armed, or a `pr-merge/lane-armed` check run at `head` proves +// pr-merge's gate passed them. The proof is the run, not the check: any +// workflow with checks: write posts as the GitHub Actions app and can copy a +// real run's id, so the check run must name, in both external_id and +// details_url, a run of pr-merge.yml on the default branch, started by +// schedule or workflow_dispatch (the lane-armed job runs on nothing else), +// running or succeeded, that +// uploaded the unexpired artifact laneArmedArtifact(pr, head), which only +// that run's own steps can; its record must cover every lane-armed path +// changed now. head_branch is only a ref name, and a tag can share the +// branch's name, so the run's head_sha must also be the default branch's tip +// or an ancestor of it (the compare API's `identical` or `ahead` against the +// branch tip's SHA). Any read that fails or answers anything else holds. +export async function laneArmedHold(github, repo, head, files, pr) { + const armed = laneArmedPaths(files); + if (armed.length === 0) { + return null; + } + const hold = (why) => `it changes ${armed.join(", ")}, the merge gate's own code, and ${why}; pr-merge posts ${LANE_ARMED_CHECK} once its gate passes this head on clean main-run reviews, then re-runs ci-status`; + if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(String(repo)) || !/^[0-9a-f]{40}$/.test(String(head)) || !Number.isSafeInteger(pr) || pr < 1) { + return hold("its attestation cannot be looked up"); + } + let checks; + let branch; + try { + branch = (await github("GET", `/repos/${repo}`))?.default_branch; + checks = (await github("GET", `/repos/${repo}/commits/${head}/check-runs?check_name=${encodeURIComponent(LANE_ARMED_CHECK)}&filter=all&per_page=100`))?.check_runs; + } catch { + return hold("its attestation could not be read"); + } + if (typeof branch !== "string" || !branch || !Array.isArray(checks)) { + return hold("its attestation could not be read"); + } + const candidates = checks.filter( + (check) => + check?.name === LANE_ARMED_CHECK && + check.app?.slug === "github-actions" && + check.head_sha === head && + check.status === "completed" && + check.conclusion === "success" && + /^[1-9][0-9]*$/.test(String(check.external_id)) && + String(check.details_url ?? "").endsWith(`/${repo}/actions/runs/${check.external_id}`), + ); + for (const check of candidates) { + const record = laneArmedRecord(check.output?.summary); + if (!record || record.head !== head || record.pr !== pr || String(record.run) !== String(check.external_id) || !Array.isArray(record.paths)) { + continue; + } + if (!armed.every((path) => record.paths.some((covered) => String(covered).toLowerCase() === path.toLowerCase()))) { + continue; + } + let run; + let artifacts; + try { + run = await github("GET", `/repos/${repo}/actions/runs/${check.external_id}`); + artifacts = (await github("GET", `/repos/${repo}/actions/runs/${check.external_id}/artifacts?name=${encodeURIComponent(laneArmedArtifact(pr, head))}&per_page=100`))?.artifacts; + } catch { + continue; + } + const uploaded = (Array.isArray(artifacts) ? artifacts : []).some( + (artifact) => artifact?.name === laneArmedArtifact(pr, head) && artifact.expired === false && String(artifact.workflow_run?.id) === String(check.external_id), + ); + const live = run?.status === "in_progress" || (run?.status === "completed" && run?.conclusion === "success"); + if ( + !( + String(run?.id) === String(check.external_id) && + run?.path === LANE_ARMED_WORKFLOW && + run?.head_branch === branch && + /^[0-9a-f]{40}$/.test(String(run?.head_sha)) && + LANE_ARMED_EVENTS.includes(run?.event) && + run?.repository?.full_name === repo && + run?.head_repository?.full_name === repo && + live && + uploaded + ) + ) { + continue; + } + if (await onBranch(github, repo, branch, run.head_sha)) { + return null; + } + } + return hold(candidates.length === 0 ? `no ${LANE_ARMED_CHECK} check run from the GitHub Actions app is at this head` : `no ${LANE_ARMED_CHECK} check run at this head names a pr-merge.yml run on ${branch} that covers them`); +} + +// Whether commit `sha` is the tip of `branch` or an ancestor of it. The tip +// comes from the branches API, never from the bare name, which a tag can +// share; any read that fails answers false. +async function onBranch(github, repo, branch, sha) { + try { + const tip = (await github("GET", `/repos/${repo}/branches/${encodeURIComponent(branch)}`))?.commit?.sha; + if (!/^[0-9a-f]{40}$/.test(String(tip))) { + return false; + } + const status = (await github("GET", `/repos/${repo}/compare/${sha}...${tip}`))?.status; + return status === "identical" || status === "ahead"; + } catch { + return false; + } +} + // Why the changed `files` hold it for the owner's cost decision, or null when // none is a cost path or `decision` (costDecision's link) clears it. As // glob_match does, a glob also covers everything under it, and `*` matches a @@ -905,18 +1120,19 @@ export const CARRY_REFUSES = Object.freeze([".github/workflows/**", ".github/act // Whether the compare from...H' `merges` touches no hold path and no file a // hold path loads, old paths of renames included: pathHold with `holdPaths` -// and HOLD_PATH_FLOOR, REVIEW_FLOOR's enforcement globs (the floored code the +// and HOLD_PATH_FLOOR, REVIEW_FLOOR's enforcement globs and LANE_ARMED_FLOOR (the floored code the // gate holds for a security review, which a person's approval never // covered) and CARRY_REFUSES. H' must be ahead of `from` with nothing behind, // its commit list read whole, and fewer than the compare API's 300 files // listed, each named. Any edit to such a file, a test or a comment included, // needs a new approval. -export function ungatedSince(merges, holdPaths) { +// `armed` is LANE_ARMED_FLOOR, a parameter so tests can give it rows. +export function ungatedSince(merges, holdPaths, armed = LANE_ARMED_FLOOR) { const files = merges?.files; if (merges?.status !== "ahead" || merges.behind_by !== 0 || listedCommits(merges) === null || !Array.isArray(files) || files.length >= 300) return false; if (!files.every((f) => typeof f?.filename === "string")) return false; const paths = files.flatMap((f) => [f.filename, ...(typeof f.previous_filename === "string" ? [f.previous_filename] : [])]); - const loaded = [...REVIEW_FLOOR, ...CARRY_REFUSES]; + const loaded = [...REVIEW_FLOOR, ...armed, ...CARRY_REFUSES]; return Array.isArray(holdPaths) && pathHold(paths, holdPaths) === null && !paths.some((path) => loaded.some((glob) => matchesHoldPath(path, glob))); } @@ -1544,12 +1760,21 @@ export async function main(argv = process.argv.slice(2), { env = process.env, gi } const widened = actorsHold(await withActorContents(github, repo, changes, env.BASE_SHA, head)); const held = pathHold(files, holdPaths); + // A trusted person's approval lifts it as it lifts a hold path. + const armedHold = await laneArmedHold(github, repo, head, files, Number(number)); let lifted = { lifters: [] }; - if (!approved.length && ((held && Array.isArray(holdPaths)) || widened) && /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repo)) { + if (!approved.length && ((held && Array.isArray(holdPaths)) || widened || armedHold) && /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repo)) { lifted = await liftersOf(github, repo, Number(number), head, humans, holdPaths); } const lifters = approved.length ? approved : lifted.lifters; const reasons = holds(body, items, files, holdPaths, lifters, decision, byPerson, widened); + if (armedHold && !lifters.length) { + reasons.push(`${armedHold}; or an approving review of this head by a person on the trusted-actor list lifts it, then re-run ci-status`); + } else if (armedHold) { + console.log(`#${number}: ${lifters.join(", ")} approved, which lifts its lane-armed hold (${escape(armedHold)}).`); + } else if (laneArmedPaths(files).length) { + console.log(`#${number}: ${LANE_ARMED_CHECK} at this head, from a pr-merge.yml run on the default branch, covers ${laneArmedPaths(files).join(", ")}.`); + } const how = approved.length ? "approved this head" : "approved an earlier head whose every change since is a base merge"; if (held && lifters.length && Array.isArray(holdPaths)) { console.log(`#${number}: ${lifters.join(", ")} ${how}, which lifts its hold path (${escape(held)}).`); diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 7a7c0519b7..146607156c 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -20,7 +20,7 @@ import { join } from "node:path"; import { test } from "node:test"; import * as holdsModule from "./merge-holds.mjs"; -import { BODY_WRITERS_QUERY, CARRY_REFUSES, COMMENT_EDITED_QUERY, approvalCarry, ungatedSince, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, PULL_OPENED_QUERY, REVIEW_FLOOR, baseMergeCarry, baseMergesOnly, changeOf, mergedParents, costDecision, costHold, costNotices, decidedHead, evidenceHold, laneChange, linkHold, main, pathHold, sameNetChange, unranHead } from "./merge-holds.mjs"; +import { BODY_WRITERS_QUERY, CARRY_REFUSES, COMMENT_EDITED_QUERY, approvalCarry, ungatedSince, HOLD_PATH_CLASSES, HOLD_PATH_FLOOR, LANE_ARM_GATE, LANE_ARMED, LANE_ARMED_CHECK, LANE_ARMED_FLOOR, PERSON_HELD, PULL_OPENED_QUERY, REVIEW_FLOOR, laneArmedArtifact, laneArmedHold, laneArmedRecord, baseMergeCarry, baseMergesOnly, changeOf, mergedParents, costDecision, costHold, costNotices, decidedHead, evidenceHold, laneChange, linkHold, main, pathHold, sameNetChange, unranHead } from "./merge-holds.mjs"; import { tempDir } from "../../../lib/test-tmp.mjs"; const REPO = "owner/repo"; @@ -312,7 +312,7 @@ test("costHold holds a cost path, case-folded, until a decision", () => { // maps a ref (BASE, HEAD) to the trusted-actor list's text there. // `queue` adds the merge queue's verdict inputs: { verdicts, link, evidence }, // each written as given (a string as it is), `evidence` left out when undefined. -function ci(files, { reviews = [], body = "Closes #40", options, denied = [], writers = "fail", head = HEAD, list = {}, queue } = {}) { +function ci(files, { reviews = [], body = "Closes #40", options, denied = [], writers = "fail", head = HEAD, list = {}, queue, attest } = {}) { const dir = tempDir("merge-holds-"); writeFileSync(join(dir, "body"), body); writeFileSync(join(dir, "discussion"), reviews.map((row) => JSON.stringify(row)).join("\n")); @@ -332,6 +332,10 @@ function ci(files, { reviews = [], body = "Closes #40", options, denied = [], wr } const { github: decisions } = world(options); const github = async (method, path, data) => { + const attested = attest?.(method, path); + if (attested !== undefined) { + return attested; + } const listed = /^\/repos\/owner\/repo\/contents\/\.github\/standards\/trusted-actors\/trusted-actors\.json\?ref=([0-9a-f]{40})$/.exec(path); if (listed) { assert.equal(method, "GET"); @@ -381,24 +385,28 @@ const FLOOR_FILES = [ ".github/actions/request-lane-token/action.yml", ".github/actions/check-trusted-trigger/check-trusted-trigger.mjs", ".github/actions/check-kill-switch/check-kill-switch.mjs", - ".github/actions/log-api-budget/rate-guard.mjs", - ".github/actions/log-api-budget/rate-limits.json", ".github/CODEOWNERS", "plugins/foo/codeowners", ".gitmodules", "vendor/sub/.gitmodules", - ".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/gate-check.mjs", - ".github/scripts/pr-merge/who.mjs", - ".github/scripts/pr-merge/rate-stop.bash", + // The person-held `gate` rows (merge.person-held and every gated workflow). + ".github/workflows/pr-merge.yml", + ".github/workflows/pr-require-checks.yml", + ".github/workflows/pr-review-hosted.yml", + ".github/workflows/pr-review-security-hosted.yml", + ".github/workflows/pr-review-main.yml", + ".github/scripts/pr-review-main/write-verdict.mjs", + ".github/workflows/pr-verify.yml", "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", "docs/conventions/pr-pipeline.yaml", + ".github/actions/log-api-budget/rate-guard.mjs", + ".github/actions/report-check-run/posting-run.mjs", + ".claude/settings.json", + "AGENTS.md", + "REVIEW.md", + "docs/standards/README.md", ]; const ENFORCEMENT_FILES = [ "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py", @@ -426,24 +434,195 @@ const ENFORCEMENT_FILES = [ ".github/scripts/work-resolve-issue/stop-routes.json", ".github/scripts/work-resolve-issue/stop-routes.mjs", ]; -test("the floor's enforcement globs are the merge gate's, every other one holds here", () => { +// Owner decision Q5: the merge gate's own code, lane-armed. +const LANE_ARMED_FILES = [ + ".github/scripts/pr-merge/merge-holds.mjs", + ".github/scripts/pr-merge/clear-escalations.mjs", + ".github/scripts/pr-merge/who.mjs", + ".github/scripts/pr-merge/rate-stop.bash", + ".github/scripts/pr-merge/lane-armed.mjs", + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", +]; +test("the floor's enforcement globs and the merge gate's own code are not hold paths here, every other one is", () => { 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( + HOLD_PATH_FLOOR, + HOLD_PATH_CLASSES.filter(([glob, kind]) => kind !== "enforcement" && !LANE_ARMED_FLOOR.includes(glob)).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("the switch lane-arms babysit_risk.py and the pr-merge scripts, and every other trust and gate row is person-held", () => { + assert.equal(LANE_ARM_GATE, true); + assert.deepEqual(LANE_ARMED_FLOOR, [...LANE_ARMED]); + const kind = new Map(HOLD_PATH_CLASSES); + assert.deepEqual( + [...LANE_ARMED].sort(), + HOLD_PATH_CLASSES.filter(([glob, k]) => k !== "enforcement" && (glob.startsWith(".github/scripts/pr-merge/") || glob.endsWith("/babysit_risk.py"))) + .map(([glob]) => glob) + .sort(), + ); + for (const glob of PERSON_HELD) { + assert.ok(["trust", "gate"].includes(kind.get(glob)), glob); + assert.ok(HOLD_PATH_FLOOR.includes(glob), glob); + } + for (const path of LANE_ARMED_FILES) { + assert.equal(pathHold([path], []), null, path); + } +}); + +// A world in which `head` carries a `pr-merge/lane-armed` check run from +// `run`, each field overridable to forge it. +const RUN_ID = 4242; +const MAIN_TIP = "d".repeat(40); +const RUN_SHA = "e".repeat(40); +const attestation = ({ head = HEAD, paths = LANE_ARMED_FILES, check = {}, run = {}, record = {}, branch = "main", artifact = {}, compare = "ahead", tip = MAIN_TIP, fail } = {}) => { + const line = `lane-armed ${JSON.stringify({ v: 1, pr: 99, head, paths, lanes: { "claude-review": 0, "claude-security-review": 0 }, threads: 0, run: RUN_ID, ...record })}`; + const runs = [ + { + name: LANE_ARMED_CHECK, + app: { slug: "github-actions" }, + head_sha: head, + status: "completed", + conclusion: "success", + external_id: String(RUN_ID), + details_url: `https://github.com/${REPO}/actions/runs/${RUN_ID}`, + output: { summary: `x\n\n\`\`\`text\n${line}\n\`\`\`` }, + ...check, + }, + ]; + const workflowRun = { + id: RUN_ID, + path: ".github/workflows/pr-merge.yml", + head_branch: "main", + head_sha: RUN_SHA, + event: "schedule", + status: "in_progress", + conclusion: null, + repository: { full_name: REPO }, + head_repository: { full_name: REPO }, + ...run, + }; + return (method, path) => { + if (fail?.(path)) throw new Error("unreadable"); + if (path === `/repos/${REPO}`) return { default_branch: branch }; + if (path.startsWith(`/repos/${REPO}/commits/`) && path.includes("/check-runs?check_name=pr-merge%2Flane-armed")) return { check_runs: path.includes(`/commits/${HEAD}/`) ? runs : [] }; + if (path === `/repos/${REPO}/actions/runs/${RUN_ID}`) return workflowRun; + if (path === `/repos/${REPO}/branches/main`) return { name: "main", commit: { sha: tip } }; + if (path === `/repos/${REPO}/compare/${workflowRun.head_sha}...${MAIN_TIP}`) return { status: compare }; + if (path === `/repos/${REPO}/actions/runs/${RUN_ID}/artifacts?name=pr-merge-lane-armed-99-${HEAD}&per_page=100`) { + return { artifacts: artifact === null ? [] : [{ name: `pr-merge-lane-armed-99-${HEAD}`, expired: false, workflow_run: { id: RUN_ID }, ...artifact }] }; + } + return undefined; + }; +}; +const reader = (world) => async (method, path) => { + const answer = world(method, path); + if (answer === undefined) throw new Error(`unexpected ${path}`); + return answer; +}; + +test("laneArmedHold passes the merge gate's own code only on a pr-merge.yml run of main that attested this head", async () => { + assert.equal(await laneArmedHold(reader(attestation()), REPO, HEAD, LANE_ARMED_FILES, 99), null); + // The run's commit is main's tip, or an ancestor of it. + assert.equal(await laneArmedHold(reader(attestation({ compare: "identical" })), REPO, HEAD, LANE_ARMED_FILES, 99), null); + for (const run of [{ event: "workflow_dispatch" }, { status: "completed", conclusion: "success" }]) { + assert.equal(await laneArmedHold(reader(attestation({ run })), REPO, HEAD, LANE_ARMED_FILES, 99), null, JSON.stringify(run)); + } + // A path off the lane-armed set needs no attestation and reads nothing. + assert.equal(await laneArmedHold(async () => assert.fail("read"), REPO, HEAD, ["docs/a.md", ".github/workflows/pr-merge.yml"], 99), null); +}); + +test("laneArmedHold holds on a forged, stale or unreadable attestation", async () => { + const forged = { + "another app's check run": { check: { app: { slug: "evil-app" } } }, + "a check run that did not succeed": { check: { conclusion: "neutral" } }, + "a check run still in progress": { check: { status: "in_progress", conclusion: null } }, + "a check run at another head": { check: { head_sha: "c".repeat(40) } }, + "a check run with no run id": { check: { external_id: "pr-merge-gate" } }, + "a details_url naming another run": { check: { details_url: `https://github.com/${REPO}/actions/runs/1` } }, + "a record of another head": { record: { head: "c".repeat(40) } }, + "a record naming another run": { record: { run: 1 } }, + "a record of another version": { record: { v: 2 } }, + "a record of another pull request": { record: { pr: 98 } }, + // The real pr-merge.yml run of main, copied into a check run it never + // posted: that run uploaded no attestation of this head. + "a genuine pr-merge run that attested nothing here": { artifact: null }, + "an expired attestation artifact": { artifact: { expired: true } }, + "an artifact of another run": { artifact: { workflow_run: { id: 1 } } }, + "an artifact of another name": { artifact: { name: `pr-merge-lane-armed-98-${HEAD}` } }, + "an artifact read that fails": { fail: (path) => path.includes("/artifacts?") }, + // A tag named main: head_branch reads "main", but the run's commit is + // not on the main branch. + "a run whose commit is not on main": { compare: "diverged" }, + "a run whose commit is ahead of main": { compare: "behind" }, + "a run with no commit": { run: { head_sha: null } }, + "a branch tip that is not a SHA": { tip: "main" }, + "a branch read that fails": { fail: (path) => path.endsWith("/branches/main") }, + "a compare read that fails": { fail: (path) => path.includes("/compare/") }, + "a record missing a changed lane-armed path": { paths: LANE_ARMED_FILES.slice(1) }, + "a run of another workflow": { run: { path: ".github/workflows/pr-refine.yml" } }, + "a run from a pull request branch": { run: { head_branch: "feat/x" } }, + "a pull_request_target run": { run: { event: "pull_request_target" } }, + "a pull_request run": { run: { event: "pull_request" } }, + // The lane-armed job never runs on workflow_run, so no such run attests. + "a workflow_run run": { run: { event: "workflow_run" } }, + "a run that failed": { run: { status: "completed", conclusion: "failure" } }, + "a queued run": { run: { status: "queued" } }, + "a run in a fork": { run: { head_repository: { full_name: "fork/repo" } } }, + "a run of another repository": { run: { repository: { full_name: "other/repo" } } }, + "a run whose id differs": { run: { id: 1 } }, + "a check-run read that fails": { fail: (path) => path.includes("/check-runs") }, + "a run read that fails": { fail: (path) => path.endsWith(`/actions/runs/${RUN_ID}`) }, + "a default branch read that fails": { fail: (path) => path === `/repos/${REPO}` }, + "no default branch": { branch: "" }, + }; + for (const [name, overrides] of Object.entries(forged)) { + const held = await laneArmedHold(reader(attestation(overrides)), REPO, HEAD, LANE_ARMED_FILES, 99); + assert.match(String(held), /the merge gate's own code/, name); + } + // No check run at all, and a malformed head. + assert.match(await laneArmedHold(reader(attestation()), REPO, "c".repeat(40), LANE_ARMED_FILES, 99), /no pr-merge\/lane-armed check run/); + assert.match(await laneArmedHold(reader(attestation()), REPO, "HEAD", LANE_ARMED_FILES, 99), /cannot be looked up/); + assert.match(await laneArmedHold(reader(attestation()), REPO, HEAD, LANE_ARMED_FILES, 0), /cannot be looked up/); + assert.equal(laneArmedArtifact(99, HEAD), `pr-merge-lane-armed-99-${HEAD}`); +}); + +test("laneArmedRecord reads only a version-1 record line", () => { + assert.deepEqual(laneArmedRecord('a\nlane-armed {"v":1,"pr":1}\nb'), { v: 1, pr: 1 }); + for (const text of ["", "lane-armed {", 'lane-armed {"v":2}', 'x lane-armed {"v":1}', null]) { + assert.equal(laneArmedRecord(text), null, String(text)); + } +}); + +test("ci-status holds the merge gate's own code until pr-merge attests the head, or a trusted person approves it", async () => { + for (const path of LANE_ARMED_FILES) { + const held = await ci([{ filename: path, status: "modified" }], { attest: attestation({ check: { app: { slug: "evil-app" } } }) }); + assert.equal(held.code, 1, `${path}: ${held.out}`); + assert.ok(held.out.includes(`it changes ${path}, the merge gate's own code`), `${path}: ${held.out}`); + const attested = await ci([{ filename: path, status: "modified" }], { attest: attestation() }); + assert.equal(attested.code, 0, `${path}: ${attested.out}`); + assert.match(attested.out, /pr-merge\/lane-armed at this head, from a pr-merge\.yml run on the default branch, covers/); + const approved = await ci([{ filename: path, status: "modified" }], { reviews: [approve], attest: attestation({ run: { head_branch: "feat/x" } }) }); + assert.equal(approved.code, 0, `${path}: ${approved.out}`); + } + // An attestation never lifts a person-held path beside it. + const both = await ci([{ filename: LANE_ARMED_FILES[0], status: "modified" }, { filename: "AGENTS.md", status: "modified" }], { attest: attestation() }); + assert.equal(both.code, 1, both.out); + assert.ok(both.out.includes("it changes AGENTS.md, a hold path"), both.out); +}); 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" }]); + const both = await ci([{ filename: ENFORCEMENT_FILES[0], status: "modified" }, { filename: ".github/workflows/pr-verify.yml", 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); + assert.ok(both.out.includes("it changes .github/workflows/pr-verify.yml, 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) { @@ -1430,6 +1609,10 @@ test("Q10: a push that touches any hold path, a configured one or a file a hold ["a script ci-status runs", ["scripts/check-stale-base-overlap.sh"], []], ["an action a hold-path workflow uses", [".github/actions/report-check-run/report-check-run.mjs"], []], ["a workflow", [".github/workflows/pr-review-hosted.yml"], []], + // A lane-armed gate path holds nothing in ci-status, but an approval never + // carries over one (owner decision Q7). + ["a lane-armed agent instruction file", ["AGENTS.md"], []], + ["a lane-armed agent settings file", [".claude/settings.json"], []], ]) { const merges = pushed(files); if (name === "a rename off a gated file") merges.files[0].previous_filename = ".github/actions/request-lane-token/action.yml"; @@ -1437,6 +1620,10 @@ test("Q10: a push that touches any hold path, a configured one or a file a hold assert.equal(ungatedSince(merges, holdPaths), false, name); } assert.ok(CARRY_REFUSES.includes("scripts/**")); + // A lane-armed path holds nothing in pathHold, yet an approval never + // carries over a push touching one (owner decision Q7). + assert.equal(ungatedSince(pushed(["docs/armed/a.md"]), [], ["docs/armed/**"]), false); + assert.equal(ungatedSince(pushed(["docs/armed/a.md"]), [], []), true); }); test("Q10: a compare not read whole, not ahead, or with a dismissal the push did not make refuses", () => { diff --git a/.github/scripts/pr-review-main/workflow.test.mjs b/.github/scripts/pr-review-main/workflow.test.mjs index fdffccc0db..db2349c149 100644 --- a/.github/scripts/pr-review-main/workflow.test.mjs +++ b/.github/scripts/pr-review-main/workflow.test.mjs @@ -420,6 +420,43 @@ test("only a lane-source pull request is reviewed, by the gate's own predicate", assert.match(step("Write the diff"), /^ {8}if: steps\.select\.outputs\.review == 'true'$/m); }); +// Owner decision Q5: after a red pr-require-checks run, only a pull request +// changing main's lane-armed code is reviewed, and a cut-short list reviews +// nothing; after a green one, any lane source is. +test("after a red ci-status each leg reviews only main's lane-armed code", () => { + const root = path.resolve(new URL("../../..", import.meta.url).pathname); + const dir = mkdtempSync(path.join(tmpdir(), "pr-review-main-select-")); + try { + symlinkSync(root, path.join(dir, ".base")); + for (const job of LEGS) { + assert.match(jobText(job), /\(github\.event\.workflow_run\.conclusion == 'success' \|\| github\.event\.workflow_run\.conclusion == 'failure'\)/, job); + const select = step("Select a lane-source pull request", job); + const program = /python3 -I -c '\n([\s\S]*?)\n {10}' "\$FILES"/.exec(select)[1].replace(/^ {10}/gm, ""); + // The program sits in a single-quoted shell string: a quote ends it. + assert.doesNotMatch(program, /'/, job); + const armed = "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py"; + for (const [upstream, files, want] of [ + ["success", ["AGENTS.md"], "true"], + ["success", [armed], "true"], + ["success", ["docs/a.md"], "false"], + ["success", Array.from({ length: 3000 }, (_, i) => `docs/${i}.md`), "true"], + ["failure", [armed], "true"], + ["failure", ["docs/a.md", ".github/scripts/pr-merge/who.mjs"], "true"], + ["failure", ["AGENTS.md"], "false"], + ["failure", [".github/workflows/pr-merge.yml"], "false"], + ["failure", [armed, ...Array.from({ length: 2999 }, (_, i) => `docs/${i}.md`)], "false"], + ]) { + const list = path.join(dir, "files.txt"); + writeFileSync(list, `${files.join("\n")}\n`); + const ran = spawnSync("python3", ["-I", "-c", program, list], { cwd: dir, env: { ...process.env, UPSTREAM: upstream }, encoding: "utf8" }); + assert.equal(ran.stdout.trim(), want, `${job} ${upstream} ${files[0]}: ${ran.stderr}`); + } + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + // The review lanes' credential order, read from their callers' `secrets:` // blocks, never from this workflow: each names the reusable's secret and the // repository secret it passes. diff --git a/.github/trust-path-hold.json b/.github/trust-path-hold.json index 6e71b17432..6db37f58f5 100644 --- a/.github/trust-path-hold.json +++ b/.github/trust-path-hold.json @@ -25,6 +25,8 @@ "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", + "plugins/review/skills/*", + "plugins/review/reference/*", ".claude/*", "*/.claude/*", ".claude-lane/*", diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 6b5708f36f..6d96dee848 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1532,6 +1532,7 @@ jobs: ready: (.ready == true), enqueued: (.enqueued == true), merged: (.merged == true), reportOnly: .unattended.reportOnly, reason: (.unattended.reason // .error), dispatched: .unattended.dispatched, blockers: (.blockers // [])} + + (if (.unattended.laneArmed | type) == "object" then {laneArmed: .unattended.laneArmed} else {} end) ' "$out" >>"$results" save reached=$((reached + 1)) @@ -2590,6 +2591,94 @@ jobs: with: tokens: github-token + # Owner decision Q5 (#10743): the first candidate whose gate passed its + # lane-armed paths (the merge gate's own code) on clean main-run reviews + # gets a `pr-merge/lane-armed` check run at its head and this run's + # artifact pr-merge-lane-armed--, which ci-status requires for + # those paths (merge-holds.mjs laneArmedHold), then its red ci-status job + # is re-run. Rules: .github/scripts/pr-merge/lane-armed.mjs. ci-status + # trusts them only from a run of this file on the default branch started by + # schedule or workflow_dispatch, so a pull_request_target run, whose + # head_branch is the pull request's, attests nothing. A job of its own: + # checks: write and actions: write, no lane token. + lane-armed: + needs: [merge] + if: >- + always() && needs.merge.outputs.gate-rows == 'true' + && github.ref == 'refs/heads/main' + && (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') + concurrency: + group: pr-merge-lane-armed + queue: max + runs-on: ubuntu-24.04 + timeout-minutes: 5 + permissions: + actions: write # re-run a red ci-status job + checks: write # post pr-merge/lane-armed + contents: read # check out main; read the floor files at base and head + pull-requests: read # each candidate's current head and base + env: + API_BUDGET_TOKEN: github-token + steps: + - name: Check out the default branch tip + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: refs/heads/main + path: .base + persist-credentials: false + + - name: Set up Node + uses: ./.base/.github/actions/setup-pinned-node # zizmor: ignore[self-repository] + with: + node-version-file: .base/.node-version + + - name: Read the gate's rows + id: rows + continue-on-error: true + timeout-minutes: 2 + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 + with: + name: pr-merge-gate-rows + path: ${{ runner.temp }}/rows + + - name: Attest a lane-armed head the gate passed + id: attest + if: steps.rows.outcome == 'success' + timeout-minutes: 3 + env: + GH_TOKEN: ${{ github.token }} + SUMMARY_PATH: ${{ runner.temp }}/rows/pr-merge-summary.json + LANE_ARMED_OUT: ${{ runner.temp }}/lane-armed + run: node .base/.github/scripts/pr-merge/lane-armed.mjs attest + + # The proof ci-status reads: only this run's own steps can upload it. + - name: Upload the attestation + id: upload + if: steps.attest.outputs.artifact != '' + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: ${{ steps.attest.outputs.artifact }} + path: ${{ runner.temp }}/lane-armed/lane-armed.json + retention-days: 7 + if-no-files-found: error + + # After the upload, so the re-run reads the artifact. + - name: Re-run the attested head's red ci-status + if: steps.upload.outcome == 'success' + timeout-minutes: 2 + env: + GH_TOKEN: ${{ github.token }} + LANE_ARMED_HEAD: ${{ steps.attest.outputs.head }} + run: node .base/.github/scripts/pr-merge/lane-armed.mjs rerun + + - name: Log the API budget at the end + if: always() + continue-on-error: true + timeout-minutes: 1 + uses: ./.base/.github/actions/log-api-budget # zizmor: ignore[self-repository] + with: + tokens: github-token + # Each candidate the gate judged shows its verdict on its head as one # `pr-merge / gate` check run: neutral, titled with the top refusal, or # success over a refusal once enqueued or merged. Not `ci-status`, so never diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 3be5b6bf9b..314b4cae37 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4775,7 +4775,7 @@ jobs: held_paths() { # The trust-gate paths, as bash patterns (`*` crosses `/`). # BEGIN hold-floor hold_patterns: generated by scripts/sync-hold-floor.py - hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.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' '.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/last-report.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-merge/listing-carry.mjs' '.github/actions/report-check-run/posting-run.mjs' 'scripts/sync-hold-floor.py' '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' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/escalation-routes.mjs' '.github/scripts/pr-unblock/escalation-routes.json' '.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/scripts/pr-unblock/report-escalation.sh' '.github/scripts/pr-unblock/adjudicate-feedback.mjs' '.github/scripts/pr-unblock/publish-feedback.mjs' '.github/scripts/pr-unblock/request-budget.mjs' '.github/scripts/pr-unblock/dependency-classify.mjs' '.github/scripts/apply-patch/apply-patch.mjs' '.github/scripts/apply-patch/predicates.mjs' '.github/scripts/apply-patch/protected-paths.mjs' '.github/scripts/apply-patch/retry-fetch.mjs' '.github/scripts/pr-unblock/fix-ci-context.mjs' '.github/scripts/pr-unblock/feedback-dispatch.mjs' '.github/scripts/pr-unblock/verify-notes.mjs' '.github/scripts/pr-unblock/limit-stop.mjs' '.github/scripts/pr-unblock/dispatch-record.mjs' '.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs' '.github/scripts/work-resolve-issue/stop-routes.json' '.github/scripts/work-resolve-issue/stop-routes.mjs' '.github/scripts/pr-review-main/*' '.claude/*' '*/.claude/*' '.claude-lane/*' '*/.claude-lane/*' 'docs/conventions/instruction-exception-register/*' 'docs/conventions/untrusted-content/*' '.mcp.json' '*/.mcp.json' 'CLAUDE.md' '*/CLAUDE.md' 'CLAUDE.local.md' '*/CLAUDE.local.md' 'AGENTS.md' '*/AGENTS.md' 'REVIEW.md' 'docs/standards/*' '.github/workflows/dependabot-fragments.yml' '.github/workflows/eval-browser-dispatch.yml' '.github/workflows/intake-reenter.yml' '.github/workflows/intake-triage.yml' '.github/workflows/maintenance-audit-lane-runs.yml' '.github/workflows/maintenance-reconcile-lost-runs.yml' '.github/workflows/post-merge-demotion-alert.yml' '.github/workflows/post-merge-test-mutation.yml' '.github/workflows/pr-feedback-dispatch.yml' '.github/workflows/pr-merge.yml' '.github/workflows/pr-refine.yml' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-review-hosted.yml' '.github/workflows/pr-review-main.yml' '.github/workflows/pr-review-security-hosted.yml' '.github/workflows/pr-run-activity-read.yml' '.github/workflows/pr-run-activity-write.yml' '.github/workflows/pr-unblock-dispatch.yml' '.github/workflows/pr-unblock.yml' '.github/workflows/pr-verify.yml' '.github/workflows/release-publish-plugins.yml' '.github/workflows/work-resolve-issue.yml' '.github/workflows/work-unblock.yml') + hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.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' '.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/last-report.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-merge/listing-carry.mjs' '.github/scripts/pr-merge/lane-armed.mjs' '.github/scripts/pr-merge/floor-ast.py' '.github/actions/report-check-run/posting-run.mjs' 'scripts/sync-hold-floor.py' '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' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/escalation-routes.mjs' '.github/scripts/pr-unblock/escalation-routes.json' '.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/scripts/pr-unblock/report-escalation.sh' '.github/scripts/pr-unblock/adjudicate-feedback.mjs' '.github/scripts/pr-unblock/publish-feedback.mjs' '.github/scripts/pr-unblock/request-budget.mjs' '.github/scripts/pr-unblock/dependency-classify.mjs' '.github/scripts/apply-patch/apply-patch.mjs' '.github/scripts/apply-patch/predicates.mjs' '.github/scripts/apply-patch/protected-paths.mjs' '.github/scripts/apply-patch/retry-fetch.mjs' '.github/scripts/pr-unblock/fix-ci-context.mjs' '.github/scripts/pr-unblock/feedback-dispatch.mjs' '.github/scripts/pr-unblock/verify-notes.mjs' '.github/scripts/pr-unblock/limit-stop.mjs' '.github/scripts/pr-unblock/dispatch-record.mjs' '.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs' '.github/scripts/work-resolve-issue/stop-routes.json' '.github/scripts/work-resolve-issue/stop-routes.mjs' 'plugins/review/skills/*' 'plugins/review/reference/*' '.github/scripts/pr-review-main/*' '.claude/*' '*/.claude/*' '.claude-lane/*' '*/.claude-lane/*' 'docs/conventions/instruction-exception-register/*' 'docs/conventions/untrusted-content/*' '.mcp.json' '*/.mcp.json' 'CLAUDE.md' '*/CLAUDE.md' 'CLAUDE.local.md' '*/CLAUDE.local.md' 'AGENTS.md' '*/AGENTS.md' 'REVIEW.md' 'docs/standards/*' '.github/workflows/dependabot-fragments.yml' '.github/workflows/eval-browser-dispatch.yml' '.github/workflows/intake-reenter.yml' '.github/workflows/intake-triage.yml' '.github/workflows/maintenance-audit-lane-runs.yml' '.github/workflows/maintenance-reconcile-lost-runs.yml' '.github/workflows/post-merge-demotion-alert.yml' '.github/workflows/post-merge-test-mutation.yml' '.github/workflows/pr-feedback-dispatch.yml' '.github/workflows/pr-merge.yml' '.github/workflows/pr-refine.yml' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-review-hosted.yml' '.github/workflows/pr-review-main.yml' '.github/workflows/pr-review-security-hosted.yml' '.github/workflows/pr-run-activity-read.yml' '.github/workflows/pr-run-activity-write.yml' '.github/workflows/pr-unblock-dispatch.yml' '.github/workflows/pr-unblock.yml' '.github/workflows/pr-verify.yml' '.github/workflows/release-publish-plugins.yml' '.github/workflows/work-resolve-issue.yml' '.github/workflows/work-unblock.yml') # END hold-floor hold_patterns if [[ ! $PR_NUMBER =~ ^[1-9][0-9]*$ ]]; then echo "::error::the pull request number '$PR_NUMBER' is not a number." diff --git a/.github/workflows/pr-review-hosted.yml b/.github/workflows/pr-review-hosted.yml index ca664a1c88..41262753ea 100644 --- a/.github/workflows/pr-review-hosted.yml +++ b/.github/workflows/pr-review-hosted.yml @@ -231,6 +231,8 @@ jobs: .github/scripts/pr-merge/post-unarmed-status.mjs .github/scripts/pr-merge/verify-answers.mjs .github/scripts/pr-merge/listing-carry.mjs + .github/scripts/pr-merge/lane-armed.mjs + .github/scripts/pr-merge/floor-ast.py .github/actions/report-check-run/posting-run.mjs scripts/sync-hold-floor.py plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -278,6 +280,8 @@ jobs: .github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs .github/scripts/work-resolve-issue/stop-routes.json .github/scripts/work-resolve-issue/stop-routes.mjs + plugins/review/skills/** + plugins/review/reference/** .github/scripts/pr-review-main/** **/.claude/** **/.claude-lane/** diff --git a/.github/workflows/pr-review-main.yml b/.github/workflows/pr-review-main.yml index 4c9e9a509c..1a70cef275 100644 --- a/.github/workflows/pr-review-main.yml +++ b/.github/workflows/pr-review-main.yml @@ -6,7 +6,10 @@ name: pr-review-main # pull_request review lanes run the head's copy of their callers and post as # github-actions[bot], so a head can rewrite or forge their result; the merge # gate binds such a pull request's review only on this run's verdict -# (babysit_review_evidence `_main_bound`), never on a comment. +# (babysit_review_evidence `_main_bound`), never on a comment. It runs after +# a green pr-require-checks run, and after a red one only for a pull request +# that changes main's lane-armed code (babysit_risk.LANE_ARMED_FLOOR), whose +# ci-status stays red until pr-merge attests this review (owner decision Q5). # # workflow_run runs this file from main, never a head's copy. No pull request # code runs here: the diff, the trusted pull request text and the lane's @@ -70,7 +73,7 @@ jobs: if: >- github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' - && github.event.workflow_run.conclusion == 'success' + && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') && github.event.workflow_run.head_repository.full_name == github.repository strategy: fail-fast: false @@ -165,16 +168,25 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ steps.gate.outputs.pr-number }} FILES: ${{ runner.temp }}/pr-review-main-files.txt + UPSTREAM: ${{ github.event.workflow_run.conclusion }} run: | gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" \ --jq '.[] | .filename, (.previous_filename // empty)' >"$FILES" review=$(python3 -I -c ' - import sys + import os, sys sys.path.insert(0, ".base/plugins/source-control/skills/babysit-prs/scripts") import babysit_review_evidence as ev + from babysit_risk import LANE_ARMED_FLOOR, glob_match paths = [line.rstrip("\n") for line in open(sys.argv[1], encoding="utf-8")] - # The files API lists at most 3000; a longer list may hide one. - print("true" if len(paths) >= 3000 or any(ev.lane_source(p) for p in paths) else "false") + if os.environ.get("UPSTREAM") == "success": + # The files API lists at most 3000; a longer list may hide one. + print("true" if len(paths) >= 3000 or any(ev.lane_source(p) for p in paths) else "false") + else: + # Owner decision Q5: after a red ci-status, only a pull request + # changing the lane-armed code on main is reviewed, so pr-merge can + # attest it; a list that may be cut short reviews nothing. + armed = any(glob_match(g, p) for g in LANE_ARMED_FLOOR for p in paths) + print("true" if len(paths) < 3000 and armed else "false") ' "$FILES") echo "review=$review" >>"$GITHUB_OUTPUT" echo "lane source changed: $review" @@ -558,7 +570,7 @@ jobs: && needs.review-handoffs.result == 'success' && github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' - && github.event.workflow_run.conclusion == 'success' + && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') && github.event.workflow_run.head_repository.full_name == github.repository strategy: fail-fast: false @@ -643,16 +655,25 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ steps.gate.outputs.pr-number }} FILES: ${{ runner.temp }}/pr-review-main-files.txt + UPSTREAM: ${{ github.event.workflow_run.conclusion }} run: | gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" \ --jq '.[] | .filename, (.previous_filename // empty)' >"$FILES" review=$(python3 -I -c ' - import sys + import os, sys sys.path.insert(0, ".base/plugins/source-control/skills/babysit-prs/scripts") import babysit_review_evidence as ev + from babysit_risk import LANE_ARMED_FLOOR, glob_match paths = [line.rstrip("\n") for line in open(sys.argv[1], encoding="utf-8")] - # The files API lists at most 3000; a longer list may hide one. - print("true" if len(paths) >= 3000 or any(ev.lane_source(p) for p in paths) else "false") + if os.environ.get("UPSTREAM") == "success": + # The files API lists at most 3000; a longer list may hide one. + print("true" if len(paths) >= 3000 or any(ev.lane_source(p) for p in paths) else "false") + else: + # Owner decision Q5: after a red ci-status, only a pull request + # changing the lane-armed code on main is reviewed, so pr-merge can + # attest it; a list that may be cut short reviews nothing. + armed = any(glob_match(g, p) for g in LANE_ARMED_FLOOR for p in paths) + print("true" if len(paths) < 3000 and armed else "false") ' "$FILES") echo "review=$review" >>"$GITHUB_OUTPUT" echo "lane source changed: $review" @@ -1035,7 +1056,7 @@ jobs: && needs.subscription-handoffs.result == 'success' && github.ref == 'refs/heads/main' && github.event.workflow_run.event == 'pull_request' - && github.event.workflow_run.conclusion == 'success' + && (github.event.workflow_run.conclusion == 'success' || github.event.workflow_run.conclusion == 'failure') && github.event.workflow_run.head_repository.full_name == github.repository strategy: fail-fast: false @@ -1113,16 +1134,25 @@ jobs: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ steps.gate.outputs.pr-number }} FILES: ${{ runner.temp }}/pr-review-main-files.txt + UPSTREAM: ${{ github.event.workflow_run.conclusion }} run: | gh api --paginate "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files?per_page=100" \ --jq '.[] | .filename, (.previous_filename // empty)' >"$FILES" review=$(python3 -I -c ' - import sys + import os, sys sys.path.insert(0, ".base/plugins/source-control/skills/babysit-prs/scripts") import babysit_review_evidence as ev + from babysit_risk import LANE_ARMED_FLOOR, glob_match paths = [line.rstrip("\n") for line in open(sys.argv[1], encoding="utf-8")] - # The files API lists at most 3000; a longer list may hide one. - print("true" if len(paths) >= 3000 or any(ev.lane_source(p) for p in paths) else "false") + if os.environ.get("UPSTREAM") == "success": + # The files API lists at most 3000; a longer list may hide one. + print("true" if len(paths) >= 3000 or any(ev.lane_source(p) for p in paths) else "false") + else: + # Owner decision Q5: after a red ci-status, only a pull request + # changing the lane-armed code on main is reviewed, so pr-merge can + # attest it; a list that may be cut short reviews nothing. + armed = any(glob_match(g, p) for g in LANE_ARMED_FLOOR for p in paths) + print("true" if len(paths) < 3000 and armed else "false") ' "$FILES") echo "review=$review" >>"$GITHUB_OUTPUT" echo "lane source changed: $review" diff --git a/.github/workflows/pr-review-security-hosted.yml b/.github/workflows/pr-review-security-hosted.yml index 2ac4cd2584..d35110be64 100644 --- a/.github/workflows/pr-review-security-hosted.yml +++ b/.github/workflows/pr-review-security-hosted.yml @@ -231,6 +231,8 @@ jobs: .github/scripts/pr-merge/post-unarmed-status.mjs .github/scripts/pr-merge/verify-answers.mjs .github/scripts/pr-merge/listing-carry.mjs + .github/scripts/pr-merge/lane-armed.mjs + .github/scripts/pr-merge/floor-ast.py .github/actions/report-check-run/posting-run.mjs scripts/sync-hold-floor.py plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -278,6 +280,8 @@ jobs: .github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs .github/scripts/work-resolve-issue/stop-routes.json .github/scripts/work-resolve-issue/stop-routes.mjs + plugins/review/skills/** + plugins/review/reference/** .github/scripts/pr-review-main/** **/.claude/** **/.claude-lane/** diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index 84a90d798e..4df8b716f9 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -958,6 +958,38 @@ merge: - melodic-agent[bot] # true: report each decision and write nothing. report-only: false + # Owner decisions Q7 and Q5 (#10743): true lane-arms the merge gate's own + # code (babysit_risk.py and every .github/scripts/pr-merge/ script on the + # floor, `trust` rows included, less `person-held` below). A pull request + # held only by those paths merges with no person once both review lanes are + # bound to the main-run review of its head with no finding, no review + # thread is open, and pr-merge's `pr-merge/lane-armed` check run, posted by + # a pr-merge.yml run on main, is at the head; ci-status checks that check + # run and the run behind it (README.md, Lane-armed gate paths). Anything but + # true lane-arms nothing. scripts/sync-hold-floor.py copies the lane-armed + # and person-held lists into babysit_risk.py and merge-holds.mjs; its + # --check fails on a stale copy. + lane-arm-gate: true + # The `gate` rows that wait for a person whatever lane-arm-gate says: every + # `gate` row outside the merge gate's own code must be here, or the script + # stops. It adds every gated workflow (babysit_risk.gated_workflow) and every + # `trust` row outside the merge gate's code. Each entry must be a `trust` or + # `gate` floor row. + person-held: + - .github/actions/log-api-budget/rate-guard.mjs + - .github/actions/log-api-budget/rate-limits.json + - .github/actions/report-check-run/posting-run.mjs + - .github/scripts/pr-review-main/** + - "**/.claude/**" + - "**/.claude-lane/**" + - docs/conventions/instruction-exception-register/** + - docs/conventions/untrusted-content/** + - "**/.mcp.json" + - "**/CLAUDE.md" + - "**/CLAUDE.local.md" + - "**/AGENTS.md" + - REVIEW.md + - docs/standards/** # true: post-merge-dispatch starts a full run of main inside its 30-minute # cap when an unverified lane merge has no started full run covering it, # one run for the whole batch, so the lane's in-flight room frees after one @@ -1068,7 +1100,9 @@ merge: majors: review # No path is held for being trusted alone (owner direction 2026-10-24) # except the narrow path floor (babysit_risk.py FLOOR_CLASSES, owner - # decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B). Its `trust` + # decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B; merge.lane-arm-gate + # above lets the merge gate's own code merge on a clean main-run review and + # pr-merge's attestation, README.md Lane-armed gate paths). 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, this file, babysit_merge*, babysit_risk.py, diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index e7d132ebff..dc02d7a85b 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -12,6 +12,7 @@ - [Outputs and skips](#outputs-and-skips) - [Merge authority](#merge-authority) - [Review-lane holds](#review-lane-holds) +- [Lane-armed gate paths](#lane-armed-gate-paths) - [Trust-root paths](#trust-root-paths) - [The lane sweep](#the-lane-sweep) - [The monitoring lane](#the-monitoring-lane) @@ -997,8 +998,10 @@ the rules that only tighten in `.github/scripts/pr-merge/merge-holds.mjs`, `scripts/sync-hold-floor.py`, which writes the floor's copies, and the pr-merge scripts that apply and enforce the holds: `enable-auto-merge.mjs`, `superseded.mjs`, `clear-escalations.mjs`, `gate-check.mjs` and - `head-ref-map.mjs`. - - `gate`, a person merges it: the rest of the merge gate's own code, which is the rate guard + `head-ref-map.mjs`. `babysit_risk.py` and those pr-merge scripts are lane-armed + ([Lane-armed gate paths](#lane-armed-gate-paths)); every other `trust` row is person-held. + - `gate`, a person merges it, except the pr-merge scripts, which are lane-armed + ([Lane-armed gate paths](#lane-armed-gate-paths)): the rest of the merge gate's own code, which is the rate guard (`.github/actions/log-api-budget/rate-guard.mjs`) with its rate-limit defaults (`.github/actions/log-api-budget/rate-limits.json`) and every other script of `.github/scripts/pr-merge/`. And the review's own surface (owner direction 2026-10-28): the @@ -1012,7 +1015,8 @@ the rules that only tighten in named by file, and each workflow whose job token has `actions: write` (or `write-all`), which can dispatch those credentialed workflows, `intake-reenter.yml` and `pr-feedback-dispatch.yml` among them: a logic edit inside scopes it already holds trips no - patch hold, so a model verdict alone never clears it. 23 workflows are gated. + patch hold, so a model verdict alone never clears it; every gated workflow is person-held. + 23 workflows are gated. `scripts/sync-hold-floor.py` derives these rows from the workflow files with `babysit_risk.gated_workflow`, so a workflow that gains a secret or `actions: write` joins the gate, and `--check` fails on a stale table. A workflow whose job token writes other scopes @@ -1042,7 +1046,9 @@ the rules that only tighten in reads, `.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 stop routes `report-outcome.mjs` loads, `stop-routes.mjs` and `stop-routes.json`, and `fix-ci-context.mjs` - with the `.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs` it loads), the scripts that act on a pr-unblock + with the `.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs` it loads), the + review criteria `pr-review-main.yml` copies from main for the model (`plugins/review/skills/**` + and `plugins/review/reference/**`), the scripts that act on a pr-unblock stop's route (`report-escalation.sh`, which labels a held reason, `adjudicate-feedback.mjs` and `publish-feedback.mjs`, which decide and resolve threads, and `.github/scripts/apply-patch/apply-patch.mjs`, which holds fix-ci escalations) with every module @@ -2452,6 +2458,93 @@ fails while the queued PR carries `pr-verify-failed`, which a pass at a new comm the next head, so the pass that removes it would never come. `fix-ci` keys on `ci-lanes`, so a hold or a pending review never summons it. +## Lane-armed gate paths + +Owner decisions Q7 ([#10743](https://github.com/melodic-software/claude-code-plugins/pull/10743#issuecomment-6486023647)) +and Q5 (2026-10-29, option 1): `merge.lane-arm-gate: true` lets a pull request held only by the +merge gate's own code merge with no person. The lane-armed set is exactly +`plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py` and every +`.github/scripts/pr-merge/` script on the floor, `trust` rows included +(`scripts/sync-hold-floor.py` `lane_armable`). A change that keeps every hold path but weakens +these files' logic merges on the two bound AI reviews plus pr-merge's `pr-merge/lane-armed` +attestation, with no person. + +Every other `trust` and `gate` row is person-held: every workflow, the credential and token paths +(`request-lane-token`, the trusted-actor list, `check-trusted-trigger`, `check-kill-switch`, +`resolve-config`), the rate guard and its limits, the check-run poster, the main-run review's +scripts, the review criteria and agent files (`REVIEW.md`, `docs/standards/**`, `**/.claude/**`, +the untrusted-content and instruction-exception conventions), the rest of babysit-prs +(`babysit_merge*.py`, `babysit_review_evidence.py`), and `scripts/sync-hold-floor.py` and +`docs/conventions/pr-pipeline.yaml`, which define the floor and `merge.person-held` itself. + +A lane-armed path clears only when all of these hold at the current head: + +1. Both review lanes are bound by the main-run review (`pr-review-main.yml`), never by a + pull_request run: a lane-armed path is a lane source (`babysit_review_evidence.lane_source`). + `pr-review-main.yml` reviews such a pull request after a red ci-status too, deciding from + main's lane-armed globs and the full file list (a list that may be cut short reviews nothing), + so neither the red ci-status nor the human-gated label blocks the review. +2. Each lane counts no open finding, any severity, and the pull request has no unresolved review + thread (`babysit_risk.lane_armed`). +3. The merge gate's risk score and verdict pass (`babysit_risk.lane_arm_record`). +4. The change does not weaken the hold floor (`lane-armed.mjs` `floorChange`). The head's text is + never parsed: a decoy marker pair in a docstring or block comment, or a row inside an + expression that drops it, would fool any text parse. The pull request's patch to each + floor-definition file (`babysit_risk.py`, `merge-holds.mjs`, `lane-armed.mjs`) is judged + against the base's marked regions (`floor-rows FLOOR_CLASSES` in `babysit_risk.py`, + `hold-floor HOLD_PATH_CLASSES`, and `hold-floor LANE_ARM` in both), which main's copy holds + in the exact generated shape. A changed line refuses when it names a floor marker; when, + outside those regions, it names a floor table or a set derived from one (`FLOOR_CLASSES`, + `HOLD_PATH_CLASSES`, `HOLD_PATH_FLOOR`, `HIGH_FLOOR`, `REVIEW_FLOOR`, `PERSON_HELD`, + `LANE_ARM*` and the rest); or when, inside them, it is anything but an added row of the + generated shape or an added person-held entry. An added line also refuses when it holds a + non-ASCII character (Python folds a fullwidth identifier to ASCII). A removed row is a shrink. + For `babysit_risk.py`, the base checkout's `floor-ast.py` also parses the base and head copies + with `ast` and runs neither. It refuses a head that references a reflective name (`globals`, + `locals`, `vars`, `setattr`, `getattr`, `exec`, `eval`, `compile`, `__import__`, `importlib`, + `builtins`, `__dict__`, `__setattr__`, `modules` and the rest) or a frame name (`f_globals`, + `_getframe`, `currentframe`, `inspect` and the rest) more often than the base, or that holds + more dunder names (`__globals__` and any other) or more imports, so an alias or a split line + still counts. It also refuses a head that binds a floor table or a set + derived from one other than once, at module level, by one assignment: a table to a literal, a + derived set to the base's expression. `merge-holds.mjs` binds its tables with `const` and + `Object.freeze`, which no later line can rebind by name. Its head copy, and `lane-armed.mjs`'s, + also refuses when it holds more `eval(`, `Function(`, `globalThis`, `Reflect.`, + `Object.defineProperty`, `__proto__`, `import(`, `require(`, `vm.` or `process.binding` than the + base copy. Any other logic edit there stays the residual below. `floor-ast.py` holds no table: + its patch is judged for markers and hold names only. + The base copy and the patches come from one compare of the base branch with the head, and the + base copy is read at its merge base, the commit GitHub measures the patch from. A missing + patch, a renamed or deleted floor file, or a file list the compare cuts short (it lists at most + 300) refuses. + +Then pr-merge's `lane-armed` job, on a scheduled or hand-started run of main, posts the check run +`pr-merge/lane-armed` at the head with the run's id as `external_id` and in `details_url`, and +re-runs the head's red ci-status job. ci-status (`merge-holds.mjs` `laneArmedHold`, read from the +base commit, on pull_request and merge_group runs alike, keyed on the pull request's head) passes a +lane-armed path only when that check run is from the GitHub Actions app, succeeded, carries a +record of this head and of every lane-armed path changed now, and names a run of +`.github/workflows/pr-merge.yml` on the default branch, started by schedule or +workflow_dispatch, running or succeeded, whose commit is the branch tip or an ancestor of it (the +compare API against the tip the branches API reads, since a tag can share the branch's name), and +that uploaded the artifact. Any workflow with `checks: write` posts as the same app, so the run is +the proof, not the check run. Anything missing or unreadable holds. A trusted +person's approval of the head lifts it as it lifts a hold path. + +The residual Q5 accepts: none of these checks reads what a lane-armed file's logic does. A head +that keeps every floor row and weakens that logic merges on the two bound AI reviews plus the +attestation. + +Each attestation is one countable record, the line +`lane-armed {"v":1,"pr":N,"head":"","paths":[...],"lanes":{"claude-review":0,"claude-security-review":0},"threads":0,"run":}` +in the check run's summary and as a `::notice::` in the pr-merge run. A head the job declines +logs `::notice::lane-armed refused {"v":1,"pr":N,"head":"","reason":"..."}` with the +failed condition. + +clear-escalations lifts a human-gated label recorded on lane-armed paths alone at the same head +once the gate's reader refuses nothing there (`armedCause`), and an approval never carries over +a push that touches a lane-armed path (`ungatedSince`). + ## Trust-root paths A lane's token grant and the rules it is judged by come from these default-branch paths, so a diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index b472e6e0f7..adc6ce0d4b 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -1138,6 +1138,18 @@ "default": [], "description": "[bot] logins of GitHub Apps whose touch puts a trust-gate path change in a person's hands. ci-status fails a same-repository pull request that changes a trust-gate path when one of them opened it, authored or committed one of its commits, or pushed to its head branch, as it fails an exempt author's (README.md, Both lanes review the head). It is read at the base commit. Empty or absent: no App is held this way." }, + "lane-arm-gate": { + "type": "boolean", + "default": false, + "description": "true: a pull request held only by the merge gate's own code (babysit_risk.py and the .github/scripts/pr-merge/ scripts on the floor, less `person-held`) merges without a person when both review lanes are bound to the main-run review at its head with no finding, no review thread is open, and pr-merge's `pr-merge/lane-armed` check run from a pr-merge.yml run on the default branch is at the head (README.md, Lane-armed gate paths). Every other `trust` and `gate` path waits for a person. Absent, false or any other value: every one waits for a person. scripts/sync-hold-floor.py copies it into babysit_risk.py and merge-holds.mjs." + }, + "person-held": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "uniqueItems": true, + "default": [], + "description": "The `gate` floor globs outside the merge gate's own code, each waiting for a person whatever lane-arm-gate says (README.md, Lane-armed gate paths); a merge-gate row listed here stays with a person too. scripts/sync-hold-floor.py adds every gated workflow and every other `trust` row, and refuses an entry that is not a `trust` or `gate` floor row or a `gate` row outside the merge gate's code left unlisted." + }, "diff-check": { "type": "object", "additionalProperties": false, diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index 0b682e76ca..e5a4494a6d 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -668,6 +668,22 @@ def project(thread: dict[str, Any]) -> dict[str, object]: return [cast(dict[str, object], record) for record in records] +def with_open_threads( + repo: str, number: int, risk_input: babysit_risk.RiskInput +) -> babysit_risk.RiskInput: + """`risk_input` with its unresolved review thread count, read only when + that count could lane-arm a `gate` path (`babysit_risk.lane_arm_pending`). + A read that fails or is not served GraphQL leaves it None, which keeps + the person hold.""" + if not babysit_risk.lane_arm_pending(risk_input): + return risk_input + try: + threads = unresolved_threads(repo, number) + except (RuntimeError, ValueError, json.JSONDecodeError): + threads = None + return replace(risk_input, open_threads=None if threads is None else len(threads)) + + def repository_default_branch(repo: str) -> str | None: """Return the repository's default branch, or None when it cannot be read. @@ -3583,8 +3599,10 @@ def score_unattended( public=True if private is False else None, pulumi_dirs=merge_policy.pulumi_dirs_at(repo, pr), base_sha=json_object(pr.get("base")).get("sha"), + lanes_app=eligible.adoption is None and eligible.dependabot is None, **lane_switches, ) + risk_input = with_open_threads(repo, number, risk_input) risk = babysit_risk.score(risk_input) if any(r.code == babysit_risk.COST_CODE for r in risk.reasons): # Read only for a cost hold: an owner decision the body cites clears it. @@ -3634,6 +3652,11 @@ def score_unattended( if verdict.refusal: inputs["own_refusal"] = inputs.get("own_refusal") or verdict.refusal reason = verdict.refusal + # What gate-check's `pr-merge/lane-armed` attestation records (owner + # decision Q5): only with no refusal of the gate's own. + record = babysit_risk.lane_arm_record(risk_input, risk) + if record is not None: + details["laneArmed"] = record | {"refused": record["refused"] or reason} return replace(policy, reason=reason, details=details, inputs=inputs) diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py index 50910860ff..9d82b790b0 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py @@ -42,7 +42,13 @@ from datetime import datetime from typing import Any -from babysit_risk import FLOOR, LONG_LINE_BYTES, LaneEvidence, glob_match +from babysit_risk import ( + FLOOR, + LANE_ARMED_FLOOR, + LONG_LINE_BYTES, + LaneEvidence, + glob_match, +) # Lane id -> (caller workflow path, lane job name suffix). Fixed in code: a # weaker table could only come from a file a pull request can edit. @@ -417,7 +423,8 @@ def _main_lane( pull_request review left open (`_carried_blocking`), and the most any main-run review recorded on another head of this pull request (`_recorded_critical`), so a finding any of them reported holds, bound - or not.""" + or not. Bound evidence is marked `main_run`, the only binding that + lane-arms a `gate` path (`babysit_risk.lane_armed`).""" try: found = _main_bound(lane, head, records, context) except Exception: # noqa: BLE001 - any doubt is unbound @@ -426,7 +433,7 @@ def _main_lane( if lane != SEVERITY_LANE: if found is None or recorded is None: return _unbound(lane, "lane-source-changed", head, *carried) - return LaneEvidence(lane, True, None, found[0], False, None) + return LaneEvidence(lane, True, None, found[0], False, None, main_run=True) runs, comments, threads, is_human, head_file = carried left = _carried_blocking(head, runs, comments, threads, is_human, head_file) if found is None or recorded is None: @@ -436,7 +443,9 @@ def _main_lane( ) findings, critical = found blocking = max(critical, left or 0, recorded) - return LaneEvidence(lane, True, None, max(findings, blocking), False, blocking) + return LaneEvidence( + lane, True, None, max(findings, blocking), False, blocking, main_run=True + ) def _recorded_critical(lane: str, context: Mapping[str, Any] | None) -> int | None: @@ -582,7 +591,9 @@ def _full_name(repository: Any) -> Any: def lane_source(path: Any) -> bool: """Whether a changed path is one the pull request's checkout feeds its - own review; a path that is not a string counts as one.""" + own review, or the merge gate's own code the switch lane-arms + (`babysit_risk.LANE_ARMED_FLOOR`), which only a main-run review may clear + (owner decision Q5); a path that is not a string counts as one.""" if not isinstance(path, str): return True # A bare directory path (a symlink in its place) counts as the directory. @@ -591,6 +602,7 @@ def lane_source(path: Any) -> bool: f"{path}/".startswith(LANE_SOURCE_PREFIXES) or LANE_SOURCE_SEGMENT in parts or parts[-1] in LANE_SOURCE_NAMES + or any(glob_match(glob, path) for glob in LANE_ARMED_FLOOR) ) 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 27ad675b8f..60f4ae5704 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -189,6 +189,10 @@ class LaneEvidence: # by babysit_review_evidence.py); None when every one does. A # claude-review finding never holds, whatever this says. blocking_findings: int | None = None + # Bound by a main-run review of the head (babysit_review_evidence + # `_main_lane`), the only binding that lane-arms a `gate` path + # (`lane_armed`). + main_run: bool = False @dataclass(frozen=True) @@ -228,6 +232,14 @@ class RiskInput: # read-only workflow's head text (`ChangedFile.contents`) shows the # main-run review does not bind it. None holds as if it did. base_sha: str | None = None + # Unresolved review threads on the pull request, read only when a + # lane-armed path could clear (`lane_arm_pending`); None (unread) keeps + # every one with a person. + open_threads: int | None = None + # The lanes App's own pull request, the only kind ci-status holds for + # pr-merge's attestation (merge-holds.mjs laneArmedHold): any other keeps + # its lane-armed paths with a person. + lanes_app: bool = False @dataclass(frozen=True) @@ -282,7 +294,9 @@ class RiskResult: # pr-unblock stop's route (report-escalation.sh, adjudicate-feedback.mjs, # publish-feedback.mjs, apply-patch.mjs) with every module they import and # the routing table they read, so a held reason cannot be dropped unreviewed. -# A `trust` or `gate` path holds for a person (HIGH_FLOOR). An `enforcement` +# A `trust` or `gate` path holds for a person (HIGH_FLOOR), except the merge +# gate's own code `merge.lane-arm-gate` lane-arms (LANE_ARMED_FLOOR, +# `lane_armed`). 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. So # does a `gate` path on a trusted person's own pull request that only their @@ -293,8 +307,11 @@ class RiskResult: # pull request, always waits for a person. # The one table: scripts/sync-hold-floor.py writes it into merge-holds.mjs # HOLD_PATH_CLASSES, the review callers' hold-paths and ci-status's -# hold_patterns, and test_merge_holds_sync.py runs its --check. +# hold_patterns, and test_merge_holds_sync.py runs its --check. The +# floor-rows markers bound the rows pr-merge's lane-armed.mjs compares +# between base and head. FLOOR_CLASSES: tuple[tuple[str, str], ...] = ( + # BEGIN floor-rows FLOOR_CLASSES (".github/actions/check-trusted-trigger/**", "trust"), ("**/CODEOWNERS", "trust"), ("**/.gitmodules", "trust"), @@ -320,6 +337,8 @@ class RiskResult: (".github/scripts/pr-merge/post-unarmed-status.mjs", "gate"), (".github/scripts/pr-merge/verify-answers.mjs", "gate"), (".github/scripts/pr-merge/listing-carry.mjs", "gate"), + (".github/scripts/pr-merge/lane-armed.mjs", "gate"), + (".github/scripts/pr-merge/floor-ast.py", "gate"), (".github/actions/report-check-run/posting-run.mjs", "gate"), ("scripts/sync-hold-floor.py", "trust"), ("plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", "trust"), @@ -367,6 +386,9 @@ class RiskResult: (".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", "enforcement"), (".github/scripts/work-resolve-issue/stop-routes.json", "enforcement"), (".github/scripts/work-resolve-issue/stop-routes.mjs", "enforcement"), + # The review criteria pr-review-main.yml copies from main for the model. + ("plugins/review/skills/**", "enforcement"), + ("plugins/review/reference/**", "enforcement"), (".github/scripts/pr-review-main/**", "gate"), # Agent settings and instruction files, whatever their name or depth: # settings, hooks and the scripts settings registers, skills, agents, @@ -408,9 +430,97 @@ class RiskResult: (".github/workflows/work-resolve-issue.yml", "gate"), (".github/workflows/work-unblock.yml", "gate"), # END hold-floor GATE_WORKFLOWS + # END floor-rows FLOOR_CLASSES ) # fmt: skip PERSON_CLASSES = frozenset({"trust", "gate"}) +# Owner decisions Q7 and Q5 (#10743): with `merge.lane-arm-gate` on, the merge +# gate's own code (LANE_ARMED: babysit_risk.py and the pr-merge scripts) waits +# for no person here once both review lanes are bound to a main-run review of +# the head with no open finding and no review thread is open (`lane_armed`). +# ci-status still holds it until pr-merge's `pr-merge/lane-armed` attestation +# is at the head (merge-holds.mjs `laneArmedHold`). Every other `trust` and +# `gate` row is person-held (`merge.person-held`, with every gated workflow). +# Copied here by scripts/sync-hold-floor.py, whose --check fails on a stale +# copy. +# BEGIN hold-floor LANE_ARM: generated by scripts/sync-hold-floor.py +LANE_ARM_GATE = True +LANE_ARMED: tuple[str, ...] = ( + ".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/last-report.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-merge/listing-carry.mjs", + ".github/scripts/pr-merge/lane-armed.mjs", + ".github/scripts/pr-merge/floor-ast.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", +) +PERSON_HELD: 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/actions/report-check-run/posting-run.mjs", + "scripts/sync-hold-floor.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_review_evidence.py", + "docs/conventions/pr-pipeline.yaml", + ".github/scripts/pr-review-main/**", + "**/.claude/**", + "**/.claude-lane/**", + "docs/conventions/instruction-exception-register/**", + "docs/conventions/untrusted-content/**", + "**/.mcp.json", + "**/CLAUDE.md", + "**/CLAUDE.local.md", + "**/AGENTS.md", + "REVIEW.md", + "docs/standards/**", + ".github/workflows/dependabot-fragments.yml", + ".github/workflows/eval-browser-dispatch.yml", + ".github/workflows/intake-reenter.yml", + ".github/workflows/intake-triage.yml", + ".github/workflows/maintenance-audit-lane-runs.yml", + ".github/workflows/maintenance-reconcile-lost-runs.yml", + ".github/workflows/post-merge-demotion-alert.yml", + ".github/workflows/post-merge-test-mutation.yml", + ".github/workflows/pr-feedback-dispatch.yml", + ".github/workflows/pr-merge.yml", + ".github/workflows/pr-refine.yml", + ".github/workflows/pr-require-checks.yml", + ".github/workflows/pr-review-hosted.yml", + ".github/workflows/pr-review-main.yml", + ".github/workflows/pr-review-security-hosted.yml", + ".github/workflows/pr-run-activity-read.yml", + ".github/workflows/pr-run-activity-write.yml", + ".github/workflows/pr-unblock-dispatch.yml", + ".github/workflows/pr-unblock.yml", + ".github/workflows/pr-verify.yml", + ".github/workflows/release-publish-plugins.yml", + ".github/workflows/work-resolve-issue.yml", + ".github/workflows/work-unblock.yml", +) +# END hold-floor LANE_ARM +# The globs that lane-arm: none with the switch off. +LANE_ARMED_FLOOR: tuple[str, ...] = LANE_ARMED if LANE_ARM_GATE is True else () + # A secret the job token is not: any `secrets` reference but GITHUB_TOKEN # itself. The union of the reference forms (`secrets.X`, `secrets.*`, # `secrets['X']`, `toJSON(secrets)`, `secrets: inherit`) and any `secrets` @@ -846,7 +956,11 @@ def _high_path_reasons( path for f in files for path in f.paths if path and path not in inp.floor_exempt ] reviewed = _author_reviewed(inp.author_reviewed, paths, configured) + # A configured glob is never lane-armed, even when it repeats a floor row. + armed = lane_armed(inp) for pattern in dict.fromkeys((*HIGH_FLOOR, *configured)): + if armed and pattern in LANE_ARMED_FLOOR and pattern not in configured: + continue hits = [ path for path in paths @@ -880,6 +994,113 @@ def _high_path_reasons( return reasons +def lanes_clean_on_main(raw_lanes: Any) -> bool: + """Whether both review lanes (`LANES`) each have one evidence entry bound + to the head by a main-run review (`main_run`), with no unbound reason and + no open finding at all. The evidence counts only findings and CRITICAL + ones, so any open finding, a SUGGESTION included, keeps a person.""" + lanes = _sequence(raw_lanes) + if lanes is None: + return False + for name in LANES: + entries = [lane for lane in lanes if getattr(lane, "lane", None) == name] + if len(entries) != 1: + return False + lane = entries[0] + total = getattr(lane, "open_findings", None) + count = blocking(lane) + if not ( + getattr(lane, "bound", None) is True + and getattr(lane, "main_run", 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 + ): + return False + return True + + +def lane_arm_pending(inp: RiskInput) -> bool: + """Whether reading the open review threads could lane-arm `inp`: the + switch is on, a changed path matches a lane-armed `gate` glob, and both + lanes are clean on main. The caller reads them only then and scores again + with `open_threads`.""" + if ( + not LANE_ARMED_FLOOR + or inp.lanes_app is not True + or not lanes_clean_on_main(inp.lanes) + ): + return False + files = _sequence(inp.files) or () + paths = [ + p + for f in files + for p in (getattr(f, "path", None), getattr(f, "previous_path", None)) + ] + return any( + isinstance(p, str) and glob_match(g, p) for p in paths for g in LANE_ARMED_FLOOR + ) + + +def lane_armed(inp: RiskInput) -> bool: + """Whether the `LANE_ARMED_FLOOR` globs wait for no person: the switch is + on, no review thread is open (`open_threads` read as 0), and both lanes + are clean on main (`lanes_clean_on_main`). Anything unread holds.""" + threads = inp.open_threads + return ( + bool(LANE_ARMED_FLOOR) + and inp.lanes_app is True + and _is_count(threads) + and threads == 0 + and lanes_clean_on_main(inp.lanes) + ) + + +def lane_arm_record(inp: RiskInput, result: RiskResult) -> dict[str, Any] | None: + """What pr-merge's `pr-merge/lane-armed` attestation would record for + `inp`, or None when it changes no lane-armed path: the lane-armed paths, + each lane's open findings, the open threads, and `refused`, the first + condition that fails (None when the gate attests).""" + files = _sequence(inp.files) or () + paths = sorted( + { + p + for f in files + for p in (getattr(f, "path", None), getattr(f, "previous_path", None)) + if isinstance(p, str) and any(glob_match(g, p) for g in LANE_ARMED_FLOOR) + } + ) + if not paths: + return None + lanes = { + getattr(lane, "lane", None): getattr(lane, "open_findings", None) + for lane in _sequence(inp.lanes) or () + } + threads = inp.open_threads + if inp.lanes_app is not True: + refused = "it is not the lanes App's pull request" + elif not lanes_clean_on_main(inp.lanes): + refused = ( + "both review lanes are not bound to a clean main-run review of the head" + ) + elif not _is_count(threads): + refused = "the open review threads could not be read" + elif threads: + refused = f"{threads} review thread(s) open" + elif holds(result): + refused = f"the risk score holds ({result.score.name})" + else: + refused = None + return { + "paths": paths, + "lanes": {name: lanes.get(name) for name in LANES}, + "threads": threads, + "refused": refused, + } + + def _author_reviewed( raw: Any, paths: list[str], configured: Iterable[str] ) -> list[str]: diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py index cf37990ec8..18c4cfed76 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py @@ -2471,7 +2471,10 @@ def lane_risk_refusal( high_floor=config.high_path_floor, max_changed_files=config.max_changed_files, lines_optional=True, + # Only the lanes App's own pull request lane-arms (owner decision Q5). + lanes_app=json_object(pr.get("user")).get("login") == identity, ) + risk_input = babysit_merge.with_open_threads(repo, number, risk_input) risk = babysit_risk.score(risk_input) if any(r.code == babysit_risk.COST_CODE for r in risk.reasons): decision = merge_policy.cost_decision( diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py index ec8139806e..c886e434c7 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py @@ -456,6 +456,24 @@ def test_a_review_plugin_change_beside_lane_source_still_unbinds(self) -> None: for lane in LANES: self.assertUnbound(result[lane], "lane-source-changed") + def test_the_merge_gates_lane_armed_code_binds_only_on_a_main_run(self) -> None: + # Owner decision Q5: only a main-run review lane-arms it, so its + # pull_request reviews bind nothing; the rest of babysit-prs does. + for path in ( + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", + ".github/scripts/pr-merge/clear-escalations.mjs", + ".GitHub/Scripts/PR-Merge/who.mjs", + ): + with self.subTest(path): + result = self.bound("app.py", path) + for lane in LANES: + self.assertUnbound(result[lane], "lane-source-changed") + result = self.bound( + "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py" + ) + for lane in LANES: + self.assertTrue(result[lane].bound, lane) + def test_a_path_that_is_not_a_string_binds_no_lane(self) -> None: result = self.bound("app.py", None) # type: ignore[arg-type] for lane in LANES: @@ -482,6 +500,8 @@ def test_github_files_the_review_takes_from_main_bind(self) -> None: result = self.bound(path) for lane in LANES: self.assertTrue(result[lane].bound, lane) + # A pull_request-bound review never lane-arms (Q7). + self.assertIs(result[lane].main_run, False, lane) def test_unrelated_changes_bind_as_before(self) -> None: result = self.bound( @@ -1028,6 +1048,8 @@ def test_a_clean_main_run_review_of_the_head_binds_both_lanes(self) -> None: self.assertTrue(result[lane].bound, lane) self.assertIsNone(result[lane].unbound_reason) self.assertEqual(result[lane].open_findings, 0) + # Owner decision Q7: only this binding lane-arms a `gate` path. + self.assertIs(result[lane].main_run, True, lane) self.assertEqual(result["claude-security-review"].blocking_findings, 0) def test_a_verdict_on_a_diff_with_a_line_past_the_cap_does_not_bind(self) -> None: @@ -1059,6 +1081,7 @@ def test_one_lane_reviewed_from_main_binds_only_that_lane(self) -> None: result = self.main_bind([main_review("claude-review")]) self.assertTrue(result["claude-review"].bound) self.assertUnbound(result["claude-security-review"], "lane-source-changed") + self.assertIs(result["claude-security-review"].main_run, False) def test_a_review_of_another_head_or_tree_does_not_bind(self) -> None: for review in ( @@ -1068,6 +1091,7 @@ def test_a_review_of_another_head_or_tree_does_not_bind(self) -> None: with self.subTest(review["verdict"]): result = self.main_bind([review]) self.assertUnbound(result["claude-review"], "lane-source-changed") + self.assertIs(result["claude-review"].main_run, False) def test_a_run_not_from_mains_definition_does_not_bind(self) -> None: for fields in ( 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 2de170daee..60281248af 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 @@ -283,7 +283,7 @@ def test_floor_paths_are_high_whatever_the_switch(self): # generated copies hold; here the count of each class. self.assertEqual( Counter(c for _, c in babysit_risk.FLOOR_CLASSES), - {"trust": 18, "gate": 48, "enforcement": 40}, + {"trust": 18, "gate": 50, "enforcement": 42}, ) self.assertEqual( babysit_risk.HIGH_FLOOR, @@ -2074,13 +2074,34 @@ def _score(self, path, 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), 40) + self.assertEqual(len(babysit_risk.REVIEW_FLOOR), 42) for glob in babysit_risk.REVIEW_FLOOR: with self.subTest(glob): - result = self._score(glob, (_lane("claude-security-review"),)) + path = glob.replace("**", "x.md") + result = self._score(path, (_lane("claude-security-review"),)) self.assertEqual(result.score, Risk.LOW, _codes(result)) self.assertFalse(holds(result)) + def test_the_review_criteria_main_runs_copy_hold_until_a_clean_review(self): + # pr-review-main.yml copies these from main as the model's criteria: + # a change to them waits for a clean bound security review, no person. + for path, code in ( + ( + "plugins/review/skills/security-review/SKILL.md", + "review-path:plugins/review/skills/**", + ), + ( + "plugins/review/reference/severity.md", + "review-path:plugins/review/reference/**", + ), + ): + with self.subTest(path): + held = self._score(path, ()) + self.assertIn(code, _codes(held)) + self.assertFalse(person_reasons(held.reasons)) + cleared = self._score(path, (_lane("claude-security-review"),)) + self.assertFalse(holds(cleared), _codes(cleared)) + def test_an_enforcement_path_holds_without_a_clean_bound_review(self): security = "claude-security-review" cases = { diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_lane_arm.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_lane_arm.py new file mode 100644 index 0000000000..a7f8c0ebb8 --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_lane_arm.py @@ -0,0 +1,432 @@ +"""Owner decisions Q7 and Q5 (#10743): `merge.lane-arm-gate`. + +With the switch on, a pull request held only by the merge gate's own code +(babysit_risk.py and the pr-merge scripts) needs no person once both +review lanes are bound to a main-run review of the head with no open finding +and no review thread is open. A `trust` path, a person-held path and every +doubt (switch off, evidence unbound, bound by a pull_request run, a finding, +an open or unread thread count) keep the person hold. The expected outcomes +come from the decision record +(https://github.com/melodic-software/claude-code-plugins/pull/10743#issuecomment-6486023647) +and the follow-up brief, never from running the scorer. +""" + +from __future__ import annotations + +import importlib.util +import pathlib +import subprocess +import sys +import tempfile +import unittest +from dataclasses import replace +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +import babysit_merge +import babysit_risk +from babysit_risk import ( + ChangedFile, + LaneEvidence, + RiskInput, + holds, + person_reasons, + score, +) + +ROOT = pathlib.Path(__file__).resolve().parents[6] + + +def _file(path: str) -> ChangedFile: + return ChangedFile( + path=path, + previous_path=None, + status="modified", + additions=1, + deletions=1, + has_patch=True, + mode="100644", + previous_mode="100644", + patch="@@ -1 +1 @@\n-# a\n+# b", + contents=("# a\n", "# b\n"), + ) + + +def _lanes(*, main_run: bool = True, findings: int = 0, critical: int = 0): + return ( + LaneEvidence( + "claude-review", True, None, findings, False, None, main_run=main_run + ), + LaneEvidence( + "claude-security-review", + True, + None, + critical, + False, + critical, + main_run=main_run, + ), + ) + + +def _input( + *paths: str, threads: int | None = 0, lanes_app: bool = True, **lanes +) -> RiskInput: + return RiskInput( + files=tuple(_file(p) for p in paths), + files_complete=True, + max_changed_lines=None, + high_paths=(), + lanes=_lanes(**lanes), + public=True, + lines_optional=True, + max_changed_files=None, + open_threads=threads, + lanes_app=lanes_app, + ) + + +def _high(result) -> list[str]: + return [r.code for r in result.reasons if r.code.startswith("high-path:")] + + +# Owner decision Q5: the switch lane-arms the merge gate's own code alone. +SAMPLE_FLOOR = babysit_risk.LANE_ARMED_FLOOR +ARMED = ( + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", + ".github/scripts/pr-merge/clear-escalations.mjs", + ".github/scripts/pr-merge/merge-holds.mjs", + ".github/scripts/pr-merge/who.mjs", + ".github/scripts/pr-merge/lane-armed.mjs", +) +# Everything else on the person floor: a person whatever the lanes say. +PERSON = ( + ".github/workflows/pr-require-checks.yml", + ".github/workflows/pr-merge.yml", + ".github/workflows/pr-review-main.yml", + ".github/workflows/pr-review-security-hosted.yml", + ".github/scripts/pr-review-main/write-verdict.mjs", + "plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py", + "scripts/sync-hold-floor.py", + "docs/conventions/pr-pipeline.yaml", + ".github/actions/request-lane-token/action.yml", + ".github/actions/log-api-budget/rate-guard.mjs", + ".github/actions/report-check-run/posting-run.mjs", + "AGENTS.md", + ".claude/settings.json", + "REVIEW.md", + "docs/standards/README.md", +) + + +# A lane-armed sample path, the glob that names it. +WHO = ".github/scripts/pr-merge/who.mjs" + + +class TheSwitchLaneArmsGatePaths(unittest.TestCase): + def setUp(self) -> None: + patch = mock.patch.object(babysit_risk, "LANE_ARMED_FLOOR", SAMPLE_FLOOR) + patch.start() + self.addCleanup(patch.stop) + + def test_a_gate_path_with_both_lanes_clean_on_main_and_no_thread_needs_no_person( + self, + ) -> None: + for path in ARMED: + with self.subTest(path): + result = score(_input(path)) + self.assertEqual(_high(result), []) + self.assertFalse(holds(result), result.reasons) + + def test_with_the_switch_off_every_gate_path_holds_for_a_person(self) -> None: + with mock.patch.object(babysit_risk, "LANE_ARMED_FLOOR", ()): + for path in ARMED: + with self.subTest(path): + result = score(_input(path)) + self.assertEqual(len(_high(result)), 1, result.reasons) + self.assertTrue(person_reasons(result.reasons)) + + def test_each_doubt_keeps_the_person_hold(self) -> None: + cases = { + "an open review thread": _input(WHO, threads=1), + "an unread thread count": _input(WHO, threads=None), + "a malformed thread count": _input(WHO, threads=True), # type: ignore[arg-type] + "a code-review finding, any severity": _input(WHO, findings=1), + "a CRITICAL security finding": _input(WHO, critical=1), + # ci-status holds only the lanes App's pull requests for the + # attestation, so any other author's keeps the person hold. + "another author's pull request": _input(WHO, lanes_app=False), + "bound by a pull_request run, not main": _input(WHO, main_run=False), + } + for name, inp in cases.items(): + with self.subTest(name): + result = score(inp) + self.assertIn(f"high-path:{WHO}", _high(result)) + self.assertTrue(holds(result)) + unbound = replace( + _input(WHO), + lanes=( + _lanes()[0], + LaneEvidence( + "claude-security-review", False, "lane-source-changed", None + ), + ), + ) + # An IMPORTANT or SUGGESTION security finding: open but not blocking. + below = replace( + _input(WHO), + lanes=( + _lanes()[0], + LaneEvidence( + "claude-security-review", True, None, 1, False, 0, main_run=True + ), + ), + ) + missing = replace(_input(WHO), lanes=_lanes()[:1]) + doubled = replace(_input(WHO), lanes=(*_lanes(), _lanes()[0])) + for name, inp in ( + ("unbound", unbound), + ("a security finding below CRITICAL", below), + ("missing", missing), + ("doubled", doubled), + ): + with self.subTest(name): + self.assertIn(f"high-path:{WHO}", _high(score(inp))) + + def test_a_person_held_or_trust_path_holds_even_with_everything_clean(self) -> None: + for path in PERSON: + with self.subTest(path): + result = score(_input(path)) + self.assertEqual(len(_high(result)), 1, result.reasons) + self.assertTrue(person_reasons(result.reasons)) + # Beside a lane-armed path, only the person-held one is named. + result = score(_input(WHO, ".github/workflows/pr-require-checks.yml")) + self.assertEqual( + _high(result), ["high-path:.github/workflows/pr-require-checks.yml"] + ) + + def test_a_configured_denied_path_is_never_lane_armed(self) -> None: + inp = replace(_input(WHO), high_paths=(WHO,)) + self.assertEqual(_high(score(inp)), [f"high-path:{WHO}"]) + + def test_the_thread_count_is_read_only_when_it_could_lane_arm(self) -> None: + self.assertTrue(babysit_risk.lane_arm_pending(_input(WHO, threads=None))) + for name, inp in ( + ( + "a person-held path", + _input(".github/workflows/pr-merge.yml", threads=None), + ), + ("a path off the floor", _input("docs/a.md", threads=None)), + ("a finding", _input(WHO, threads=None, findings=1)), + ("not main-run", _input(WHO, threads=None, main_run=False)), + ): + with self.subTest(name): + self.assertFalse(babysit_risk.lane_arm_pending(inp)) + with mock.patch.object(babysit_risk, "LANE_ARMED_FLOOR", ()): + self.assertFalse(babysit_risk.lane_arm_pending(_input(WHO))) + + def test_the_gate_reads_the_threads_and_an_unreadable_read_keeps_the_hold( + self, + ) -> None: + pending = _input(WHO, threads=None) + for threads, expected in (([], 0), ([{"path": WHO}], 1), (None, None)): + with ( + self.subTest(threads=threads), + mock.patch.object( + babysit_merge, "unresolved_threads", return_value=threads + ) as read, + ): + self.assertEqual( + babysit_merge.with_open_threads("o/r", 1, pending).open_threads, + expected, + ) + read.assert_called_once_with("o/r", 1) + with mock.patch.object( + babysit_merge, "unresolved_threads", side_effect=RuntimeError("x") + ): + self.assertIsNone( + babysit_merge.with_open_threads("o/r", 1, pending).open_threads + ) + with mock.patch.object(babysit_merge, "unresolved_threads") as read: + person = _input(".github/workflows/pr-merge.yml", threads=None) + self.assertIs(babysit_merge.with_open_threads("o/r", 1, person), person) + read.assert_not_called() + + +class TheAttestationRecord(unittest.TestCase): + """What gate-check's `pr-merge/lane-armed` attestation records, and the + first failed condition when it must not attest (owner decision Q5).""" + + def test_a_clean_head_records_its_lane_armed_paths_and_no_refusal(self) -> None: + inp = _input(WHO, "docs/a.md") + self.assertEqual( + babysit_risk.lane_arm_record(inp, score(inp)), + { + "paths": [WHO], + "lanes": {"claude-review": 0, "claude-security-review": 0}, + "threads": 0, + "refused": None, + }, + ) + + def test_each_failed_condition_is_named(self) -> None: + for inp, refused in ( + (_input(WHO, lanes_app=False), "not the lanes App's pull request"), + (_input(WHO, main_run=False), "both review lanes are not bound"), + (_input(WHO, findings=1), "both review lanes are not bound"), + (_input(WHO, threads=None), "could not be read"), + (_input(WHO, threads=2), "2 review thread(s) open"), + ( + _input(WHO, ".github/workflows/pr-merge.yml"), + "the risk score holds (HIGH)", + ), + ): + with self.subTest(refused): + record = babysit_risk.lane_arm_record(inp, score(inp)) + self.assertIn(refused, record["refused"]) + + def test_a_head_with_no_lane_armed_path_records_nothing(self) -> None: + inp = _input("AGENTS.md") + self.assertIsNone(babysit_risk.lane_arm_record(inp, score(inp))) + with mock.patch.object(babysit_risk, "LANE_ARMED_FLOOR", ()): + inp = _input(WHO) + self.assertIsNone(babysit_risk.lane_arm_record(inp, score(inp))) + + +class TheMergeEnforcementList(unittest.TestCase): + def test_the_switch_lane_arms_exactly_the_merge_gates_own_code(self) -> None: + # Owner decision Q5: babysit_risk.py and the pr-merge scripts, and + # nothing more; every other trust and gate row is person-held. + self.assertIs(babysit_risk.LANE_ARM_GATE, True) + pr_merge = [ + g + for g, c in babysit_risk.FLOOR_CLASSES + if c in ("trust", "gate") and g.startswith(".github/scripts/pr-merge/") + ] + self.assertEqual( + sorted(babysit_risk.LANE_ARMED_FLOOR), + sorted( + [ + *pr_merge, + "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py", + ] + ), + ) + person = [g for g, c in babysit_risk.FLOOR_CLASSES if c in ("trust", "gate")] + self.assertEqual( + sorted(babysit_risk.PERSON_HELD), + sorted(g for g in person if g not in babysit_risk.LANE_ARMED_FLOOR), + ) + for path in PERSON: + with self.subTest(path): + self.assertTrue( + any( + babysit_risk.glob_match(g, path) + for g in babysit_risk.PERSON_HELD + ) + ) + self.assertFalse( + any(babysit_risk.glob_match(g, path) for g in SAMPLE_FLOOR) + ) + + def test_every_gated_workflow_is_person_held_without_being_listed(self) -> None: + on, held, armed = _sync_module().lane_arm() + self.assertIs(on, True) + self.assertEqual(tuple(held), babysit_risk.PERSON_HELD) + self.assertEqual(tuple(armed), babysit_risk.LANE_ARMED_FLOOR) + workflows = [ + g + for g, c in babysit_risk.FLOOR_CLASSES + if c == "gate" and g.startswith(".github/workflows/") + ] + self.assertGreater(len(workflows), 0) + for glob in workflows: + self.assertIn(glob, babysit_risk.PERSON_HELD, glob) + + def test_the_copies_match_pr_pipeline_yaml(self) -> None: + proc = subprocess.run( + [sys.executable, "-I", str(ROOT / "scripts/sync-hold-floor.py"), "--check"], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr) + + +def _sync_module(): + spec = importlib.util.spec_from_file_location( + "sync_hold_floor", ROOT / "scripts/sync-hold-floor.py" + ) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class TheGeneratorReadsTheSwitchFailClosed(unittest.TestCase): + """The brief: a switch absent or unreadable keeps the person hold.""" + + def lane_arm(self, merge_block: str): + sync = _sync_module() + with tempfile.TemporaryDirectory() as tmp: + path = pathlib.Path(tmp) / "pr-pipeline.yaml" + path.write_text(f"version: 2\nmerge:\n{merge_block}", encoding="utf-8") + with mock.patch.object(sync, "PIPELINE", path): + return sync.lane_arm() + + # Every `gate` row outside the merge gate's code and the workflows. + FULL = " person-held:\n" + "".join( + f' - "{g}"\n' + for g, c in babysit_risk.FLOOR_CLASSES + if c == "gate" + and not g.startswith((".github/workflows/", ".github/scripts/pr-merge/")) + ) + RISK = "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py" + + def test_only_a_literal_true_turns_it_on(self) -> None: + on, held, armed = self.lane_arm(f" lane-arm-gate: true\n{self.FULL}") + self.assertIs(on, True) + self.assertIn(".github/workflows/pr-require-checks.yml", held) + self.assertIn(self.RISK, armed) + # The shared reader unquotes a scalar, so 'true' reads as true. + for value in (None, "false", "yes", "True", "1", "on", "~"): + with self.subTest(value=value): + line = "" if value is None else f" lane-arm-gate: {value}\n" + on, held, armed = self.lane_arm(f"{line}{self.FULL}") + self.assertIs(on, False) + self.assertEqual(armed, []) + self.assertIn(self.RISK, held) + + def test_a_gate_row_outside_the_merge_gates_code_left_unlisted_stops_it( + self, + ) -> None: + short = self.FULL.replace(' - "**/AGENTS.md"\n', "") + for switch in ("true", "false"): + with self.subTest(switch=switch), self.assertRaises(SystemExit): + self.lane_arm(f" lane-arm-gate: {switch}\n{short}") + + def test_a_listed_merge_gate_row_stays_with_a_person(self) -> None: + on, held, armed = self.lane_arm( + f" lane-arm-gate: true\n{self.FULL} - {self.RISK}\n" + ) + self.assertNotIn(self.RISK, armed) + self.assertIn(self.RISK, held) + self.assertIn(".github/scripts/pr-merge/who.mjs", armed) + + def test_a_person_held_entry_off_the_floor_or_repeated_stops_it(self) -> None: + for entries in ( + ["docs/held.md"], + ["plugins/source-control/scripts/source-control-babysit-merge"], + # An `enforcement` row is not a person's. + ["plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py"], + [".github/workflows/pr-merge.yml", ".github/workflows/pr-merge.yml"], + ): + with self.subTest(entries=entries): + block = "".join(f" - {e}\n" for e in entries) + with self.assertRaises(SystemExit): + self.lane_arm(f" lane-arm-gate: true\n person-held:\n{block}") + + +if __name__ == "__main__": + unittest.main() 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 3a73f81eab..a7ddc3133a 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 @@ -15,7 +15,10 @@ `babysit_risk.FLOOR_CLASSES`, `COST_PATHS` and `merge.trust-path-hold-apps`. - The floor (owner decisions Q-FLOOR (B), Q-ENFORCE (A) and Q26 option B): `pathHold` holds, with no configured glob, exactly the paths `glob_match` - matches over `babysit_risk.HIGH_FLOOR` (its `trust` and `gate` globs), case + matches over `NATIVE_FLOOR`, `babysit_risk.HIGH_FLOOR` (its `trust` and + `gate` globs) less the merge gate's own code `merge.lane-arm-gate` + lane-arms (`babysit_risk.LANE_ARMED_FLOOR`), which ci-status holds until + pr-merge's attestation instead (`laneArmedHold`), 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` or, for the scripts that apply the @@ -25,6 +28,9 @@ match every floor entry, whatever its class. The gate's trusted-author lift (`babysit_risk.AUTHOR_REVIEW_FLOOR`) is exactly the `gate` class, and `pathHold`, which judges only lanes App pull requests, holds all of it. +- The enforcement closure: what every `trust` row and the merge-enforcement + `gate` scripts import or run sits on a `trust` or `gate` row, less the + pinned `OFF_FLOOR_ALLOWED` list awaiting the owner. - The cost paths: the same paths matched as `babysit_risk.COST_PATHS` (`glob_match` folds case and covers what sits under a glob). - The cost decision: `costDecision` accepts the same comments as `babysit_merge_policy.cost_decision`, with the owner a trusted person in @@ -63,6 +69,9 @@ ROOT = pathlib.Path(__file__).resolve().parents[6] MODULE = ROOT / ".github/scripts/pr-merge/merge-holds.mjs" +# What pathHold holds for a person: the person floor less what the switch +# lane-arms (owner decisions Q7 and Q5), which laneArmedHold holds instead. +NATIVE_FLOOR = tuple(g for g in risk.HIGH_FLOOR if g not in risk.LANE_ARMED_FLOOR) # The floor actions' entry points and the floored scripts that enforce the # floor (owner decision Q-ENFORCE (A)): every module they load is code main # runs with their verdict, so the floor holds it too. @@ -340,8 +349,8 @@ def test_the_cost_paths_are_the_same(self) -> None: def test_the_floor_holds_the_same_paths_on_both_sides(self) -> None: answer = module(FLOOR_PATHS) - self.assertEqual(len(risk.FLOOR_CLASSES), 106) - self.assertEqual(len(risk.REVIEW_FLOOR), 40) + self.assertEqual(len(risk.FLOOR_CLASSES), 110) + self.assertEqual(len(risk.REVIEW_FLOOR), 42) self.assertEqual( {kind for _, kind in risk.FLOOR_CLASSES}, {"trust", "gate", "enforcement"} ) @@ -349,7 +358,7 @@ def test_the_floor_holds_the_same_paths_on_both_sides(self) -> None: FLOOR_PATHS, answer["held"], answer["configured"], strict=True ): with self.subTest(path): - on_floor = any(risk.glob_match(g, path) for g in risk.HIGH_FLOOR) + on_floor = any(risk.glob_match(g, path) for g in NATIVE_FLOOR) self.assertEqual(held, on_floor) self.assertEqual( configured, @@ -393,17 +402,21 @@ def test_the_author_lift_is_the_gate_class_and_the_lanes_app_never_gets_it( if g not in surface and not g.startswith(".github/workflows/") ], ) - self.assertEqual(len(risk.AUTHOR_REVIEW_FLOOR), 14) + self.assertEqual(len(risk.AUTHOR_REVIEW_FLOOR), 16) samples = [g.removeprefix("**/").replace("**", "x.mjs") for g in gate] - self.assertEqual(module(samples)["held"], [True] * len(samples)) + self.assertEqual( + module(samples)["held"], + [any(risk.glob_match(g, p) for g in NATIVE_FLOOR) for p in samples], + ) step = (ROOT / ".github/workflows/pr-require-checks.yml").read_text( encoding="utf-8" ) self.assertIn("is not the lanes App's; pr-merge's gate or a person", step) def test_the_person_floor_is_the_same_list(self) -> None: - # HOLD_PATH_FLOOR (merge-holds.mjs) is babysit_risk.HIGH_FLOOR, entry - # for entry and in order: the trust and gate globs a person clears. + # HOLD_PATH_FLOOR (merge-holds.mjs) is NATIVE_FLOOR, entry for entry + # and in order: the trust and gate globs a person clears, less the + # lane-armed ones (owner decision Q5). script = ( "import { HOLD_PATH_FLOOR } from %s; console.log(JSON.stringify(HOLD_PATH_FLOOR));" % json.dumps(MODULE.as_uri()) @@ -414,8 +427,9 @@ def test_the_person_floor_is_the_same_list(self) -> None: text=True, check=True, ) - self.assertEqual(tuple(json.loads(proc.stdout)), risk.HIGH_FLOOR) - self.assertEqual(len(risk.HIGH_FLOOR), 66) + self.assertEqual(tuple(json.loads(proc.stdout)), NATIVE_FLOOR) + self.assertEqual(len(risk.HIGH_FLOOR), 68) + self.assertEqual(len(NATIVE_FLOOR), 68 - len(risk.LANE_ARMED_FLOOR)) def test_the_floor_holds_what_its_actions_load(self) -> None: files = import_closure(FLOOR_ENTRIES) @@ -432,7 +446,7 @@ def test_the_floor_holds_what_its_actions_load(self) -> None: with self.subTest(path): 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) + held, any(risk.glob_match(g, path) for g in NATIVE_FLOOR) ) def test_the_floor_holds_the_code_that_acts_on_a_stop_route(self) -> None: @@ -445,7 +459,7 @@ def test_the_floor_holds_the_code_that_acts_on_a_stop_route(self) -> None: with self.subTest(path): 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) + held, any(risk.glob_match(g, path) for g in NATIVE_FLOOR) ) def test_every_pr_merge_script_is_held_for_a_person(self) -> None: @@ -485,7 +499,9 @@ def test_the_floor_holds_the_python_that_decides_the_holds(self) -> None: "babysit_risk.py", "babysit_review_evidence.py", } - self.assertEqual(held, gate) + # babysit_risk.py is lane-armed (owner decision Q5): ci-status + # holds it until pr-merge's attestation, not as a hold path. + self.assertEqual(held, gate and name != "babysit_risk.py") self.assertEqual(path in risk.REVIEW_FLOOR, not gate) # Nothing else of that directory: the floor tracks the closure. self.assertEqual( @@ -1774,3 +1790,139 @@ def test_every_file_a_hold_path_loads_stops_the_carry(self) -> None: f"{path} is loaded by a hold path but neither held nor in CARRY_REFUSES", ) self.assertFalse(policy.ungated_since(_pushed([path]), []), path) + + +# The merge-enforcement code: every `trust` row, and the `gate` rows of the +# scripts and actions that decide or post the holds (the pr-merge scripts, +# the rate guard, the check-run poster, the main-run review's scripts). +ENFORCEMENT_DIRS = ( + ".github/scripts/pr-merge/", + ".github/actions/", + ".github/scripts/pr-review-main/", +) +# Code only: pr-pipeline.yaml names scripts a workflow runs, not this code. +CODE = (".mjs", ".js", ".py", ".sh", ".bash", "/action.yml") + + +def enforcement_roots() -> list[str]: + globs = [ + g + for g, c in risk.FLOOR_CLASSES + if c == "trust" or (c == "gate" and g.startswith(ENFORCEMENT_DIRS)) + ] + return sorted( + f + for f in tracked() + if f.endswith(CODE) + and ".test." not in f + and "/fixtures/" not in f + and not f.startswith(".github/workflows/") + and any(risk.glob_match(g, f) for g in globs) + ) + + +def enforcement_closure() -> list[str]: + """What the merge-enforcement code imports or runs: relative JS imports + and module-relative reads, the Python a module or script names, scripts an + action or shell script runs, and the babysit-prs modules Python imports.""" + in_git = tracked() + seen: set[str] = set() + todo = [ROOT / f for f in enforcement_roots()] + while todo: + file = todo.pop().resolve() + if not file.is_file(): + continue + name = file.relative_to(ROOT).as_posix() + if name in seen: + continue + seen.add(name) + text = file.read_text(encoding="utf-8") + if file.suffix in (".mjs", ".js"): + todo.extend(file.parent / spec for spec in IMPORT.findall(text)) + todo.extend(ROOT / p for p, _ in PY_RUN.findall(text) if p) + elif file.suffix == ".py": + for node in ast.walk(ast.parse(text)): + names = ( + [a.name for a in node.names] + if isinstance(node, ast.Import) + else [node.module] + if isinstance(node, ast.ImportFrom) + and not node.level + and node.module + else [] + ) + todo.extend( + d / f"{n}.py" + for n in names + for d in (file.parent, SCRIPTS) + if (d / f"{n}.py").is_file() + ) + else: + todo.extend(ROOT / p for p in py_runs(text)) + todo.extend(ROOT / s for s in RUNS.findall(text) if s in in_git) + return sorted(seen) + + +def enforcement_closure_off_floor() -> list[str]: + person = [g for g, c in risk.FLOOR_CLASSES if c in ("trust", "gate")] + return [ + f + for f in enforcement_closure() + if not any(risk.glob_match(g, f) for g in person) + ] + + +# The merge-enforcement code's closure today reaches these paths, which no +# person holds: `enforcement` rows the security review holds, and +# lib/publish-gate.mjs, on no floor row. Whether they become `gate` rows is +# the owner's decision (#10943); this pins the list, so a new escape fails and +# a fixed one must leave it. +OFF_FLOOR_ALLOWED: tuple[str, ...] = ( + ".github/scripts/pr-unblock/address-feedback.mjs", + ".github/scripts/pr-unblock/escalation-routes.json", + ".github/scripts/pr-unblock/escalation-routes.mjs", + ".github/scripts/pr-unblock/gate-source.mjs", + ".github/scripts/pr-unblock/retry-fetch.mjs", + ".github/scripts/pr-unblock/thread-scope.mjs", + ".github/scripts/work-resolve-issue/limits.mjs", + ".github/scripts/work-resolve-issue/report-outcome.mjs", + ".github/scripts/work-resolve-issue/retry-fetch.mjs", + ".github/scripts/work-resolve-issue/stop-routes.json", + ".github/scripts/work-resolve-issue/stop-routes.mjs", + "lib/publish-gate.mjs", + "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_postmerge.py", + "plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.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", +) + + +class TheEnforcementClosureStaysOnThePersonFloor(unittest.TestCase): + def test_what_the_merge_enforcement_code_loads_is_trust_or_gate(self) -> None: + # Security review of #10966: a file the merge-enforcement code imports + # or runs decides what that code does, so it needs the same person + # hold. A new escape fails here; so does a listed one that is gone. + closure = enforcement_closure() + self.assertIn(".github/scripts/pr-merge/rotation.mjs", closure) + self.assertIn(f"{_SCRIPTS_DIR}/babysit_merge_policy.py", closure) + self.assertEqual(enforcement_closure_off_floor(), list(OFF_FLOOR_ALLOWED)) + + def test_the_check_fails_on_a_loaded_path_off_the_person_floor(self) -> None: + loaded = ".github/scripts/pr-merge/rotation.mjs" + floor = tuple(g for g, _ in risk.FLOOR_CLASSES if g != loaded) + with mock.patch.object( + risk, + "FLOOR_CLASSES", + tuple((g, c) for g, c in risk.FLOOR_CLASSES if g in floor), + ): + self.assertIn(loaded, enforcement_closure_off_floor()) diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index cefbe78a09..fa2e93663e 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -488,16 +488,22 @@ for path in CODEOWNERS docs/CODEOWNERS .gitmodules .github/actions/resolve-confi .github/actions/check-trusted-trigger/.x.mjs .github/actions/request-lane-token/action.yml \ .github/actions/check-kill-switch/check-kill-switch.mjs .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/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_merge.py \ plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py docs/conventions/pr-pipeline.yaml \ .github/workflows/pr-merge.yml .github/workflows/pr-review-main.yml \ .github/scripts/pr-review-main/write-verdict.mjs; do STUB_FILES="[[\"$path\"]]" STUB_PULL="$(pull 'Closes #1')" expect "the floor holds $path" 1 3 "it changes $path, a hold path" done +# Owner decision Q5: the merge gate's own code is lane-armed, no hold path, +# and holds until pr-merge's attestation, which no stub here serves. +for path in .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/gate-check.mjs .github/scripts/pr-merge/who.mjs \ + plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py; do + STUB_FILES="[[\"$path\"]]" STUB_PULL="$(pull 'Closes #1')" + expect "the attestation holds $path" 1 3 "it changes $path, the merge gate's own code, and its attestation could not be read" +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 \ diff --git a/scripts/ci-review-lanes.test.sh b/scripts/ci-review-lanes.test.sh index 122f677dea..78bd0adff1 100755 --- a/scripts/ci-review-lanes.test.sh +++ b/scripts/ci-review-lanes.test.sh @@ -113,8 +113,10 @@ fi # An exempt author's pull request, which no review lane reads, still fails on # the trust-gate paths in bash form: `**/X` is X or */X, a trailing `**` is -# `*`. The security review's own files in hold-paths are scrutiny only. -want_patterns="$(grep -v '^plugins/review/' <<<"$first_paths" | sed -E 's#/\*\*$#/*#; s#^\*\*/(.*)$#\1\n*/\1#' | sort)" +# `*`. The security review's own files at the end of hold-paths are scrutiny +# only; the review criteria floor rows (plugins/review/skills/**, +# plugins/review/reference/**) hold like every other floor row. +want_patterns="$(grep -vxF -e 'plugins/review/skills/security-review/**' -e 'plugins/review/evals/security-review-*/**' <<<"$first_paths" | sed -E 's#/\*\*$#/*#; s#^\*\*/(.*)$#\1\n*/\1#' | sort)" got_patterns="$(sed -nE 's/^ *hold_patterns=\((.*)\)$/\1/p' "$TMP_ROOT/body.sh" | tr ' ' '\n' | tr -d "'" | sort)" if [[ -n "$got_patterns" && "$got_patterns" == "$want_patterns" ]]; then ok "the exempt-author hold patterns are the trust-gate paths" @@ -404,12 +406,13 @@ for changed in .github/standards/trusted-actors/trusted-actors.json .github/acti .github/actions/resolve-config/package-lock.json .github/actions/request-lane-token/action.yml .github/actions/check-kill-switch/check-kill-switch.mjs \ .github/scripts/pr-merge/merge-holds.mjs plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py \ .gitmodules vendor/sub/.gitmodules 'docs/new.md<.github/CODEOWNERS' \ + plugins/review/skills/security-review/SKILL.md plugins/review/reference/x.md \ .GitHub/Actions/Request-Lane-Token/action.yml vendor/sub/.GitModules docs/CodeOwners Scripts/Sync-Hold-Floor.PY; do STUB_FILES="$(files README.md "$changed")" AUTHOR_ID=300666570 expect "an exempt author changing $changed fails" 1 "::error::#77 is by an author no review lane reviews and changes trust-gate paths:" done for changed in .github/standards/other.json CODEOWNERS.md docs/gitmodules.md .github/actions/check-trusted-trigger-x/a.mjs .github/actions/check-wip-cap/action.yml .github/scripts/pr-merge/gate.mjs \ - plugins/review/skills/security-review/SKILL.md plugins/review/evals/security-review-kill-switch-narrowed/prompt.md; do + plugins/review/evals/security-review-kill-switch-narrowed/prompt.md; do STUB_FILES="$(files "$changed")" STUB_CHANGED=1 AUTHOR_ID=300666570 expect "an exempt author changing $changed passes" 0 "no trust-gate path changed" done diff --git a/scripts/sync-hold-floor.py b/scripts/sync-hold-floor.py index 412e77f211..66fc09d0a7 100755 --- a/scripts/sync-hold-floor.py +++ b/scripts/sync-hold-floor.py @@ -16,6 +16,13 @@ security review's own files (scrutiny only). - ci-status's exempt-author `hold_patterns`: the same less those two, as bash patterns (`**/X` is `X` and `*/X`; a trailing `/**` is `/*`). +- babysit_risk.py and merge-holds.mjs `LANE_ARM`: `merge.lane-arm-gate` (on only when it + reads exactly `true`), the lane-armed rows (the merge gate's own code, `lane_armable`, less + `merge.person-held`) and every other `trust` and `gate` row as person-held, read from + docs/conventions/pr-pipeline.yaml through the shared YAML reader. A person-held entry that + is not a `trust` or `gate` floor row, a repeated one, a file the reader rejects, or a `gate` + row outside the merge gate's code that is neither listed nor a gated workflow stops the + script. - `.github/trust-path-hold.json`, the whole file, which ci-workflows' pr-require-checks-trust-paths reads at the base SHA: `.github/**`, then the `hold_patterns` outside `.github/`, and `merge.trust-path-hold-apps` from pr-pipeline.yaml as `apps`. Its @@ -106,6 +113,97 @@ def hold_patterns(indent: str) -> list[str]: return [f"{indent}hold_patterns=({quoted})"] +PIPELINE = ROOT / "docs/conventions/pr-pipeline.yaml" +SWITCH_KEY = "merge.lane-arm-gate" +HELD_KEY = "merge.person-held" + + +# The merge gate's own code, the only rows the switch may lane-arm (owner +# decision Q5): babysit_risk.py and the pr-merge scripts, `trust` rows +# included. Every other `trust` row is person-held. +LANE_ARMABLE_FILE = "plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py" +LANE_ARMABLE_DIR = ".github/scripts/pr-merge/" + + +def lane_armable(glob: str) -> bool: + return glob == LANE_ARMABLE_FILE or ( + glob.startswith(LANE_ARMABLE_DIR) and "/" not in glob[len(LANE_ARMABLE_DIR) :] + ) + + +def lane_arm() -> tuple[bool, list[str], list[str]]: + """(`merge.lane-arm-gate` is exactly `true`, the person-held globs, the + lane-armed globs). The lane-armed globs are the `lane_armable` floor rows + not in `merge.person-held`, none with the switch off; the person-held + globs are every other `trust` and `gate` row. Each `merge.person-held` + entry must be a `trust` or `gate` floor row, and every `gate` row outside + `lane_armable` must be in it or be a gated workflow (`gate_workflows`), so + a workflow that gains a credential is person-held at once.""" + import babysit_merge_policy # noqa: PLC0415 + + text = PIPELINE.read_text(encoding="utf-8") + values = babysit_merge_policy._concern_values( + text, [(SWITCH_KEY, False), (HELD_KEY, True)] + ) + if values is None: + raise SystemExit(f"sync-hold-floor: the shared YAML reader rejects {PIPELINE}") + held = values[HELD_KEY] + rows = dict(floor_classes()) + bad = [g for g in held if rows.get(g) not in ("trust", "gate")] + if bad or len(set(held)) != len(held): + raise SystemExit( + f"sync-hold-floor: {HELD_KEY} holds a repeated entry or one that is not a " + f"trust or gate floor row: {bad or held}" + ) + on = values[SWITCH_KEY] == ["true"] + listed = {*held, *gate_workflows()} + loose = [ + g + for g, c in floor_classes() + if c == "gate" and not lane_armable(g) and g not in listed + ] + if loose: + raise SystemExit( + f"sync-hold-floor: {loose} are `gate` rows outside the merge gate's own " + f"code: add them to {HELD_KEY}" + ) + armed = [ + g + for g, c in floor_classes() + if on and c in ("trust", "gate") and lane_armable(g) and g not in held + ] + person = [ + g for g, c in floor_classes() if c in ("trust", "gate") and g not in armed + ] + return on, person, armed + + +def risk_lane_arm(indent: str) -> list[str]: + on, held, armed = lane_arm() + return [ + f"{indent}LANE_ARM_GATE = {on}", + f"{indent}LANE_ARMED: tuple[str, ...] = (", + *(f"{indent} {json.dumps(g)}," for g in armed), + f"{indent})", + f"{indent}PERSON_HELD: tuple[str, ...] = (", + *(f"{indent} {json.dumps(g)}," for g in held), + f"{indent})", + ] + + +def mjs_lane_arm(indent: str) -> list[str]: + on, held, armed = lane_arm() + return [ + f"{indent}export const LANE_ARM_GATE = {json.dumps(on)};", + f"{indent}export const LANE_ARMED = Object.freeze([", + *(f"{indent} {json.dumps(g)}," for g in armed), + f"{indent}]);", + f"{indent}export const PERSON_HELD = Object.freeze([", + *(f"{indent} {json.dumps(g)}," for g in held), + f"{indent}]);", + ] + + def trust_path_hold() -> str: found = subprocess.run( ["bash", str(ROOT / "lib/parse-concern-value.sh"), "--strict", "--list"] @@ -125,11 +223,14 @@ def trust_path_hold() -> str: return json.dumps({"paths": paths, "apps": apps}, indent=2) + "\n" -PIPELINE = "docs/conventions/pr-pipeline.yaml" MERGE_HOLDS = ".github/scripts/pr-merge/merge-holds.mjs" BLOCKS = { - RISK: {"GATE_WORKFLOWS": gate_workflow_rows}, - MERGE_HOLDS: {"COST_PATHS": cost_rows, "HOLD_PATH_CLASSES": class_rows}, + RISK: {"GATE_WORKFLOWS": gate_workflow_rows, "LANE_ARM": risk_lane_arm}, + MERGE_HOLDS: { + "COST_PATHS": cost_rows, + "HOLD_PATH_CLASSES": class_rows, + "LANE_ARM": mjs_lane_arm, + }, ".github/workflows/pr-review-hosted.yml": {"hold-paths": hold_paths}, ".github/workflows/pr-review-security-hosted.yml": {"hold-paths": hold_paths}, ".github/workflows/pr-require-checks.yml": {"hold_patterns": hold_patterns},