Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions packages/merge-steward/src/github/pr-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,41 @@ export class GitHubPRClient implements GitHubPRApi {
}
}

async listOpenPRsByBase(baseBranch: string): Promise<Array<{ number: number; branch: string; headSha: string; baseBranch: string }>> {
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<Array<{
number: number;
head: { ref: string; sha: string };
base: { ref: string };
}>>;
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<void> {
await exec("gh", [
"api", "--method", "PATCH",
`repos/${this.repoFullName}/pulls/${prNumber}`,
"-f", `base=${baseBranch}`,
], { githubRepoFullName: this.repoFullName });
}

async deleteBranch(prNumber: number): Promise<void> {
const status = await this.getStatus(prNumber);
await exec("gh", [
Expand Down
4 changes: 4 additions & 0 deletions packages/merge-steward/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,12 @@ export interface GitHubPRApi {
setLabels(prNumber: number, opts: { add?: string[]; remove?: string[] }): Promise<void>;
/** List open PRs for restart-time eligibility scans. */
listOpenPRs(): Promise<Array<{ number: number; branch: string; headSha: string; baseBranch: string }>>;
/** List every open PR whose current base is the given branch. */
listOpenPRsByBase(baseBranch: string): Promise<Array<{ number: number; branch: string; headSha: string; baseBranch: string }>>;
/** Find the open PR number for a branch, or null if none exists. */
findPRByBranch(branch: string): Promise<number | null>;
/** Retarget a PR before deleting a branch used as its current base. */
setBaseBranch(prNumber: number, baseBranch: string): Promise<void>;
/** Delete the PR's head branch from the remote (best-effort cleanup). */
deleteBranch(prNumber: number): Promise<void>;
/** List open PRs with a specific label (for startup scan). */
Expand Down
24 changes: 21 additions & 3 deletions packages/merge-steward/src/reconciler-merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReturnType<ReconcileContext["github"]["getStatus"]>> | 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 {
Expand Down
10 changes: 10 additions & 0 deletions packages/merge-steward/src/sim/github-sim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ export class GitHubSim implements GitHubPRApi {
return null;
}

async setBaseBranch(prNumber: number, baseBranch: string): Promise<void> {
const pr = this.prs.get(prNumber);
if (!pr) throw new Error(`PR #${prNumber} not found`);
pr.baseRefName = baseBranch;
}

async listLabels(prNumber: number): Promise<string[]> {
const pr = this.prs.get(prNumber);
if (!pr) return [];
Expand All @@ -150,6 +156,10 @@ export class GitHubSim implements GitHubPRApi {
}));
}

async listOpenPRsByBase(baseBranch: string): Promise<Array<{ number: number; branch: string; headSha: string; baseBranch: string }>> {
return (await this.listOpenPRs()).filter((pr) => pr.baseBranch === baseBranch);
}

async deleteBranch(_prNumber: number): Promise<void> {
// No-op in sim — branch deletion is cosmetic cleanup.
}
Expand Down
1 change: 1 addition & 0 deletions packages/merge-steward/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
44 changes: 44 additions & 0 deletions packages/merge-steward/test/github-pr-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 });
}
Expand Down
72 changes: 72 additions & 0 deletions packages/merge-steward/test/reconciler-merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
Expand All @@ -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;
Expand Down
Loading