Fix #2398: share one synthetic session across slice integration branch creation

orchestrator/gateway_client.py

@@ -1611,39 +1611,15 @@ def create_slice_integration_branch(
             # No-op: integration branch already exists at parent's tip.
             return True
 
-        # Refresh the local remote-tracking ref + odb so the parent's
-        # commit object is available for the push below.  ``git push
-        # <sha>:refs/heads/...`` requires the source object to be
-        # locally reachable; the fetch makes that true even when the
-        # worktree was just created and has never seen this ref.
-        # Best-effort: if it fails the object may already be local
-        # from a prior step, so we still attempt the ls-remote / push.
-        self.fetch_branch(
-            pipeline_id,
-            repo_path,
-            args=[f"+refs/heads/{parent_branch}:refs/remotes/origin/{parent_branch}"],
-            mode=mode,
-        )
-
-        # Resolve the parent to a SHA on origin.  Failing fast here
-        # produces a clear "parent not found" error instead of git's
-        # confusing ``src refspec X does not match any``.
-        parent_sha = self.get_remote_branch_sha(
-            pipeline_id,
-            repo_path,
-            f"refs/heads/{parent_branch}",
-            mode=mode,
-        )
-        if not parent_sha:
-            logger.warning(
-                "Parent branch not found on origin; cannot create slice integration branch",
-                pipeline_id=pipeline_id,
-                integration_branch=integration_branch,
-                parent_branch=parent_branch,
-            )
-            return False
-
+        # Register one synthetic session up front and share it across
+        # the fetch, ls-remote, and push (#2398).  The session is
+        # tagged for the push (branch=integration_branch + agent_role
+        # — required for the gateway's slice integration-branch
+        # exemption and branch-ownership check); the fetch and
+        # ls-remote endpoints accept any synthetic session, so the
+        # extra metadata is harmless for those calls.
         temp_container_id = f"{pipeline_id}-slice-branch-{integration_branch.replace('/', '-')}"
+        parent_sha: str | None = None
         session_token: str | None = None
         try:
             session = self.register_session(
@@ -1657,6 +1633,41 @@ def create_slice_integration_branch(
             )
             session_token = session.session_token
 
+            # Refresh the local remote-tracking ref + odb so the
+            # parent's commit object is available for the push below.
+            # ``git push <sha>:refs/heads/...`` requires the source
+            # object to be locally reachable; the fetch makes that
+            # true even when the worktree was just created and has
+            # never seen this ref.  Best-effort: if it fails the
+            # object may already be local from a prior step, so we
+            # still attempt the ls-remote / push.
+            self.fetch_branch(
+                pipeline_id,
+                repo_path,
+                args=[f"+refs/heads/{parent_branch}:refs/remotes/origin/{parent_branch}"],
+                mode=mode,
+                bearer_token=session_token,
+            )
+
+            # Resolve the parent to a SHA on origin.  Failing fast
+            # here produces a clear "parent not found" error instead
+            # of git's confusing ``src refspec X does not match any``.
+            parent_sha = self.get_remote_branch_sha(
+                pipeline_id,
+                repo_path,
+                f"refs/heads/{parent_branch}",
+                mode=mode,
+                bearer_token=session_token,
+            )
+            if not parent_sha:
+                logger.warning(
+                    "Parent branch not found on origin; cannot create slice integration branch",
+                    pipeline_id=pipeline_id,
+                    integration_branch=integration_branch,
+                    parent_branch=parent_branch,
+                )
+                return False
+
             refspec = f"{parent_sha}:refs/heads/{integration_branch}"
             self._make_request(
                 "/api/v1/git/push",
@@ -1935,30 +1946,45 @@ def fetch_branch(
         repo_path: str,
         args: list[str] | None = None,
         mode: Literal["public", "private"] = "public",
+        *,
+        bearer_token: str | None = None,
     ) -> bool:
         """Fetch with custom args using a temporary session.
 
         Best-effort operation used to fetch specific refs from remote.
 
         Args:
-            pipeline_id: Pipeline ID (used as container_id for the temp session)
+            pipeline_id: Pipeline ID; used as ``container_id`` for the
+                temp session and for log fields.  When ``bearer_token``
+                is supplied no session is registered, so it's only used
+                for log fields in that case.
             repo_path: Path to the repo directory
             args: Additional args for git fetch (e.g., ["+remote:local"])
+            mode: Network mode for the temp session.  Ignored when
+                ``bearer_token`` is supplied — the supplied session's
+                mode was fixed at its register time.
+            bearer_token: Pre-registered synthetic session token to reuse
+                (#2398).  When provided, skip the internal
+                ``register_session``/``delete_session`` and authenticate
+                the fetch with the supplied token — lets a caller share
+                one session across several gateway calls.
 
         Returns:
             True if fetch succeeded, False otherwise
         """
-        temp_container_id = f"{pipeline_id}-state-fetch"
-        session_token: str | None = None
+        owns_session = bearer_token is None
+        session_token: str | None = bearer_token
         try:
-            session = self.register_session(
-                container_id=temp_container_id,
-                container_ip=self.self_ip,
-                mode=mode,
-                pipeline_id=pipeline_id,
-                synthetic=True,
-            )
-            session_token = session.session_token
+            if owns_session:
+                temp_container_id = f"{pipeline_id}-state-fetch"
+                session = self.register_session(
+                    container_id=temp_container_id,
+                    container_ip=self.self_ip,
+                    mode=mode,
+                    pipeline_id=pipeline_id,
+                    synthetic=True,
+                )
+                session_token = session.session_token
 
             # Do NOT include container_id — repo_path is already the
             # resolved path; the synthetic container_id has no real
@@ -1989,7 +2015,7 @@ def fetch_branch(
             )
             return False
         finally:
-            if session_token:
+            if owns_session and session_token:
                 try:
                     self.delete_session(session_token)
                 except Exception:
@@ -2063,6 +2089,8 @@ def get_remote_branch_sha(
         repo_path: str,
         ref: str,
         mode: Literal["public", "private"] = "public",
+        *,
+        bearer_token: str | None = None,
     ) -> str | None:
         """Resolve a remote ref to its commit SHA via ``git ls-remote``.
 
@@ -2073,18 +2101,26 @@ def get_remote_branch_sha(
         different SHA than ``origin/<base_branch>``, the branch carries
         prior-pipeline commits and starting on top of it would inherit
         them — so refuse with a hint to ``cancel_task(cleanup=true)``.
+
+        ``bearer_token`` lets a caller pass in a pre-registered synthetic
+        session to share across several gateway calls (#2398).  When
+        provided, the per-call ``register_session``/``delete_session``
+        round-trip is skipped, and the ``mode`` argument is ignored —
+        the supplied session's mode was fixed at its register time.
         """
-        temp_container_id = f"{pipeline_id}-state-ls-remote-sha"
-        session_token: str | None = None
+        owns_session = bearer_token is None
+        session_token: str | None = bearer_token
         try:
-            session = self.register_session(
-                container_id=temp_container_id,
-                container_ip=self.self_ip,
-                mode=mode,
-                pipeline_id=pipeline_id,
-                synthetic=True,
-            )
-            session_token = session.session_token
+            if owns_session:
+                temp_container_id = f"{pipeline_id}-state-ls-remote-sha"
+                session = self.register_session(
+                    container_id=temp_container_id,
+                    container_ip=self.self_ip,
+                    mode=mode,
+                    pipeline_id=pipeline_id,
+                    synthetic=True,
+                )
+                session_token = session.session_token
 
             result = self._make_request(
                 "/api/v1/git/fetch",
@@ -2113,7 +2149,7 @@ def get_remote_branch_sha(
             )
             return None
         finally:
-            if session_token:
+            if owns_session and session_token:
                 try:
                     self.delete_session(session_token)
                 except Exception:

orchestrator/tests/test_create_slice_integration_branch.py

@@ -18,7 +18,7 @@
 """
 
 from datetime import datetime, timedelta
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, call, patch
 
 import pytest
 from gateway_client import GatewayClient, SessionInfo
@@ -347,3 +347,73 @@ def test_synthetic_session_carries_integration_branch_and_role(self, gateway_cli
         assert kwargs["agent_role"] == "coder"
         assert kwargs["mode"] == "private"
         assert kwargs["pipeline_id"] == "pipe-1"
+
+    def test_one_session_shared_across_fetch_lsremote_and_push(self, gateway_client):
+        """#2398: the fetch, ls-remote, and push must reuse a single
+        synthetic session — exactly one ``register_session`` and one
+        ``delete_session`` per call, with the same bearer token
+        forwarded to ``fetch_branch``, ``get_remote_branch_sha``, and
+        the push request."""
+        register_spy = MagicMock(return_value=_session_info("shared-tok"))
+        delete_spy = MagicMock(return_value=True)
+        fetch_spy = MagicMock(return_value=True)
+        ls_spy = MagicMock(return_value="cafebabe" * 5)
+
+        push_tokens: list[str | None] = []
+
+        def fake_make_request(endpoint, method=None, data=None, **kwargs):
+            if endpoint == "/api/v1/git/push":
+                push_tokens.append(kwargs.get("bearer_token"))
+            return {"success": True, "data": {}}
+
+        with (
+            patch.object(gateway_client, "register_session", side_effect=register_spy),
+            patch.object(gateway_client, "delete_session", side_effect=delete_spy),
+            patch.object(gateway_client, "fetch_branch", side_effect=fetch_spy),
+            patch.object(gateway_client, "get_remote_branch_sha", side_effect=ls_spy),
+            patch.object(gateway_client, "_make_request", side_effect=fake_make_request),
+        ):
+            ok = gateway_client.create_slice_integration_branch(
+                "pipe-1",
+                "/repo",
+                integration_branch="egg/issue-2393/slice-1",
+                parent_branch="egg/issue-2393",
+            )
+
+        assert ok is True
+        assert register_spy.call_count == 1, "session must be registered exactly once"
+        assert delete_spy.call_args_list == [call("shared-tok")], (
+            "session must be deleted exactly once with the shared token"
+        )
+        assert fetch_spy.call_args.kwargs.get("bearer_token") == "shared-tok"
+        assert ls_spy.call_args.kwargs.get("bearer_token") == "shared-tok"
+        assert push_tokens == ["shared-tok"]
+
+    def test_session_cleaned_up_when_parent_missing(self, gateway_client):
+        """With the shared-session refactor (#2398) ``register_session``
+        runs before the ``ls-remote`` SHA lookup, so a missing parent
+        still has to clean up the session it just registered."""
+        register_spy = MagicMock(return_value=_session_info("orphan-tok"))
+        delete_spy = MagicMock(return_value=True)
+
+        with (
+            patch.object(gateway_client, "register_session", side_effect=register_spy),
+            patch.object(gateway_client, "delete_session", side_effect=delete_spy),
+            patch.object(gateway_client, "fetch_branch", return_value=True),
+            patch.object(gateway_client, "get_remote_branch_sha", return_value=None),
+            patch.object(
+                gateway_client,
+                "_make_request",
+                return_value={"success": True, "data": {}},
+            ),
+        ):
+            ok = gateway_client.create_slice_integration_branch(
+                "pipe-1",
+                "/repo",
+                integration_branch="egg/issue-2393/slice-1",
+                parent_branch="egg/issue-2393",
+            )
+
+        assert ok is False
+        assert register_spy.call_count == 1
+        assert delete_spy.call_args_list == [call("orphan-tok")]