diff --git a/.changes/pr-pipeline/feat-lane-prs-native-8d31c5a0.md b/.changes/pr-pipeline/feat-lane-prs-native-8d31c5a0.md new file mode 100644 index 0000000000..c91df99396 --- /dev/null +++ b/.changes/pr-pipeline/feat-lane-prs-native-8d31c5a0.md @@ -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)). diff --git a/.github/actions/resolve-config/merge-authority.test.mjs b/.github/actions/resolve-config/merge-authority.test.mjs index 7b0c375e56..78c46fe947 100644 --- a/.github/actions/resolve-config/merge-authority.test.mjs +++ b/.github/actions/resolve-config/merge-authority.test.mjs @@ -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", () => { @@ -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); diff --git a/.github/actions/resolve-config/resolve.mjs b/.github/actions/resolve-config/resolve.mjs index e4c1487772..55a75e3e1b 100644 --- a/.github/actions/resolve-config/resolve.mjs +++ b/.github/actions/resolve-config/resolve.mjs @@ -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", }; } diff --git a/.github/actions/resolve-config/resolve.test.mjs b/.github/actions/resolve-config/resolve.test.mjs index 9b373b3014..fb001f8467 100644 --- a/.github/actions/resolve-config/resolve.test.mjs +++ b/.github/actions/resolve-config/resolve.test.mjs @@ -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", diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs index 83dfc4b83a..ac68dcb512 100755 --- a/.github/scripts/pr-merge/clear-escalations.mjs +++ b/.github/scripts/pr-merge/clear-escalations.mjs @@ -43,8 +43,10 @@ // newest lanes App comment carrying each of // pr-merge's own markers (PR_MERGE_MARKERS) names, in its `at head ```, // 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 @@ -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 @@ -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)) { @@ -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 { @@ -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; diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs index dabace88da..16aa3b8d22 100644 --- a/.github/scripts/pr-merge/clear-escalations.test.mjs +++ b/.github/scripts/pr-merge/clear-escalations.test.mjs @@ -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 }); @@ -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 }; @@ -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; } @@ -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], ``); }); +// 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) => `\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}\`.`; diff --git a/.github/scripts/pr-merge/merge-job.test.mjs b/.github/scripts/pr-merge/merge-job.test.mjs index e355327ca1..96d01bb66d 100644 --- a/.github/scripts/pr-merge/merge-job.test.mjs +++ b/.github/scripts/pr-merge/merge-job.test.mjs @@ -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"); @@ -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") : "", }; @@ -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); diff --git a/.github/workflows/pr-merge.yml b/.github/workflows/pr-merge.yml index 6fb997ffb8..a904d9128d 100644 --- a/.github/workflows/pr-merge.yml +++ b/.github/workflows/pr-merge.yml @@ -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' @@ -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")" @@ -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 @@ -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) @@ -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") # With adoption.idle-hours set, each trusted-actors.json account, a # person by login and an App as app/: the gate adopts their # pull requests (babysit_merge_policy.adoption_refusal) and checks @@ -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" @@ -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 diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index 8f3c4e64a5..75871f3d39 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -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 diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index be45636549..bba2cf59e3 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -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 diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index fca83f4fa4..6473ac54c1 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -863,6 +863,11 @@ "default": false, "description": "true: the merge authority reports each decision and writes nothing (no enqueue, dequeue or comment). The pr-merge lane starts true; the owner sets false to let it merge (README.md, Merge authority)." }, + "lane-prs": { + "enum": ["native", "gate"], + "default": "native", + "description": "How the agent identity's own pull requests merge. native: GitHub's auto-merge, armed by the enable-auto-merge activity and gated by ci-status; the pr-merge gate does not list, score or enqueue them. gate: the pr-merge gate lists, scores and enqueues them as well (README.md, Native auto-merge)." + }, "require-verifier-verdict": { "type": "boolean", "default": false,