From e594eced9c60aa594091f111172ec24b5e35832d Mon Sep 17 00:00:00 2001 From: Alex Sehl Date: Tue, 29 Sep 2026 15:29:56 +0500 Subject: [PATCH] fix(sync): stop when a merged pull request is still in the stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a squash or rebase merge, the original commit stays in local history until the branch is rebased. Syncing then re-pushed the merged branch (recreating it if GitHub had deleted it), matched the change to its merged PR, and left the merged changes in every PR above it. Sync now checks this during planning, before anything is pushed or edited, and asks the user to rebase onto the remote base first. Dry run reports the same error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 (1M context) bstack-id: 70c558ad2f27440984a1cd55d30e71fd --- src/commands/sync.ts | 28 ++++++++++++++++++++++++++-- test/sync.test.ts | 24 +++++++++++++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/commands/sync.ts b/src/commands/sync.ts index 2c483f2..da5c9ae 100644 --- a/src/commands/sync.ts +++ b/src/commands/sync.ts @@ -96,7 +96,7 @@ export class SyncCommand { run(options: SyncOptions): SyncResult { const { base, remote, stack } = this.prepareStack(options); - const plan = this.plan(stack, base); + const plan = this.plan(stack, base, remote); if (options.dryRun) { return this.dryRunResult(stack, plan, base, remote); @@ -163,7 +163,7 @@ export class SyncCommand { // Everything the sync will do is decided here with read-only lookups, so // a dry run can report it without changing anything. - private plan(stack: Stack, base: string): SyncPlan { + private plan(stack: Stack, base: string, remote: string): SyncPlan { this.reporter.progress("Reading the previous stack state"); const state = this.stateStore.read(); @@ -187,6 +187,8 @@ export class SyncCommand { current: this.github.pullRequestForBranch(change.remoteBranch), })); + assertNoMergedChanges(existing, `${remote}/${base}`); + const omitted = this.omittedPullRequests(transition, stack.changes); return { state, previousEntry, transition, existing, omitted }; @@ -674,6 +676,28 @@ export class SyncCommand { } } +// A squash or rebase merge leaves the original commit in local history until +// the branch is rebased. Syncing it would re-push the merged branch and keep +// the merged changes in every PR above it. +function assertNoMergedChanges( + existing: readonly ExistingPullRequest[], + remoteBase: string, +) { + const merged = existing.filter(({ current }) => current?.state === "MERGED"); + + if (merged.length === 0) { + return; + } + + const pullRequests = merged + .map(({ change, current }) => `#${current!.number} (${change.subject})`) + .join(", "); + + throw new Error( + `${merged.length === 1 ? "Pull request" : "Pull requests"} ${pullRequests} ${merged.length === 1 ? "is" : "are"} already merged but still in the local stack. Rebase onto ${remoteBase} and run bstack again`, + ); +} + function branchUpdates(changes: readonly StackChange[]) { return changes.map((change) => ({ name: change.remoteBranch, diff --git a/test/sync.test.ts b/test/sync.test.ts index 6f4f9f6..68c03ad 100644 --- a/test/sync.test.ts +++ b/test/sync.test.ts @@ -98,6 +98,27 @@ describe("stack sync", () => { expect(github.mutations).toEqual([]); }); + test("refuses to sync a change whose pull request is already merged", () => { + const repository = new SyncRepository([commit("one"), commit("two")]); + const github = new SyncGitHub(); + github.mergedBranches.add("bstack/test-user/one"); + const stateStore = new RecordingStateStore(emptyState); + + expect(() => + command(repository, github, stateStore).run({ + base: "main", + remote: "origin", + draft: false, + dryRun: false, + }), + ).toThrow( + "Pull request #1 (Change one) is already merged but still in the local stack. Rebase onto origin/main and run bstack again", + ); + expect(repository.pushCalls).toEqual([]); + expect(github.mutations).toEqual([]); + expect(stateStore.writes).toEqual([]); + }); + test("rebuilds unchanged pull requests when the stack base changes", () => { const repository = new SyncRepository([commit("one"), commit("two")]); const github = new SyncGitHub(); @@ -384,6 +405,7 @@ class SyncGitHub { draft: boolean; }> = []; failNextLinkWith: Error | undefined; + readonly mergedBranches = new Set(); assertReady() {} @@ -402,7 +424,7 @@ class SyncGitHub { return { number, url: `https://example.test/pull/${number}`, - state: "OPEN", + state: this.mergedBranches.has(branch) ? "MERGED" : "OPEN", title: `Change ${id}`, body: "", isDraft: false,