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
7 changes: 7 additions & 0 deletions .changes/pr-pipeline/feat-lane-prs-native-8d31c5a0.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: minor
---

### Changed

- `merge.lane-prs` (default `native`) leaves the lanes App's pull requests to GitHub's native auto-merge: pr-merge's `merge` job no longer lists them, so its gate never scores, escalates or enqueues one. Adopted owner pull requests and Dependabot's are gated as before. `gate` restores the earlier listing, and this repository sets it until `ci-status` or `enable-auto-merge` enforces the pr-verify verdict and the post-merge check judges native merges ([README.md, Native auto-merge](https://github.com/melodic-software/claude-code-plugins/blob/main/docs/conventions/pr-pipeline/README.md#native-auto-merge)).
13 changes: 12 additions & 1 deletion .github/actions/resolve-config/merge-authority.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ const rejectedFor = (config, pattern) =>
);

// The merge section valid.yaml resolves to: no merge section, so rung off,
// manual landing, and only the forced denied paths.
// manual landing, lane pull requests on native auto-merge, and only the
// forced denied paths.
const VALID_MERGE = {
rung: "off",
"diff-check": { "denied-paths": [CONFIG_PATH, ".github/**", TRUSTED_ACTORS] },
"stack-landing": "manual",
"lane-prs": "native",
};

test("authority local-babysit without agent-identity fails the schema on the missing identity", () => {
Expand Down Expand Up @@ -251,6 +253,15 @@ test("any other merge.dependabot.mode or key fails the schema", () => {
);
});

test("merge.lane-prs takes native or gate, and the schema's default is the resolver's", () => {
for (const mode of ["native", "gate"]) {
assert.equal(run(withMerge({ "lane-prs": mode })).merge["lane-prs"], mode);
}
rejectedFor(withMerge({ "lane-prs": "off" }), /must be equal to one of the allowed values/);
const schema = json("../../../docs/conventions/pr-pipeline/pr-pipeline.schema.json");
assert.equal(schema.$defs.merge.properties["lane-prs"].default, run(withMerge({})).merge["lane-prs"]);
});

test("merge.dependabot.majors takes hold or review", () => {
for (const majors of ["hold", "review"]) {
assert.doesNotThrow(() => run(withMerge({ dependabot: { mode: "enforce", majors } })), majors);
Expand Down
1 change: 1 addition & 0 deletions .github/actions/resolve-config/resolve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,7 @@ function resolveMerge(merge = {}, configPath) {
rung: merge.rung ?? "off",
"diff-check": { ...diffCheck, "denied-paths": denied },
"stack-landing": merge["stack-landing"] ?? "manual",
"lane-prs": merge["lane-prs"] ?? "native",
};
}

Expand Down
3 changes: 2 additions & 1 deletion .github/actions/resolve-config/resolve.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,12 @@ test("wip: the schema's defaults are the readers' defaults", () => {
assert.deepEqual(WIP_DEFAULTS, WIP);
});

test("defaults are materialized: loop 2/3/2/3, merge off and manual, slot scope diff and enabled", () => {
test("defaults are materialized: loop 2/3/2/3, merge off, manual and native, slot scope diff and enabled", () => {
const resolved = run(fixture("valid.yaml"), { activity: "run-tests" });
assert.deepEqual(resolved.loop, { "review-rounds": 2, "no-progress": 3, "dependency-review-rounds": 2, "fix-ci-rounds": 3 });
assert.equal(resolved.merge.rung, "off");
assert.equal(resolved.merge["stack-landing"], "manual");
assert.equal(resolved.merge["lane-prs"], "native");
assert.equal(resolved.version, 1);
assert.deepEqual(resolved.source, {
"base-path": ".base",
Expand Down
15 changes: 11 additions & 4 deletions .github/scripts/pr-merge/clear-escalations.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@
// newest lanes App comment carrying each of
// pr-merge's own markers (PR_MERGE_MARKERS) names, in its `at head `<sha>``,
// a head that is not the PR's current one. pr-merge then judges the new head
// and escalates again if its refusal still holds. A marker whose head does not
// parse as 40-hex keeps the label, and so does any other marker, as above.
// and escalates again if its refusal still holds. Under `merge.lane-prs:
// native` pr-merge never judges a lanes App pull request, so one of those
// keeps the label: nothing would escalate it again. A marker whose head does
// not parse as 40-hex keeps the label, and so does any other marker, as above.
//
// Before the label comes off, the App posts one comment naming the cleared
// reason and the head; a sweep that finds that comment for this escalation
Expand All @@ -56,6 +58,8 @@
// KILL_SWITCH `proceed` from the kill-switch step; anything else stops
// TRACKER_CONFIG the base .work-item-tracker.json, for the human-gated
// label (default .base/.work-item-tracker.json)
// LANE_PRS merge.lane-prs as resolve-config resolved it; only
// `native` keeps a lanes App PR's head-moved label
// LANES_BOT_LOGIN the lanes App bot login (default
// melodic-automation-lanes[bot])
// GITHUB_API_URL
Expand Down Expand Up @@ -187,7 +191,7 @@ function spoken(item) {
}

// What to clear on `number`, or { reason } saying why nothing is.
async function verdict({ github, repo, number, label, bot, mainSha }) {
async function verdict({ github, repo, number, label, bot, mainSha, lanePrs }) {
const timeline = await paginate(github, `/repos/${repo}/issues/${number}/timeline`);
const { labeled, since, lifted } = labelHistory(timeline, label);
if (!isApp(labeled?.actor, bot)) {
Expand Down Expand Up @@ -247,6 +251,9 @@ async function verdict({ github, repo, number, label, bot, mainSha }) {
if (merged.some(({ head }) => head === pull.head.sha)) {
return { reason: `a pr-merge escalation names the current head ${pull.head.sha}` };
}
if (lanePrs === "native" && isApp(pull.user, bot)) {
return { reason: "under merge.lane-prs native pr-merge does not judge a lanes App pull request's new head" };
}
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 {
Expand Down Expand Up @@ -295,7 +302,7 @@ export async function main({ env = process.env, github } = {}) {
continue;
}
try {
const result = await verdict({ github, repo, number, label, bot, mainSha });
const result = await verdict({ github, repo, number, label, bot, mainSha, lanePrs: env.LANE_PRS });
if (result.reason !== null) {
console.log(`clear-escalations: #${number} kept: ${result.reason}`);
continue;
Expand Down
24 changes: 20 additions & 4 deletions .github/scripts/pr-merge/clear-escalations.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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.
function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [] }) {
function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable = false, pullFiles = [], author }) {
const calls = [];
const github = async (method, path, body) => {
calls.push({ method, path, body });
Expand All @@ -45,7 +45,7 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable =
return path.endsWith("page=1") ? timeline : [];
}
if (method === "GET" && route === `/repos/${REPO}/pulls/7`) {
return { number: 7, state: "open", base: { ref: "main" }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable };
return { number: 7, state: "open", user: author, base: { ref: "main" }, head: { sha: HEAD }, labels: [{ name: "needs-human" }], mergeable };
}
if (method === "GET" && route === `/repos/${REPO}/compare/${HEAD}...${MAIN}`) {
return { files: mainFiles };
Expand All @@ -61,12 +61,12 @@ function world({ timeline, mainFiles = [{ filename: "README.md" }], mergeable =
return { github, calls, writes: () => calls.filter((call) => call.method !== "GET") };
}

const run = async (w) => {
const run = async (w, env = {}) => {
const logs = [];
const original = console.log;
console.log = (line) => logs.push(line);
try {
return { code: await main({ env: ENV, github: w.github }), logs };
return { code: await main({ env: { ...ENV, ...env }, github: w.github }), logs };
} finally {
console.log = original;
}
Expand Down Expand Up @@ -250,6 +250,22 @@ test("a pr-merge risk comment at an old head comes off as head-moved", async ()
assert.equal(w.writes()[0].body.body.split("\n")[0], `<!-- pr-pipeline needs-human-cleared reason=head-moved head=${HEAD} -->`);
});

// Under merge.lane-prs native pr-merge never lists a lanes App PR, so nothing
// would judge its new head and escalate again: the label stays.
test("a pr-merge escalation at an old head on a lanes App PR is kept under lane-prs native, cleared under gate", async () => {
const native = world({ timeline: mergeEscalated(riskBody(OLD)), author: BOT });
const { logs } = await run(native, { LANE_PRS: "native" });
assert.equal(native.writes().length, 0);
assert.ok(logs.some((line) => /#7 kept: under merge\.lane-prs native/.test(line)), logs.join("\n"));
const gate = world({ timeline: mergeEscalated(riskBody(OLD)), author: BOT });
await run(gate, { LANE_PRS: "gate" });
assert.equal(gate.writes().length, 2);
// An adopted owner PR is still gated under native, so it clears.
const owner = world({ timeline: mergeEscalated(riskBody(OLD)), author: OWNER });
await run(owner, { LANE_PRS: "native" });
assert.equal(owner.writes().length, 2);
});

const sharedCopyBody = (head) =>
`<!-- babysit-merge-shared-copy-escalation -->\nUnattended merge: the shared-library canonical \`lib/a.sh\` changed on \`main\` since merge base \`${OLD}\`, so the copies this pull request carries are stale at head \`${head}\`.`;

Expand Down
59 changes: 56 additions & 3 deletions .github/scripts/pr-merge/merge-job.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ test("the early re-dispatch keeps the trusted mechanics and skips untrusted even
// fails its first `failures` calls, with `.base` pointing at this checkout,
// or, given `edit`, at a copy whose pr-pipeline.yaml is `edit(text)` and whose
// trusted-actors.json is `actors`, when given, else this checkout's.
const runListing = (failures, edit, actors) => {
// `env` overrides the step's environment: merge.lane-prs is `gate` unless it
// says otherwise, the mode that lists the lanes App's own pull requests.
const runListing = (failures, edit, actors, env = {}) => {
const dir = mkdtempSync(join(tmpdir(), "pr-merge-listing-"));
try {
const bin = join(dir, "bin");
Expand Down Expand Up @@ -124,14 +126,19 @@ const runListing = (failures, edit, actors) => {
GITHUB_OUTPUT: output,
REPO: "o/r",
DEPENDABOT: "off",
LANE_PRS: "gate",
...env,
},
});
const read = (name) => (existsSync(join(dir, name)) ? readFileSync(join(dir, name), "utf8") : "");
return {
status: result.status,
stdout: result.stdout,
stderr: result.stderr,
calls: Number(readFileSync(join(dir, "calls"), "utf8")),
args: readFileSync(join(dir, "args"), "utf8"),
calls: Number(read("calls") || 0),
args: read("args"),
candidates: read("candidates.json"),
drafts: read("drafts.json"),
prefiltered: existsSync(join(dir, "prefilter.json")),
output: existsSync(output) ? readFileSync(output, "utf8") : "",
};
Expand Down Expand Up @@ -175,6 +182,52 @@ test("without an adoption section no trusted actor is listed", () => {
assert.deepEqual(authorsOf(noIdle.args), ["app/melodic-automation-lanes"]);
});

// merge.lane-prs: native leaves the lanes App's pull requests to GitHub's
// auto-merge: the listing never asks for them, so the gate never reads them.
test("under lane-prs native the listing skips the lanes App and keeps every other author", () => {
const actors = [
{ id: 1, login: "a-person", kind: "human" },
{ id: 3, login: "melodic-automation-lanes[bot]", kind: "bot" },
];
const native = runListing(0, (config) => config, actors, { LANE_PRS: "native", DEPENDABOT: "enforce" });
assert.equal(native.status, 0, native.stderr);
assert.deepEqual(authorsOf(native.args), ["a-person", "app/dependabot"]);
assert.match(native.output, /^count=1$/m);
// gate lists the lanes App first, as before the setting existed.
const gate = runListing(0, (config) => config, actors, { LANE_PRS: "gate", DEPENDABOT: "enforce" });
assert.equal(gate.status, 0, gate.stderr);
assert.deepEqual(authorsOf(gate.args), ["app/melodic-automation-lanes", "a-person", "app/dependabot"]);
});

test("under lane-prs native with no other author the listing calls nothing and lists no candidate", () => {
const run = runListing(0, (config) => config.replace(/^adoption:\n( {2}.*\n)*/m, ""), undefined, { LANE_PRS: "native" });
assert.equal(run.status, 0, run.stderr);
assert.equal(run.calls, 0);
assert.equal(run.candidates.trim(), "[]");
assert.equal(run.drafts.trim(), "[]");
assert.match(run.output, /^count=0$/m);
});

test("the mode step outputs merge.lane-prs as resolve-config resolved it, and the listing reads it", () => {
for (const mode of ["native", "gate"]) {
const dir = mkdtempSync(join(tmpdir(), "pr-merge-mode-"));
try {
symlinkSync(root, join(dir, ".base"));
writeFileSync(join(dir, "resolved-config.json"), JSON.stringify({ merge: { "lane-prs": mode } }));
const result = spawnSync("bash", ["-c", runScript("Read report-only")], {
cwd: dir,
encoding: "utf8",
env: { ...process.env, RUNNER_TEMP: dir, GITHUB_OUTPUT: join(dir, "output") },
});
assert.equal(result.status, 0, result.stderr);
assert.match(readFileSync(join(dir, "output"), "utf8"), new RegExp(`^lane-prs=${mode}$`, "m"));
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
assert.match(step("List the candidate PRs"), /\n {10}LANE_PRS: \$\{\{ steps\.mode\.outputs\.lane-prs \}\}\n/);
});

test("the listing fails with one clear error after three failed tries", () => {
const run = runListing(3);
assert.equal(run.status, 1);
Expand Down
18 changes: 13 additions & 5 deletions .github/workflows/pr-merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,8 @@ jobs:
# lane's own PRs only `false` (or no setting at all) is live; for
# Dependabot PRs only `merge.dependabot.mode: enforce` is, and only
# `report` or `enforce` lists them at all. While neither is live this
# run starts no CI run either.
# run starts no CI run either. lane-prs is merge.lane-prs as
# resolve-config resolved it (absent: native).
- name: Read report-only
id: mode
if: steps.config.outcome == 'success'
Expand All @@ -538,6 +539,7 @@ jobs:
echo "report-only=$report_only"
echo "dependabot=$dependabot"
echo "live=$live"
echo "lane-prs=$(jq -r '.merge."lane-prs"' "$RUNNER_TEMP/resolved-config.json")"
echo "gate-budget-minutes=$(yq '.merge."gate-budget-minutes" // ""' "$config")"
echo "gate-candidate-minutes=$(yq '.merge."gate-candidate-minutes" // ""' "$config")"
echo "gate-deferred-minutes=$(yq '.merge."gate-deferred-minutes" // ""' "$config")"
Expand Down Expand Up @@ -587,7 +589,10 @@ jobs:

# Candidates: open, non-draft PRs into main that the lanes App
# (merge.agent-identity on main) or the owner account opened, plus
# Dependabot's unless merge.dependabot.mode is off. This is only a shortlist; the gate checks
# Dependabot's unless merge.dependabot.mode is off. The lanes App's are
# listed only under `merge.lane-prs: gate`; under native they merge
# through GitHub's auto-merge (the enable-auto-merge job), so the gate
# never reads them. This is only a shortlist; the gate checks
# the author, the closing issue and verifier verdict or the Dependabot
# tier, and every other rule again. A candidate the gate would refuse on
# its listed labels or body alone (the `do-not-merge` label, the
Expand All @@ -602,6 +607,7 @@ jobs:
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
DEPENDABOT: ${{ steps.mode.outputs.dependabot }}
LANE_PRS: ${{ steps.mode.outputs.lane-prs }}
run: |
set -euo pipefail
identity=$(yq '.merge."agent-identity" // ""' .base/docs/conventions/pr-pipeline.yaml)
Expand All @@ -610,7 +616,8 @@ jobs:
echo "::error::merge.agent-identity is not an App bot login: $identity"
exit 1
fi
authors=("app/$slug")
authors=()
[ "$LANE_PRS" = native ] || authors+=("app/$slug")
Comment thread
kyle-sexton marked this conversation as resolved.
# With adoption.idle-hours set, each trusted-actors.json account, a
# person by login and an App as app/<slug>: the gate adopts their
# pull requests (babysit_merge_policy.adoption_refusal) and checks
Expand Down Expand Up @@ -656,14 +663,14 @@ jobs:
sleep $((attempt * 15))
done
done
jq -cs '[add[] | select(.isDraft | not)
jq -cs '[(add // [])[] | select(.isDraft | not)
| {number, head: .headRefOid, updatedAt, labels: [.labels[].name], body}]
| unique_by(.number)' "$listing" |
python3 .base/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_prefilter.py \
--human-gated="$human_gated" >"$RUNNER_TEMP/prefilter.json"
jq -c .candidates "$RUNNER_TEMP/prefilter.json" >"$RUNNER_TEMP/candidates.json"
jq -c .held "$RUNNER_TEMP/prefilter.json" >"$RUNNER_TEMP/held.json"
jq -cs '[add[] | select(.isDraft) | {number, id, head: .headRefOid, updatedAt,
jq -cs '[(add // [])[] | select(.isDraft) | {number, id, head: .headRefOid, updatedAt,
labels: [.labels[].name]}] | unique_by(.number)' "$listing" >"$RUNNER_TEMP/drafts.json"
echo "count=$(jq length "$RUNNER_TEMP/candidates.json")" >>"$GITHUB_OUTPUT"

Expand Down Expand Up @@ -1193,6 +1200,7 @@ jobs:
env:
GH_TOKEN: ${{ steps.lane-token.outputs.token }}
KILL_SWITCH: ${{ steps.kill.outputs.proceed == 'true' && 'proceed' || '' }}
LANE_PRS: ${{ steps.mode.outputs.lane-prs }}
run: node .base/.github/scripts/pr-merge/clear-escalations.mjs

- name: Log the API budget at job end
Expand Down
14 changes: 12 additions & 2 deletions docs/conventions/pr-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,18 @@ merge:
agent-identity: melodic-automation-lanes[bot]
# true: report each decision and write nothing.
report-only: false
# A PR merges only with a passing pr-verify.yml verdict at its head. Must be
# true at a rung above off: the gate refuses otherwise.
# native: the lanes App's pull requests merge through GitHub's auto-merge
# (the enable-auto-merge activity arms them, ci-status gates them), and the
# gate below neither lists, scores nor enqueues them. gate: the gate also
# lists, scores and enqueues them. Owner PRs and Dependabot's are gated in
# both. Absent is native. This repository stays on gate until ci-status or
# enable-auto-merge requires the pr-verify verdict and the post-merge check
# judges a native merge with no pin comment: until then the gate is the only
# place either holds for a lanes App PR.
lane-prs: gate
# A PR the gate lists merges only with a passing pr-verify.yml verdict at its
# head (lane PRs under lane-prs native are not listed). Must be true at a
# rung above off: the gate refuses otherwise.
require-verifier-verdict: true
# Lane merges in flight at once (queued plus merged and not yet verified).
# The merge queue tests each PR against the latest base plus the PRs ahead
Expand Down
8 changes: 8 additions & 0 deletions docs/conventions/pr-pipeline/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,14 @@ missed. The token broker mints `merge` only for `pr-merge.yml`, and outside the
listed human's run, so an issue run the lanes App started is armed by the next scheduled run. While
`merge.report-only` is true it reports each decision and writes nothing.

`merge.lane-prs` (default `native`) decides whether the merge gate still handles the lanes App's
pull requests. Under `native` the `merge` job's candidate listing does not ask for them, so the gate
never scores, escalates, toggles or enqueues one; native auto-merge and `ci-status` are their whole
merge path. Owner pull requests the lane adopts and Dependabot's are listed and gated as before,
and the post-merge check, the full-run dispatch, re-firing refused runs and clearing escalations
run unchanged. `gate` lists the lanes App's pull requests for the gate as well, which is how the
lane ran before this setting; the `enable-auto-merge` job arms them in both modes.

`ci-status` enforces the same holds on every lanes App pull request, the two text holds and the
hold paths, so one added after arming stops the merge without waiting for this job: its step
`Fail a lanes App pull request its body or a member holds` runs
Expand Down
Loading
Loading