From 5dcbb1f3a27dd0cac1ac96e7961f1f9cca6e2f95 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:42:38 -0400 Subject: [PATCH 1/5] feat(pr-pipeline): delete stale branches daily by the branch-cleanup rule Owner decision W-7: no non-default branch lives forever. A daily maintenance-delete-stale-branches.yml sorts every branch: closed or merged PR and closed linked issue at least min-age-days (14) ago deletes, at most max-deletes-per-run (25) a run; an open issue, an open PR, a protected or ruleset branch, and a stopped lane run's branch are never deleted; anything unreadable or without a PR is listed for a person. Each delete re-reads the tip and open PRs and writes a ledger line first. Manual runs default to dry-run, which refuses every non-GET request. Co-Authored-By: Claude Opus 5.5 --- .../feat-branch-cleanup-w7-663cf524.md | 14 + .../actions/resolve-config/resolve-config.mjs | 8 + .github/scripts/branch-cleanup/cleanup.mjs | 269 +++++++++++++++++ .../scripts/branch-cleanup/cleanup.test.mjs | 275 ++++++++++++++++++ .github/scripts/branch-cleanup/package.json | 7 + .github/scripts/branch-cleanup/rules.mjs | 74 +++++ .../scripts/branch-cleanup/workflow.test.mjs | 43 +++ .../maintenance-delete-stale-branches.yml | 110 +++++++ docs/conventions/pr-pipeline.yaml | 10 + docs/conventions/pr-pipeline/README.md | 40 +++ .../pr-pipeline/pr-pipeline.schema.json | 21 ++ scripts/outside-node-packages.txt | 1 + 12 files changed, 872 insertions(+) create mode 100644 .changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md create mode 100644 .github/scripts/branch-cleanup/cleanup.mjs create mode 100644 .github/scripts/branch-cleanup/cleanup.test.mjs create mode 100644 .github/scripts/branch-cleanup/package.json create mode 100644 .github/scripts/branch-cleanup/rules.mjs create mode 100644 .github/scripts/branch-cleanup/workflow.test.mjs create mode 100644 .github/workflows/maintenance-delete-stale-branches.yml diff --git a/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md b/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md new file mode 100644 index 0000000000..3251cc1713 --- /dev/null +++ b/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md @@ -0,0 +1,14 @@ +--- +bump: minor +--- + +### Added + +- A `branch-cleanup` leaf (`min-age-days`, default 14; `max-deletes-per-run`, default 25) and + the daily maintenance-delete-stale-branches.yml that applies it: a branch whose pull request + closed or merged and whose linked issue closed at least `min-age-days` ago is deleted, oldest + first and at most `max-deletes-per-run` a run, after a re-read of its tip and open pull + requests and a ledger line in the step summary and an artifact. A branch with an open issue + or pull request, a protected or ruleset branch, and a stopped lane run's branch are never + deleted; a branch that never had a pull request, or whose tip moved after its pull request + closed with commits ahead, is listed for a person. A manual run is a dry-run by default. diff --git a/.github/actions/resolve-config/resolve-config.mjs b/.github/actions/resolve-config/resolve-config.mjs index dcb621eb68..12630f9395 100644 --- a/.github/actions/resolve-config/resolve-config.mjs +++ b/.github/actions/resolve-config/resolve-config.mjs @@ -377,6 +377,14 @@ export function redMainFromBase(env) { return { redMain: doc["red-main"] ?? {}, agentIdentity: doc.merge?.["agent-identity"] ?? "" }; } +// The base config's `branch-cleanup` leaf as set, likewise; its defaults have +// one copy, in .github/scripts/branch-cleanup/rules.mjs. +export function branchCleanupFromBase(env) { + const basePath = env.BASE_PATH || DEFAULT_BASE_PATH; + const configPath = env.CONFIG_PATH || DEFAULT_CONFIG_PATH; + return loadConfig(filesUnderBase(basePath, configPath), { configPath })["branch-cleanup"] ?? {}; +} + // A run that is no lane activity resolves only its model route. function resolveRouteFromBase(env) { if (env.LANE) { diff --git a/.github/scripts/branch-cleanup/cleanup.mjs b/.github/scripts/branch-cleanup/cleanup.mjs new file mode 100644 index 0000000000..90532e4155 --- /dev/null +++ b/.github/scripts/branch-cleanup/cleanup.mjs @@ -0,0 +1,269 @@ +// maintenance-delete-stale-branches.yml's job: sorts every branch of this +// repository into delete, keep or report by the branch-cleanup rule +// (docs/conventions/pr-pipeline/README.md, "Branch cleanup"), prints how many +// it will delete, and deletes at most `max-deletes-per-run` of them. Before +// each delete it re-reads the branch tip and its open pull requests, skips the +// branch when either changed, and appends the ledger line. In dry-run the +// client refuses every request but GET, so nothing is written. REST only, +// paced, within a call budget. No model. +// +// Environment: GITHUB_TOKEN, REPOSITORY, DRY_RUN (`true` or `false`), +// BASE_PATH (main's checkout), LEDGER_PATH (JSON Lines file), CALL_BUDGET, +// LANES_BOT_LOGIN, GITHUB_API_URL, GITHUB_STEP_SUMMARY. +import { appendFileSync } from "node:fs"; +import process from "node:process"; +import { setTimeout as delay } from "node:timers/promises"; +import { pathToFileURL } from "node:url"; + +import { parseOutcomeMarker } from "../work-resolve-issue/report-outcome.mjs"; +import { ageDays, excluded, laneBranch, lastPull, linkedIssues, resolveBranchCleanup, stoppedRun } from "./rules.mjs"; + +export function client(env, { pace = () => delay(200), dryRun = true } = {}) { + let used = 0; + const budget = Number(env.CALL_BUDGET ?? 600); + return async (method, path) => { + if (dryRun && method !== "GET") throw new Error(`dry-run refuses ${method} ${path}`); + if (used >= budget) throw new Error(`call budget of ${budget} spent`); + used += 1; + await pace(); + const response = await fetch(`${env.GITHUB_API_URL ?? "https://api.github.com"}${path}`, { + method, + headers: { + accept: "application/vnd.github+json", + authorization: `Bearer ${env.GITHUB_TOKEN}`, + "x-github-api-version": "2022-11-28", + }, + }); + if (response.status === 404 && method === "GET") return null; + if (!response.ok) throw new Error(`${method} ${path.split("?")[0]}: HTTP ${response.status}`); + return response.status === 204 ? null : response.json(); + }; +} + +async function pages(call, path, max = 20) { + const items = []; + for (let page = 1; page <= max; page += 1) { + const batch = (await call("GET", `${path}${path.includes("?") ? "&" : "?"}per_page=100&page=${page}`)) ?? []; + items.push(...batch); + if (batch.length < 100) return items; + } + throw new Error(`${path.split("?")[0]}: more than ${max} pages`); +} + +const refPath = (name) => name.split("/").map(encodeURIComponent).join("/"); + +// One branch's verdict: { verdict: delete | keep | report, rule or reason, +// issue, pr, closedAt }. Reads run cheapest first and stop at the first +// verdict; a failed read reports the branch. +export async function evaluate(call, { repo, owner, bot, defaultBranch, config, now }, branch) { + const { name, sha } = branch; + const keep = (reason, extra = {}) => ({ verdict: "keep", reason, ...extra }); + const report = (reason, extra = {}) => ({ verdict: "report", reason, ...extra }); + if (branch.protected) return keep("protected"); + const pulls = await pages(call, `/repos/${repo}/pulls?state=all&head=${encodeURIComponent(`${owner}:${name}`)}`); + const openPull = pulls.find((p) => p.state === "open"); + if (openPull) return keep("open pull request", { pr: openPull.number }); + const pull = lastPull(pulls); + const pr = pull?.number ?? null; + const lane = laneBranch(name); + + const closedTimes = []; + let issue = null; + if (lane) { + issue = lane.issue; + const read = await call("GET", `/repos/${repo}/issues/${issue}`); + if (!read) return report("its issue could not be read", { issue, pr }); + if (read.state !== "closed") return keep("its issue is open", { issue, pr }); + const markers = (await pages(call, `/repos/${repo}/issues/${issue}/comments`)) + .filter((c) => c.user?.login === bot) + .map((c) => parseOutcomeMarker(c.body)) + .filter(Boolean); + if (stoppedRun(markers, lane.run)) return report("a stopped lane run's branch: a person decides", { issue, pr }); + closedTimes.push(read.closed_at); + } else { + if (!pull) return report("never had a pull request", {}); + for (const number of linkedIssues(pull.body, repo)) { + const read = await call("GET", `/repos/${repo}/issues/${number}`); + if (!read) return report(`linked issue #${number} could not be read`, { issue: number, pr }); + if (read.state !== "closed") return keep(`linked issue #${number} is open`, { issue: number, pr }); + issue ??= number; + closedTimes.push(read.closed_at); + } + } + + let rule; + if (!pull) { + rule = "lane-issue-closed"; + } else { + closedTimes.push(pull.closed_at); + const tipMoved = pull.head?.sha !== sha; + if (pull.merged_at && !tipMoved) { + rule = "pr-merged"; + } else if (pull.merged_at) { + const compare = await call( + "GET", + `/repos/${repo}/compare/${encodeURIComponent(defaultBranch)}...${encodeURIComponent(sha)}`, + ); + if (!Number.isSafeInteger(compare?.ahead_by)) return report("its commits ahead of the default branch could not be read", { issue, pr }); + if (compare.ahead_by > 0) return report(`merged, then ${compare.ahead_by} commit(s) ahead of ${defaultBranch}`, { issue, pr }); + rule = "pr-merged-no-commits-ahead"; + } else if (!tipMoved) { + rule = "pr-closed"; + } else { + return report("pull request closed unmerged, then the tip moved", { issue, pr }); + } + } + + const age = ageDays(closedTimes, now); + if (age === null) return report("a close time could not be read", { issue, pr }); + if (age < config["min-age-days"]) { + return keep(`closed ${Math.floor(age)} d ago, under min-age-days (${config["min-age-days"]})`, { issue, pr }); + } + const rules = await call("GET", `/repos/${repo}/rules/branches/${refPath(name)}`); + if (!Array.isArray(rules)) return report("the rules for it could not be read", { issue, pr }); + if (rules.length > 0) return keep("a ruleset applies to it", { issue, pr }); + return { verdict: "delete", rule, issue, pr, ageDays: Math.floor(age) }; +} + +// Right before the delete: the tip still at the listed SHA, and no open pull +// request from it or into it. +async function stillDeletable(call, { repo, owner }, { name, sha }) { + const ref = await call("GET", `/repos/${repo}/git/ref/heads/${refPath(name)}`); + if (ref?.object?.sha !== sha) return `the tip moved to ${ref?.object?.sha ?? "nothing"} since the listing`; + const from = await call("GET", `/repos/${repo}/pulls?state=open&head=${encodeURIComponent(`${owner}:${name}`)}&per_page=1`); + if (!Array.isArray(from) || from.length > 0) return "a pull request from it is open"; + const into = await call("GET", `/repos/${repo}/pulls?state=open&base=${encodeURIComponent(name)}&per_page=1`); + if (!Array.isArray(into) || into.length > 0) return "a pull request into it is open"; + return null; +} + +const row = (cells) => `| ${cells.map((c) => String(c ?? "").replaceAll("|", "\\|")).join(" | ")} |`; + +export async function cleanup(env, { call, clock, loadConfig, append, log = console.log }) { + const repo = env.REPOSITORY; + if (!/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repo ?? "")) throw new Error("REPOSITORY is not owner/repo"); + const dryRun = env.DRY_RUN !== "false"; + const config = resolveBranchCleanup(loadConfig()); + const owner = repo.split("/")[0]; + const bot = env.LANES_BOT_LOGIN || "melodic-automation-lanes[bot]"; + const now = clock(); + const defaultBranch = (await call("GET", `/repos/${repo}`))?.default_branch; + if (typeof defaultBranch !== "string" || defaultBranch === "") throw new Error("the default branch could not be read"); + const context = { repo, owner, bot, defaultBranch, config, now }; + + const branches = await pages(call, `/repos/${repo}/branches`); + const results = []; + for (const branch of branches) { + const name = branch.name; + const sha = branch.commit?.sha; + const skip = excluded(name, defaultBranch); + if (skip) { + results.push({ name, sha, verdict: "excluded", reason: skip }); + continue; + } + try { + results.push({ name, sha, ...(await evaluate(call, context, { name, sha, protected: branch.protected })) }); + } catch (error) { + results.push({ name, sha, verdict: "report", reason: `a read failed: ${error.message}` }); + } + } + + // Oldest first, so a capped run deletes the longest-closed branches. + const candidates = results.filter((r) => r.verdict === "delete").sort((a, b) => b.ageDays - a.ageDays); + const cap = config["max-deletes-per-run"]; + const planned = candidates.slice(0, cap); + log( + `${dryRun ? "dry-run: would delete" : "will delete"} ${planned.length} of ${candidates.length} deletable branch(es) (cap ${cap}); ${branches.length} branch(es) listed`, + ); + + const deleted = []; + const skipped = []; + for (const candidate of planned) { + if (dryRun) continue; + let reason; + try { + reason = await stillDeletable(call, context, candidate); + } catch (error) { + reason = `a re-read failed: ${error.message}`; + } + if (reason) { + skipped.push({ ...candidate, reason }); + continue; + } + const entry = { + repo, + branch: candidate.name, + sha: candidate.sha, + rule: candidate.rule, + issue: candidate.issue ?? null, + pr: candidate.pr ?? null, + deletedAt: clock(), + }; + append(entry); + try { + await call("DELETE", `/repos/${repo}/git/refs/heads/${refPath(candidate.name)}`); + deleted.push(entry); + } catch (error) { + skipped.push({ ...candidate, reason: `the delete failed: ${error.message}` }); + } + } + + const count = (verdict) => results.filter((r) => r.verdict === verdict).length; + const lines = [ + `### Branch cleanup${dryRun ? " (dry-run: nothing deleted)" : ""}`, + "", + `${branches.length} branches: ${candidates.length} deletable, ${count("keep")} kept, ${count("report")} for a person, ${count("excluded")} excluded. min-age-days ${config["min-age-days"]}, cap ${cap}.`, + "", + dryRun ? `#### Would delete (${planned.length})` : `#### Deleted (${deleted.length})`, + "", + row(["Branch", "SHA", "Rule", "Issue", "PR", "Closed (days)"]), + row(["---", "---", "---", "---", "---", "---"]), + ...(dryRun ? planned : planned.filter((c) => deleted.some((d) => d.branch === c.name))).map((c) => + row([`\`${c.name}\``, c.sha.slice(0, 12), c.rule, c.issue ? `#${c.issue}` : "", c.pr ? `#${c.pr}` : "", c.ageDays]), + ), + ]; + if (candidates.length > planned.length) { + lines.push("", `${candidates.length - planned.length} more deletable branch(es) wait for the next run (cap ${cap}).`); + } + if (skipped.length > 0) { + lines.push("", "#### Skipped at delete time", "", ...skipped.map((s) => `- \`${s.name}\`: ${s.reason}`)); + } + lines.push( + "", + `#### For a person (${count("report")})`, + "", + ...results.filter((r) => r.verdict === "report").map((r) => `- \`${r.name}\`: ${r.reason}`), + "", + `#### Kept (${count("keep")})`, + "", + ...results.filter((r) => r.verdict === "keep").map((r) => `- \`${r.name}\`: ${r.reason}`), + ); + return { dryRun, results, candidates, planned, deleted, skipped, summary: lines.join("\n") }; +} + +async function main(env = process.env) { + const { branchCleanupFromBase } = await import( + pathToFileURL(`${env.BASE_PATH}/.github/actions/resolve-config/resolve-config.mjs`).href + ); + const dryRun = env.DRY_RUN !== "false"; + const result = await cleanup(env, { + call: client(env, { dryRun }), + clock: () => new Date().toISOString(), + loadConfig: () => branchCleanupFromBase({ BASE_PATH: env.BASE_PATH }), + append: (entry) => { + const line = `${JSON.stringify(entry)}\n`; + appendFileSync(env.LEDGER_PATH, line); + if (env.GITHUB_STEP_SUMMARY) appendFileSync(env.GITHUB_STEP_SUMMARY, `ledger: \`${line.trim()}\`\n\n`); + }, + }); + if (env.GITHUB_STEP_SUMMARY) appendFileSync(env.GITHUB_STEP_SUMMARY, `${result.summary}\n`); + else console.log(result.summary); + if (result.skipped.some((s) => s.reason.startsWith("the delete failed"))) process.exitCode = 1; +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { + main().catch((error) => { + console.log(`::error::${error.message}`); + process.exitCode = 1; + }); +} diff --git a/.github/scripts/branch-cleanup/cleanup.test.mjs b/.github/scripts/branch-cleanup/cleanup.test.mjs new file mode 100644 index 0000000000..dbe758416f --- /dev/null +++ b/.github/scripts/branch-cleanup/cleanup.test.mjs @@ -0,0 +1,275 @@ +// cleanup() against a fake GitHub: hand-built branches, pull requests and +// issues, with each expected verdict taken from the branch-cleanup rule +// (docs/conventions/pr-pipeline/README.md, "Branch cleanup"). +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { cleanup } from "./cleanup.mjs"; +import { linkedIssues } from "./rules.mjs"; + +const REPO = "melodic-software/claude-code-plugins"; +const BOT = "melodic-automation-lanes[bot]"; +const NOW = "2026-10-21T12:00:00Z"; +const OLD = "2026-09-01T00:00:00Z"; // 50 days before NOW +const RECENT = "2026-10-15T00:00:00Z"; // 6 days before NOW +const sha = (c) => c.repeat(40); + +// `branches` are { name, sha, protected }; `pulls` are keyed by branch name; +// `issues` and `comments` by number; `ahead` maps a SHA to its commits ahead +// of main; `rules` maps a branch to its ruleset rules; `moves` maps a branch +// to the tip a re-read returns; `openLater` names branches whose open PR +// appears only at the delete-time re-read. +function world({ branches, pulls = {}, issues = {}, comments = {}, ahead = {}, rules = {}, moves = {}, openLater = [] }) { + const writes = []; + const call = async (method, path) => { + if (method !== "GET") { + writes.push({ method, path }); + return null; + } + const url = new URL(`https://api.test${path}`); + const p = url.pathname; + const q = url.searchParams; + let m; + if (p === `/repos/${REPO}`) return { default_branch: "main" }; + if (p === `/repos/${REPO}/branches`) { + return q.get("page") === "1" ? branches.map((b) => ({ name: b.name, protected: !!b.protected, commit: { sha: b.sha } })) : []; + } + if (p === `/repos/${REPO}/pulls`) { + if (q.get("base")) return []; + const name = q.get("head").slice("melodic-software:".length); + const list = pulls[name] ?? []; + if (q.get("state") === "open") return openLater.includes(name) ? [{ number: 999, state: "open" }] : list.filter((x) => x.state === "open"); + return q.get("page") === "1" ? list : []; + } + if ((m = /^\/repos\/[^/]+\/[^/]+\/issues\/(\d+)$/.exec(p))) return issues[m[1]] ?? null; + if ((m = /^\/repos\/[^/]+\/[^/]+\/issues\/(\d+)\/comments$/.exec(p))) return q.get("page") === "1" ? (comments[m[1]] ?? []) : []; + if ((m = /^\/repos\/[^/]+\/[^/]+\/compare\/main\.\.\.([0-9a-f]{40})$/.exec(p))) return { ahead_by: ahead[m[1]] ?? 0 }; + if ((m = /^\/repos\/[^/]+\/[^/]+\/rules\/branches\/(.+)$/.exec(p))) return rules[decodeURIComponent(m[1])] ?? []; + if ((m = /^\/repos\/[^/]+\/[^/]+\/git\/ref\/heads\/(.+)$/.exec(p))) { + const name = m[1].split("/").map(decodeURIComponent).join("/"); + const branch = branches.find((b) => b.name === name); + return { object: { sha: moves[name] ?? branch.sha } }; + } + throw new Error(`unexpected GET ${path}`); + }; + return { call, writes }; +} + +async function run(w, { dryRun = false, config = {} } = {}) { + const ledger = []; + const result = await cleanup( + { REPOSITORY: REPO, DRY_RUN: dryRun ? "true" : "false", LANES_BOT_LOGIN: BOT }, + { call: w.call, clock: () => NOW, loadConfig: () => config, append: (e) => ledger.push(e), log: () => {} }, + ); + const verdict = (name) => result.results.find((r) => r.name === name); + return { ...result, ledger, verdict, deletes: w.writes.filter((x) => x.method === "DELETE").map((x) => x.path) }; +} + +const closedIssue = (closedAt) => ({ state: "closed", closed_at: closedAt }); +const openIssue = { state: "open", closed_at: null }; +const mergedPull = (number, head, at, body = "") => ({ + number, + state: "closed", + merged_at: at, + closed_at: at, + created_at: at, + head: { sha: head }, + body, +}); +const deletePath = (name) => `/repos/${REPO}/git/refs/heads/${name}`; +const lane = (issue) => `work-resolve-issue/issue-${issue}-100-1`; + +test("a lane branch whose issue is open is kept, however old its merged PR", async () => { + const name = lane(10); + const w = world({ + branches: [{ name, sha: sha("a") }], + pulls: { [name]: [mergedPull(1, sha("a"), OLD)] }, + issues: { 10: openIssue }, + }); + const r = await run(w); + assert.equal(r.verdict(name).verdict, "keep"); + assert.deepEqual(r.deletes, []); +}); + +test("a stopped lane run's branch is never deleted, even after its issue closed", async () => { + const name = lane(11); + const w = world({ + branches: [{ name, sha: sha("b") }], + issues: { 11: closedIssue(OLD) }, + comments: { + 11: [{ user: { login: BOT }, body: "\nPartial work is on the branch." }], + }, + }); + const r = await run(w); + assert.equal(r.verdict(name).verdict, "report"); + assert.deepEqual(r.deletes, []); +}); + +test("a stopped-run marker from someone other than the lanes App protects nothing", async () => { + const name = lane(12); + const w = world({ + branches: [{ name, sha: sha("c") }], + issues: { 12: closedIssue(OLD) }, + comments: { 12: [{ user: { login: "someone" }, body: "" }] }, + }); + const r = await run(w); + assert.equal(r.verdict(name).verdict, "delete"); + assert.deepEqual(r.deletes, [deletePath(name)]); +}); + +test("a lane branch with no PR is kept while its issue closed under 14 days ago, deleted at or past it", async () => { + const young = lane(13); + const old = lane(14); + const w = world({ + branches: [ + { name: young, sha: sha("d") }, + { name: old, sha: sha("e") }, + ], + issues: { 13: closedIssue(RECENT), 14: closedIssue(OLD) }, + }); + const r = await run(w); + assert.equal(r.verdict(young).verdict, "keep"); + assert.equal(r.verdict(old).verdict, "delete"); + assert.equal(r.verdict(old).rule, "lane-issue-closed"); + assert.deepEqual(r.deletes, [deletePath(old)]); + assert.deepEqual(r.ledger, [ + { repo: REPO, branch: old, sha: sha("e"), rule: "lane-issue-closed", issue: 14, pr: null, deletedAt: NOW }, + ]); +}); + +test("the age counts from the later of PR close and issue close", async () => { + const name = lane(15); + const w = world({ + branches: [{ name, sha: sha("f") }], + pulls: { [name]: [mergedPull(2, sha("f"), OLD)] }, + issues: { 15: closedIssue(RECENT) }, + }); + const r = await run(w); + assert.equal(r.verdict(name).verdict, "keep"); +}); + +test("a branch with an open PR is kept", async () => { + const w = world({ + branches: [{ name: "feat/open", sha: sha("1") }], + pulls: { "feat/open": [{ number: 3, state: "open", created_at: OLD, head: { sha: sha("1") } }] }, + }); + const r = await run(w); + assert.equal(r.verdict("feat/open").verdict, "keep"); + assert.deepEqual(r.deletes, []); +}); + +test("a merged PR's branch is deleted; one whose tip moved is deleted only with no commits ahead", async () => { + const w = world({ + branches: [ + { name: "feat/merged", sha: sha("2") }, + { name: "feat/moved-empty", sha: sha("3") }, + { name: "feat/moved-ahead", sha: sha("4") }, + ], + pulls: { + "feat/merged": [mergedPull(4, sha("2"), OLD)], + "feat/moved-empty": [mergedPull(5, sha("9"), OLD)], + "feat/moved-ahead": [mergedPull(6, sha("8"), OLD)], + }, + ahead: { [sha("3")]: 0, [sha("4")]: 2 }, + }); + const r = await run(w); + assert.equal(r.verdict("feat/merged").rule, "pr-merged"); + assert.equal(r.verdict("feat/moved-empty").rule, "pr-merged-no-commits-ahead"); + assert.equal(r.verdict("feat/moved-ahead").verdict, "report"); + assert.deepEqual(r.deletes.sort(), [deletePath("feat/merged"), deletePath("feat/moved-empty")]); +}); + +test("a non-lane branch is kept while an issue its last PR body links is open", async () => { + const w = world({ + branches: [{ name: "fix/linked", sha: sha("5") }], + pulls: { "fix/linked": [mergedPull(7, sha("5"), OLD, "Refs: #20\n\nCloses #21")] }, + issues: { 20: closedIssue(OLD), 21: openIssue }, + }); + const r = await run(w); + assert.equal(r.verdict("fix/linked").verdict, "keep"); +}); + +test("a human branch that never had a PR, and an unreadable issue, are reported, never deleted", async () => { + const w = world({ + branches: [ + { name: "wip/idea", sha: sha("6") }, + { name: lane(30), sha: sha("7") }, + ], + }); + const r = await run(w); + assert.equal(r.verdict("wip/idea").verdict, "report"); + assert.equal(r.verdict(lane(30)).verdict, "report"); + assert.deepEqual(r.deletes, []); +}); + +test("excluded, protected and ruleset branches are never deleted", async () => { + const old = (s) => [mergedPull(8, s, OLD)]; + const w = world({ + branches: [ + { name: "main", sha: sha("a") }, + { name: "release/plugins", sha: sha("b") }, + { name: "dependabot/npm/x", sha: sha("c") }, + { name: "gh-readonly-queue/main/pr-1", sha: sha("d") }, + { name: "feat/protected", sha: sha("e"), protected: true }, + { name: "feat/ruled", sha: sha("f") }, + ], + pulls: { + "release/plugins": old(sha("b")), + "feat/protected": old(sha("e")), + "feat/ruled": old(sha("f")), + }, + rules: { "feat/ruled": [{ type: "deletion" }] }, + }); + const r = await run(w); + assert.deepEqual(r.deletes, []); + assert.equal(r.verdict("feat/ruled").verdict, "keep"); + assert.equal(r.verdict("feat/protected").verdict, "keep"); +}); + +test("a branch whose tip moved, or that gained an open PR, between listing and delete is skipped", async () => { + const w = world({ + branches: [ + { name: "feat/moved", sha: sha("1") }, + { name: "feat/reopened", sha: sha("2") }, + ], + pulls: { + "feat/moved": [mergedPull(9, sha("1"), OLD)], + "feat/reopened": [mergedPull(10, sha("2"), OLD)], + }, + moves: { "feat/moved": sha("9") }, + openLater: ["feat/reopened"], + }); + const r = await run(w); + assert.deepEqual(r.deletes, []); + assert.deepEqual(r.ledger, []); + assert.deepEqual(r.skipped.map((s) => s.name).sort(), ["feat/moved", "feat/reopened"]); +}); + +test("the per-run cap is honored, oldest-closed first", async () => { + const branches = []; + const pulls = {}; + for (let i = 1; i <= 4; i += 1) { + const name = `feat/b${i}`; + branches.push({ name, sha: sha(String(i)) }); + pulls[name] = [mergedPull(i, sha(String(i)), `2026-0${i}-01T00:00:00Z`)]; + } + const r = await run(world({ branches, pulls }), { config: { "max-deletes-per-run": 2 } }); + assert.equal(r.candidates.length, 4); + assert.deepEqual(r.deletes, [deletePath("feat/b1"), deletePath("feat/b2")]); +}); + +test("dry-run deletes nothing and writes no ledger line", async () => { + const w = world({ + branches: [{ name: "feat/merged", sha: sha("2") }], + pulls: { "feat/merged": [mergedPull(4, sha("2"), OLD)] }, + }); + const r = await run(w, { dryRun: true }); + assert.equal(r.planned.length, 1); + assert.deepEqual(w.writes, []); + assert.deepEqual(r.ledger, []); +}); + +test("linkedIssues reads same-repository closing and Refs links only", () => { + const body = `Closes #1, fixes: #2\nResolves ${REPO}#3\nRefs https://github.com/${REPO}/issues/4\nCloses other/repo#5\nsee #6`; + assert.deepEqual(linkedIssues(body, REPO), [1, 2, 3, 4]); +}); diff --git a/.github/scripts/branch-cleanup/package.json b/.github/scripts/branch-cleanup/package.json new file mode 100644 index 0000000000..f2a304ee68 --- /dev/null +++ b/.github/scripts/branch-cleanup/package.json @@ -0,0 +1,7 @@ +{ + "name": "branch-cleanup", + "private": true, + "scripts": { + "test": "node --test" + } +} diff --git a/.github/scripts/branch-cleanup/rules.mjs b/.github/scripts/branch-cleanup/rules.mjs new file mode 100644 index 0000000000..3db017792c --- /dev/null +++ b/.github/scripts/branch-cleanup/rules.mjs @@ -0,0 +1,74 @@ +// The branch-cleanup rule as pure functions: which branches the job never +// touches, which issues a branch is linked to, and whether a closed branch is +// old enough to delete. docs/conventions/pr-pipeline/README.md, "Branch +// cleanup", states the rule; cleanup.mjs reads GitHub and calls these. +import { DIMENSION_STEPS } from "../work-resolve-issue/report-outcome.mjs"; + +// The `branch-cleanup` leaf of pr-pipeline.yaml (pr-pipeline.schema.json); +// the one copy of its defaults. +export const BRANCH_CLEANUP_DEFAULTS = Object.freeze({ + "min-age-days": 14, + "max-deletes-per-run": 25, +}); + +const VALID = { + "min-age-days": (value) => Number.isInteger(value) && value >= 1 && value <= 365, + "max-deletes-per-run": (value) => Number.isInteger(value) && value >= 0 && value <= 100, +}; + +// Every leaf, each valid set value over its default. +export const resolveBranchCleanup = (leaf) => + Object.fromEntries( + Object.entries(BRANCH_CLEANUP_DEFAULTS).map(([key, value]) => [key, VALID[key](leaf?.[key]) ? leaf[key] : value]), + ); + +const EXCLUDED_PREFIXES = ["release/", "dependabot/", "gh-readonly-queue/"]; + +// Why the job never looks at a branch, or null. +export function excluded(name, defaultBranch) { + if (name === defaultBranch) return "the default branch"; + const prefix = EXCLUDED_PREFIXES.find((p) => name.startsWith(p)); + return prefix ? `a ${prefix}* branch` : null; +} + +// {issue, run} for a work-resolve-issue lane branch +// (work-resolve-issue/issue---), else null. +const LANE = /^work-resolve-issue\/issue-([1-9][0-9]*)-([1-9][0-9]*)-([1-9][0-9]*)$/; +export function laneBranch(name) { + const match = LANE.exec(name); + return match ? { issue: Number(match[1]), run: match[2] } : null; +} + +// Issue numbers in this repository a pull request body links with Closes, +// Fixes, Resolves or Refs (any case, optional colon), as `#n`, +// `owner/repo#n` or an issue URL. A link to another repository is ignored. +export function linkedIssues(body, repo) { + const escaped = repo.replaceAll(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const ref = `(?:#|${escaped}#|https://github\\.com/${escaped}/issues/)([1-9][0-9]*)\\b`; + const pattern = new RegExp(`\\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|refs?)\\b:?\\s+${ref}`, "gi"); + return [...new Set([...String(body ?? "").matchAll(pattern)].map((m) => Number(m[1])))]; +} + +// Whether a lanes-App outcome marker on the issue records a stopped run +// (needs a human, blocked, limited, funding, turn-cap) whose work may sit on +// this lane branch. A marker naming this run, naming no run, or left by a +// resumed run that kept no branch of its own (its work is on an earlier lane +// branch) counts; a dimension skip ran nothing and does not. +export function stoppedRun(markers, run) { + return markers.some( + (m) => + !Object.hasOwn(DIMENSION_STEPS, m.outcome) && (m.run === run || m.run === "" || m.run === "0" || m.onResumed), + ); +} + +const DAY_MS = 86_400_000; + +// Days since the latest of `times` (ISO strings), or null when one is unreadable. +export function ageDays(times, now) { + const parsed = times.map((t) => Date.parse(t ?? "")); + if (parsed.length === 0 || parsed.some((t) => !Number.isFinite(t))) return null; + return (Date.parse(now) - Math.max(...parsed)) / DAY_MS; +} + +// The branch's pull request the rule reads: the newest by creation. +export const lastPull = (pulls) => [...pulls].sort((a, b) => String(b.created_at).localeCompare(String(a.created_at)))[0] ?? null; diff --git a/.github/scripts/branch-cleanup/workflow.test.mjs b/.github/scripts/branch-cleanup/workflow.test.mjs new file mode 100644 index 0000000000..f4947727c5 --- /dev/null +++ b/.github/scripts/branch-cleanup/workflow.test.mjs @@ -0,0 +1,43 @@ +// maintenance-delete-stale-branches.yml's hardening (README.md, "Branch +// cleanup"): the kill switch stops it before it reads, its token writes +// contents only, a manual run is a dry-run by default, and the ledger is +// uploaded even after a failed step. +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { test } from "node:test"; + +const text = readFileSync(new URL("../../workflows/maintenance-delete-stale-branches.yml", import.meta.url), "utf8"); + +test("the job's token writes contents and reads issues and pull requests, nothing else", () => { + assert.match(text, /\npermissions: \{\}\n/); + const block = /\n {4}permissions:\n((?: {6}.+\n)+)/.exec(text)[1]; + const grants = block.trim().split("\n").map((line) => line.trim().replace(/ #.*$/, "")).sort(); + assert.deepEqual(grants, ["contents: write", "issues: read", "pull-requests: read"]); +}); + +test("the kill switch gates every step that reads or deletes", () => { + assert.match(text, /uses: \.\/\.base\/\.github\/actions\/check-kill-switch/); + for (const step of ["Install resolve-config's dependencies", "Delete stale branches", "Upload the ledger"]) { + const body = text.slice(text.indexOf(`- name: ${step}`)).split("\n - name:")[0]; + assert.match(body, /steps\.kill\.outputs\.proceed == 'true'/, step); + } +}); + +test("a manual run is a dry-run unless set, and the schedule deletes", () => { + assert.match(text, /dry-run:\n {8}description: .+\n {8}type: boolean\n {8}default: true\n/); + assert.match( + text, + /DRY_RUN: \$\{\{ github\.event_name == 'workflow_dispatch' && format\('\{0\}', inputs\.dry-run\) \|\| 'false' \}\}/, + ); +}); + +test("the ledger artifact uploads after a failed step too", () => { + const body = text.slice(text.indexOf("- name: Upload the ledger")); + assert.match(body, /if: always\(\) && steps\.kill\.outputs\.proceed == 'true'/); + assert.match(body, /path: \$\{\{ runner\.temp \}\}\/branch-cleanup-ledger\.jsonl/); +}); + +test("it runs only on main and checks out no branch code", () => { + assert.match(text, /if: github\.ref == 'refs\/heads\/main'/); + assert.match(text, /ref: \$\{\{ github\.sha \}\}\n {10}path: \.base\n {10}persist-credentials: false/); +}); diff --git a/.github/workflows/maintenance-delete-stale-branches.yml b/.github/workflows/maintenance-delete-stale-branches.yml new file mode 100644 index 0000000000..e74a2dc7f3 --- /dev/null +++ b/.github/workflows/maintenance-delete-stale-branches.yml @@ -0,0 +1,110 @@ +name: maintenance-delete-stale-branches + +# Deletes the branches of this repository whose work is finished, so no +# non-default branch lives forever (owner decision W-7). Daily it sorts every +# branch by the branch-cleanup rule (docs/conventions/pr-pipeline/README.md, +# "Branch cleanup"): a branch whose pull request closed or merged and whose +# linked issue closed at least `branch-cleanup.min-age-days` ago is deleted; +# one with an open issue, an open pull request, a stopped lane run, a +# protection or ruleset, or that is the default, release/*, dependabot/* or +# gh-readonly-queue/* branch is never deleted; anything else is listed for a +# person. It prints how many it will delete first and deletes at most +# `branch-cleanup.max-deletes-per-run`. Before each delete it re-reads the tip +# and the open pull requests, and appends {repo, branch, sha, rule, issue, pr, +# deletedAt} to the ledger: the step summary and the branch-cleanup-ledger +# artifact. A deleted branch is restored from its ledger SHA with +# `git push origin :refs/heads/`. No model, and no branch code is +# checked out. Rules: .github/scripts/branch-cleanup/. +# +# The schedule deletes; a manual run is a dry-run unless dry-run is set to +# false. How to stop it: set the organization or repository lanes kill-switch +# variable to anything but "false". + +on: + schedule: + - cron: "17 6 * * *" + workflow_dispatch: + inputs: + dry-run: + description: "true: report what would be deleted and delete nothing" + type: boolean + default: true + +permissions: {} + +concurrency: + group: maintenance-delete-stale-branches + cancel-in-progress: false + +defaults: + run: + shell: bash + +jobs: + clean: + if: github.ref == 'refs/heads/main' + runs-on: ubuntu-24.04 + timeout-minutes: 20 + permissions: + contents: write # list branches, compare, delete a branch ref + issues: read # read linked issues and the lanes App's outcome markers + pull-requests: read # read each branch's pull requests + steps: + - name: Check out the run's main commit + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.sha }} + path: .base + persist-credentials: false + + - name: Set up Node + uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version-file: .base/.node-version + package-manager-cache: false + + - name: Check the kill switch + id: kill + uses: ./.base/.github/actions/check-kill-switch # zizmor: ignore[self-repository] + with: + value: ${{ vars.CLAUDE_LANES_DISABLED }} + repo-value: ${{ vars.CLAUDE_LANES_REPO_DISABLED }} + + - name: Report the kill switch + if: steps.kill.outputs.proceed != 'true' + env: + REASON: ${{ steps.kill.outputs.reason }} + run: echo "::notice::the lanes kill switch stops this run (reason ${REASON:-unreadable}); nothing read or deleted" + + # cleanup.mjs reads the branch-cleanup leaf through the base's + # resolve-config, which needs its dependencies. + - name: Install resolve-config's dependencies + if: steps.kill.outputs.proceed == 'true' + run: npm ci --ignore-scripts --omit=dev --prefix .base/.github/actions/resolve-config + + # A workflow token's REST limit is per repository per hour + # (https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api); + # 600 calls covers about three reads for each of 200 branches. + - name: Delete stale branches + if: steps.kill.outputs.proceed == 'true' + env: + GITHUB_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + BASE_PATH: .base + CALL_BUDGET: "600" + DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && format('{0}', inputs.dry-run) || 'false' }} + LEDGER_PATH: ${{ runner.temp }}/branch-cleanup-ledger.jsonl + run: | + : > "$LEDGER_PATH" + node .base/.github/scripts/branch-cleanup/cleanup.mjs + + # Uploaded after a failed step too, so every line written before a + # delete survives. + - name: Upload the ledger + if: always() && steps.kill.outputs.proceed == 'true' + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: branch-cleanup-ledger-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ runner.temp }}/branch-cleanup-ledger.jsonl + if-no-files-found: ignore + retention-days: 90 diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index bd66e333cd..a0299cba2b 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -543,6 +543,16 @@ red-main: # (16) plus queue time. confirm-wait-minutes: 40 +# The daily branch cleanup (owner decision W-7, 2026-10-21): no non-default +# branch lives forever. maintenance-delete-stale-branches.yml deletes a branch +# whose pull request closed and whose linked issue closed at least +# min-age-days ago, and never one with an open issue, an open pull request +# or a stopped lane run on it. +branch-cleanup: + min-age-days: 14 + # A run deletes at most this many, oldest-closed first. + max-deletes-per-run: 25 + merge: # The lane treats every PR the lanes App opened for one closing issue as # class C2 (plugins/autonomy/reference/guardrails/work-classes.md) and reads diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 7535292d9e..3983de966d 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -20,6 +20,7 @@ - [Limits](#limits) - [WIP cap](#wip-cap) - [Red main](#red-main) +- [Branch cleanup](#branch-cleanup) - [Planning lane work](#planning-lane-work) - [Changing this config](#changing-this-config) - [Outside this repository](#outside-this-repository) @@ -1576,6 +1577,45 @@ calls, and stops on any failed read. A range of more than 25 merges pins no culp sent. Owner decision 2026-10-20: `dry-run` stays true for 3 days of reports with no wrong blame before any revert merges. +## Branch cleanup + +No non-default branch lives forever (owner decision W-7, 2026-10-21). +[`maintenance-delete-stale-branches.yml`](../../../.github/workflows/maintenance-delete-stale-branches.yml) +runs daily and sorts every branch; its rules and tests are in +[`.github/scripts/branch-cleanup/`](../../../.github/scripts/branch-cleanup/). It runs no model. + +1. **Never touched.** The default branch, `release/*`, `dependabot/*` and `gh-readonly-queue/*`. +2. **Kept.** A protected branch or one a ruleset applies to; one with an open pull request; a lane + branch (`work-resolve-issue/issue--...`) while issue `n` is open; any other branch while an + issue its newest pull request's body links with `Closes`, `Fixes`, `Resolves` or `Refs` is + open; and a branch whose pull request or issue closed under `min-age-days` ago. +3. **Never deleted, reported.** A lane branch of a stopped run: a lanes-App outcome marker on its + issue names its run, names no run, or comes from a resumed run (needs a human, blocked, + limited, funding, turn-cap), even after the issue closed, since the next run resumes from it. + Also a branch that never had a pull request, a merged branch with commits ahead of the default + branch after its tip moved, a closed-unmerged branch whose tip moved, and any branch a read + failed on. The step summary lists them for a person. +4. **Deleted.** The newest pull request closed unmerged or merged at the branch tip, or merged + with the tip moved and 0 commits ahead of the default branch, and every linked issue closed; + or a lane branch with no pull request whose issue closed. The later of those closes is at + least `min-age-days` ago. + +The job prints how many it will delete, then deletes at most `max-deletes-per-run`, oldest-closed +first. Before each delete it re-reads the tip and the open pull requests from and into the branch, +skips it when the tip moved or one opened, and appends `{repo, branch, sha, rule, issue, pr, +deletedAt}` to the step summary and the run's `branch-cleanup-ledger--` artifact. The +delete is `DELETE git/refs/heads/` with the job's own `GITHUB_TOKEN` (`contents: write`, +`issues` and `pull-requests` read); the API takes no expected SHA, so the delete also removes a push that lands between the re-read +and the delete; the pusher's own copy restores it. The ledger SHA restores the deleted tip: +`git push origin :refs/heads/`. A scheduled run +deletes; a manual run is a dry-run unless `dry-run` is set to false, and in dry-run the client +refuses every request but `GET`. The kill switch stops it. + +| Key | Default | What it sets | +|---|---|---| +| `min-age-days` | 14 | Days since the later of the pull request and issue closes; 1 to 365. | +| `max-deletes-per-run` | 25 | Deletes per run at most; 0 to 100, and 0 deletes none. | + ## Planning lane work Size the process to the change: diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index 1f4df05118..43cf7ac02f 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -247,6 +247,27 @@ } } }, + "branch-cleanup": { + "type": "object", + "additionalProperties": false, + "description": "The scheduled branch cleanup: how long a closed branch waits before the job deletes it, and how many it deletes in one run. Contract: README.md, Branch cleanup.", + "properties": { + "min-age-days": { + "type": "integer", + "minimum": 1, + "maximum": 365, + "default": 14, + "description": "Days since the later of the branch's pull request closing and its issue closing before the branch is deleted." + }, + "max-deletes-per-run": { + "type": "integer", + "minimum": 0, + "maximum": 100, + "default": 25, + "description": "Branches one run deletes at most, oldest-closed first; the rest wait for the next run. 0 deletes none." + } + } + }, "merge": { "$ref": "#/$defs/merge" }, "models": { "$ref": "#/$defs/models" } }, diff --git a/scripts/outside-node-packages.txt b/scripts/outside-node-packages.txt index 8f1155e138..134e9f39c4 100644 --- a/scripts/outside-node-packages.txt +++ b/scripts/outside-node-packages.txt @@ -16,6 +16,7 @@ .github/actions/seal-lane-output .github/actions/select-trusted-text .github/scripts/apply-patch +.github/scripts/branch-cleanup .github/scripts/intake-reenter .github/scripts/maintenance-audit-lane-runs .github/scripts/maintenance-monitor-signals From e8cc27244880c42bedaf9aebf6cd2f056807382e Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:46:08 -0400 Subject: [PATCH 2/5] test(pr-pipeline): pin the min-age-days boundary in the branch-cleanup suite Co-Authored-By: Claude Opus 5.5 --- .github/scripts/branch-cleanup/cleanup.test.mjs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/scripts/branch-cleanup/cleanup.test.mjs b/.github/scripts/branch-cleanup/cleanup.test.mjs index dbe758416f..2be86a5b20 100644 --- a/.github/scripts/branch-cleanup/cleanup.test.mjs +++ b/.github/scripts/branch-cleanup/cleanup.test.mjs @@ -137,6 +137,13 @@ test("a lane branch with no PR is kept while its issue closed under 14 days ago, ]); }); +test("a branch closed exactly min-age-days ago is deleted", async () => { + const name = lane(16); + const w = world({ branches: [{ name, sha: sha("9") }], issues: { 16: closedIssue("2026-10-07T12:00:00Z") } }); + const r = await run(w); + assert.deepEqual(r.deletes, [deletePath(name)]); +}); + test("the age counts from the later of PR close and issue close", async () => { const name = lane(15); const w = world({ From eaad8a9db1389cbd987c7d2c7443ef492aedf348 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:35:10 -0400 Subject: [PATCH 3/5] feat(pr-pipeline): keep a branch only while its issue is open Owner decision on #8958: no dates. A branch lives as long as its issue is open and is deleted once the issue closes. Remove the min-age-days key and every age check, and the stopped-run keep: a lane branch whose issue closed and that has no open pull request is deletable whatever stop or resume marker the lanes App left. The cap now takes branches in listing order. Everything else holds: open issue or open pull request keeps, merged-with-commits-ahead and never-had-a-PR branches go to a person, exclusions, cap, kill switch, dry-run and ledger. Co-Authored-By: Claude Opus 5.5 --- .../feat-branch-cleanup-w7-663cf524.md | 17 +++-- .github/scripts/branch-cleanup/cleanup.mjs | 47 ++++-------- .../scripts/branch-cleanup/cleanup.test.mjs | 73 ++++++------------- .github/scripts/branch-cleanup/rules.mjs | 38 ++-------- .../maintenance-delete-stale-branches.yml | 19 +++-- docs/conventions/pr-pipeline.yaml | 12 ++- docs/conventions/pr-pipeline/README.md | 24 +++--- .../pr-pipeline/pr-pipeline.schema.json | 11 +-- 8 files changed, 79 insertions(+), 162 deletions(-) diff --git a/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md b/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md index 3251cc1713..5a0454f9c0 100644 --- a/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md +++ b/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md @@ -4,11 +4,12 @@ bump: minor ### Added -- A `branch-cleanup` leaf (`min-age-days`, default 14; `max-deletes-per-run`, default 25) and - the daily maintenance-delete-stale-branches.yml that applies it: a branch whose pull request - closed or merged and whose linked issue closed at least `min-age-days` ago is deleted, oldest - first and at most `max-deletes-per-run` a run, after a re-read of its tip and open pull - requests and a ledger line in the step summary and an artifact. A branch with an open issue - or pull request, a protected or ruleset branch, and a stopped lane run's branch are never - deleted; a branch that never had a pull request, or whose tip moved after its pull request - closed with commits ahead, is listed for a person. A manual run is a dry-run by default. +- A `branch-cleanup` leaf (`max-deletes-per-run`, default 25) and the daily + maintenance-delete-stale-branches.yml that applies it: a branch lives as long as its issue is + open, and one whose pull request closed or merged and whose linked issue closed is deleted, at + most `max-deletes-per-run` a run, after a re-read of its tip and open pull requests and a + ledger line in the step summary and an artifact. A lane run's stop marker does not keep a + branch once its issue closed. A branch with an open issue or pull request, and a protected or + ruleset branch, are never deleted; a branch that never had a pull request, or whose tip moved + after its pull request closed with commits ahead, is listed for a person. A manual run is a + dry-run by default. diff --git a/.github/scripts/branch-cleanup/cleanup.mjs b/.github/scripts/branch-cleanup/cleanup.mjs index 90532e4155..b31ffd689c 100644 --- a/.github/scripts/branch-cleanup/cleanup.mjs +++ b/.github/scripts/branch-cleanup/cleanup.mjs @@ -9,14 +9,13 @@ // // Environment: GITHUB_TOKEN, REPOSITORY, DRY_RUN (`true` or `false`), // BASE_PATH (main's checkout), LEDGER_PATH (JSON Lines file), CALL_BUDGET, -// LANES_BOT_LOGIN, GITHUB_API_URL, GITHUB_STEP_SUMMARY. +// GITHUB_API_URL, GITHUB_STEP_SUMMARY. import { appendFileSync } from "node:fs"; import process from "node:process"; import { setTimeout as delay } from "node:timers/promises"; import { pathToFileURL } from "node:url"; -import { parseOutcomeMarker } from "../work-resolve-issue/report-outcome.mjs"; -import { ageDays, excluded, laneBranch, lastPull, linkedIssues, resolveBranchCleanup, stoppedRun } from "./rules.mjs"; +import { excluded, laneIssue, lastPull, linkedIssues, resolveBranchCleanup } from "./rules.mjs"; export function client(env, { pace = () => delay(200), dryRun = true } = {}) { let used = 0; @@ -53,9 +52,9 @@ async function pages(call, path, max = 20) { const refPath = (name) => name.split("/").map(encodeURIComponent).join("/"); // One branch's verdict: { verdict: delete | keep | report, rule or reason, -// issue, pr, closedAt }. Reads run cheapest first and stop at the first +// issue, pr }. Reads run cheapest first and stop at the first // verdict; a failed read reports the branch. -export async function evaluate(call, { repo, owner, bot, defaultBranch, config, now }, branch) { +export async function evaluate(call, { repo, owner, defaultBranch }, branch) { const { name, sha } = branch; const keep = (reason, extra = {}) => ({ verdict: "keep", reason, ...extra }); const report = (reason, extra = {}) => ({ verdict: "report", reason, ...extra }); @@ -65,21 +64,11 @@ export async function evaluate(call, { repo, owner, bot, defaultBranch, config, if (openPull) return keep("open pull request", { pr: openPull.number }); const pull = lastPull(pulls); const pr = pull?.number ?? null; - const lane = laneBranch(name); - - const closedTimes = []; - let issue = null; - if (lane) { - issue = lane.issue; + let issue = laneIssue(name); + if (issue) { const read = await call("GET", `/repos/${repo}/issues/${issue}`); if (!read) return report("its issue could not be read", { issue, pr }); if (read.state !== "closed") return keep("its issue is open", { issue, pr }); - const markers = (await pages(call, `/repos/${repo}/issues/${issue}/comments`)) - .filter((c) => c.user?.login === bot) - .map((c) => parseOutcomeMarker(c.body)) - .filter(Boolean); - if (stoppedRun(markers, lane.run)) return report("a stopped lane run's branch: a person decides", { issue, pr }); - closedTimes.push(read.closed_at); } else { if (!pull) return report("never had a pull request", {}); for (const number of linkedIssues(pull.body, repo)) { @@ -87,7 +76,6 @@ export async function evaluate(call, { repo, owner, bot, defaultBranch, config, if (!read) return report(`linked issue #${number} could not be read`, { issue: number, pr }); if (read.state !== "closed") return keep(`linked issue #${number} is open`, { issue: number, pr }); issue ??= number; - closedTimes.push(read.closed_at); } } @@ -95,7 +83,6 @@ export async function evaluate(call, { repo, owner, bot, defaultBranch, config, if (!pull) { rule = "lane-issue-closed"; } else { - closedTimes.push(pull.closed_at); const tipMoved = pull.head?.sha !== sha; if (pull.merged_at && !tipMoved) { rule = "pr-merged"; @@ -114,15 +101,10 @@ export async function evaluate(call, { repo, owner, bot, defaultBranch, config, } } - const age = ageDays(closedTimes, now); - if (age === null) return report("a close time could not be read", { issue, pr }); - if (age < config["min-age-days"]) { - return keep(`closed ${Math.floor(age)} d ago, under min-age-days (${config["min-age-days"]})`, { issue, pr }); - } const rules = await call("GET", `/repos/${repo}/rules/branches/${refPath(name)}`); if (!Array.isArray(rules)) return report("the rules for it could not be read", { issue, pr }); if (rules.length > 0) return keep("a ruleset applies to it", { issue, pr }); - return { verdict: "delete", rule, issue, pr, ageDays: Math.floor(age) }; + return { verdict: "delete", rule, issue, pr }; } // Right before the delete: the tip still at the listed SHA, and no open pull @@ -145,11 +127,9 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons const dryRun = env.DRY_RUN !== "false"; const config = resolveBranchCleanup(loadConfig()); const owner = repo.split("/")[0]; - const bot = env.LANES_BOT_LOGIN || "melodic-automation-lanes[bot]"; - const now = clock(); const defaultBranch = (await call("GET", `/repos/${repo}`))?.default_branch; if (typeof defaultBranch !== "string" || defaultBranch === "") throw new Error("the default branch could not be read"); - const context = { repo, owner, bot, defaultBranch, config, now }; + const context = { repo, owner, defaultBranch }; const branches = await pages(call, `/repos/${repo}/branches`); const results = []; @@ -168,8 +148,7 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons } } - // Oldest first, so a capped run deletes the longest-closed branches. - const candidates = results.filter((r) => r.verdict === "delete").sort((a, b) => b.ageDays - a.ageDays); + const candidates = results.filter((r) => r.verdict === "delete"); const cap = config["max-deletes-per-run"]; const planned = candidates.slice(0, cap); log( @@ -212,14 +191,14 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons const lines = [ `### Branch cleanup${dryRun ? " (dry-run: nothing deleted)" : ""}`, "", - `${branches.length} branches: ${candidates.length} deletable, ${count("keep")} kept, ${count("report")} for a person, ${count("excluded")} excluded. min-age-days ${config["min-age-days"]}, cap ${cap}.`, + `${branches.length} branches: ${candidates.length} deletable, ${count("keep")} kept, ${count("report")} for a person, ${count("excluded")} excluded. Cap ${cap}.`, "", dryRun ? `#### Would delete (${planned.length})` : `#### Deleted (${deleted.length})`, "", - row(["Branch", "SHA", "Rule", "Issue", "PR", "Closed (days)"]), - row(["---", "---", "---", "---", "---", "---"]), + row(["Branch", "SHA", "Rule", "Issue", "PR"]), + row(["---", "---", "---", "---", "---"]), ...(dryRun ? planned : planned.filter((c) => deleted.some((d) => d.branch === c.name))).map((c) => - row([`\`${c.name}\``, c.sha.slice(0, 12), c.rule, c.issue ? `#${c.issue}` : "", c.pr ? `#${c.pr}` : "", c.ageDays]), + row([`\`${c.name}\``, c.sha.slice(0, 12), c.rule, c.issue ? `#${c.issue}` : "", c.pr ? `#${c.pr}` : ""]), ), ]; if (candidates.length > planned.length) { diff --git a/.github/scripts/branch-cleanup/cleanup.test.mjs b/.github/scripts/branch-cleanup/cleanup.test.mjs index 2be86a5b20..5a1947ec87 100644 --- a/.github/scripts/branch-cleanup/cleanup.test.mjs +++ b/.github/scripts/branch-cleanup/cleanup.test.mjs @@ -10,8 +10,7 @@ import { linkedIssues } from "./rules.mjs"; const REPO = "melodic-software/claude-code-plugins"; const BOT = "melodic-automation-lanes[bot]"; const NOW = "2026-10-21T12:00:00Z"; -const OLD = "2026-09-01T00:00:00Z"; // 50 days before NOW -const RECENT = "2026-10-15T00:00:00Z"; // 6 days before NOW +const OLD = "2026-09-01T00:00:00Z"; const sha = (c) => c.repeat(40); // `branches` are { name, sha, protected }; `pulls` are keyed by branch name; @@ -58,7 +57,7 @@ function world({ branches, pulls = {}, issues = {}, comments = {}, ahead = {}, r async function run(w, { dryRun = false, config = {} } = {}) { const ledger = []; const result = await cleanup( - { REPOSITORY: REPO, DRY_RUN: dryRun ? "true" : "false", LANES_BOT_LOGIN: BOT }, + { REPOSITORY: REPO, DRY_RUN: dryRun ? "true" : "false" }, { call: w.call, clock: () => NOW, loadConfig: () => config, append: (e) => ledger.push(e), log: () => {} }, ); const verdict = (name) => result.results.find((r) => r.name === name); @@ -91,68 +90,42 @@ test("a lane branch whose issue is open is kept, however old its merged PR", asy assert.deepEqual(r.deletes, []); }); -test("a stopped lane run's branch is never deleted, even after its issue closed", async () => { +// Owner decision on #8958: a branch lives while its issue is open and is +// deleted once it closes, whatever the lanes App's outcome markers say. +const stopMarker = { user: { login: BOT }, body: "\nPartial work is on the branch." }; + +test("a stopped lane run's branch is kept while its issue is open", async () => { const name = lane(11); - const w = world({ - branches: [{ name, sha: sha("b") }], - issues: { 11: closedIssue(OLD) }, - comments: { - 11: [{ user: { login: BOT }, body: "\nPartial work is on the branch." }], - }, - }); + const w = world({ branches: [{ name, sha: sha("b") }], issues: { 11: openIssue }, comments: { 11: [stopMarker] } }); const r = await run(w); - assert.equal(r.verdict(name).verdict, "report"); + assert.equal(r.verdict(name).verdict, "keep"); assert.deepEqual(r.deletes, []); }); -test("a stopped-run marker from someone other than the lanes App protects nothing", async () => { +test("a stopped lane run's branch is deleted once its issue closes", async () => { const name = lane(12); - const w = world({ - branches: [{ name, sha: sha("c") }], - issues: { 12: closedIssue(OLD) }, - comments: { 12: [{ user: { login: "someone" }, body: "" }] }, - }); + const w = world({ branches: [{ name, sha: sha("c") }], issues: { 12: closedIssue(NOW) }, comments: { 12: [stopMarker] } }); const r = await run(w); - assert.equal(r.verdict(name).verdict, "delete"); + assert.equal(r.verdict(name).rule, "lane-issue-closed"); assert.deepEqual(r.deletes, [deletePath(name)]); }); -test("a lane branch with no PR is kept while its issue closed under 14 days ago, deleted at or past it", async () => { - const young = lane(13); - const old = lane(14); - const w = world({ - branches: [ - { name: young, sha: sha("d") }, - { name: old, sha: sha("e") }, - ], - issues: { 13: closedIssue(RECENT), 14: closedIssue(OLD) }, - }); +test("a lane branch with no PR is deleted the run after its issue closes, with a ledger line", async () => { + const name = lane(14); + const w = world({ branches: [{ name, sha: sha("e") }], issues: { 14: closedIssue(NOW) } }); const r = await run(w); - assert.equal(r.verdict(young).verdict, "keep"); - assert.equal(r.verdict(old).verdict, "delete"); - assert.equal(r.verdict(old).rule, "lane-issue-closed"); - assert.deepEqual(r.deletes, [deletePath(old)]); + assert.deepEqual(r.deletes, [deletePath(name)]); assert.deepEqual(r.ledger, [ - { repo: REPO, branch: old, sha: sha("e"), rule: "lane-issue-closed", issue: 14, pr: null, deletedAt: NOW }, + { repo: REPO, branch: name, sha: sha("e"), rule: "lane-issue-closed", issue: 14, pr: null, deletedAt: NOW }, ]); }); -test("a branch closed exactly min-age-days ago is deleted", async () => { - const name = lane(16); - const w = world({ branches: [{ name, sha: sha("9") }], issues: { 16: closedIssue("2026-10-07T12:00:00Z") } }); +test("a branch whose PR closed unmerged at its tip, linking no issue, is deleted", async () => { + const pull = { ...mergedPull(2, sha("f"), NOW), merged_at: null }; + const w = world({ branches: [{ name: "feat/abandoned", sha: sha("f") }], pulls: { "feat/abandoned": [pull] } }); const r = await run(w); - assert.deepEqual(r.deletes, [deletePath(name)]); -}); - -test("the age counts from the later of PR close and issue close", async () => { - const name = lane(15); - const w = world({ - branches: [{ name, sha: sha("f") }], - pulls: { [name]: [mergedPull(2, sha("f"), OLD)] }, - issues: { 15: closedIssue(RECENT) }, - }); - const r = await run(w); - assert.equal(r.verdict(name).verdict, "keep"); + assert.equal(r.verdict("feat/abandoned").rule, "pr-closed"); + assert.deepEqual(r.deletes, [deletePath("feat/abandoned")]); }); test("a branch with an open PR is kept", async () => { @@ -252,7 +225,7 @@ test("a branch whose tip moved, or that gained an open PR, between listing and d assert.deepEqual(r.skipped.map((s) => s.name).sort(), ["feat/moved", "feat/reopened"]); }); -test("the per-run cap is honored, oldest-closed first", async () => { +test("a run deletes at most max-deletes-per-run, the rest wait", async () => { const branches = []; const pulls = {}; for (let i = 1; i <= 4; i += 1) { diff --git a/.github/scripts/branch-cleanup/rules.mjs b/.github/scripts/branch-cleanup/rules.mjs index 3db017792c..ecf3f5e151 100644 --- a/.github/scripts/branch-cleanup/rules.mjs +++ b/.github/scripts/branch-cleanup/rules.mjs @@ -1,18 +1,15 @@ // The branch-cleanup rule as pure functions: which branches the job never -// touches, which issues a branch is linked to, and whether a closed branch is -// old enough to delete. docs/conventions/pr-pipeline/README.md, "Branch -// cleanup", states the rule; cleanup.mjs reads GitHub and calls these. -import { DIMENSION_STEPS } from "../work-resolve-issue/report-outcome.mjs"; +// touches and which issues a branch is linked to. +// docs/conventions/pr-pipeline/README.md, "Branch cleanup", states the rule; +// cleanup.mjs reads GitHub and calls these. // The `branch-cleanup` leaf of pr-pipeline.yaml (pr-pipeline.schema.json); // the one copy of its defaults. export const BRANCH_CLEANUP_DEFAULTS = Object.freeze({ - "min-age-days": 14, "max-deletes-per-run": 25, }); const VALID = { - "min-age-days": (value) => Number.isInteger(value) && value >= 1 && value <= 365, "max-deletes-per-run": (value) => Number.isInteger(value) && value >= 0 && value <= 100, }; @@ -31,12 +28,12 @@ export function excluded(name, defaultBranch) { return prefix ? `a ${prefix}* branch` : null; } -// {issue, run} for a work-resolve-issue lane branch +// The issue number of a work-resolve-issue lane branch // (work-resolve-issue/issue---), else null. -const LANE = /^work-resolve-issue\/issue-([1-9][0-9]*)-([1-9][0-9]*)-([1-9][0-9]*)$/; -export function laneBranch(name) { +const LANE = /^work-resolve-issue\/issue-([1-9][0-9]*)-[1-9][0-9]*-[1-9][0-9]*$/; +export function laneIssue(name) { const match = LANE.exec(name); - return match ? { issue: Number(match[1]), run: match[2] } : null; + return match ? Number(match[1]) : null; } // Issue numbers in this repository a pull request body links with Closes, @@ -49,26 +46,5 @@ export function linkedIssues(body, repo) { return [...new Set([...String(body ?? "").matchAll(pattern)].map((m) => Number(m[1])))]; } -// Whether a lanes-App outcome marker on the issue records a stopped run -// (needs a human, blocked, limited, funding, turn-cap) whose work may sit on -// this lane branch. A marker naming this run, naming no run, or left by a -// resumed run that kept no branch of its own (its work is on an earlier lane -// branch) counts; a dimension skip ran nothing and does not. -export function stoppedRun(markers, run) { - return markers.some( - (m) => - !Object.hasOwn(DIMENSION_STEPS, m.outcome) && (m.run === run || m.run === "" || m.run === "0" || m.onResumed), - ); -} - -const DAY_MS = 86_400_000; - -// Days since the latest of `times` (ISO strings), or null when one is unreadable. -export function ageDays(times, now) { - const parsed = times.map((t) => Date.parse(t ?? "")); - if (parsed.length === 0 || parsed.some((t) => !Number.isFinite(t))) return null; - return (Date.parse(now) - Math.max(...parsed)) / DAY_MS; -} - // The branch's pull request the rule reads: the newest by creation. export const lastPull = (pulls) => [...pulls].sort((a, b) => String(b.created_at).localeCompare(String(a.created_at)))[0] ?? null; diff --git a/.github/workflows/maintenance-delete-stale-branches.yml b/.github/workflows/maintenance-delete-stale-branches.yml index e74a2dc7f3..274e684aaf 100644 --- a/.github/workflows/maintenance-delete-stale-branches.yml +++ b/.github/workflows/maintenance-delete-stale-branches.yml @@ -1,14 +1,13 @@ name: maintenance-delete-stale-branches -# Deletes the branches of this repository whose work is finished, so no -# non-default branch lives forever (owner decision W-7). Daily it sorts every -# branch by the branch-cleanup rule (docs/conventions/pr-pipeline/README.md, -# "Branch cleanup"): a branch whose pull request closed or merged and whose -# linked issue closed at least `branch-cleanup.min-age-days` ago is deleted; -# one with an open issue, an open pull request, a stopped lane run, a -# protection or ruleset, or that is the default, release/*, dependabot/* or -# gh-readonly-queue/* branch is never deleted; anything else is listed for a -# person. It prints how many it will delete first and deletes at most +# Deletes the branches of this repository whose work is finished: a branch +# lives as long as its issue is open (owner decisions W-7 and #8958). Daily it +# sorts every branch by the branch-cleanup rule +# (docs/conventions/pr-pipeline/README.md, "Branch cleanup"): a branch whose +# pull request closed or merged and whose linked issue closed is deleted; one +# with an open issue, an open pull request, a protection or ruleset, or that +# is the default, release/*, dependabot/* or gh-readonly-queue/* branch is +# never deleted; anything else is listed for a person. It prints how many it will delete first and deletes at most # `branch-cleanup.max-deletes-per-run`. Before each delete it re-reads the tip # and the open pull requests, and appends {repo, branch, sha, rule, issue, pr, # deletedAt} to the ledger: the step summary and the branch-cleanup-ledger @@ -47,7 +46,7 @@ jobs: timeout-minutes: 20 permissions: contents: write # list branches, compare, delete a branch ref - issues: read # read linked issues and the lanes App's outcome markers + issues: read # read linked issues pull-requests: read # read each branch's pull requests steps: - name: Check out the run's main commit diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index a0299cba2b..0b9305b762 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -543,14 +543,12 @@ red-main: # (16) plus queue time. confirm-wait-minutes: 40 -# The daily branch cleanup (owner decision W-7, 2026-10-21): no non-default -# branch lives forever. maintenance-delete-stale-branches.yml deletes a branch -# whose pull request closed and whose linked issue closed at least -# min-age-days ago, and never one with an open issue, an open pull request -# or a stopped lane run on it. +# The daily branch cleanup (owner decisions W-7, 2026-10-21, and #8958): a +# branch lives as long as its issue is open. maintenance-delete-stale-branches.yml +# deletes a branch once its pull request and linked issue are closed, and never +# one with an open issue or an open pull request. branch-cleanup: - min-age-days: 14 - # A run deletes at most this many, oldest-closed first. + # A run deletes at most this many; the rest wait for the next run. max-deletes-per-run: 25 merge: diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 3983de966d..31e53ba057 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1579,7 +1579,8 @@ before any revert merges. ## Branch cleanup -No non-default branch lives forever (owner decision W-7, 2026-10-21). +A branch lives as long as its issue is open and is deleted once the issue closes (owner decisions +W-7, 2026-10-21, and #8958). No dates are involved. [`maintenance-delete-stale-branches.yml`](../../../.github/workflows/maintenance-delete-stale-branches.yml) runs daily and sorts every branch; its rules and tests are in [`.github/scripts/branch-cleanup/`](../../../.github/scripts/branch-cleanup/). It runs no model. @@ -1588,20 +1589,18 @@ runs daily and sorts every branch; its rules and tests are in 2. **Kept.** A protected branch or one a ruleset applies to; one with an open pull request; a lane branch (`work-resolve-issue/issue--...`) while issue `n` is open; any other branch while an issue its newest pull request's body links with `Closes`, `Fixes`, `Resolves` or `Refs` is - open; and a branch whose pull request or issue closed under `min-age-days` ago. -3. **Never deleted, reported.** A lane branch of a stopped run: a lanes-App outcome marker on its - issue names its run, names no run, or comes from a resumed run (needs a human, blocked, - limited, funding, turn-cap), even after the issue closed, since the next run resumes from it. - Also a branch that never had a pull request, a merged branch with commits ahead of the default - branch after its tip moved, a closed-unmerged branch whose tip moved, and any branch a read - failed on. The step summary lists them for a person. + open. +3. **Never deleted, reported.** A branch that never had a pull request (a lane branch excepted), a + merged branch with commits ahead of the default branch after its tip moved, a closed-unmerged + branch whose tip moved, and any branch a read failed on. The step summary lists them for a + person. 4. **Deleted.** The newest pull request closed unmerged or merged at the branch tip, or merged with the tip moved and 0 commits ahead of the default branch, and every linked issue closed; - or a lane branch with no pull request whose issue closed. The later of those closes is at - least `min-age-days` ago. + or a lane branch with no pull request whose issue closed. A lane run's stop or resume marker + on the issue does not keep the branch once the issue is closed. -The job prints how many it will delete, then deletes at most `max-deletes-per-run`, oldest-closed -first. Before each delete it re-reads the tip and the open pull requests from and into the branch, +The job prints how many it will delete, then deletes at most `max-deletes-per-run`, in listing +order. Before each delete it re-reads the tip and the open pull requests from and into the branch, skips it when the tip moved or one opened, and appends `{repo, branch, sha, rule, issue, pr, deletedAt}` to the step summary and the run's `branch-cleanup-ledger--` artifact. The delete is `DELETE git/refs/heads/` with the job's own `GITHUB_TOKEN` (`contents: write`, @@ -1613,7 +1612,6 @@ refuses every request but `GET`. The kill switch stops it. | Key | Default | What it sets | |---|---|---| -| `min-age-days` | 14 | Days since the later of the pull request and issue closes; 1 to 365. | | `max-deletes-per-run` | 25 | Deletes per run at most; 0 to 100, and 0 deletes none. | ## Planning lane work diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index 43cf7ac02f..29d6438a5e 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -250,21 +250,14 @@ "branch-cleanup": { "type": "object", "additionalProperties": false, - "description": "The scheduled branch cleanup: how long a closed branch waits before the job deletes it, and how many it deletes in one run. Contract: README.md, Branch cleanup.", + "description": "The scheduled branch cleanup: how many branches it deletes in one run. Contract: README.md, Branch cleanup.", "properties": { - "min-age-days": { - "type": "integer", - "minimum": 1, - "maximum": 365, - "default": 14, - "description": "Days since the later of the branch's pull request closing and its issue closing before the branch is deleted." - }, "max-deletes-per-run": { "type": "integer", "minimum": 0, "maximum": 100, "default": 25, - "description": "Branches one run deletes at most, oldest-closed first; the rest wait for the next run. 0 deletes none." + "description": "Branches one run deletes at most; the rest wait for the next run. 0 deletes none." } } }, From 7cbca139a0c0a3d8f47ecb0be14453271c9a2b6b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:48:36 -0400 Subject: [PATCH 4/5] fix(pr-pipeline): reserve the delete calls before branch evaluation Evaluation spent the shared 600-call budget, so with about 300 deletable branches every planned delete failed its re-read and nothing was ever deleted (Codex review on #8959). cleanup.mjs now reserves 4 calls per planned delete from the client's budget, evaluates with the rest, and stops evaluating once it has found max-deletes-per-run candidates or spent its share; the remaining branches wait for the next run. Co-Authored-By: Claude Opus 5.5 --- .../feat-branch-cleanup-w7-663cf524.md | 2 +- .github/scripts/branch-cleanup/cleanup.mjs | 54 +++++++++++++++---- .../scripts/branch-cleanup/cleanup.test.mjs | 30 +++++++++-- .../maintenance-delete-stale-branches.yml | 3 +- docs/conventions/pr-pipeline/README.md | 4 +- 5 files changed, 76 insertions(+), 17 deletions(-) diff --git a/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md b/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md index 5a0454f9c0..4d13967353 100644 --- a/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md +++ b/.changes/pr-pipeline/feat-branch-cleanup-w7-663cf524.md @@ -7,7 +7,7 @@ bump: minor - A `branch-cleanup` leaf (`max-deletes-per-run`, default 25) and the daily maintenance-delete-stale-branches.yml that applies it: a branch lives as long as its issue is open, and one whose pull request closed or merged and whose linked issue closed is deleted, at - most `max-deletes-per-run` a run, after a re-read of its tip and open pull requests and a + most `max-deletes-per-run` a run with the calls those deletes need reserved first, after a re-read of its tip and open pull requests and a ledger line in the step summary and an artifact. A lane run's stop marker does not keep a branch once its issue closed. A branch with an open issue or pull request, and a protected or ruleset branch, are never deleted; a branch that never had a pull request, or whose tip moved diff --git a/.github/scripts/branch-cleanup/cleanup.mjs b/.github/scripts/branch-cleanup/cleanup.mjs index b31ffd689c..1f5bd85fbb 100644 --- a/.github/scripts/branch-cleanup/cleanup.mjs +++ b/.github/scripts/branch-cleanup/cleanup.mjs @@ -5,7 +5,8 @@ // each delete it re-reads the branch tip and its open pull requests, skips the // branch when either changed, and appends the ledger line. In dry-run the // client refuses every request but GET, so nothing is written. REST only, -// paced, within a call budget. No model. +// paced, within a call budget, of which the deletes' share is reserved before +// any branch is evaluated. No model. // // Environment: GITHUB_TOKEN, REPOSITORY, DRY_RUN (`true` or `false`), // BASE_PATH (main's checkout), LEDGER_PATH (JSON Lines file), CALL_BUDGET, @@ -17,12 +18,14 @@ import { pathToFileURL } from "node:url"; import { excluded, laneIssue, lastPull, linkedIssues, resolveBranchCleanup } from "./rules.mjs"; +export class BudgetSpent extends Error {} + export function client(env, { pace = () => delay(200), dryRun = true } = {}) { let used = 0; const budget = Number(env.CALL_BUDGET ?? 600); - return async (method, path) => { + const call = async (method, path) => { if (dryRun && method !== "GET") throw new Error(`dry-run refuses ${method} ${path}`); - if (used >= budget) throw new Error(`call budget of ${budget} spent`); + if (used >= budget) throw new BudgetSpent(`call budget of ${budget} spent`); used += 1; await pace(); const response = await fetch(`${env.GITHUB_API_URL ?? "https://api.github.com"}${path}`, { @@ -37,6 +40,8 @@ export function client(env, { pace = () => delay(200), dryRun = true } = {}) { if (!response.ok) throw new Error(`${method} ${path.split("?")[0]}: HTTP ${response.status}`); return response.status === 204 ? null : response.json(); }; + call.remaining = () => budget - used; + return call; } async function pages(call, path, max = 20) { @@ -119,6 +124,10 @@ async function stillDeletable(call, { repo, owner }, { name, sha }) { return null; } +// Calls one delete takes: the tip and the open pull requests from and into +// the branch re-read (stillDeletable), then the delete. +const CALLS_PER_DELETE = 4; + const row = (cells) => `| ${cells.map((c) => String(c ?? "").replaceAll("|", "\\|")).join(" | ")} |`; export async function cleanup(env, { call, clock, loadConfig, append, log = console.log }) { @@ -127,13 +136,31 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons const dryRun = env.DRY_RUN !== "false"; const config = resolveBranchCleanup(loadConfig()); const owner = repo.split("/")[0]; - const defaultBranch = (await call("GET", `/repos/${repo}`))?.default_branch; + const cap = config["max-deletes-per-run"]; + // Evaluation gets what the planned deletes leave of the client's budget. + const evalLimit = (call.remaining?.() ?? Number.POSITIVE_INFINITY) - cap * CALLS_PER_DELETE; + if (evalLimit <= 0) throw new Error(`the call budget does not cover ${cap} deletes and any reads`); + let evalUsed = 0; + const read = async (method, path) => { + if (evalUsed >= evalLimit) throw new BudgetSpent(`the evaluation share of ${evalLimit} calls is spent`); + evalUsed += 1; + return call(method, path); + }; + const defaultBranch = (await read("GET", `/repos/${repo}`))?.default_branch; if (typeof defaultBranch !== "string" || defaultBranch === "") throw new Error("the default branch could not be read"); const context = { repo, owner, defaultBranch }; - const branches = await pages(call, `/repos/${repo}/branches`); + const branches = await pages(read, `/repos/${repo}/branches`); const results = []; - for (const branch of branches) { + // Evaluation stops once the cap is filled or its share is spent; the + // branches after that wait for the next run. + let unevaluated = 0; + let found = 0; + for (const [index, branch] of branches.entries()) { + if (cap > 0 && found >= cap) { + unevaluated = branches.length - index; + break; + } const name = branch.name; const sha = branch.commit?.sha; const skip = excluded(name, defaultBranch); @@ -142,14 +169,19 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons continue; } try { - results.push({ name, sha, ...(await evaluate(call, context, { name, sha, protected: branch.protected })) }); + const verdict = await evaluate(read, context, { name, sha, protected: branch.protected }); + if (verdict.verdict === "delete") found += 1; + results.push({ name, sha, ...verdict }); } catch (error) { + if (error instanceof BudgetSpent) { + unevaluated = branches.length - index; + break; + } results.push({ name, sha, verdict: "report", reason: `a read failed: ${error.message}` }); } } const candidates = results.filter((r) => r.verdict === "delete"); - const cap = config["max-deletes-per-run"]; const planned = candidates.slice(0, cap); log( `${dryRun ? "dry-run: would delete" : "will delete"} ${planned.length} of ${candidates.length} deletable branch(es) (cap ${cap}); ${branches.length} branch(es) listed`, @@ -201,8 +233,8 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons row([`\`${c.name}\``, c.sha.slice(0, 12), c.rule, c.issue ? `#${c.issue}` : "", c.pr ? `#${c.pr}` : ""]), ), ]; - if (candidates.length > planned.length) { - lines.push("", `${candidates.length - planned.length} more deletable branch(es) wait for the next run (cap ${cap}).`); + if (unevaluated > 0) { + lines.push("", `${unevaluated} branch(es) not evaluated this run (cap ${cap} reached or call budget spent); the next run continues.`); } if (skipped.length > 0) { lines.push("", "#### Skipped at delete time", "", ...skipped.map((s) => `- \`${s.name}\`: ${s.reason}`)); @@ -217,7 +249,7 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons "", ...results.filter((r) => r.verdict === "keep").map((r) => `- \`${r.name}\`: ${r.reason}`), ); - return { dryRun, results, candidates, planned, deleted, skipped, summary: lines.join("\n") }; + return { dryRun, results, candidates, planned, deleted, skipped, unevaluated, summary: lines.join("\n") }; } async function main(env = process.env) { diff --git a/.github/scripts/branch-cleanup/cleanup.test.mjs b/.github/scripts/branch-cleanup/cleanup.test.mjs index 5a1947ec87..b3b8da93fb 100644 --- a/.github/scripts/branch-cleanup/cleanup.test.mjs +++ b/.github/scripts/branch-cleanup/cleanup.test.mjs @@ -4,7 +4,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { cleanup } from "./cleanup.mjs"; +import { cleanup, client } from "./cleanup.mjs"; import { linkedIssues } from "./rules.mjs"; const REPO = "melodic-software/claude-code-plugins"; @@ -225,7 +225,7 @@ test("a branch whose tip moved, or that gained an open PR, between listing and d assert.deepEqual(r.skipped.map((s) => s.name).sort(), ["feat/moved", "feat/reopened"]); }); -test("a run deletes at most max-deletes-per-run, the rest wait", async () => { +test("a run deletes at most max-deletes-per-run and leaves the rest unevaluated for the next run", async () => { const branches = []; const pulls = {}; for (let i = 1; i <= 4; i += 1) { @@ -234,8 +234,8 @@ test("a run deletes at most max-deletes-per-run, the rest wait", async () => { pulls[name] = [mergedPull(i, sha(String(i)), `2026-0${i}-01T00:00:00Z`)]; } const r = await run(world({ branches, pulls }), { config: { "max-deletes-per-run": 2 } }); - assert.equal(r.candidates.length, 4); assert.deepEqual(r.deletes, [deletePath("feat/b1"), deletePath("feat/b2")]); + assert.equal(r.unevaluated, 2); }); test("dry-run deletes nothing and writes no ledger line", async () => { @@ -253,3 +253,27 @@ test("linkedIssues reads same-repository closing and Refs links only", () => { const body = `Closes #1, fixes: #2\nResolves ${REPO}#3\nRefs https://github.com/${REPO}/issues/4\nCloses other/repo#5\nsee #6`; assert.deepEqual(linkedIssues(body, REPO), [1, 2, 3, 4]); }); + +// The real client over a stubbed fetch, so its call budget applies (#8959 +// review: evaluation spent the whole budget and every delete then failed). +test("300 deletable branches under a 600-call budget: the run deletes the cap of 25", async (t) => { + const branches = []; + const pulls = {}; + for (let i = 1; i <= 300; i += 1) { + const name = `feat/old-${i}`; + const tip = i.toString(16).padStart(40, "0"); + branches.push({ name, sha: tip }); + pulls[name] = [mergedPull(i, tip, OLD)]; + } + const w = world({ branches, pulls }); + t.mock.method(globalThis, "fetch", async (url, { method }) => { + const body = await w.call(method, String(url).slice("https://api.test".length)); + if (method !== "GET") return new Response(null, { status: 204 }); + return body === null ? new Response(null, { status: 404 }) : Response.json(body); + }); + const env = { REPOSITORY: REPO, DRY_RUN: "false", CALL_BUDGET: "600", GITHUB_API_URL: "https://api.test" }; + const call = client(env, { pace: async () => {}, dryRun: false }); + const result = await cleanup(env, { call, clock: () => NOW, loadConfig: () => ({}), append: () => {}, log: () => {} }); + assert.equal(result.deleted.length, 25); + assert.equal(w.writes.length, 25); +}); diff --git a/.github/workflows/maintenance-delete-stale-branches.yml b/.github/workflows/maintenance-delete-stale-branches.yml index 274e684aaf..a2a2930fc6 100644 --- a/.github/workflows/maintenance-delete-stale-branches.yml +++ b/.github/workflows/maintenance-delete-stale-branches.yml @@ -83,7 +83,8 @@ jobs: # A workflow token's REST limit is per repository per hour # (https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api); - # 600 calls covers about three reads for each of 200 branches. + # cleanup.mjs reserves 4 calls for each of the max-deletes-per-run + # deletes and evaluates branches with the rest, stopping at the cap. - name: Delete stale branches if: steps.kill.outputs.proceed == 'true' env: diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 206f3a65c4..396b4f6622 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1601,7 +1601,9 @@ runs daily and sorts every branch; its rules and tests are in on the issue does not keep the branch once the issue is closed. The job prints how many it will delete, then deletes at most `max-deletes-per-run`, in listing -order. Before each delete it re-reads the tip and the open pull requests from and into the branch, +order. It reserves four calls per planned delete from its call budget before evaluating, and stops +evaluating once it has found `max-deletes-per-run` deletable branches or spent the rest of the +budget; the branches after that wait for the next run. Before each delete it re-reads the tip and the open pull requests from and into the branch, skips it when the tip moved or one opened, and appends `{repo, branch, sha, rule, issue, pr, deletedAt}` to the step summary and the run's `branch-cleanup-ledger--` artifact. The delete is `DELETE git/refs/heads/` with the job's own `GITHUB_TOKEN` (`contents: write`, From 6c1af60ab297afe536c22f9df65d3a0b658777b0 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:37:20 -0400 Subject: [PATCH 5/5] fix(pr-pipeline): rotate where branch evaluation starts each run Evaluation always started at the head of the branch listing, so kept or reported branches early in it spent the evaluation share every run and the later branches were never read. Each run now starts at GITHUB_RUN_NUMBER times a quarter of the evaluation share, modulo the listing length, wrapping around, so successive runs cover the whole listing. The summary says the next run starts elsewhere instead of claiming it continues. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/branch-cleanup/cleanup.mjs | 19 ++++++---- .../scripts/branch-cleanup/cleanup.test.mjs | 38 +++++++++++++++++++ docs/conventions/pr-pipeline/README.md | 3 +- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/.github/scripts/branch-cleanup/cleanup.mjs b/.github/scripts/branch-cleanup/cleanup.mjs index 1f5bd85fbb..3a56db1b75 100644 --- a/.github/scripts/branch-cleanup/cleanup.mjs +++ b/.github/scripts/branch-cleanup/cleanup.mjs @@ -10,7 +10,7 @@ // // Environment: GITHUB_TOKEN, REPOSITORY, DRY_RUN (`true` or `false`), // BASE_PATH (main's checkout), LEDGER_PATH (JSON Lines file), CALL_BUDGET, -// GITHUB_API_URL, GITHUB_STEP_SUMMARY. +// GITHUB_API_URL, GITHUB_STEP_SUMMARY, GITHUB_RUN_NUMBER. import { appendFileSync } from "node:fs"; import process from "node:process"; import { setTimeout as delay } from "node:timers/promises"; @@ -151,14 +151,19 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons const context = { repo, owner, defaultBranch }; const branches = await pages(read, `/repos/${repo}/branches`); + // Evaluation stops once the cap is filled or its share is spent, so each run + // starts at a point the run number moves by about a quarter of what one run + // can read (a branch takes up to about four reads), wrapping around: kept + // branches early in the listing cannot hide the later ones from every run. + const stride = Number.isFinite(evalLimit) ? Math.max(1, Math.floor(evalLimit / 4)) : 0; + const start = branches.length === 0 ? 0 : ((Number.parseInt(env.GITHUB_RUN_NUMBER ?? "", 10) || 0) * stride) % branches.length; + const ordered = [...branches.slice(start), ...branches.slice(0, start)]; const results = []; - // Evaluation stops once the cap is filled or its share is spent; the - // branches after that wait for the next run. let unevaluated = 0; let found = 0; - for (const [index, branch] of branches.entries()) { + for (const [index, branch] of ordered.entries()) { if (cap > 0 && found >= cap) { - unevaluated = branches.length - index; + unevaluated = ordered.length - index; break; } const name = branch.name; @@ -174,7 +179,7 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons results.push({ name, sha, ...verdict }); } catch (error) { if (error instanceof BudgetSpent) { - unevaluated = branches.length - index; + unevaluated = ordered.length - index; break; } results.push({ name, sha, verdict: "report", reason: `a read failed: ${error.message}` }); @@ -234,7 +239,7 @@ export async function cleanup(env, { call, clock, loadConfig, append, log = cons ), ]; if (unevaluated > 0) { - lines.push("", `${unevaluated} branch(es) not evaluated this run (cap ${cap} reached or call budget spent); the next run continues.`); + lines.push("", `${unevaluated} branch(es) not evaluated this run (cap ${cap} reached or call budget spent); the next run starts at a different point in the listing.`); } if (skipped.length > 0) { lines.push("", "#### Skipped at delete time", "", ...skipped.map((s) => `- \`${s.name}\`: ${s.reason}`)); diff --git a/.github/scripts/branch-cleanup/cleanup.test.mjs b/.github/scripts/branch-cleanup/cleanup.test.mjs index b3b8da93fb..a5bc1b693c 100644 --- a/.github/scripts/branch-cleanup/cleanup.test.mjs +++ b/.github/scripts/branch-cleanup/cleanup.test.mjs @@ -277,3 +277,41 @@ test("300 deletable branches under a 600-call budget: the run deletes the cap of assert.equal(result.deleted.length, 25); assert.equal(w.writes.length, 25); }); + +// #8976 review: kept branches ahead of the deletable ones spent the +// evaluation share every run, so the branches after them were never read. +test("493 kept branches ahead of 8 deletable: successive runs reach and delete all 8", async (t) => { + const branches = []; + const pulls = {}; + for (let i = 1; i <= 501; i += 1) { + const name = `feat/b-${i}`; + const tip = i.toString(16).padStart(40, "0"); + branches.push({ name, sha: tip }); + // A kept branch costs two reads: its pull requests and its open linked issue. + pulls[name] = [mergedPull(i, tip, OLD, i <= 493 ? "Closes #7000" : "")]; + } + const w = world({ branches, pulls, issues: { 7000: openIssue } }); + t.mock.method(globalThis, "fetch", async (url, { method }) => { + const path = String(url).slice("https://api.test".length); + const body = await w.call(method, path); + if (method !== "GET") { + const name = decodeURIComponent(path.slice(`/repos/${REPO}/git/refs/heads/`.length)); + branches.splice(branches.findIndex((b) => b.name === name), 1); + return new Response(null, { status: 204 }); + } + return body === null ? new Response(null, { status: 404 }) : Response.json(body); + }); + for (let runNumber = 1; runNumber <= 5; runNumber += 1) { + const env = { + REPOSITORY: REPO, + DRY_RUN: "false", + CALL_BUDGET: "600", + GITHUB_API_URL: "https://api.test", + GITHUB_RUN_NUMBER: String(runNumber), + }; + const call = client(env, { pace: async () => {}, dryRun: false }); + await cleanup(env, { call, clock: () => NOW, loadConfig: () => ({}), append: () => {}, log: () => {} }); + } + assert.equal(w.writes.length, 8); + assert.equal(branches.length, 493); +}); diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 396b4f6622..9dd7712179 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1603,7 +1603,8 @@ runs daily and sorts every branch; its rules and tests are in The job prints how many it will delete, then deletes at most `max-deletes-per-run`, in listing order. It reserves four calls per planned delete from its call budget before evaluating, and stops evaluating once it has found `max-deletes-per-run` deletable branches or spent the rest of the -budget; the branches after that wait for the next run. Before each delete it re-reads the tip and the open pull requests from and into the branch, +budget. Each run starts at a different point in the listing, moved by the run number and wrapping +around, so branches kept early in the listing do not hide the later ones from every run. Before each delete it re-reads the tip and the open pull requests from and into the branch, skips it when the tip moved or one opened, and appends `{repo, branch, sha, rule, issue, pr, deletedAt}` to the step summary and the run's `branch-cleanup-ledger--` artifact. The delete is `DELETE git/refs/heads/` with the job's own `GITHUB_TOKEN` (`contents: write`,