From e217b2454f85b129e086959071b1eeedfac49ea3 Mon Sep 17 00:00:00 2001
From: Patrick Roza <contact@patrickroza.com>
Date: Mon, 27 Jul 2026 15:04:19 +0200
Subject: [PATCH] fix(stack): treat already-based overlays as rebase success

The post-sync overlay gate compared skip reasons against strings that
never match rebaseOpenFeaturePullRequests ("already based on new
fork/changes"). Overlays that were already on the tip were reported as
incomplete, hard-failing the stack job and blocking compose/dispatch.

Accept the actual reason strings via isSuccessfulFeatureRebaseSkip so a
no-op cascade still composes integration overlays.
---
 scripts/rebase-pr-stack.test.ts | 54 +++++++++++++++++++++++++++++++++
 scripts/rebase-pr-stack.ts      | 23 ++++++++++++--
 2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/scripts/rebase-pr-stack.test.ts b/scripts/rebase-pr-stack.test.ts
index c3f7b082e2d..412a9703aa0 100644
--- a/scripts/rebase-pr-stack.test.ts
+++ b/scripts/rebase-pr-stack.test.ts
@@ -8,6 +8,7 @@ import * as NodePath from "node:path";
 
 import {
   baseHistoryPushArgs,
+  isSuccessfulFeatureRebaseSkip,
   rebaseOpenFeaturePullRequests,
   RebaseConflictError,
   resumeStack,
@@ -20,6 +21,59 @@ import {
   validatePullRequestSnapshots,
 } from "./rebase-pr-stack.ts";
 
+describe("isSuccessfulFeatureRebaseSkip", () => {
+  it("treats actual already-based reason strings as success", () => {
+    // rebaseOpenFeaturePullRequests emits these exact strings when an overlay
+    // (or feature) already contains the new parent tip. The post-sync overlay
+    // gate must not treat them as incomplete — that bug hard-failed stack
+    // runs after #97 whenever overlays needed no rewrite.
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip("already based on fork/changes", "fork/changes"),
+      true,
+    );
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip(
+        "remote already based on fork/changes after concurrent update",
+        "fork/changes",
+      ),
+      true,
+    );
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip("rebase produced identical tip", "fork/changes"),
+      true,
+    );
+  });
+
+  it("does not accept the historical mistyped allowlist that never matched", () => {
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip("already based on new fork/changes", "fork/changes"),
+      false,
+    );
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip(
+        "remote already based on new fork/changes after concurrent update",
+        "fork/changes",
+      ),
+      false,
+    );
+  });
+
+  it("still fails incomplete recovery / missing-branch skips", () => {
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip(
+        "cannot recover old fork/changes tip (no known historical base tip is an ancestor of this head)",
+        "fork/changes",
+      ),
+      false,
+    );
+    assert.equal(isSuccessfulFeatureRebaseSkip("missing remote branch", "fork/changes"), false);
+    assert.equal(
+      isSuccessfulFeatureRebaseSkip("parent branch fork/changes was not rebased", "fork/changes"),
+      false,
+    );
+  });
+});
+
 describe("baseHistoryPushArgs", () => {
   it("force-updates the blob ref while leasing its observed remote value", () => {
     assert.deepEqual(baseHistoryPushArgs("abc123"), [
diff --git a/scripts/rebase-pr-stack.ts b/scripts/rebase-pr-stack.ts
index 4e6c94a2474..da9c769dbf8 100644
--- a/scripts/rebase-pr-stack.ts
+++ b/scripts/rebase-pr-stack.ts
@@ -1537,6 +1537,8 @@ export async function syncStack(options: StackRunOptions): Promise<StackRunResul
         // Integration overlays must be based on the new tip for compose. Surface a
         // hard error when a registered overlay could not be rebased, instead of
         // failing later with a less actionable compose-time message.
+        // "Already based" / identical-tip skips are success — see
+        // isSuccessfulFeatureRebaseSkip (must match actual skip reason strings).
         if (manifest.integrationOverlays.length > 0) {
           const overlayBranches = new Set(manifest.integrationOverlays.map(({ branch }) => branch));
           const failedOverlays = featureResult.conflicts.filter((entry) =>
@@ -1545,9 +1547,7 @@ export async function syncStack(options: StackRunOptions): Promise<StackRunResul
           const skippedOverlays = featureResult.skipped.filter(
             (entry) =>
               overlayBranches.has(entry.branch) &&
-              entry.reason !== "already based on new fork/changes" &&
-              entry.reason !== "remote already based on new fork/changes after concurrent update" &&
-              entry.reason !== "rebase produced identical tip",
+              !isSuccessfulFeatureRebaseSkip(entry.reason, manifest.forkChangesBranch),
           );
           if (failedOverlays.length > 0 || skippedOverlays.length > 0) {
             const details = [
@@ -1584,6 +1584,23 @@ export async function syncStack(options: StackRunOptions): Promise<StackRunResul
   return result;
 }
 
+/**
+ * Skip reasons from {@link rebaseOpenFeaturePullRequests} that mean the branch
+ * is already correctly based on its parent (no further work needed).
+ *
+ * Keep these strings in sync with the `skipped.push({ reason: ... })` sites in
+ * that function. The post-sync overlay gate must treat them as success, not as
+ * "incomplete" failures — otherwise a no-op cascade hard-fails when overlays
+ * are already on the new tip and blocks compose/dispatch.
+ */
+export function isSuccessfulFeatureRebaseSkip(reason: string, baseBranch: string): boolean {
+  return (
+    reason === `already based on ${baseBranch}` ||
+    reason === `remote already based on ${baseBranch} after concurrent update` ||
+    reason === "rebase produced identical tip"
+  );
+}
+
 /**
  * Append fork/changes tips to the durable base-history ref and push it.
  * Newest tips first so multi-generation recovery prefers the most recent base