diff --git a/packages/merge-steward/src/github/pr-client.ts b/packages/merge-steward/src/github/pr-client.ts index 3e784e80..a3d810df 100644 --- a/packages/merge-steward/src/github/pr-client.ts +++ b/packages/merge-steward/src/github/pr-client.ts @@ -158,6 +158,41 @@ export class GitHubPRClient implements GitHubPRApi { } } + async listOpenPRsByBase(baseBranch: string): Promise> { + const result = await exec("gh", [ + "api", "--method", "GET", + `repos/${this.repoFullName}/pulls`, + "-f", "state=open", + "-f", `base=${baseBranch}`, + "-f", "per_page=100", + "--paginate", "--slurp", + ], { githubRepoFullName: this.repoFullName }); + + try { + const pages = JSON.parse(result.stdout) as Array>; + return pages.flat().map((pr) => ({ + number: pr.number, + branch: pr.head.ref, + headSha: pr.head.sha, + baseBranch: pr.base.ref, + })); + } catch (error) { + throw new Error("GitHub returned malformed stack child PR data", { cause: error }); + } + } + + async setBaseBranch(prNumber: number, baseBranch: string): Promise { + await exec("gh", [ + "api", "--method", "PATCH", + `repos/${this.repoFullName}/pulls/${prNumber}`, + "-f", `base=${baseBranch}`, + ], { githubRepoFullName: this.repoFullName }); + } + async deleteBranch(prNumber: number): Promise { const status = await this.getStatus(prNumber); await exec("gh", [ diff --git a/packages/merge-steward/src/interfaces.ts b/packages/merge-steward/src/interfaces.ts index e3102374..ed5a2294 100644 --- a/packages/merge-steward/src/interfaces.ts +++ b/packages/merge-steward/src/interfaces.ts @@ -53,8 +53,12 @@ export interface GitHubPRApi { setLabels(prNumber: number, opts: { add?: string[]; remove?: string[] }): Promise; /** List open PRs for restart-time eligibility scans. */ listOpenPRs(): Promise>; + /** List every open PR whose current base is the given branch. */ + listOpenPRsByBase(baseBranch: string): Promise>; /** Find the open PR number for a branch, or null if none exists. */ findPRByBranch(branch: string): Promise; + /** Retarget a PR before deleting a branch used as its current base. */ + setBaseBranch(prNumber: number, baseBranch: string): Promise; /** Delete the PR's head branch from the remote (best-effort cleanup). */ deleteBranch(prNumber: number): Promise; /** List open PRs with a specific label (for startup scan). */ diff --git a/packages/merge-steward/src/reconciler-merge.ts b/packages/merge-steward/src/reconciler-merge.ts index 6328a487..401dbee6 100644 --- a/packages/merge-steward/src/reconciler-merge.ts +++ b/packages/merge-steward/src/reconciler-merge.ts @@ -506,15 +506,33 @@ export async function deletePrBranchAfterGitHubMarksMerged( const delayMs = options.delayMs ?? DEFAULT_PR_MERGED_POLL_DELAY_MS; for (let attempt = 0; attempt < attempts; attempt++) { - let merged = false; + let mergedStatus: Awaited> | null = null; try { - merged = (await ctx.github.getStatus(entry.prNumber)).merged; + const status = await ctx.github.getStatus(entry.prNumber); + if (status.merged) mergedStatus = status; } catch { // Keep polling briefly. If GitHub is unavailable, branch cleanup is // cosmetic; preserving correct PR merge classification matters more. } - if (merged) { + if (mergedStatus) { + let children: Array<{ number: number; branch: string; headSha: string; baseBranch: string }>; + try { + children = await ctx.github.listOpenPRsByBase(mergedStatus.branch); + for (const child of children) { + await ctx.github.setBaseBranch(child.number, ctx.baseBranch); + } + } catch { + emit(ctx, entry, "pr_branch_cleanup_deferred", { + detail: "preserving the merged PR branch because stacked children could not be safely retargeted", + }); + return; + } + if (children.length > 0) { + emit(ctx, entry, "stack_children_retargeted", { + detail: `retargeted child PR ${children.map((child) => `#${child.number}`).join(", ")} to ${ctx.baseBranch} before deleting ${mergedStatus.branch}`, + }); + } try { await ctx.github.deleteBranch(entry.prNumber); } catch { diff --git a/packages/merge-steward/src/sim/github-sim.ts b/packages/merge-steward/src/sim/github-sim.ts index 481040ee..41d028be 100644 --- a/packages/merge-steward/src/sim/github-sim.ts +++ b/packages/merge-steward/src/sim/github-sim.ts @@ -129,6 +129,12 @@ export class GitHubSim implements GitHubPRApi { return null; } + async setBaseBranch(prNumber: number, baseBranch: string): Promise { + const pr = this.prs.get(prNumber); + if (!pr) throw new Error(`PR #${prNumber} not found`); + pr.baseRefName = baseBranch; + } + async listLabels(prNumber: number): Promise { const pr = this.prs.get(prNumber); if (!pr) return []; @@ -150,6 +156,10 @@ export class GitHubSim implements GitHubPRApi { })); } + async listOpenPRsByBase(baseBranch: string): Promise> { + return (await this.listOpenPRs()).filter((pr) => pr.baseBranch === baseBranch); + } + async deleteBranch(_prNumber: number): Promise { // No-op in sim — branch deletion is cosmetic cleanup. } diff --git a/packages/merge-steward/src/types.ts b/packages/merge-steward/src/types.ts index 39b7dff9..f526972d 100644 --- a/packages/merge-steward/src/types.ts +++ b/packages/merge-steward/src/types.ts @@ -301,6 +301,7 @@ export type ReconcileAction = | "main_pending_bypassed" // main rerun pending for an already-validated merge commit | "pr_branch_cleanup_deferred" // GitHub has not yet classified the PR as merged | "pr_branch_cleanup_failed" // branch cleanup failed after merge; non-blocking + | "stack_children_retargeted" // direct children moved to the merged parent's base before cleanup | "sanitized_closed" // entry terminalized: PR closed on GitHub | "sanitized_duplicate" // older duplicate entry superseded | "post_merge_verification_started" diff --git a/packages/merge-steward/test/github-pr-client.test.ts b/packages/merge-steward/test/github-pr-client.test.ts index a2b21401..d3bce1a1 100644 --- a/packages/merge-steward/test/github-pr-client.test.ts +++ b/packages/merge-steward/test/github-pr-client.test.ts @@ -52,6 +52,7 @@ exit 1 { name: "Tests", conclusion: "success", appId: 42, runId: 101, url: "https://github.com/owner/repo/checks/1" }, { name: "AI Review", conclusion: "skipped", appId: 7, runId: 102, url: "https://github.com/owner/repo/checks/2" }, ]); + await client.setBaseBranch(101, "main"); } finally { if (previousPath === undefined) { delete process.env.PATH; @@ -68,6 +69,49 @@ exit 1 const log = readFileSync(logPath, "utf8"); assert.match(log, /pr view 101 --repo owner\/repo --json number,title,headRefName,headRefOid,baseRefName,reviewDecision,reviews,state/); assert.match(log, /api repos\/owner\/repo\/commits\/sha-101\/check-runs --jq \.check_runs/); + assert.match(log, /api --method PATCH repos\/owner\/repo\/pulls\/101 -f base=main/); + } finally { + rmSync(baseDir, { recursive: true, force: true }); + } +}); + +test("GitHubPRClient lists every open child for one base with paginated REST", async () => { + const baseDir = mkdtempSync(path.join(tmpdir(), "ms-gh-pr-children-")); + const ghPath = path.join(baseDir, "gh"); + const logPath = path.join(baseDir, "gh.log"); + + try { + writeFileSync( + ghPath, + `#!/bin/sh +echo "$*" >> "$GH_LOG" +printf '[[{"number":102,"head":{"ref":"child-one","sha":"sha-102"},"base":{"ref":"renamed-parent"}}],[{"number":103,"head":{"ref":"child-two","sha":"sha-103"},"base":{"ref":"renamed-parent"}}]]' +`, + "utf8", + ); + chmodSync(ghPath, 0o755); + + const previousPath = process.env.PATH; + const previousLog = process.env.GH_LOG; + process.env.PATH = `${baseDir}${path.delimiter}${previousPath ?? ""}`; + process.env.GH_LOG = logPath; + try { + const children = await new GitHubPRClient("owner/repo").listOpenPRsByBase("renamed-parent"); + assert.deepEqual(children, [ + { number: 102, branch: "child-one", headSha: "sha-102", baseBranch: "renamed-parent" }, + { number: 103, branch: "child-two", headSha: "sha-103", baseBranch: "renamed-parent" }, + ]); + } finally { + if (previousPath === undefined) delete process.env.PATH; + else process.env.PATH = previousPath; + if (previousLog === undefined) delete process.env.GH_LOG; + else process.env.GH_LOG = previousLog; + } + + assert.match( + readFileSync(logPath, "utf8"), + /api --method GET repos\/owner\/repo\/pulls -f state=open -f base=renamed-parent -f per_page=100 --paginate --slurp/, + ); } finally { rmSync(baseDir, { recursive: true, force: true }); } diff --git a/packages/merge-steward/test/reconciler-merge.test.ts b/packages/merge-steward/test/reconciler-merge.test.ts index 3917115d..37f453b6 100644 --- a/packages/merge-steward/test/reconciler-merge.test.ts +++ b/packages/merge-steward/test/reconciler-merge.test.ts @@ -101,6 +101,9 @@ test("branch cleanup runs after GitHub reports the PR as merged", async () => { merged: true, }; }, + async listOpenPRsByBase() { + return []; + }, async deleteBranch() { deleteCalls += 1; }, @@ -116,6 +119,75 @@ test("branch cleanup runs after GitHub reports the PR as merged", async () => { assert.equal(events.length, 0); }); +test("branch cleanup retargets direct stack children before deleting their parent branch", async () => { + const calls: string[] = []; + const events: ReconcileEvent[] = []; + const github = { + async getStatus() { + return { + number: 764, + branch: "renamed-feature", + headSha: "head", + mergeable: false, + reviewApproved: true, + merged: true, + }; + }, + async listOpenPRsByBase(baseBranch: string) { + calls.push(`discover:${baseBranch}`); + return [ + { number: 765, branch: "child", headSha: "child-head", baseBranch: baseBranch }, + ]; + }, + async setBaseBranch(prNumber: number, baseBranch: string) { + calls.push(`retarget:${prNumber}:${baseBranch}`); + }, + async deleteBranch(prNumber: number) { + calls.push(`delete:${prNumber}`); + }, + } as GitHubPRApi; + + await deletePrBranchAfterGitHubMarksMerged(buildContext(github, events), makeEntry(), { + attempts: 1, + delayMs: 0, + }); + + assert.deepEqual(calls, ["discover:renamed-feature", "retarget:765:main", "delete:764"]); + assert.equal(events.at(-1)?.action, "stack_children_retargeted"); +}); + +test("branch cleanup preserves the parent branch when stack children cannot be inspected", async () => { + let deleteCalls = 0; + const events: ReconcileEvent[] = []; + const github = { + async getStatus() { + return { + number: 764, + branch: "feature", + headSha: "head", + mergeable: false, + reviewApproved: true, + merged: true, + }; + }, + async listOpenPRsByBase() { + throw new Error("GitHub unavailable"); + }, + async deleteBranch() { + deleteCalls += 1; + }, + } as GitHubPRApi; + + await deletePrBranchAfterGitHubMarksMerged(buildContext(github, events), makeEntry(), { + attempts: 1, + delayMs: 0, + }); + + assert.equal(deleteCalls, 0); + assert.equal(events.at(-1)?.action, "pr_branch_cleanup_deferred"); + assert.match(events.at(-1)?.detail ?? "", /stacked children/); +}); + function protectedBranchError(): Error & { stderr: string; exitCode: number } { const error = new Error("Command failed: git push origin mq-spec:main") as Error & { stderr: string;