Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changes/pr-pipeline/fix-needs-human-h38-routing-b0d69f43.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
bump: minor
---

### Changed

- `enable-auto-merge.mjs` adds the human-gated label only for a cause a person must clear (owner rule 2026-10-26): a refusal the gate marks `needsHuman` (an untrusted spec, the repeated-FAIL cap, a pull request too old to verify), the trust hold, a risk hold the gate marks `reviewsPerson`, a hold path, a cost hold, a listing past 300 files (a fourth page that is not empty; no later read completes it), or the lane's own hold line. A changed spec and a changed-file page that did not read get the comment alone, and the next scheduled runs judge them again. `merge-holds.pathHoldKind` reports a path hold's kind, so the routing reads no prose.
- A refusal the lanes clear that has stood at the head for `merge.risk-escalation-minutes`, timed from the first lane refusal noted there (the comment's `pr-merge-unarmed-since` line), shows as a failing `pr-merge/unarmed` commit status on the head; an arm, the refusal becoming a person's, or a member's veto or a person turning auto-merge off standing in for it posts a passing one over it; a run that read nothing new (no time left, unread hold paths, a verdict that did not read) leaves the status alone. The new `unarmed-status` job in `pr-merge.yml` posts them with the job's own token (`contents: read`, `statuses: write`, no lane token) and trusts only a status the GitHub Actions bot posted; the status is informational, not `ci-status`.
14 changes: 14 additions & 0 deletions .changes/source-control/fix-needs-human-h38-routing-b0d69f43.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
bump: minor
---

### Changed

- The merge gate pulls a person in only for a reason a person must clear (owner rule 2026-10-26): `babysit_risk.person_reasons` (`PERSON_CODES`: a CRITICAL security finding, a trusted-actor widening, a cost hold, every high path, and `files-capped`, a listing past the files API's 3000-file cap) is the one list the gate and `enable-auto-merge.mjs` read. A risk hold the lanes clear, such as review evidence not bound to the head or a listing a read did not finish, keeps its reasons comment and no longer gets the human-gated label however long it stands.
- A blocking security finding a bound review left open holds as `security-finding` after the lane unbinds (a new head, a newer marker, a retargeted base): the binder carries the count from the newest head of the pull request the security lane's evidence binds at, from the runs, comments and threads it already read. The gate's head-file reader sends no read again after a contents or comparison read fails, a rate limit included; a path it only cannot prove removed (`HeadFileUnproven`) stops nothing else.
- `babysit_verify_evidence.NEEDS_HUMAN` drops `verify:spec-changed`, which the lanes judge again; the terminal repeated-FAIL cap, `verify:too-old` and `verify:untrusted-spec` stay a person's.
- `evidence --lane-reviews` reports `reviewsPerson`, whether only a person clears the risk hold it names.

### Fixed

- The merge gate re-runs a rate-limited AI review only when the run's attempt 1 concluded `failure`, the condition under which the review binding takes attempt 2, so a re-run never spends the one allowed attempt on a hold.
196 changes: 148 additions & 48 deletions .github/scripts/pr-merge/enable-auto-merge.mjs

Large diffs are not rendered by default.

160 changes: 144 additions & 16 deletions .github/scripts/pr-merge/enable-auto-merge.test.mjs

Large diffs are not rendered by default.

14 changes: 9 additions & 5 deletions .github/scripts/pr-merge/merge-holds.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -277,19 +277,23 @@ export const HOLD_PATH_FLOOR = Object.freeze([

// Why the head's changed `files` hold it, or null: one matching `holdPaths`
// or HOLD_PATH_FLOOR. Null `holdPaths` or `files` (unreadable) holds it.
export function pathHold(files, holdPaths) {
export const pathHold = (files, holdPaths) => pathHoldKind(files, holdPaths)?.reason ?? null;

// pathHold's answer with its kind: `hold-path` for a changed file a hold
// path matches, `unread` for a list that could not be read or checked.
export function pathHoldKind(files, holdPaths) {
if (!Array.isArray(holdPaths)) {
return "the hold paths could not be read";
return { kind: "unread", reason: "the hold paths could not be read" };
}
if (!Array.isArray(files)) {
return "its changed files could not all be read";
return { kind: "unread", reason: "its changed files could not all be read" };
}
if (typeof posix.matchesGlob !== "function") {
return "this Node has no path.matchesGlob, so its hold paths cannot be checked";
return { kind: "unread", reason: "this Node has no path.matchesGlob, so its hold paths cannot be checked" };
}
const globs = [...holdPaths, ...HOLD_PATH_FLOOR];
const held = files.find((path) => globs.some((glob) => matchesHoldPath(path, glob)));
return held === undefined ? null : `it changes ${held}, a hold path`;
return held === undefined ? null : { kind: "hold-path", reason: `it changes ${held}, a hold path` };
}

// Why the changed `files` hold it for the owner's cost decision, or null when
Expand Down
108 changes: 108 additions & 0 deletions .github/scripts/pr-merge/post-unarmed-status.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
#!/usr/bin/env node
// post-unarmed-status.mjs: posts the `pr-merge/unarmed` commit statuses that
// enable-auto-merge.mjs hands on (UNARMED_OUTPUT): `failure` on a lanes App
// pull request's head whose refusal the lanes clear has stood
// merge.risk-escalation-minutes unchanged, `success` on a head it armed. No
// model.
//
// The status shows a hold the lanes own on the pull request, in place of the
// human-gated label (owner rule 2026-10-26: a person only for an intent or
// design change or a CRITICAL security issue). It is informational:
// its context is not `ci-status`, the only check the main ruleset requires,
// so it neither holds nor passes a merge. Each head's newest status in the
// context is read first, trusted only when the GitHub Actions bot posted it
// (any token with statuses: write can post the context): one already in that
// state with that description gets no write, and a `success` is written only
// over a status already there, so an unchanged run writes nothing.
//
// Environment:
// GH_TOKEN the job's own token, with statuses: write
// GITHUB_REPOSITORY owner/repo
// STATUSES enable-auto-merge's UNARMED_OUTPUT, a JSON array
// GITHUB_RUN_ID, GITHUB_SERVER_URL, GITHUB_API_URL
import process from "node:process";
import { pathToFileURL } from "node:url";

import { createGitHub } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs";
import { UNARMED_CONTEXT } from "./enable-auto-merge.mjs";

const REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/;
const SHA = /^[0-9a-f]{40}$/;
const STATES = new Set(["failure", "success"]);
// GitHub's limit on a status description.
const DESCRIPTION_CHARS = 140;
// Statuses read and written in one run at most (judgment: more open lane
// pull requests than this is a run to look at).
export const MAX_STATUSES = 100;
// The GitHub Actions bot's user id, which every GITHUB_TOKEN posts as
// (fix-ci-dispatch.mjs ACTIONS_BOT_ID).
const ACTIONS_BOT_ID = 41898282;

const escapeCommand = (text) => String(text).replaceAll(/[\r\n%]/g, " ");

// The statuses `text` hands on, well-formed only, one per head (its last).
export function parse(text) {
let list;
try {
list = JSON.parse(text);
} catch {
return [];
}
const byHead = new Map();
for (const item of Array.isArray(list) ? list : []) {
if (SHA.test(String(item?.sha)) && STATES.has(item?.state) && typeof item.description === "string") {
byHead.set(item.sha, { sha: item.sha, state: item.state, description: Array.from(item.description).slice(0, DESCRIPTION_CHARS).join("") });
}
}
return [...byHead.values()].slice(0, MAX_STATUSES);
}

// Whether `status` changes what its head shows, given the head's newest
// status in the context (`current`, undefined for none).
export const changes = (status, current) =>
current === undefined ? status.state === "failure" : current.state !== status.state || current.description !== status.description;

export async function main({ env = process.env, github } = {}) {
try {
if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) {
throw new Error("GITHUB_REPOSITORY is not owner/repo");
}
const repo = env.GITHUB_REPOSITORY;
github ??= createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined });
const target = /^[1-9][0-9]*$/.test(env.GITHUB_RUN_ID ?? "") ? `${env.GITHUB_SERVER_URL || "https://github.com"}/${repo}/actions/runs/${env.GITHUB_RUN_ID}` : null;
let failed = 0;
let written = 0;
for (const status of parse(env.STATUSES ?? "")) {
try {
// Newest first, so the first in the context is the one shown.
const listed = await github("GET", `/repos/${repo}/commits/${status.sha}/statuses?per_page=100`);
const newest = (Array.isArray(listed) ? listed : []).find((item) => item?.context === UNARMED_CONTEXT);
// Another creator's status in the context is not this job's: never
// read it as already shown.
const current = newest?.creator?.id === ACTIONS_BOT_ID ? newest : undefined;
if (!changes(status, current)) {
continue;
}
await github("POST", `/repos/${repo}/statuses/${status.sha}`, {
state: status.state,
context: UNARMED_CONTEXT,
description: status.description,
...(target ? { target_url: target } : {}),
});
written += 1;
} catch (error) {
failed += 1;
console.log(`::error title=post-unarmed-status::${status.sha}: ${escapeCommand(error?.message ?? error)}`);
}
}
console.log(`post-unarmed-status: ${written} written, ${failed} failed`);
return failed === 0 ? 0 : 1;
} catch (error) {
console.log(`::error title=post-unarmed-status::${escapeCommand(error?.message ?? error)}`);
return 1;
}
}

if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) {
process.exitCode = await main();
}
111 changes: 111 additions & 0 deletions .github/scripts/pr-merge/post-unarmed-status.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// Expected values come from the owner rule of 2026-10-26: a hold the lanes
// clear ends in a visible failing status, never the human-gated label, and a
// head that arms shows it passed.
import assert from "node:assert/strict";
import { test } from "node:test";

import { UNARMED_CONTEXT } from "./enable-auto-merge.mjs";
import { changes, MAX_STATUSES, main, parse } from "./post-unarmed-status.mjs";

const REPO = "melodic-software/claude-code-plugins";
const A = "a".repeat(40);
const B = "b".repeat(40);
const ACTIONS = { id: 41898282, login: "github-actions[bot]" };
const ENV = { GITHUB_REPOSITORY: REPO, GITHUB_RUN_ID: "9", GITHUB_SERVER_URL: "https://github.com" };

// `shown`: each head's statuses, newest first, as GitHub lists them.
async function run(statuses, shown = {}, { failRead = false } = {}) {
const posts = [];
const github = async (method, path, body) => {
const read = /^\/repos\/melodic-software\/claude-code-plugins\/commits\/([0-9a-f]{40})\/statuses\?per_page=100$/.exec(path);
if (method === "GET" && read) {
if (failRead) {
throw new Error("GET statuses failed with status 502");
}
return shown[read[1]] ?? [];
}
assert.equal(method, "POST");
posts.push({ path, body });
return {};
};
const log = console.log;
console.log = () => {};
try {
const code = await main({ env: { ...ENV, STATUSES: JSON.stringify(statuses) }, github });
return { code, posts };
} finally {
console.log = log;
}
}

test("a failing status is posted on its head with the run as its target", async () => {
const { code, posts } = await run([{ sha: A, state: "failure", description: "pr-merge's gate refuses its verifier verdict (verify:missing)" }]);
assert.equal(code, 0);
assert.deepEqual(posts, [
{
path: `/repos/${REPO}/statuses/${A}`,
body: {
state: "failure",
context: UNARMED_CONTEXT,
description: "pr-merge's gate refuses its verifier verdict (verify:missing)",
target_url: `https://github.com/${REPO}/actions/runs/9`,
},
},
]);
});

test("an unchanged status writes nothing, and a success only replaces a status already shown", async () => {
const failing = { context: UNARMED_CONTEXT, state: "failure", description: "x", creator: ACTIONS };
const { posts } = await run(
[
{ sha: A, state: "failure", description: "x" },
{ sha: B, state: "success", description: "armed for native auto-merge" },
],
{ [A]: [failing] },
);
assert.deepEqual(posts, []);
const cleared = await run([{ sha: A, state: "success", description: "armed for native auto-merge" }], { [A]: [{ context: "other", state: "success", creator: ACTIONS }, failing] });
assert.deepEqual(
cleared.posts.map(({ body }) => body.state),
["success"],
);
});

// Any token with statuses: write can post the context (fix-ci-dispatch.mjs
// reads the creator the same way).
test("a status in the context another creator posted is not read as this job's", async () => {
const forged = { context: UNARMED_CONTEXT, state: "failure", description: "x", creator: { id: 7, login: "someone" } };
const { posts } = await run([{ sha: A, state: "failure", description: "x" }], { [A]: [forged] });
assert.deepEqual(
posts.map(({ body }) => body.state),
["failure"],
);
const skipped = await run([{ sha: A, state: "success", description: "armed for native auto-merge" }], { [A]: [forged] });
assert.deepEqual(skipped.posts, []);
});

test("only well-formed statuses are read, one per head and at most MAX_STATUSES", () => {
assert.deepEqual(parse("not json"), []);
assert.deepEqual(parse(JSON.stringify({ sha: A })), []);
assert.deepEqual(
parse(
JSON.stringify([
{ sha: "A".repeat(40), state: "failure", description: "x" },
{ sha: A, state: "pending", description: "x" },
{ sha: A, state: "failure", description: 7 },
{ sha: A, state: "failure", description: "first" },
{ sha: A, state: "success", description: "y".repeat(200) },
]),
),
[{ sha: A, state: "success", description: "y".repeat(140) }],
);
const many = Array.from({ length: MAX_STATUSES + 5 }, (_, index) => ({ sha: index.toString(16).padStart(40, "0"), state: "failure", description: "x" }));
assert.equal(parse(JSON.stringify(many)).length, MAX_STATUSES);
assert.equal(changes({ state: "success", description: "x" }, undefined), false);
});

test("a read that fails writes nothing for that head and fails the step", async () => {
const { code, posts } = await run([{ sha: A, state: "failure", description: "x" }], {}, { failRead: true });
assert.equal(code, 1);
assert.deepEqual(posts, []);
});
25 changes: 24 additions & 1 deletion .github/scripts/pr-merge/workflow.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ test("the merge and enable-auto-merge jobs' kept-file readers share their reads"

test("the last verifier read enable-auto-merge starts ends inside its step's timeout", () => {
const arm = job("enable-auto-merge");
const step = /- name: Arm and disarm auto-merge\n {8}if: .*\n {8}timeout-minutes: ([0-9]+)\n/.exec(arm);
const step = /- name: Arm and disarm auto-merge\n {8}id: arm\n {8}if: .*\n {8}timeout-minutes: ([0-9]+)\n/.exec(arm);
assert.ok(step, "the arm step names its timeout");
// The room read, then the verdict budget and the last read it starts.
assert.ok(LANE_ROOM_TIMEOUT_MS + VERIFY_BUDGET_MS + VERIFY_TIMEOUT_MS < Number(step[1]) * 60_000);
Expand Down Expand Up @@ -360,6 +360,29 @@ test("enable-auto-merge's step timeouts fit inside its job's", () => {
assert.ok(steps.reduce((sum, minutes) => sum + minutes, 0) < total, `${steps.join(" + ")} inside ${total}`);
});

// pr-unblock's report-conflict-status precedent: statuses: write only on a
// job that holds no lane token and runs only main's script.
test("the pr-merge/unarmed statuses are posted by a job of their own, with statuses: write and nothing it could run from a pull request", () => {
const arm = job("enable-auto-merge");
assert.match(arm, /\n {4}outputs:\n {6}unarmed-statuses: \$\{\{ steps\.arm\.outputs\.unarmed-statuses \}\}\n/);
assert.match(arm, /\n {6}- name: Arm and disarm auto-merge\n {8}id: arm\n/);
assert.match(arm, /RISK_ESCALATION_MINUTES=\$\(yq '\.merge\."risk-escalation-minutes" \/\/ ""' "\$config"\)\n/);
const post = job("unarmed-status");
assert.match(post, /^ {4}needs: \[enable-auto-merge\]\n/);
// It runs whenever enable-auto-merge handed it statuses, after a failed
// step too, and only then.
assert.match(post, /\n {4}if: \$\{\{ !cancelled\(\) && needs\.enable-auto-merge\.outputs\.unarmed-statuses != '' \}\}\n/);
const block = /\n {4}permissions:\n((?: {6}[a-z-]+: [a-z]+.*\n)+)/.exec(post)[1];
assert.deepEqual(
[...block.matchAll(/ {6}([a-z-]+): ([a-z]+)/g)].map((m) => `${m[1]}: ${m[2]}`),
["contents: read", "statuses: write"],
);
assert.doesNotMatch(post, /lane-token|request-lane-token|id-token/);
assert.match(post, /\n {10}ref: refs\/heads\/main\n {10}path: \.base\n {10}persist-credentials: false\n/);
// The statuses reach the script through its environment, never the shell.
assert.match(post, /\n {10}STATUSES: \$\{\{ needs\.enable-auto-merge\.outputs\.unarmed-statuses \}\}\n {8}run: node \.base\/\.github\/scripts\/pr-merge\/post-unarmed-status\.mjs\n/);
});

// The merge job's sizing comment: about a minute before the loop, then the
// longer of the gate budget (10) plus a candidate's read limit (4) and a
// deferred candidate's ceiling (20), plus one read's own timeout (4); the
Expand Down
Loading
Loading