From 16982ce887d6b5fc6ff2a54a90075a6a75d81ed5 Mon Sep 17 00:00:00 2001 From: Renn F Date: Thu, 4 Jun 2026 02:08:36 +0200 Subject: [PATCH] =?UTF-8?q?fix(git):=20idempotent=20branch=20creation=20?= =?UTF-8?q?=E2=80=94=20checkout=20existing=20branch=20instead=20of=20faili?= =?UTF-8?q?ng=20128?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prior claim attempt can create the branch on disk before the DB records branch_name (the claim rolls its fields back, but the on-disk branch persists). A plain checkout -b then fails 'already exists' (exit 128), and the resulting error-handling cascade is how branch creation spiraled into INTERNAL_ERROR. Fall back to checkout when checkout -b returns non-zero. --- roboco/services/git.py | 12 ++++++++- tests/unit/services/test_git.py | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/roboco/services/git.py b/roboco/services/git.py index 47d8c33b..b9bf9338 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -835,7 +835,17 @@ class GitService(BaseService): token=project_token, timeout=_network_git_timeout(), ) - await self._run_git(workspace, ["checkout", "-b", branch_name]) + # Idempotent branch creation: a prior attempt may have created the + # branch on disk but failed before the DB recorded branch_name (the + # claim rolls back its fields, but the on-disk branch persists). A + # plain `checkout -b` then fails "already exists" (exit 128), and the + # resulting error-handling cascade is how a retry spirals. Switch to + # the existing branch instead. + created = await self._run_git( + workspace, ["checkout", "-b", branch_name], check=False + ) + if created.returncode != 0: + await self._run_git(workspace, ["checkout", branch_name]) await self._run_git( workspace, ["push", "-u", "origin", branch_name], diff --git a/tests/unit/services/test_git.py b/tests/unit/services/test_git.py index 2035b5d5..d9c48b0f 100644 --- a/tests/unit/services/test_git.py +++ b/tests/unit/services/test_git.py @@ -299,3 +299,51 @@ async def test_commit_uses_longer_timeout_for_staging_and_commit() -> None: assert timeouts_by_subcmd["commit"] == settings.git_commit_timeout_seconds # ...while the cheap read-only ops kept the default (None → default budget). assert timeouts_by_subcmd["log"] is None + + +@pytest.mark.asyncio +async def test_create_branch_idempotent_when_branch_already_exists() -> None: + # A prior attempt may have created the branch on disk before the DB recorded + # branch_name; `checkout -b` then fails 128. create_branch must switch to the + # existing branch instead of raising (the raise triggered a retry cascade). + from roboco.api.schemas.git import GitCreateBranchRequest + + branch = "feature/backend/abc12345--def67890" + svc = _service() + object.__setattr__(svc, "_resolve_base_branch", AsyncMock(return_value="master")) + object.__setattr__(svc, "_project_default_branch", AsyncMock(return_value="master")) + object.__setattr__(svc, "_token_for_project", AsyncMock(return_value=None)) + object.__setattr__( + svc, "_checkout_base_with_fallback", AsyncMock(return_value="master") + ) + + calls: list[list[str]] = [] + + async def fake_run_git(_workspace: object, args: list[str], **_kw: object) -> object: + calls.append(list(args)) + rc = 1 if list(args[:2]) == ["checkout", "-b"] else 0 + return MagicMock(stdout="", returncode=rc) + + object.__setattr__(svc, "_run_git", fake_run_git) + + with ( + patch("roboco.services.git.build_branch_name", AsyncMock(return_value=branch)), + patch( + "roboco.services.git.get_task_service", + MagicMock(return_value=MagicMock(update=AsyncMock())), + ), + ): + await svc.create_branch( + Path("/tmp/ws"), + "backend", + GitCreateBranchRequest( + project_slug="roboco-api", + task_id=uuid4(), + branch_type="feature", + agent_id=str(uuid4()), + parent_branch=None, + ), + ) + + assert ["checkout", "-b", branch] in calls, "checkout -b attempted" + assert ["checkout", branch] in calls, "fell back to existing branch on 128"