From 2731084831f61dc21b03a2af8394b7a837506eb2 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:36:43 -0400 Subject: [PATCH 1/4] feat(pr-pipeline): lift a lanes App needs-human once trusted pushes clear its cause clear-escalations.mjs now clears more pr-unblock stops without a person, from evidence comment text cannot forge (the branch activity API and check runs, read with the job token; actors matched by id against the base trusted-actors.json): - workflow-staged: also once a trusted move took the head off every resume key and the head contains main as it was at the escalation. - ci-fix-*: once a trusted move took the head off every resume key and no check but ci-status fails, runs or was cancelled at the new head. - unexpected-path-staged, conflict-patch-*: once GitHub reports the PR mergeable. Security, intent and decision stops still wait for a person. Co-Authored-By: Claude Opus 5.5 --- .../feat-lift-stale-needs-human-3b7d9c51.md | 7 + .../scripts/pr-merge/clear-escalations.mjs | 192 +++++++++++++-- .../pr-merge/clear-escalations.test.mjs | 221 +++++++++++++++++- .github/workflows/pr-merge.yml | 5 +- docs/conventions/pr-pipeline/README.md | 20 +- 5 files changed, 417 insertions(+), 28 deletions(-) create mode 100644 .changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md diff --git a/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md b/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md new file mode 100644 index 0000000000..66c408f4ad --- /dev/null +++ b/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md @@ -0,0 +1,7 @@ +--- +bump: minor +--- + +### Added + +- `clear-escalations.mjs` lifts more pr-unblock stops once their cause is gone: `workflow-staged` also clears once a trusted move took the head off the resume key and the head contains main as it was at the escalation; the `ci-fix-*` stops clear once a trusted move took the head off the resume key and no check but `ci-status` fails, runs or was cancelled at the new head; and `unexpected-path-staged` and the `conflict-patch-*` stops clear once GitHub reports the PR mergeable. A trusted move is read with the job token from the branch's activity, never from comment text: every push since the label is by a `human` of the base `trusted-actors.json` or the lanes App's entry, matched by id. Security, intent and decision stops still wait for a person. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 335dcb8aea..8167503a85 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -28,13 +28,28 @@ // - Every recorded reason no longer holds at the current head: // workflow-staged main changed nothing under .github/workflows/ // since its merge base with the head, so merging -// main into the head stages no workflow change +// main into the head stages no workflow change; +// or a trusted move (below) and the head +// contains main as it was at the escalation // unresolved-conflict, GitHub reports the PR mergeable: no conflict -// protected-path-conflict is left to resolve +// protected-path-conflict, is left to resolve +// unexpected-path-staged, +// conflict-patch-* // feedback-protected-path the PR changes nothing under .github/workflows/ +// ci-fix-* a trusted move, and at the new head no check +// but ci-status (the label fails it) fails, runs +// or was cancelled, and one passed +// A trusted move: the head is this repository's and none of the recorded +// resume keys, and every activity on its branch since the label (the +// activity API, read with the job token) is by an id trusted-actors.json +// lists as a `human`, or the lanes App's entry, the newest landing the head. +// Comment text never counts. Removing the label approves nothing: ci-status +// fails a review-lane hold an App lifted, and a hold path needs a trusted +// person's approving review. // A listing that may be cut short, or a mergeable state not yet computed, // counts as the reason still holding. -// Any other reason is decided by a person and is never cleared here. +// Any other reason is decided by a person and is never cleared here: a +// security, intent or decision stop, or one with no cause-gone evidence. // // With no pr-unblock escalation since the label, a pr-merge escalation clears // as `head-moved` instead, but only when the marked lanes App comment written @@ -83,15 +98,20 @@ // `native` re-judges enable-auto-merge's refusal // LANES_BOT_LOGIN the lanes App bot login (default // melodic-automation-lanes[bot]) +// TRUSTED_ACTORS_PATH the base trusted-actors.json (default +// .base/.github/standards/trusted-actors/trusted-actors.json) // GITHUB_API_URL // GITHUB_RUN_NUMBER where the sweep starts (rotated) // BABYSIT_VERIFY_GH_TOKEN, BABYSIT_VERIFY_LISTING, BABYSIT_VERIFY_HEAD_REFS, // BABYSIT_RUN_CACHE the gate reader's Actions token and the run's shared -// reads, as enable-auto-merge passes them +// reads, as enable-auto-merge passes them; the job +// token also reads branch activity, check runs and +// main's history for a trusted move import process from "node:process"; import { pathToFileURL } from "node:url"; -import { createGitHub, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; +import { createGitHub, loadTrustedList, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; +import { failingChecks } from "../pr-unblock/fix-ci-context.mjs"; import { humanGatedLabelFrom } from "../pr-unblock/address-feedback.mjs"; import { CLEARED, @@ -130,6 +150,12 @@ const prMergeLine = (line) => PR_MERGE_MARKERS.includes(line) || line === PR_MER const AT_HEAD = / at head `([^`]*)`/g; const HEAD_MOVED = "head-moved"; const REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; +// report-escalation.sh's fix-ci stops, each judged by ciPasses. +const CI_FIX = Object.freeze(["ci-fix-round-cap", "ci-fix-empty", "ci-fix-invalid", "ci-fix-protected-path", "ci-fix-not-applied", "ci-fix-escalated"]); +// fix-ci's aggregate check, which the human-gated label itself fails. +const AGGREGATE = "ci-status"; +// Check-run pages read at a head; a head with more stays unread. +const CHECK_PAGES = 3; const SHA = /^[0-9a-f]{40}$/; // The compare API lists at most 300 files, the PR files API at most 3000. const COMPARE_CAP = 300; @@ -169,24 +195,37 @@ function laneCause(kind, reason) { return codes.every((code) => LANE_CAUSES[kind].some((entry) => (entry.endsWith(":") ? code.startsWith(entry) : code === entry))); } +const reasonOf = ({ body }) => REASON.exec(body.join("\n"))?.[1]; const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " "); const reportError = (number, error) => console.log(`::error title=clear-escalations::#${number}: ${escapeCommand(error?.message ?? error)}`); // Why each reason no longer holds at `pull`'s head, or null while it holds. +// `moved` and `mainMerged` read with the job token (`reader`) and answer null +// on any doubt, an unreadable read included. const CHECKS = Object.freeze({ - "workflow-staged": async ({ github, repo, pull, mainSha }) => { + "workflow-staged": async (context) => { + const { github, repo, pull, mainSha } = context; const files = (await github("GET", `/repos/${repo}/compare/${pull.head.sha}...${mainSha}`))?.files; - if (!Array.isArray(files) || files.length >= COMPARE_CAP) { - return null; - } const workflow = (path) => /^\.github\/workflows\//.test(String(path ?? "")); - return files.some((file) => workflow(file.filename) || workflow(file.previous_filename)) - ? null - : `main changes nothing under .github/workflows/ since its merge base with ${pull.head.sha}`; + if (Array.isArray(files) && files.length < COMPARE_CAP && !files.some((file) => workflow(file.filename) || workflow(file.previous_filename))) { + return `main changes nothing under .github/workflows/ since its merge base with ${pull.head.sha}`; + } + // The merge the lane would not push is in place: a trusted push moved the + // head, and the head contains main as it was when the lane escalated. + const moved = await context.moved(); + const merged = moved === null ? null : await context.mainMerged("workflow-staged"); + return merged === null ? null : `${moved}, and it contains main as of the escalation (${merged})`; }, "unresolved-conflict": async ({ pull }) => mergeable(pull), "protected-path-conflict": async ({ pull }) => mergeable(pull), + // The lane's own conflict resolution was refused; with no conflict left + // there is nothing to resolve. + "unexpected-path-staged": async ({ pull }) => mergeable(pull), + "conflict-patch-invalid": async ({ pull }) => mergeable(pull), + "conflict-patch-not-applied": async ({ pull }) => mergeable(pull), + "conflict-patch-protected-path": async ({ pull }) => mergeable(pull), + ...Object.fromEntries(CI_FIX.map((reason) => [reason, ciPasses])), "feedback-protected-path": async ({ github, repo, pull }) => { const files = await paginate(github, `/repos/${repo}/pulls/${pull.number}/files`); return files.length >= FILES_CAP || files.some((file) => isProtectedPath(file.filename) || isProtectedPath(file.previous_filename ?? "")) @@ -195,6 +234,88 @@ const CHECKS = Object.freeze({ }, }); +// A ci-fix stop: the lane could not fix CI at its resume key. Gone once a +// trusted push moved the head and, at the new head, no check fails, runs or +// was cancelled (fix-ci's failingChecks over every run, so a later skip hides +// no failure), and one passed. ci-status is left out: the label fails it. +async function ciPasses(context) { + const moved = await context.moved(); + if (moved === null) { + return null; + } + const head = context.pull.head.sha; + const runs = await context.read(async (reader) => { + const all = []; + for (let page = 1; page <= CHECK_PAGES; page += 1) { + const batch = (await reader("GET", `/repos/${context.repo}/commits/${head}/check-runs?filter=all&per_page=100&page=${page}`))?.check_runs; + if (!Array.isArray(batch)) { + return null; + } + all.push(...batch); + if (batch.length < 100) { + return all; + } + } + return null; + }); + const others = (runs ?? []).filter((run) => run?.name !== AGGREGATE); + if ( + runs === null || + failingChecks(others).length > 0 || + others.some((run) => run.status !== "completed") || + !others.some((run) => run.conclusion === "success") + ) { + return null; + } + return `${moved}, and no check other than ${AGGREGATE} fails, runs or was cancelled at it`; +} + +// Whether trusted pushes alone moved `pull`'s head off every resume key since +// the label went on: the head is this repository's (the activity API shows no +// fork push), its newest push since `since` landed the head, and every push +// or other activity on the branch since then is by an id `trusted` holds (a +// `human` of trusted-actors.json, or the lanes App's own entry). The page must +// reach back past `since`, or the move stays unread. A description, or null. +async function trustedMove({ read, repo, pull, keys, since, trusted }) { + const head = pull.head.sha; + const ref = String(pull.head?.ref ?? ""); + if (trusted === null || keys.includes(head) || pull.head?.repo?.full_name !== repo || ref === "") { + return null; + } + const activity = await read((reader) => reader("GET", `/repos/${repo}/activity?ref=${encodeURIComponent(`refs/heads/${ref}`)}&per_page=100`)); + if (!Array.isArray(activity)) { + return null; + } + const at = (item) => Date.parse(String(item?.timestamp)); + const recent = activity.filter((item) => !(at(item) <= since)).sort((a, b) => at(b) - at(a)); + if (recent.length === 0 || recent.length === activity.length || recent[0].after !== head) { + return null; + } + if (recent.some((item) => !trusted.has(item?.actor?.id))) { + return null; + } + const by = [...new Set(recent.map((item) => item.actor.login))].join(", "); + return `${by} moved the head from the resume key to ${head}`; +} + +// main's tip when the newest `reason` escalation was written, when the head +// contains it (the compare from it to the head is behind by none), or null. +async function mainMergedAt({ read, repo, pull, escalations }, reason) { + const times = escalations.filter((item) => reasonOf(item) === reason).map((item) => item.at); + if (times.length === 0 || !times.every(Number.isFinite)) { + return null; + } + const until = new Date(Math.max(...times)).toISOString(); + return read(async (reader) => { + const then = (await reader("GET", `/repos/${repo}/commits?sha=main&until=${encodeURIComponent(until)}&per_page=1`))?.[0]?.sha; + if (!SHA.test(String(then))) { + return null; + } + const compare = await reader("GET", `/repos/${repo}/compare/${then}...${pull.head.sha}`); + return compare?.behind_by === 0 && ["ahead", "identical"].includes(compare?.status) ? then : null; + }); +} + function mergeable(pull) { return pull.mergeable === true && pull.mergeable_state !== "dirty" ? "GitHub reports no conflict with main" : null; } @@ -280,7 +401,7 @@ function pairedWithPrMerge(appComments, since) { } // What to clear on `number`, or { reason } saying why nothing is. -async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, verify, reads }) { +async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, verify, reads, reader, trusted }) { const timeline = await paginate(github, `/repos/${repo}/issues/${number}/timeline`); const { labeled, since, lifted } = labelHistory(timeline, label); if (!isApp(labeled?.actor, bot)) { @@ -302,7 +423,7 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver if (escalations.length === 0 || !(escalations[0].at - since <= PAIRED_MS)) { return { reason: `no pr-unblock escalation by the lanes App right after it added ${label}` }; } - reasons = [...new Set(escalations.map(({ body }) => REASON.exec(body.join("\n"))?.[1]))]; + reasons = [...new Set(escalations.map(reasonOf))]; const decided = reasons.find((reason) => !Object.hasOwn(CHECKS, reason ?? "")); if (reasons.length === 0 || decided !== undefined || reasons.includes(undefined)) { return { reason: `an escalation reason since the label (${decided ?? "none"}) is one a person decides` }; @@ -370,8 +491,26 @@ async function verdict({ github, repo, number, label, bot, mainSha, lanePrs, ver reasons = [HEAD_MOVED]; why.push(`its escalation names head \`${[...new Set(merged.map(({ head }) => head))].join("`, `")}\`, and the head is now \`${pull.head.sha}\``); } else { + // A job-token read that fails keeps the label and leaves the run green: + // it is retried on the next schedule. + const read = async (fn) => { + if (reader === null) { + return null; + } + try { + return await fn(reader); + } catch (error) { + console.log(`::warning title=clear-escalations::#${number}: ${escapeCommand(error?.message ?? error)}`); + return null; + } + }; + const keys = escalations.map(({ body }) => RESUME_KEY.exec(body[1])?.[1]); + let movedOnce; + const context = { github, repo, pull, mainSha, read, escalations }; + context.moved = () => (movedOnce ??= trustedMove({ read, repo, pull, keys, since, trusted })); + context.mainMerged = (reason) => mainMergedAt(context, reason); for (const reason of reasons) { - const cleared = await CHECKS[reason]({ github, repo, pull, mainSha }); + const cleared = await CHECKS[reason](context); if (cleared === null) { return { reason: `${reason} still holds at ${pull.head.sha}` }; } @@ -391,7 +530,21 @@ function cleared(timeline, bot, since, reasons, head, why) { return { reason: null, recorded, head, why: why.join("; "), explained }; } -export async function main({ env = process.env, github, verify } = {}) { +// The ids whose push is a trusted move: every `human` of trusted-actors.json +// and the lanes App's own entry (`bot`), matched by id. Null when the list +// does not read. +function trustedIds(path, bot) { + try { + const { humans, logins } = loadTrustedList(path); + const app = logins.get(bot.toLowerCase()); + return new Set(app === undefined ? humans : [...humans, app]); + } catch (error) { + console.log(`clear-escalations: no stop clears on a trusted move: ${escapeCommand(error?.message ?? error)}`); + return null; + } +} + +export async function main({ env = process.env, github, verify, reader } = {}) { try { if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) { throw new Error("GITHUB_REPOSITORY is not owner/repo"); @@ -406,6 +559,11 @@ export async function main({ env = process.env, github, verify } = {}) { github ??= createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }); verify ??= pythonVerify(env); const reads = { left: NATIVE_READS_PER_RUN }; + // The job's own token reads branch activity, check runs and main's + // history; without it, or without the trusted list, no stop clears on a + // trusted move. + reader = env.BABYSIT_VERIFY_GH_TOKEN ? (reader ?? createGitHub({ token: env.BABYSIT_VERIFY_GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined })) : null; + const trusted = trustedIds(env.TRUSTED_ACTORS_PATH || ".base/.github/standards/trusted-actors/trusted-actors.json", bot); const tip = await github("GET", `/repos/${repo}/git/ref/heads/main`); const mainSha = tip?.ref === "refs/heads/main" ? tip.object?.sha : undefined; if (!SHA.test(String(mainSha))) { @@ -422,7 +580,7 @@ export async function main({ env = process.env, github, verify } = {}) { continue; } try { - const result = await verdict({ github, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS, verify, reads }); + const result = await verdict({ github, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS, verify, reads, reader, trusted }); 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 542a3c9a83..34eba32df6 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -7,7 +7,7 @@ // owner decision on #8387, a bot account speaking after the label (AI review, // another lane) does not keep it. import assert from "node:assert/strict"; -import { test } from "node:test"; +import { after, test } from "node:test"; import { main, NATIVE_READS_PER_RUN } from "./clear-escalations.mjs"; import { refusalBody } from "./enable-auto-merge.mjs"; @@ -32,7 +32,7 @@ const escalated = (reason = "workflow-staged", { user = BOT, at = "2026-10-18T10 // One open pull request (number 7) labeled needs-human; `timeline` is its timeline and // `mainFiles` the files main changed since its merge base with the head. // `evidence` is what the gate's reader (enable-auto-merge pythonVerify) answers. -function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author, evidence = () => null, numbers = [7] }) { +function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author, evidence = () => null, numbers = [7], headRepo = REPO, reader }) { const calls = []; const reads = []; const verify = async (pull, rung) => { @@ -53,7 +53,7 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = return path.endsWith("page=1") ? timeline : []; } if (method === "GET" && kind === "pulls" && numbers.includes(Number(number))) { - return { number: Number(number), state: "open", user: author, base: { ref: "main", sha: MAIN }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable }; + return { number: Number(number), state: "open", user: author, base: { ref: "main", sha: MAIN }, head: { sha: HEAD, ref: "feat/x", repo: { full_name: headRepo } }, labels: [{ name: "needs-human" }], mergeable }; } if (method === "GET" && route === `/repos/${REPO}/compare/${HEAD}...${MAIN}`) { return { files: mainFiles }; @@ -66,7 +66,7 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = } throw new Error(`unexpected ${method} ${path}`); }; - return { github, verify, reads, calls, writes: () => calls.filter((call) => call.method !== "GET") }; + return { github, verify, reader, reads, calls, writes: () => calls.filter((call) => call.method !== "GET") }; } const run = async (w, env = {}) => { @@ -74,7 +74,7 @@ const run = async (w, env = {}) => { const original = console.log; console.log = (line) => logs.push(line); try { - return { code: await main({ env: { ...ENV, ...env }, github: w.github, verify: w.verify }), logs }; + return { code: await main({ env: { ...ENV, ...env }, github: w.github, verify: w.verify, reader: w.reader }), logs }; } finally { console.log = original; } @@ -785,3 +785,214 @@ test("labels past the read cap are read on a later run", async () => { } assert.deepEqual([...read].sort((a, b) => a - b), numbers); }); + +// A trusted move: the head left every resume key through pushes the base +// trusted-actors.json trusts (a `human` entry, or the lanes App's own entry), +// read with the job token (`reader`). Expected values come from the owner rule +// of 2026-10-26: a person is needed only for intent, design or a HIGH/CRITICAL +// security finding, so a mechanical stop comes off once trusted evidence shows +// its cause gone. +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const PERSON_ID = 153232337; +const APP_ID = 337595936; +const STRANGER = { login: "drive-by", id: 99, type: "User" }; +const PERSON = { login: "kyle-sexton", id: PERSON_ID, type: "User" }; +const APP_ACTOR = { login: BOT.login, id: APP_ID, type: "Bot" }; +const MAIN_THEN = "d".repeat(40); +const TRUSTED_DIR = mkdtempSync(join(tmpdir(), "clear-escalations-")); +after(() => rmSync(TRUSTED_DIR, { recursive: true, force: true })); +const TRUSTED = join(TRUSTED_DIR, "trusted-actors.json"); +writeFileSync( + TRUSTED, + JSON.stringify({ + version: 1, + actors: [ + { id: PERSON_ID, login: "kyle-sexton", kind: "human" }, + { id: APP_ID, login: BOT.login, kind: "bot" }, + { id: 209825114, login: "claude[bot]", kind: "bot" }, + ], + }), +); +const TRUST_ENV = { TRUSTED_ACTORS_PATH: TRUSTED, BABYSIT_VERIFY_GH_TOKEN: "job-token" }; + +// The branch's activity, newest first: created before the label, then `pushes` +// ({ actor, after, at }) after it. +const activityOf = (pushes) => [ + ...pushes.map(({ actor, after, at }) => ({ activity_type: "push", actor, after, before: KEY, timestamp: at })).reverse(), + { activity_type: "branch_creation", actor: PERSON, after: KEY, before: "0".repeat(40), timestamp: "2026-10-17T09:00:00Z" }, +]; +const pushedBy = (actor, after = HEAD, at = "2026-10-18T11:00:00Z") => ({ actor, after, at }); +const run_ = (name, conclusion, id, status = "completed") => ({ id, name, status, conclusion, app: { slug: "github-actions" } }); +const GREEN = [run_("ci-status", "failure", 1), run_("test-node", "success", 2), run_("lint-repo", "skipped", 3)]; + +// The job-token reads: branch activity, main's tip when the lane escalated, +// whether the head contains it, and the head's check runs. +function readerFor({ pushes = [pushedBy(PERSON)], contains = true, checkRuns = GREEN, calls = [] } = {}) { + return async (method, path) => { + calls.push(path); + if (method !== "GET") { + throw new Error(`the reader writes nothing: ${method} ${path}`); + } + if (path === `/repos/${REPO}/activity?ref=${encodeURIComponent("refs/heads/feat/x")}&per_page=100`) { + return activityOf(pushes); + } + if (path === `/repos/${REPO}/commits?sha=main&until=${encodeURIComponent("2026-10-18T10:00:05.000Z")}&per_page=1`) { + return [{ sha: MAIN_THEN }]; + } + if (path === `/repos/${REPO}/compare/${MAIN_THEN}...${HEAD}`) { + return contains ? { status: "ahead", behind_by: 0 } : { status: "diverged", behind_by: 3 }; + } + if (path.startsWith(`/repos/${REPO}/commits/${HEAD}/check-runs?filter=all&per_page=100&page=`)) { + return { check_runs: path.endsWith("page=1") ? checkRuns : [] }; + } + throw new Error(`unexpected reader ${method} ${path}`); + }; +} + +const WORKFLOW_MAIN = [{ filename: ".github/workflows/pr-merge.yml" }]; +const stagedTimeline = () => [labeled(BOT, "2026-10-18T10:00:00Z"), escalated("workflow-staged")]; + +test("workflow-staged comes off once a trusted person moved the head past the resume key with main merged in, though main still changes workflows", async () => { + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: readerFor() }); + await run(w, TRUST_ENV); + const [comment, removal] = w.writes(); + assert.equal(w.writes().length, 2); + assert.equal(comment.body.body.split("\n")[0], ``); + assert.match(comment.body.body, /kyle-sexton/); + assert.match(comment.body.body, new RegExp(MAIN_THEN)); + assert.equal(removal.method, "DELETE"); +}); + +test("a lanes App push counts as a trusted move", async () => { + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: readerFor({ pushes: [pushedBy(APP_ACTOR)] }) }); + await run(w, TRUST_ENV); + assert.equal(w.writes().at(-1)?.method, "DELETE"); +}); + +test("workflow-staged stays when any push since the label is by an account off the trusted list", async () => { + for (const pushes of [ + [pushedBy(STRANGER)], + [pushedBy(STRANGER, "e".repeat(40), "2026-10-18T10:30:00Z"), pushedBy(PERSON)], + [pushedBy({ login: "claude[bot]", id: 209825114, type: "Bot" })], + [pushedBy({ login: BOT.login, id: 1, type: "Bot" })], + ]) { + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: readerFor({ pushes }) }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), [], JSON.stringify(pushes)); + } +}); + +test("workflow-staged stays when the head does not contain main as it was at the escalation", async () => { + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: readerFor({ contains: false }) }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), []); +}); + +test("workflow-staged stays when the newest push is not the head, or no push since the label is read", async () => { + for (const pushes of [[pushedBy(PERSON, "e".repeat(40))], []]) { + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: readerFor({ pushes }) }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), []); + } +}); + +test("workflow-staged stays when the activity page does not reach back to the label", async () => { + const pushes = Array.from({ length: 100 }, (_, index) => pushedBy(PERSON, index === 99 ? HEAD : "e".repeat(40), new Date(Date.parse("2026-10-18T11:00:00Z") + index * 60_000).toISOString())); + const reader = async (method, path) => + path.includes("/activity?") ? pushes.map(({ actor, after, at }) => ({ activity_type: "push", actor, after, timestamp: at })) : readerFor()(method, path); + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), []); +}); + +test("workflow-staged stays at the resume key, on a fork head, and without the trusted list or the job token", async () => { + const atKey = world({ + timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), commented(BOT, escalationBody("workflow-staged").replaceAll(KEY, HEAD), "2026-10-18T10:00:05Z")], + mainFiles: WORKFLOW_MAIN, + reader: readerFor(), + }); + await run(atKey, TRUST_ENV); + assert.deepEqual(atKey.writes(), []); + const fork = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, headRepo: "someone/fork", reader: readerFor() }); + await run(fork, TRUST_ENV); + assert.deepEqual(fork.writes(), []); + for (const env of [{ BABYSIT_VERIFY_GH_TOKEN: "job-token", TRUSTED_ACTORS_PATH: "/nonexistent/trusted-actors.json" }, { TRUSTED_ACTORS_PATH: TRUSTED }]) { + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: env.BABYSIT_VERIFY_GH_TOKEN ? readerFor() : undefined }); + await run(w, env); + assert.deepEqual(w.writes(), []); + } +}); + +test("an unreadable job-token read keeps the label without failing the run", async () => { + const reader = async () => { + throw new Error("HTTP 502"); + }; + const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader }); + const { code } = await run(w, TRUST_ENV); + assert.equal(code, 0); + assert.deepEqual(w.writes(), []); +}); + +const CI_FIX = ["ci-fix-round-cap", "ci-fix-empty", "ci-fix-invalid", "ci-fix-protected-path", "ci-fix-not-applied", "ci-fix-escalated"]; + +test("a ci-fix stop comes off once a trusted push moved the head and no check but ci-status fails there", async () => { + for (const reason of CI_FIX) { + const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)], reader: readerFor() }); + await run(w, TRUST_ENV); + assert.equal(w.writes().length, 2, reason); + assert.match(w.writes()[0].body.body, new RegExp(`reason=${reason} head=${HEAD}`)); + assert.match(w.writes()[0].body.body, /no check other than ci-status fails/); + } +}); + +test("a ci-fix stop stays while a check fails, runs, or none has passed at the head, or the head never moved", async () => { + for (const checkRuns of [ + [...GREEN, run_("test-bash (0)", "failure", 4)], + [...GREEN, run_("test-bash (0)", null, 4, "in_progress")], + [...GREEN, run_("test-python", "cancelled", 4)], + [run_("test-node", "failure", 2), run_("test-node", "skipped", 9)], + [run_("ci-status", "failure", 1), run_("lint-repo", "skipped", 3)], + [], + ]) { + const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated("ci-fix-escalated")], reader: readerFor({ checkRuns }) }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), [], JSON.stringify(checkRuns)); + } + const unmoved = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated("ci-fix-escalated")], reader: readerFor({ pushes: [pushedBy(STRANGER)] }) }); + await run(unmoved, TRUST_ENV); + assert.deepEqual(unmoved.writes(), []); +}); + +test("a stop on the lane's own conflict patch comes off once GitHub reports the PR mergeable", async () => { + for (const reason of ["unexpected-path-staged", "conflict-patch-invalid", "conflict-patch-not-applied", "conflict-patch-protected-path"]) { + const timeline = [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)]; + const conflicted = world({ timeline, mergeable: false }); + await run(conflicted); + assert.deepEqual(conflicted.writes(), [], reason); + const clean = world({ timeline, mergeable: true }); + await run(clean); + assert.equal(clean.writes().at(-1)?.method, "DELETE", reason); + } +}); + +test("a security, intent or decision stop is never lifted, even after a trusted move with everything green", async () => { + for (const reason of [ + "feedback-uncertain", + "feedback-disputed", + "feedback-unadjudicated", + "feedback-question-intent", + "feedback-question-design", + "feedback-question-security", + "feedback-round-cap", + "dependency-security-close", + "dependency-escalate", + "repeat-cap", + ]) { + const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)], mergeable: true, reader: readerFor() }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), [], reason); + } +}); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 2675d5a9bd..da38945a6f 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1421,7 +1421,9 @@ jobs: # A pr-unblock escalation adds the human-gated label with a reason. On # the schedule, this takes off a label the lanes App set once that # reason no longer holds at the PR's head (workflow-staged, a conflict - # reason, feedback-protected-path), after a comment naming the reason + # or conflict-patch reason, feedback-protected-path, a ci-fix stop once + # trusted pushes moved the head and its checks pass, read with this + # job's own token and the base trusted-actor list), after a comment naming the reason # and the head: never a label a person set, and never after anyone but # the App commented or reviewed since it went on. At most five a run, and not # while the lane is report-only or after the gate's rate-limit stop. @@ -1447,6 +1449,7 @@ jobs: API_BUDGET_OVERRIDE_TOKEN: github-token KILL_SWITCH: ${{ steps.kill.outputs.proceed == 'true' && 'proceed' || '' }} LANE_PRS: ${{ steps.mode.outputs.lane-prs }} + TRUSTED_ACTORS_PATH: .base/.github/standards/trusted-actors/trusted-actors.json run: node .base/.github/scripts/pr-merge/clear-escalations.mjs - name: Log the API budget at job end diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 0ecd99d87e..6fa858e199 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -534,8 +534,9 @@ head: | Reason | Cleared when | |---|---| -| `workflow-staged` | main changes nothing under `.github/workflows/` since its merge base with the head | -| `unresolved-conflict`, `protected-path-conflict` | GitHub reports the PR mergeable | +| `workflow-staged` | main changes nothing under `.github/workflows/` since its merge base with the head; or a trusted move (below) took the head off the resume key and the head contains main as it was when the lane escalated | +| `unresolved-conflict`, `protected-path-conflict`, `unexpected-path-staged`, `conflict-patch-invalid`, `conflict-patch-not-applied`, `conflict-patch-protected-path` | GitHub reports the PR mergeable | +| `ci-fix-round-cap`, `ci-fix-empty`, `ci-fix-invalid`, `ci-fix-protected-path`, `ci-fix-not-applied`, `ci-fix-escalated` | a trusted move took the head off the resume key, and at the new head no check other than `ci-status` (which the label itself fails) fails, runs or was cancelled, and one passed | | `feedback-protected-path` | the PR changes nothing under `.github/workflows/` | | `head-moved` (pr-merge's `babysit-merge-verify-escalation`, `babysit-merge-risk` and `babysit-merge-shared-copy-escalation`, with no pr-unblock escalation since the label, and the marked App comment written last before the label is pr-merge's and within a minute of it) | the newest App comment carrying each marker names, as ``at head `` ``, a 40-hex head other than the current one; pr-merge then judges the new head and escalates again if its refusal still holds | | `refusal-cleared` (under `merge.lane-prs: native`, `enable-auto-merge`'s own unedited person refusal comment on a lanes App pull request, naming the current head, whose recorded verifier or risk cause is one the lanes clear: every code on `LANE_CAUSES`) | the gate's reader at that head finds nothing refusing, or only causes the lanes clear, read at least as far as the recorded one. A recorded cause a person clears (an issue link, a security finding, a trust-gate widening, a cost hold, a high path, an untrusted spec, the repeated-fail cap, any code off that list) is never read or lifted at its head (owner decision Q-CLEAR B): a person removes the label, or the `head-moved` clear does once the head moves. At most three reader runs a run | @@ -544,9 +545,18 @@ It removes only a label whose newest `labeled` event has the lanes App as its ac when a person commented, reviewed or left a review comment since that event (a bot account, such as an AI review or another lane, does not count), or when an App comment written or edited since then carries any marker other than the escalation being judged (another -`babysit-merge-*` marker, another lane's). Every other reason -(`unexpected-path-staged`, the round and repeat caps, and the `feedback-*`, `dependency-*` and -`ci-fix-*` verdicts) waits for a person. Rules: `.github/scripts/pr-merge/clear-escalations.mjs`. +`babysit-merge-*` marker, another lane's). A trusted move is read with the job's own token, never +from comment text: the PR head is in this repository, every push on its branch since the label went +on is by a `human` of the base `trusted-actors.json` or the lanes App's entry there (matched by +id), the newest one landed the current head, and the head is none of the recorded resume keys. A +read that fails keeps the label. Every other reason waits for a person: the security, intent and +decision stops (`feedback-uncertain`, `feedback-disputed`, `feedback-unadjudicated`, +`feedback-question-*`, `feedback-round-cap`, `feedback-not-converging`, `dependency-security-close`, +`dependency-escalate`), and those with no cause-gone evidence yet (`repeat-cap`, +`feedback-not-landed`, `feedback-invalid`, `feedback-partial`, `feedback-incomplete`, and the other +`dependency-*` stops). Removing the label this way approves nothing: `ci-status` still fails a +review-lane hold the lanes App unlabeled, and hold paths still need a trusted person's approving +review. Rules: `.github/scripts/pr-merge/clear-escalations.mjs`. Accepted risk: a person's request relayed through an App posts as a Bot account (`claude[bot]` replying to an `@claude` mention that asks to wait), and so does a third-party App writing "needs From 97de5c13356f9bb4c8bb97ac4141fe7071252aa0 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:43:20 -0400 Subject: [PATCH 2/4] fix(pr-pipeline): lift only the owner-approved stops and floor the new imports Narrow clear-escalations' new lifts to the set the owner approved: workflow-staged and the ci-fix-* stops. unexpected-path-staged and the conflict-patch-* stops wait for a person again; a test pins that they stay even once the PR is mergeable after a trusted move. clear-escalations.mjs now loads pr-unblock/fix-ci-context.mjs and, through it, maintenance-reconcile-lost-runs/infra-failure.mjs. Add both to the path floor as enforcement paths, like the other pr-unblock modules it loads, in babysit_risk.FLOOR_CLASSES, merge-holds.mjs HOLD_PATH_CLASSES, the review callers' hold-paths and ci-status's hold_patterns, so the floor-closure test passes. Co-Authored-By: Claude Opus 5.5 --- .../feat-lift-stale-needs-human-3b7d9c51.md | 2 +- .../feat-lift-stale-needs-human-floor-3b7d9c51.md | 7 +++++++ .github/scripts/pr-merge/clear-escalations.mjs | 10 +--------- .github/scripts/pr-merge/clear-escalations.test.mjs | 12 ++++-------- .github/scripts/pr-merge/merge-holds.mjs | 2 ++ .github/scripts/pr-merge/merge-holds.test.mjs | 2 ++ .github/workflows/pr-require-checks.yml | 2 +- .github/workflows/pr-review-hosted.yml | 2 ++ .github/workflows/pr-review-security-hosted.yml | 2 ++ docs/conventions/pr-pipeline/README.md | 5 +++-- .../skills/babysit-prs/scripts/babysit_risk.py | 2 ++ .../babysit-prs/scripts/tests/test_babysit_risk.py | 6 +++++- .../scripts/tests/test_merge_holds_sync.py | 4 ++-- scripts/ci-lanes-pr-hold.test.sh | 3 ++- 14 files changed, 36 insertions(+), 25 deletions(-) create mode 100644 .changes/source-control/feat-lift-stale-needs-human-floor-3b7d9c51.md diff --git a/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md b/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md index 66c408f4ad..4a62ef8bf8 100644 --- a/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md +++ b/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md @@ -4,4 +4,4 @@ bump: minor ### Added -- `clear-escalations.mjs` lifts more pr-unblock stops once their cause is gone: `workflow-staged` also clears once a trusted move took the head off the resume key and the head contains main as it was at the escalation; the `ci-fix-*` stops clear once a trusted move took the head off the resume key and no check but `ci-status` fails, runs or was cancelled at the new head; and `unexpected-path-staged` and the `conflict-patch-*` stops clear once GitHub reports the PR mergeable. A trusted move is read with the job token from the branch's activity, never from comment text: every push since the label is by a `human` of the base `trusted-actors.json` or the lanes App's entry, matched by id. Security, intent and decision stops still wait for a person. +- `clear-escalations.mjs` lifts more pr-unblock stops once their cause is gone: `workflow-staged` also clears once a trusted move took the head off the resume key and the head contains main as it was at the escalation; the `ci-fix-*` stops clear once a trusted move took the head off the resume key and no check but `ci-status` fails, runs or was cancelled at the new head. A trusted move is read with the job token from the branch's activity, never from comment text: every push since the label is by a `human` of the base `trusted-actors.json` or the lanes App's entry, matched by id. Security, intent and decision stops still wait for a person. diff --git a/.changes/source-control/feat-lift-stale-needs-human-floor-3b7d9c51.md b/.changes/source-control/feat-lift-stale-needs-human-floor-3b7d9c51.md new file mode 100644 index 0000000000..0014457381 --- /dev/null +++ b/.changes/source-control/feat-lift-stale-needs-human-floor-3b7d9c51.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Changed + +- The merge gate's path floor (`babysit_risk.FLOOR_CLASSES`) holds `.github/scripts/pr-unblock/fix-ci-context.mjs` and `.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs` as `enforcement` paths, since `clear-escalations.mjs` now loads them: a change to either waits for a clean security review bound to the head. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 3b5d022f85..ede8b3876b 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -32,9 +32,7 @@ // or a trusted move (below) and the head // contains main as it was at the escalation // unresolved-conflict, GitHub reports the PR mergeable: no conflict -// protected-path-conflict, is left to resolve -// unexpected-path-staged, -// conflict-patch-* +// protected-path-conflict is left to resolve // feedback-protected-path the PR changes nothing under .github/workflows/ // ci-fix-* a trusted move, and at the new head no check // but ci-status (the label fails it) fails, runs @@ -219,12 +217,6 @@ const CHECKS = Object.freeze({ }, "unresolved-conflict": async ({ pull }) => mergeable(pull), "protected-path-conflict": async ({ pull }) => mergeable(pull), - // The lane's own conflict resolution was refused; with no conflict left - // there is nothing to resolve. - "unexpected-path-staged": async ({ pull }) => mergeable(pull), - "conflict-patch-invalid": async ({ pull }) => mergeable(pull), - "conflict-patch-not-applied": async ({ pull }) => mergeable(pull), - "conflict-patch-protected-path": async ({ pull }) => mergeable(pull), ...Object.fromEntries(CI_FIX.map((reason) => [reason, ciPasses])), "feedback-protected-path": async ({ github, repo, pull }) => { const files = await paginate(github, `/repos/${repo}/pulls/${pull.number}/files`); diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 34eba32df6..af606b1a23 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -966,15 +966,11 @@ test("a ci-fix stop stays while a check fails, runs, or none has passed at the h assert.deepEqual(unmoved.writes(), []); }); -test("a stop on the lane's own conflict patch comes off once GitHub reports the PR mergeable", async () => { +test("a stop on the lane's own conflict patch stays, even once the PR is mergeable after a trusted move", async () => { for (const reason of ["unexpected-path-staged", "conflict-patch-invalid", "conflict-patch-not-applied", "conflict-patch-protected-path"]) { - const timeline = [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)]; - const conflicted = world({ timeline, mergeable: false }); - await run(conflicted); - assert.deepEqual(conflicted.writes(), [], reason); - const clean = world({ timeline, mergeable: true }); - await run(clean); - assert.equal(clean.writes().at(-1)?.method, "DELETE", reason); + const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)], mergeable: true, reader: readerFor() }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), [], reason); } }); diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index 16a1d73231..f8cb78499c 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -300,6 +300,8 @@ export const HOLD_PATH_CLASSES = Object.freeze([ [".github/scripts/work-resolve-issue/report-outcome.mjs", "enforcement"], [".github/scripts/work-resolve-issue/limits.mjs", "enforcement"], [".github/scripts/work-resolve-issue/retry-fetch.mjs", "enforcement"], + [".github/scripts/pr-unblock/fix-ci-context.mjs", "enforcement"], + [".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", "enforcement"], ]); // The `trust` and `gate` globs (babysit_risk.HIGH_FLOOR), which hold for a // person. An `enforcement` glob (REVIEW_FLOOR) holds nothing here: the merge diff --git a/.github/scripts/pr-merge/merge-holds.test.mjs b/.github/scripts/pr-merge/merge-holds.test.mjs index 3e08fc72c0..2326e48644 100644 --- a/.github/scripts/pr-merge/merge-holds.test.mjs +++ b/.github/scripts/pr-merge/merge-holds.test.mjs @@ -348,6 +348,8 @@ const ENFORCEMENT_FILES = [ ".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/fix-ci-context.mjs", + ".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", ]; test("the floor's enforcement globs are the merge gate's, every other one holds here", () => { assert.deepEqual(REVIEW_FLOOR, HOLD_PATH_CLASSES.filter(([, kind]) => kind === "enforcement").map(([glob]) => glob)); diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index e4e9d0db8a..e701dfcd1b 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4463,7 +4463,7 @@ jobs: } if [[ $AUTHOR_ID =~ ^[0-9]+$ && " $exempt_author_ids " == *" $AUTHOR_ID "* ]]; then # The trust-gate paths, as bash patterns (`*` crosses `/`). - hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' '.github/actions/resolve-config/*' '.github/actions/request-lane-token/*' '.github/actions/check-kill-switch/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-merge/carry-run-cache.mjs' '.github/scripts/pr-merge/gate-check.mjs' '.github/scripts/pr-merge/head-ref-map.mjs' '.github/scripts/pr-merge/lane-records.mjs' '.github/scripts/pr-merge/rate-stop.bash' '.github/scripts/pr-merge/refire-refused.mjs' '.github/scripts/pr-merge/rotation.mjs' '.github/scripts/pr-merge/sweep-summary.mjs' '.github/scripts/pr-merge/who.mjs' '.github/scripts/pr-merge/post-unarmed-status.mjs' '.github/scripts/pr-merge/verify-answers.mjs' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-merge.yml' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules') + hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' '.github/actions/resolve-config/*' '.github/actions/request-lane-token/*' '.github/actions/check-kill-switch/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-merge/carry-run-cache.mjs' '.github/scripts/pr-merge/gate-check.mjs' '.github/scripts/pr-merge/head-ref-map.mjs' '.github/scripts/pr-merge/lane-records.mjs' '.github/scripts/pr-merge/rate-stop.bash' '.github/scripts/pr-merge/refire-refused.mjs' '.github/scripts/pr-merge/rotation.mjs' '.github/scripts/pr-merge/sweep-summary.mjs' '.github/scripts/pr-merge/who.mjs' '.github/scripts/pr-merge/post-unarmed-status.mjs' '.github/scripts/pr-merge/verify-answers.mjs' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/scripts/pr-unblock/fix-ci-context.mjs' '.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-merge.yml' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules') if [[ ! $PR_NUMBER =~ ^[1-9][0-9]*$ ]]; then echo "::error::the pull request number '$PR_NUMBER' is not a number." exit 1 diff --git a/.github/workflows/pr-review-hosted.yml b/.github/workflows/pr-review-hosted.yml index 8674a1c918..b6361e7f59 100644 --- a/.github/workflows/pr-review-hosted.yml +++ b/.github/workflows/pr-review-hosted.yml @@ -229,6 +229,8 @@ jobs: .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/fix-ci-context.mjs + .github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs .github/workflows/pr-require-checks.yml .github/workflows/pr-merge.yml plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py diff --git a/.github/workflows/pr-review-security-hosted.yml b/.github/workflows/pr-review-security-hosted.yml index 47fb86da95..e7f5e004b5 100644 --- a/.github/workflows/pr-review-security-hosted.yml +++ b/.github/workflows/pr-review-security-hosted.yml @@ -229,6 +229,8 @@ jobs: .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/fix-ci-context.mjs + .github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs .github/workflows/pr-require-checks.yml .github/workflows/pr-merge.yml plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 862d03f534..4e1b5d831f 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -560,7 +560,7 @@ head: | Reason | Cleared when | |---|---| | `workflow-staged` | main changes nothing under `.github/workflows/` since its merge base with the head; or a trusted move (below) took the head off the resume key and the head contains main as it was when the lane escalated | -| `unresolved-conflict`, `protected-path-conflict`, `unexpected-path-staged`, `conflict-patch-invalid`, `conflict-patch-not-applied`, `conflict-patch-protected-path` | GitHub reports the PR mergeable | +| `unresolved-conflict`, `protected-path-conflict` | GitHub reports the PR mergeable | | `ci-fix-round-cap`, `ci-fix-empty`, `ci-fix-invalid`, `ci-fix-protected-path`, `ci-fix-not-applied`, `ci-fix-escalated` | a trusted move took the head off the resume key, and at the new head no check other than `ci-status` (which the label itself fails) fails, runs or was cancelled, and one passed | | `feedback-protected-path` | the PR changes nothing under `.github/workflows/` | | `head-moved` (pr-merge's `babysit-merge-verify-escalation`, `babysit-merge-risk` and `babysit-merge-shared-copy-escalation`, with no pr-unblock escalation since the label, and the marked App comment written last before the label is pr-merge's and within a minute of it) | the newest App comment carrying each marker names, as ``at head `` ``, a 40-hex head other than the current one; pr-merge then judges the new head and escalates again if its refusal still holds | @@ -854,7 +854,8 @@ the rules that only tighten in - `enforcement`, a clean security review clears it: every other module the floored scripts import (`.github/scripts/pr-unblock/address-feedback.mjs`, `gate-source.mjs` and `thread-scope.mjs`, `.github/scripts/work-resolve-issue/report-outcome.mjs` and `limits.mjs`, - and the `retry-fetch.mjs` copy in each of those two directories) + and the `retry-fetch.mjs` copy in each of those two directories, and `fix-ci-context.mjs` with + the `.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs` it loads) and the rest of the Python that decides the holds (`babysit_verify_evidence.py`, which `enable-auto-merge.mjs` runs, and the `babysit-prs/scripts/` modules it and `babysit_risk.py` import). It scores HIGH as `review-path:` until the `claude-security-review` evidence 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 985ef2e3b2..67bf178813 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -260,6 +260,8 @@ class RiskResult: (".github/scripts/work-resolve-issue/report-outcome.mjs", "enforcement"), (".github/scripts/work-resolve-issue/limits.mjs", "enforcement"), (".github/scripts/work-resolve-issue/retry-fetch.mjs", "enforcement"), + (".github/scripts/pr-unblock/fix-ci-context.mjs", "enforcement"), + (".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", "enforcement"), ) # fmt: skip PERSON_CLASSES = frozenset({"trust", "gate"}) HIGH_FLOOR: tuple[str, ...] = tuple(g for g, c in FLOOR_CLASSES if c in PERSON_CLASSES) 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 b08074a324..cec454ad11 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 @@ -331,6 +331,8 @@ def test_floor_paths_are_high_whatever_the_switch(self): (".github/scripts/work-resolve-issue/report-outcome.mjs", "enforcement"), (".github/scripts/work-resolve-issue/limits.mjs", "enforcement"), (".github/scripts/work-resolve-issue/retry-fetch.mjs", "enforcement"), + (".github/scripts/pr-unblock/fix-ci-context.mjs", "enforcement"), + (".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", "enforcement"), ), ) # fmt: skip self.assertEqual( @@ -370,6 +372,8 @@ def test_floor_paths_are_high_whatever_the_switch(self): (".github/scripts/work-resolve-issue/report-outcome.mjs", ".github/scripts/work-resolve-issue/report-outcome.mjs"), (".github/scripts/work-resolve-issue/limits.mjs", ".github/scripts/work-resolve-issue/limits.mjs"), (".github/scripts/work-resolve-issue/retry-fetch.mjs", ".github/scripts/work-resolve-issue/retry-fetch.mjs"), + (".github/scripts/pr-unblock/fix-ci-context.mjs", ".github/scripts/pr-unblock/fix-ci-context.mjs"), + (".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs", ".github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs"), ] # fmt: skip unreviewed = ( _lane("claude-review"), @@ -1800,7 +1804,7 @@ 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), 21) + self.assertEqual(len(babysit_risk.REVIEW_FLOOR), 23) for glob in babysit_risk.REVIEW_FLOOR: with self.subTest(glob): result = self._score(glob, (_lane("claude-security-review"),)) 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 127cdc8f71..ee6150ab47 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 @@ -226,8 +226,8 @@ def test_the_floor_is_the_same_on_both_sides(self) -> None: self.assertEqual(answer["classes"], [list(row) for row in risk.FLOOR_CLASSES]) self.assertEqual(answer["floor"], list(risk.HIGH_FLOOR)) self.assertEqual(answer["review"], list(risk.REVIEW_FLOOR)) - self.assertEqual(len(risk.FLOOR_CLASSES), 51) - self.assertEqual(len(risk.REVIEW_FLOOR), 21) + self.assertEqual(len(risk.FLOOR_CLASSES), 53) + self.assertEqual(len(risk.REVIEW_FLOOR), 23) self.assertEqual( {kind for _, kind in risk.FLOOR_CLASSES}, {"trust", "gate", "enforcement"} ) diff --git a/scripts/ci-lanes-pr-hold.test.sh b/scripts/ci-lanes-pr-hold.test.sh index e9e5ec3bd4..a6e3314446 100755 --- a/scripts/ci-lanes-pr-hold.test.sh +++ b/scripts/ci-lanes-pr-hold.test.sh @@ -475,7 +475,8 @@ for path in plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evi .github/scripts/pr-unblock/address-feedback.mjs .github/scripts/pr-unblock/gate-source.mjs \ .github/scripts/pr-unblock/thread-scope.mjs .github/scripts/pr-unblock/retry-fetch.mjs \ .github/scripts/work-resolve-issue/report-outcome.mjs .github/scripts/work-resolve-issue/limits.mjs \ - .github/scripts/work-resolve-issue/retry-fetch.mjs; do + .github/scripts/work-resolve-issue/retry-fetch.mjs \ + .github/scripts/pr-unblock/fix-ci-context.mjs .github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs; do STUB_FILES="[[\"$path\"]]" STUB_PULL="$(pull 'Closes #1')" expect "the gate, not ci-status, holds $path" 0 3 "no hold path" done From c81dd1be51197b7991972037c91fbe4ea192dd26 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 18:35:51 -0400 Subject: [PATCH 3/4] fix(pr-pipeline): lift only on human pushes and passing checks, never ci-fix-protected-path Answer the security review's gaps in clear-escalations.mjs: - A trusted move counts only pushes by a `human` of trusted-actors.json. The lanes App's own push no longer counts, so it cannot lift its own stop. Every lane that sets check-trusted-trigger's denied-actor-ids denies the App's sender id, so this also applies that denial. - ciPasses allowlists conclusions: each check's latest run at the head must end success, neutral or skipped, so action_required, stale or a null conclusion keeps the label. - ci-fix-protected-path leaves CI_FIX: its fix needed a protected path, so a person decides it, like unexpected-path-staged and the conflict-patch-* stops. The pr-merge.yml step comment, the pr-pipeline README clear table and the changelog fragment now say the same. Co-Authored-By: Claude Opus 5.5 --- .../feat-lift-stale-needs-human-3b7d9c51.md | 2 +- .../scripts/pr-merge/clear-escalations.mjs | 37 +++++++++++++------ .../pr-merge/clear-escalations.test.mjs | 27 ++++++++++---- .github/workflows/pr-merge.yml | 5 ++- docs/conventions/pr-pipeline/README.md | 9 +++-- 5 files changed, 54 insertions(+), 26 deletions(-) diff --git a/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md b/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md index 4a62ef8bf8..4975962384 100644 --- a/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md +++ b/.changes/pr-pipeline/feat-lift-stale-needs-human-3b7d9c51.md @@ -4,4 +4,4 @@ bump: minor ### Added -- `clear-escalations.mjs` lifts more pr-unblock stops once their cause is gone: `workflow-staged` also clears once a trusted move took the head off the resume key and the head contains main as it was at the escalation; the `ci-fix-*` stops clear once a trusted move took the head off the resume key and no check but `ci-status` fails, runs or was cancelled at the new head. A trusted move is read with the job token from the branch's activity, never from comment text: every push since the label is by a `human` of the base `trusted-actors.json` or the lanes App's entry, matched by id. Security, intent and decision stops still wait for a person. +- `clear-escalations.mjs` lifts more pr-unblock stops once their cause is gone: `workflow-staged` also clears once a trusted move took the head off the resume key and the head contains main as it was at the escalation; the `ci-fix-*` stops other than `ci-fix-protected-path` clear once a trusted move took the head off the resume key and, at the new head, no check but `ci-status` fails, runs or was cancelled and each check's latest run ended `success`, `neutral` or `skipped`. A trusted move is read with the job token from the branch's activity, never from comment text: every push since the label is by a `human` of the base `trusted-actors.json`, matched by id, never the lanes App. Security, intent, decision, protected-path and own-patch stops still wait for a person. diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index fd6d3c5ee5..83cf77ae58 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -148,8 +148,12 @@ const prMergeLine = (line) => PR_MERGE_MARKERS.includes(line) || line === PR_MER const AT_HEAD = / at head `([^`]*)`/g; const HEAD_MOVED = "head-moved"; const REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; -// report-escalation.sh's fix-ci stops, each judged by ciPasses. -const CI_FIX = Object.freeze(["ci-fix-round-cap", "ci-fix-empty", "ci-fix-invalid", "ci-fix-protected-path", "ci-fix-not-applied", "ci-fix-escalated"]); +// report-escalation.sh's fix-ci stops, each judged by ciPasses. Not +// ci-fix-protected-path: its fix needed a protected path, so a person decides it. +const CI_FIX = Object.freeze(["ci-fix-round-cap", "ci-fix-empty", "ci-fix-invalid", "ci-fix-not-applied", "ci-fix-escalated"]); +// The only conclusions a check's latest run at the head may have for a ci-fix +// stop to clear; action_required, stale, null and any other keep it. +const PASSING = new Set(["success", "neutral", "skipped"]); // fix-ci's aggregate check, which the human-gated label itself fails. const AGGREGATE = "ci-status"; // Check-run pages read at a head; a head with more stays unread. @@ -231,7 +235,8 @@ const CHECKS = Object.freeze({ // A ci-fix stop: the lane could not fix CI at its resume key. Gone once a // trusted push moved the head and, at the new head, no check fails, runs or // was cancelled (fix-ci's failingChecks over every run, so a later skip hides -// no failure), and one passed. ci-status is left out: the label fails it. +// no failure), each check's latest run ended in PASSING, and one passed. +// ci-status is left out: the label fails it. async function ciPasses(context) { const moved = await context.moved(); if (moved === null) { @@ -253,10 +258,17 @@ async function ciPasses(context) { return null; }); const others = (runs ?? []).filter((run) => run?.name !== AGGREGATE); + const latest = new Map(); + for (const run of others) { + if (!(latest.get(run.name)?.id > run.id)) { + latest.set(run.name, run); + } + } if ( runs === null || failingChecks(others).length > 0 || others.some((run) => run.status !== "completed") || + [...latest.values()].some((run) => !PASSING.has(run.conclusion)) || !others.some((run) => run.conclusion === "success") ) { return null; @@ -268,7 +280,8 @@ async function ciPasses(context) { // the label went on: the head is this repository's (the activity API shows no // fork push), its newest push since `since` landed the head, and every push // or other activity on the branch since then is by an id `trusted` holds (a -// `human` of trusted-actors.json, or the lanes App's own entry). The page must +// `human` of trusted-actors.json; never the lanes App, whose own push would +// lift its own stop). The page must // reach back past `since`, or the move stays unread. A description, or null. async function trustedMove({ read, repo, pull, keys, since, trusted }) { const head = pull.head.sha; @@ -524,14 +537,14 @@ function cleared(timeline, bot, since, reasons, head, why) { return { reason: null, recorded, head, why: why.join("; "), explained }; } -// The ids whose push is a trusted move: every `human` of trusted-actors.json -// and the lanes App's own entry (`bot`), matched by id. Null when the list -// does not read. -function trustedIds(path, bot) { +// The ids whose push is a trusted move: every `human` of trusted-actors.json, +// matched by id. Not the lanes App: every lane that sets check-trusted-trigger's +// denied-actor-ids denies its sender id, and its own push must not clear its +// own stop. +// Null when the list does not read. +function trustedIds(path) { try { - const { humans, logins } = loadTrustedList(path); - const app = logins.get(bot.toLowerCase()); - return new Set(app === undefined ? humans : [...humans, app]); + return new Set(loadTrustedList(path).humans); } catch (error) { console.log(`clear-escalations: no stop clears on a trusted move: ${escapeCommand(error?.message ?? error)}`); return null; @@ -557,7 +570,7 @@ export async function main({ env = process.env, github, verify, reader } = {}) { // history; without it, or without the trusted list, no stop clears on a // trusted move. reader = env.BABYSIT_VERIFY_GH_TOKEN ? (reader ?? createGitHub({ token: env.BABYSIT_VERIFY_GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined })) : null; - const trusted = trustedIds(env.TRUSTED_ACTORS_PATH || ".base/.github/standards/trusted-actors/trusted-actors.json", bot); + const trusted = trustedIds(env.TRUSTED_ACTORS_PATH || ".base/.github/standards/trusted-actors/trusted-actors.json"); const tip = await github("GET", `/repos/${repo}/git/ref/heads/main`); const mainSha = tip?.ref === "refs/heads/main" ? tip.object?.sha : undefined; if (!SHA.test(String(mainSha))) { diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index 4137ba44fe..dd547c7682 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -869,10 +869,14 @@ test("workflow-staged comes off once a trusted person moved the head past the re assert.equal(removal.method, "DELETE"); }); -test("a lanes App push counts as a trusted move", async () => { - const w = world({ timeline: stagedTimeline(), mainFiles: WORKFLOW_MAIN, reader: readerFor({ pushes: [pushedBy(APP_ACTOR)] }) }); - await run(w, TRUST_ENV); - assert.equal(w.writes().at(-1)?.method, "DELETE"); +test("a lanes App push is not a trusted move: the App never lifts its own stop with its own push", async () => { + for (const pushes of [[pushedBy(APP_ACTOR)], [pushedBy(PERSON, "e".repeat(40), "2026-10-18T10:30:00Z"), pushedBy(APP_ACTOR)]]) { + for (const reason of ["workflow-staged", "ci-fix-escalated"]) { + const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)], mainFiles: WORKFLOW_MAIN, reader: readerFor({ pushes }) }); + await run(w, TRUST_ENV); + assert.deepEqual(w.writes(), [], `${reason} ${JSON.stringify(pushes)}`); + } + } }); test("workflow-staged stays when any push since the label is by an account off the trusted list", async () => { @@ -939,7 +943,7 @@ test("an unreadable job-token read keeps the label without failing the run", asy assert.deepEqual(w.writes(), []); }); -const CI_FIX = ["ci-fix-round-cap", "ci-fix-empty", "ci-fix-invalid", "ci-fix-protected-path", "ci-fix-not-applied", "ci-fix-escalated"]; +const CI_FIX = ["ci-fix-round-cap", "ci-fix-empty", "ci-fix-invalid", "ci-fix-not-applied", "ci-fix-escalated"]; test("a ci-fix stop comes off once a trusted push moved the head and no check but ci-status fails there", async () => { for (const reason of CI_FIX) { @@ -958,6 +962,9 @@ test("a ci-fix stop stays while a check fails, runs, or none has passed at the h [...GREEN, run_("test-python", "cancelled", 4)], [run_("test-node", "failure", 2), run_("test-node", "skipped", 9)], [run_("ci-status", "failure", 1), run_("lint-repo", "skipped", 3)], + [...GREEN, run_("deploy", "action_required", 4)], + [...GREEN, run_("test-go", "stale", 4)], + [...GREEN, run_("test-go", null, 4)], [], ]) { const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated("ci-fix-escalated")], reader: readerFor({ checkRuns }) }); @@ -969,8 +976,14 @@ test("a ci-fix stop stays while a check fails, runs, or none has passed at the h assert.deepEqual(unmoved.writes(), []); }); -test("a stop on the lane's own conflict patch stays, even once the PR is mergeable after a trusted move", async () => { - for (const reason of ["unexpected-path-staged", "conflict-patch-invalid", "conflict-patch-not-applied", "conflict-patch-protected-path"]) { +test("a ci-fix stop passes with a neutral check at the head", async () => { + const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated("ci-fix-escalated")], reader: readerFor({ checkRuns: [...GREEN, run_("pr-unblock / publish-feedback", "neutral", 4)] }) }); + await run(w, TRUST_ENV); + assert.equal(w.writes().at(-1)?.method, "DELETE"); +}); + +test("a protected-path or own-patch stop stays, even once the PR is mergeable after a trusted move with everything green", async () => { + for (const reason of ["ci-fix-protected-path", "unexpected-path-staged", "conflict-patch-invalid", "conflict-patch-not-applied", "conflict-patch-protected-path"]) { const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(reason)], mergeable: true, reader: readerFor() }); await run(w, TRUST_ENV); assert.deepEqual(w.writes(), [], reason); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 740e3f71b6..3c83f34120 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -1472,8 +1472,9 @@ jobs: # A pr-unblock escalation adds the human-gated label with a reason. On # the schedule, this takes off a label the lanes App set once that # reason no longer holds at the PR's head (workflow-staged, a conflict - # or conflict-patch reason, feedback-protected-path, a ci-fix stop once - # trusted pushes moved the head and its checks pass, read with this + # reason, feedback-protected-path, a ci-fix stop other than + # ci-fix-protected-path once trusted human pushes moved the head and its + # checks pass, read with this # job's own token and the base trusted-actor list), after a comment naming the reason # and the head: never a label a person set, and never after anyone but # the App commented or reviewed since it went on. At most five a run, and not diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 8d1f88d60e..8584e2b847 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -564,7 +564,7 @@ head: |---|---| | `workflow-staged` | main changes nothing under `.github/workflows/` since its merge base with the head; or a trusted move (below) took the head off the resume key and the head contains main as it was when the lane escalated | | `unresolved-conflict`, `protected-path-conflict` | GitHub reports the PR mergeable | -| `ci-fix-round-cap`, `ci-fix-empty`, `ci-fix-invalid`, `ci-fix-protected-path`, `ci-fix-not-applied`, `ci-fix-escalated` | a trusted move took the head off the resume key, and at the new head no check other than `ci-status` (which the label itself fails) fails, runs or was cancelled, and one passed | +| `ci-fix-round-cap`, `ci-fix-empty`, `ci-fix-invalid`, `ci-fix-not-applied`, `ci-fix-escalated` | a trusted move took the head off the resume key, and at the new head no check other than `ci-status` (which the label itself fails) fails, runs or was cancelled, the latest run of each ended `success`, `neutral` or `skipped`, and one passed | | `feedback-protected-path` | the PR changes nothing under `.github/workflows/` | | `head-moved` (pr-merge's `babysit-merge-verify-escalation`, `babysit-merge-risk` and `babysit-merge-shared-copy-escalation`, with no pr-unblock escalation since the label, and the marked App comment written last before the label is pr-merge's and within a minute of it) | the newest App comment carrying each marker names, as ``at head `` ``, a 40-hex head other than the current one; pr-merge then judges the new head and escalates again if its refusal still holds | | `refusal-cleared` (under `merge.lane-prs: native`, `enable-auto-merge`'s own unedited person refusal comment on a lanes App pull request, naming the current head, whose recorded verifier or risk cause is one the lanes clear: every code on `LANE_CAUSES`) | the gate's reader at that head finds nothing refusing, or only causes the lanes clear, read at least as far as the recorded one. A recorded cause a person clears (an issue link, a security finding, a trust-gate widening, a cost hold, a high path, an untrusted spec, any code off that list) is never read or lifted at its head (owner decision Q-CLEAR B): a person removes the label, or the `head-moved` clear does once the head moves. At most three reader runs a run | @@ -575,12 +575,13 @@ as an AI review or another lane, does not count), or when an App comment written or edited since then carries any marker other than the escalation being judged (another `babysit-merge-*` marker, another lane's). A trusted move is read with the job's own token, never from comment text: the PR head is in this repository, every push on its branch since the label went -on is by a `human` of the base `trusted-actors.json` or the lanes App's entry there (matched by -id), the newest one landed the current head, and the head is none of the recorded resume keys. A +on is by a `human` of the base `trusted-actors.json` (matched by id; never the lanes App, so its +own push never lifts its own stop), the newest one landed the current head, and the head is none of the recorded resume keys. A read that fails keeps the label. Every other reason waits for a person: the security, intent and decision stops (`feedback-uncertain`, `feedback-disputed`, `feedback-unadjudicated`, `feedback-question-*`, `feedback-round-cap`, `feedback-not-converging`, `dependency-security-close`, -`dependency-escalate`), and those with no cause-gone evidence yet (`repeat-cap`, +`dependency-escalate`), the protected-path and own-patch stops (`ci-fix-protected-path`, +`unexpected-path-staged`, `conflict-patch-*`), and those with no cause-gone evidence yet (`repeat-cap`, `feedback-not-landed`, `feedback-invalid`, `feedback-partial`, `feedback-incomplete`, and the other `dependency-*` stops). Removing the label this way approves nothing: `ci-status` still fails a review-lane hold the lanes App unlabeled, and hold paths still need a trusted person's approving From 0afa6ed66b43f0a6f70d0a706a32951ccab6f71f Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 27 Oct 2026 20:20:22 -0400 Subject: [PATCH 4/4] docs(pr-pipeline): say only human pushes make a trusted move in clear-escalations comments The header of clear-escalations.mjs and the trusted-move comment in its test still said the lanes App's own entry counts as a trusted pusher, and the header's ci-fix row named every ci-fix-* stop. The code counts only `human` ids, keeps ci-fix-protected-path for a person, and requires each check's latest run to end success, neutral or skipped. The comments now say so. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-merge/clear-escalations.mjs | 9 +++++---- .github/scripts/pr-merge/clear-escalations.test.mjs | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index cfcbb08248..d1042d6d62 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -34,13 +34,14 @@ // unresolved-conflict, GitHub reports the PR mergeable: no conflict // protected-path-conflict is left to resolve // feedback-protected-path the PR changes nothing under .github/workflows/ -// ci-fix-* a trusted move, and at the new head no check -// but ci-status (the label fails it) fails, runs -// or was cancelled, and one passed +// ci-fix-* but a trusted move, and at the new head no check +// ci-fix-protected-path but ci-status (the label fails it) fails, runs +// or was cancelled, each latest run ended +// success, neutral or skipped, and one passed // A trusted move: the head is this repository's and none of the recorded // resume keys, and every activity on its branch since the label (the // activity API, read with the job token) is by an id trusted-actors.json -// lists as a `human`, or the lanes App's entry, the newest landing the head. +// lists as a `human`, never the lanes App, the newest landing the head. // Comment text never counts. Removing the label approves nothing: ci-status // fails a review-lane hold an App lifted, and a hold path needs a trusted // person's approving review. diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index dd547c7682..da11b7adae 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -790,7 +790,7 @@ test("labels past the read cap are read on a later run", async () => { }); // A trusted move: the head left every resume key through pushes the base -// trusted-actors.json trusts (a `human` entry, or the lanes App's own entry), +// trusted-actors.json lists as a `human` (never the lanes App's own entry), // read with the job token (`reader`). Expected values come from the owner rule // of 2026-10-26: a person is needed only for intent, design or a HIGH/CRITICAL // security finding, so a mechanical stop comes off once trusted evidence shows