From 9157cdb0be734a58972ca7ea89d30430af4e32bb Mon Sep 17 00:00:00 2001 From: "krasnoperov-coding-agent[bot]" <322146550+krasnoperov-coding-agent[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:20:39 +0200 Subject: [PATCH 1/2] fix: retarget stack children before branch cleanup --- .../merge-steward/src/github/pr-client.ts | 8 +++ packages/merge-steward/src/interfaces.ts | 2 + .../merge-steward/src/reconciler-merge.ts | 17 +++++ packages/merge-steward/src/sim/github-sim.ts | 6 ++ packages/merge-steward/src/types.ts | 1 + .../test/github-pr-client.test.ts | 2 + .../test/reconciler-merge.test.ts | 72 +++++++++++++++++++ 7 files changed, 108 insertions(+) diff --git a/packages/merge-steward/src/github/pr-client.ts b/packages/merge-steward/src/github/pr-client.ts index 3e784e80..40066601 100644 --- a/packages/merge-steward/src/github/pr-client.ts +++ b/packages/merge-steward/src/github/pr-client.ts @@ -158,6 +158,14 @@ export class GitHubPRClient implements GitHubPRApi { } } + 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..0621366d 100644 --- a/packages/merge-steward/src/interfaces.ts +++ b/packages/merge-steward/src/interfaces.ts @@ -55,6 +55,8 @@ export interface GitHubPRApi { listOpenPRs(): 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..f7ee2270 100644 --- a/packages/merge-steward/src/reconciler-merge.ts +++ b/packages/merge-steward/src/reconciler-merge.ts @@ -515,6 +515,23 @@ export async function deletePrBranchAfterGitHubMarksMerged( } if (merged) { + let children: Array<{ number: number; branch: string; headSha: string; baseBranch: string }>; + try { + children = (await ctx.github.listOpenPRs()).filter((pr) => pr.baseBranch === entry.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 ${entry.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..eab68de8 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 []; 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..1ea5610b 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,7 @@ 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 }); } diff --git a/packages/merge-steward/test/reconciler-merge.test.ts b/packages/merge-steward/test/reconciler-merge.test.ts index 3917115d..4fabdc2f 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 listOpenPRs() { + 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: "feature", + headSha: "head", + mergeable: false, + reviewApproved: true, + merged: true, + }; + }, + async listOpenPRs() { + return [ + { number: 765, branch: "child", headSha: "child-head", baseBranch: "feature" }, + { number: 766, branch: "other", headSha: "other-head", baseBranch: "main" }, + ]; + }, + 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, ["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 listOpenPRs() { + 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; From db8923ba13af6f03d79bb6e1b1e7b18e5bd1622d Mon Sep 17 00:00:00 2001 From: "krasnoperov-coding-agent[bot]" <322146550+krasnoperov-coding-agent[bot]@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:29:58 +0200 Subject: [PATCH 2/2] fix: discover all live stack children --- .../merge-steward/src/github/pr-client.ts | 27 ++++++++++++ packages/merge-steward/src/interfaces.ts | 2 + .../merge-steward/src/reconciler-merge.ts | 11 ++--- packages/merge-steward/src/sim/github-sim.ts | 4 ++ .../test/github-pr-client.test.ts | 42 +++++++++++++++++++ .../test/reconciler-merge.test.ts | 14 +++---- 6 files changed, 88 insertions(+), 12 deletions(-) diff --git a/packages/merge-steward/src/github/pr-client.ts b/packages/merge-steward/src/github/pr-client.ts index 40066601..a3d810df 100644 --- a/packages/merge-steward/src/github/pr-client.ts +++ b/packages/merge-steward/src/github/pr-client.ts @@ -158,6 +158,33 @@ 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", diff --git a/packages/merge-steward/src/interfaces.ts b/packages/merge-steward/src/interfaces.ts index 0621366d..ed5a2294 100644 --- a/packages/merge-steward/src/interfaces.ts +++ b/packages/merge-steward/src/interfaces.ts @@ -53,6 +53,8 @@ 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. */ diff --git a/packages/merge-steward/src/reconciler-merge.ts b/packages/merge-steward/src/reconciler-merge.ts index f7ee2270..401dbee6 100644 --- a/packages/merge-steward/src/reconciler-merge.ts +++ b/packages/merge-steward/src/reconciler-merge.ts @@ -506,18 +506,19 @@ 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.listOpenPRs()).filter((pr) => pr.baseBranch === entry.branch); + children = await ctx.github.listOpenPRsByBase(mergedStatus.branch); for (const child of children) { await ctx.github.setBaseBranch(child.number, ctx.baseBranch); } @@ -529,7 +530,7 @@ export async function deletePrBranchAfterGitHubMarksMerged( } 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 ${entry.branch}`, + detail: `retargeted child PR ${children.map((child) => `#${child.number}`).join(", ")} to ${ctx.baseBranch} before deleting ${mergedStatus.branch}`, }); } try { diff --git a/packages/merge-steward/src/sim/github-sim.ts b/packages/merge-steward/src/sim/github-sim.ts index eab68de8..41d028be 100644 --- a/packages/merge-steward/src/sim/github-sim.ts +++ b/packages/merge-steward/src/sim/github-sim.ts @@ -156,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/test/github-pr-client.test.ts b/packages/merge-steward/test/github-pr-client.test.ts index 1ea5610b..d3bce1a1 100644 --- a/packages/merge-steward/test/github-pr-client.test.ts +++ b/packages/merge-steward/test/github-pr-client.test.ts @@ -74,3 +74,45 @@ exit 1 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 4fabdc2f..37f453b6 100644 --- a/packages/merge-steward/test/reconciler-merge.test.ts +++ b/packages/merge-steward/test/reconciler-merge.test.ts @@ -101,7 +101,7 @@ test("branch cleanup runs after GitHub reports the PR as merged", async () => { merged: true, }; }, - async listOpenPRs() { + async listOpenPRsByBase() { return []; }, async deleteBranch() { @@ -126,17 +126,17 @@ test("branch cleanup retargets direct stack children before deleting their paren async getStatus() { return { number: 764, - branch: "feature", + branch: "renamed-feature", headSha: "head", mergeable: false, reviewApproved: true, merged: true, }; }, - async listOpenPRs() { + async listOpenPRsByBase(baseBranch: string) { + calls.push(`discover:${baseBranch}`); return [ - { number: 765, branch: "child", headSha: "child-head", baseBranch: "feature" }, - { number: 766, branch: "other", headSha: "other-head", baseBranch: "main" }, + { number: 765, branch: "child", headSha: "child-head", baseBranch: baseBranch }, ]; }, async setBaseBranch(prNumber: number, baseBranch: string) { @@ -152,7 +152,7 @@ test("branch cleanup retargets direct stack children before deleting their paren delayMs: 0, }); - assert.deepEqual(calls, ["retarget:765:main", "delete:764"]); + assert.deepEqual(calls, ["discover:renamed-feature", "retarget:765:main", "delete:764"]); assert.equal(events.at(-1)?.action, "stack_children_retargeted"); }); @@ -170,7 +170,7 @@ test("branch cleanup preserves the parent branch when stack children cannot be i merged: true, }; }, - async listOpenPRs() { + async listOpenPRsByBase() { throw new Error("GitHub unavailable"); }, async deleteBranch() {