Fix #2871: don't mark an empty slice branch COMPLETE on restart

orchestrator/gateway_client.py

@@ -1992,6 +1992,7 @@ def is_slice_branch_merged_into_parent(
         *,
         integration_branch: str,
         parent_branch: str,
+        integration_base_sha: str | None = None,
         agent_role: str = "coder",
         mode: Literal["public", "private"] = "public",
     ) -> bool:
@@ -2011,6 +2012,15 @@ def is_slice_branch_merged_into_parent(
         * The integration branch tip equals the parent tip (``==`` is
           neither "merged" nor "diverged"; just a no-op state — let the
           regular create path handle it as a fast-forward no-op).
+        * ``integration_base_sha`` is supplied and the integration branch
+          tip still equals it (#2871): the branch never received a slice
+          commit, so it is *un-started* work, not merged work. Such a
+          branch is trivially an ancestor of any advanced parent (its tip
+          *is* the parent's old fork point), and treating that ancestry as
+          "merged → COMPLETE" silently skips a slice that never ran. We
+          can only make this call when the caller recorded the fork base;
+          ``None`` (slices provisioned before #2871) falls through to the
+          ancestor-only check, preserving the prior behaviour.
         * The ancestry check itself fails (gateway down, missing object
           after a flaky fetch). In that case we return False so the
           caller falls through to the existing create path rather than
@@ -2060,6 +2070,28 @@ def is_slice_branch_merged_into_parent(
             if parent_sha == existing_sha:
                 return False
 
+            # #2871 — empty / un-started slice branch guard. When the
+            # caller recorded the fork base (the SHA the integration
+            # branch was created at) and the branch tip still equals it,
+            # the slice never received a commit. Its tip is the parent's
+            # old fork point, so it is *trivially* an ancestor of any
+            # advanced parent — but that is un-started work, not merged
+            # work. Returning True here would mark the slice COMPLETE and
+            # skip it, running dependents without their prerequisite.
+            # ``existing_sha`` and ``integration_base_sha`` both originate
+            # from ``get_remote_branch_sha`` (full 40-char SHAs), so an
+            # exact compare is correct.
+            if integration_base_sha and existing_sha == integration_base_sha:
+                logger.info(
+                    "Slice integration branch is still at its creation base "
+                    "(no slice commits) — treating as un-started, not merged (#2871)",
+                    pipeline_id=pipeline_id,
+                    integration_branch=integration_branch,
+                    parent_branch=parent_branch,
+                    integration_base_sha=integration_base_sha,
+                )
+                return False
+
             # Both refs must be locally reachable for ``merge-base
             # --is-ancestor`` to evaluate without errors. Best-effort:
             # if either fetch fails the merge-base call will return

orchestrator/routes/pipelines.py

@@ -16651,6 +16651,10 @@ def _bootstrap_check_one(slice_obj: Any) -> tuple[str, bool]:
                     str(worktree_repo_path),
                     integration_branch=integration_branch_for_check,
                     parent_branch=parent_branch_for_check,
+                    # #2871 — pass the recorded fork base so an empty
+                    # (un-started) slice branch whose tip is still at its
+                    # creation base is not mistaken for merged work.
+                    integration_base_sha=slice_obj.integration_base_sha,
                     agent_role="coder",
                     mode=gateway_mode,  # type: ignore[arg-type]
                 )
@@ -16793,13 +16797,20 @@ def _run_one_slice_inner(slice_id: str, parent_slice_id: str | None) -> tuple[in
                 # Persist the parent-branch reference on the contract
                 # under the per-pipeline state lock so a concurrent
                 # tester / documenter contract write doesn't race with
-                # ours (reviewer_code v4 #5).
+                # ours (reviewer_code v4 #5). While we hold the contract,
+                # also read back any integration_base_sha recorded on a
+                # prior run (#2871) — on a restart this lets the race
+                # check below tell an empty branch apart from a merged
+                # one. It is ``None`` on a slice's first run (recorded
+                # only after the branch is created, just below).
+                recorded_base_sha: str | None = None
                 try:
                     with get_pipeline_state_lock(pipeline_id):
                         contract_local = load_contract(pipeline_id, worktree_repo_path)
                         for s in contract_local.slices:
                             if s.id == slice_id:
                                 s.parent_branch_at_creation = parent_branch
+                                recorded_base_sha = s.integration_base_sha
                                 break
                         save_contract(contract_local, worktree_repo_path)
                 except Exception as save_err:  # noqa: BLE001
@@ -16828,6 +16839,7 @@ def _run_one_slice_inner(slice_id: str, parent_slice_id: str | None) -> tuple[in
                             str(worktree_repo_path),
                             integration_branch=integration_branch,
                             parent_branch=parent_branch,
+                            integration_base_sha=recorded_base_sha,
                             agent_role="coder",
                             mode=gateway_mode,  # type: ignore[arg-type]
                         )
@@ -16905,6 +16917,46 @@ def _run_one_slice_inner(slice_id: str, parent_slice_id: str | None) -> tuple[in
                             f"{parent_branch}"
                         )
 
+                    # #2871 — record the integration branch's fork base
+                    # exactly once, on first creation. The branch was just
+                    # pushed at the parent's tip and no agent has been
+                    # spawned yet, so its origin tip still equals its base.
+                    # Persisting it now lets a later restart's bootstrap
+                    # reconciliation (and the race check above) tell an
+                    # *empty* slice branch — tip still at this base, hence
+                    # a trivial ancestor of an advanced parent — apart from
+                    # a genuinely *merged* one whose tip moved past it. We
+                    # only write it when unset so a restart over a branch
+                    # that already carries slice commits (#2512 recovery)
+                    # keeps its original base rather than the advanced tip.
+                    if recorded_base_sha is None:
+                        try:
+                            base_sha = spawner.gateway.get_remote_branch_sha(
+                                pipeline_id,
+                                str(worktree_repo_path),
+                                f"refs/heads/{integration_branch}",
+                                mode=gateway_mode,  # type: ignore[arg-type]
+                            )
+                            if base_sha:
+                                with get_pipeline_state_lock(pipeline_id):
+                                    contract_local = load_contract(pipeline_id, worktree_repo_path)
+                                    for s in contract_local.slices:
+                                        if s.id == slice_id:
+                                            s.integration_base_sha = base_sha
+                                            break
+                                    save_contract(contract_local, worktree_repo_path)
+                                recorded_base_sha = base_sha
+                        except Exception as base_err:  # noqa: BLE001
+                            logger.warning(
+                                "Failed to record slice integration_base_sha "
+                                "(#2871); empty-branch detection degrades to "
+                                "ancestor-only on a future restart",
+                                pipeline_id=pipeline_id,
+                                slice_id=slice_id,
+                                integration_branch=integration_branch,
+                                error=str(base_err),
+                            )
+
                 logger.info(
                     "Slice spawn",
                     pipeline_id=pipeline_id,

orchestrator/tests/test_create_slice_integration_branch.py

@@ -793,6 +793,94 @@ def fake_make_request(endpoint, method=None, data=None, **kwargs):
             "ancestor of parent, signalling 'slice merged into parent'"
         )
 
+    def test_empty_branch_at_creation_base_is_not_merged(self, gateway_client):
+        """#2871 — the slice integration branch never received a commit:
+        its tip on origin still equals the recorded ``integration_base_sha``
+        (the parent SHA it was forked at). When the parent later advances,
+        that tip is trivially an ancestor of the new parent tip — but this
+        is *un-started* work, not merged work. The empty-branch guard must
+        return False *before* the merge-base call so the slice still runs."""
+        base_sha = "abcd1234" * 5  # parent tip at fork == empty branch tip
+        parent_sha = "ef99ef99" * 5  # parent has since advanced past the fork
+
+        merge_base_calls: list[dict] = []
+
+        def fake_make_request(endpoint, method=None, data=None, **kwargs):
+            if endpoint == "/api/v1/git/execute":
+                merge_base_calls.append(dict(data or {}))
+                # If the guard failed to short-circuit, the empty branch's
+                # base IS an ancestor of the advanced parent → would
+                # wrongly report merged.
+                return {"success": True, "data": {"returncode": 0}}
+            return {"success": True, "data": {}}
+
+        with (
+            patch.object(gateway_client, "register_session", return_value=_session_info()),
+            patch.object(gateway_client, "delete_session", return_value=True),
+            patch.object(gateway_client, "fetch_branch", return_value=True),
+            patch.object(
+                gateway_client,
+                "get_remote_branch_sha",
+                # Integration branch tip == base_sha (never advanced).
+                side_effect=self._setup_remotes(parent_sha, base_sha),
+            ),
+            patch.object(gateway_client, "_make_request", side_effect=fake_make_request),
+        ):
+            merged = gateway_client.is_slice_branch_merged_into_parent(
+                "issue-2777-replan",
+                "/repo",
+                integration_branch="egg/issue-2777-replan/slice-1",
+                parent_branch="egg/issue-2777-replan/work",
+                integration_base_sha=base_sha,
+            )
+
+        assert merged is False, (
+            "an empty slice branch (tip still at its creation base) is "
+            "un-started work, not merged — #2871 false-COMPLETE regression"
+        )
+        assert merge_base_calls == [], (
+            "the empty-branch guard must short-circuit before the merge-base ancestry call runs"
+        )
+
+    def test_recorded_base_does_not_block_genuinely_merged_branch(self, gateway_client):
+        """#2871 guard is additive: when the slice branch tip has moved
+        past its recorded base (it carries slice commits) and is an
+        ancestor of the parent, the merged signal still fires True. The
+        base-SHA check only suppresses the *empty* case."""
+        base_sha = "11112222" * 5  # fork base
+        existing_sha = "33334444" * 5  # slice tip with commits (!= base)
+        parent_sha = "55556666" * 5  # parent that has merged the slice
+
+        merge_base_calls: list[dict] = []
+
+        def fake_make_request(endpoint, method=None, data=None, **kwargs):
+            if endpoint == "/api/v1/git/execute":
+                merge_base_calls.append(dict(data or {}))
+                return {"success": True, "data": {"returncode": 0}}
+            return {"success": True, "data": {}}
+
+        with (
+            patch.object(gateway_client, "register_session", return_value=_session_info()),
+            patch.object(gateway_client, "delete_session", return_value=True),
+            patch.object(gateway_client, "fetch_branch", return_value=True),
+            patch.object(
+                gateway_client,
+                "get_remote_branch_sha",
+                side_effect=self._setup_remotes(parent_sha, existing_sha),
+            ),
+            patch.object(gateway_client, "_make_request", side_effect=fake_make_request),
+        ):
+            merged = gateway_client.is_slice_branch_merged_into_parent(
+                "p",
+                "/repo",
+                integration_branch="egg/issue-1/slice-1",
+                parent_branch="egg/issue-1",
+                integration_base_sha=base_sha,
+            )
+
+        assert merged is True
+        assert len(merge_base_calls) == 1
+
     def test_returns_false_when_slice_tip_diverged_from_parent(self, gateway_client):
         """Genuinely diverged history (slice has commits parent doesn't,
         or vice versa) → not merged. Caller falls through to the regular

shared/egg_contracts/models.py

@@ -370,6 +370,22 @@ class Slice(EggContractBaseModel):
             "that have not yet been provisioned."
         ),
     )
+    integration_base_sha: str | None = Field(
+        default=None,
+        pattern=r"^[a-f0-9]{7,40}$",
+        description=(
+            "Origin SHA the slice's integration branch was forked at "
+            "when first created (#2871). Recorded once, right after "
+            "``create_slice_integration_branch`` succeeds and before any "
+            "agent is spawned, so the branch tip still equals its base. "
+            "Lets ``is_slice_branch_merged_into_parent`` distinguish an "
+            "*empty, un-started* slice branch (tip still == this base, so "
+            "trivially an ancestor of an advanced parent) from a genuinely "
+            "*merged* one (tip moved past the base). ``None`` for slices "
+            "provisioned before this field existed — the merged-check then "
+            "falls back to its prior ancestor-only behaviour."
+        ),
+    )
     commit: str | None = Field(
         default=None,
         pattern=r"^[a-f0-9]{7,40}$",
@@ -379,7 +395,7 @@ class Slice(EggContractBaseModel):
         default_factory=list, description="Feedback from reviewer"
     )
 
-    @field_validator("commit", mode="before")
+    @field_validator("commit", "integration_base_sha", mode="before")
     @classmethod
     def validate_commit(cls, v: Any) -> str | None:
         return _normalize_commit(v)