fix(sync): stop when a merged pull request is still in the stack

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,

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<string>();
 
   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,