From f6cca66afa85dc3ec67613a01903d497cab7bf8a Mon Sep 17 00:00:00 2001 From: Renzo F <45401804+rennf93@users.noreply.github.com> Date: Mon, 20 Jul 2026 20:38:53 +0200 Subject: [PATCH] fix(git): branch list classifies remote refs correctly and prunes stale ones (#610) * fix(git): branch list classifies remote refs correctly and prunes stale ones The branches route detected remote-tracking refs via a 'remotes/' prefix that --format=%(refname:short) never emits, so every origin/* ref rendered under LOCAL and origin/HEAD surfaced as a fake branch. Listing now uses the full %(refname) and classifies on refs/heads/ vs refs/remotes/. Cleanup's remote deletion worked, but no code path ever pruned the viewing clone's remote-tracking refs, so deleted branches persisted in the UI forever. The branches route now runs a best-effort 'git remote prune origin' before listing remote refs, and the manual Fetch fetches with --prune. * refactor(git): extract branch-line classifier to satisfy the complexity gate --------- Co-authored-by: Renn F --- roboco/api/routes/git.py | 42 ++++++++++++++++++++-------- roboco/services/git.py | 28 ++++++++++++++++++- tests/integration/test_git_routes.py | 26 +++++++++++++++-- 3 files changed, 81 insertions(+), 15 deletions(-) diff --git a/roboco/api/routes/git.py b/roboco/api/routes/git.py index f969fa7d..12469d5f 100644 --- a/roboco/api/routes/git.py +++ b/roboco/api/routes/git.py @@ -290,6 +290,28 @@ async def get_git_log( ) +def _parse_branch_line(line: str) -> tuple[str, bool, str | None] | None: + """Classify one `%(refname)|%(objectname:short)` line as (name, is_remote, + last_commit), or None for skippable entries (blank, origin/HEAD, other ref + namespaces). Full refname, not `:short` — a remote-tracking ref shortens to + `origin/`, indistinguishable from a local branch literally named + that; classify on the `refs/heads/` vs `refs/remotes/` prefix instead. + """ + if not line: + return None + parts = line.split("|") + ref = parts[0] + last_commit = parts[1] if len(parts) > 1 else None + if ref.startswith("refs/heads/"): + return ref.removeprefix("refs/heads/"), False, last_commit + if ref.startswith("refs/remotes/"): + _remote_name, _, name = ref.removeprefix("refs/remotes/").partition("/") + if not name or name == "HEAD": + return None # origin/HEAD is a symbolic pointer, not a branch + return name, True, last_commit + return None + + @router.get("/branches", response_model=GitBranchListResponse) async def list_branches( db: DbSession, @@ -305,8 +327,12 @@ async def list_branches( workspace = await git_service.get_workspace(project_slug, agent.agent_id) current_branch = await git_service.get_current_branch(workspace) - # Get branches - args = ["branch", "--format=%(refname:short)|%(objectname:short)"] + if include_remote: + # Self-heal orphaned remote-tracking refs (branches deleted + # upstream via the forge API) before listing them. + await git_service.prune_remote_best_effort(workspace) + + args = ["branch", "--format=%(refname)|%(objectname:short)"] if include_remote: args.append("-a") @@ -316,16 +342,10 @@ async def list_branches( branches = [] for line in branch_result.stdout.strip().split("\n"): - if not line: + parsed = _parse_branch_line(line) + if parsed is None: continue - parts = line.split("|") - name = parts[0] - last_commit = parts[1] if len(parts) > 1 else None - - is_remote = name.startswith("remotes/") - if is_remote: - name = name.replace("remotes/origin/", "") - + name, is_remote, last_commit = parsed branches.append( BranchInfo( name=name, diff --git a/roboco/services/git.py b/roboco/services/git.py index ad4dc124..e706d68a 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -648,6 +648,28 @@ class GitService(BaseService): return None return bool(result.stdout.strip()) + async def prune_remote_best_effort(self, workspace: Path) -> None: + """Drop stale `origin/*` remote-tracking refs (ref-only, no object + transfer) so a branch deleted upstream stops showing in the viewing + clone's remote branch list. Never raises — a prune failure must not + break the caller's listing, mirroring `branch_exists_on_remote`. + """ + try: + token = await self._token_for_workspace(workspace) + await self._run_git( + workspace, + ["remote", "prune", "origin"], + check=False, + token=token, + timeout=_network_git_timeout(), + ) + except Exception as exc: + self.log.warning( + "remote prune failed; continuing with existing refs", + workspace=str(workspace), + error=str(exc), + ) + # ========================================================================= # STATUS / INFO METHODS # ========================================================================= @@ -1842,6 +1864,10 @@ class GitService(BaseService): ) -> tuple[str, bool, list[str], list[str], list[str], int, int]: """Fetch changes from origin without merging and return post-fetch status. + `--prune` drops local remote-tracking refs for branches deleted + upstream, so the manual Fetch button self-heals the same staleness + `prune_remote_best_effort` targets for the branches-list route. + Uses _network_git_timeout() because the operation talks to origin. Returns: (current_branch, has_changes, staged, unstaged, untracked, @@ -1850,7 +1876,7 @@ class GitService(BaseService): token = await self._token_for_workspace(workspace) await self._run_git( workspace, - ["fetch", "origin"], + ["fetch", "origin", "--prune"], token=token, timeout=_network_git_timeout(), ) diff --git a/tests/integration/test_git_routes.py b/tests/integration/test_git_routes.py index c3bf6ef4..d9721efe 100644 --- a/tests/integration/test_git_routes.py +++ b/tests/integration/test_git_routes.py @@ -324,7 +324,7 @@ async def test_log_service_error(git_client: dict) -> None: @pytest.mark.asyncio async def test_branches_local_only(git_client: dict) -> None: branch_result = MagicMock() - branch_result.stdout = "main|abc123\nfeature/x|def456\n" + branch_result.stdout = "refs/heads/main|abc123\nrefs/heads/feature/x|def456\n" with patch("roboco.api.routes.git.get_git_service") as mock_get: svc = AsyncMock() svc.get_workspace = AsyncMock(return_value="/tmp/ws") @@ -336,12 +336,27 @@ async def test_branches_local_only(git_client: dict) -> None: headers=_HDR, ) assert response.status_code == HTTPStatus.OK + names = {b["name"]: b for b in response.json()["branches"]} + assert names["main"]["is_remote"] is False + assert names["feature/x"]["is_remote"] is False + # include_remote=False (default) never prunes. + svc.prune_remote_best_effort.assert_not_awaited() @pytest.mark.asyncio async def test_branches_with_remote(git_client: dict) -> None: + """Regression: `%(refname)` renders a remote-tracking ref as + `refs/remotes/origin/` (real git never emits the old stub's + `remotes/origin/` shape) — it must classify as remote with the + `refs/remotes/origin/` prefix stripped down to the bare branch name, and + the symbolic `origin/HEAD` ref must be dropped, not surfaced as a fake + branch named "HEAD".""" branch_result = MagicMock() - branch_result.stdout = "main|abc123\nremotes/origin/feature/y|def456\n" + branch_result.stdout = ( + "refs/heads/main|abc123\n" + "refs/remotes/origin/feature/y|def456\n" + "refs/remotes/origin/HEAD|abc123\n" + ) with patch("roboco.api.routes.git.get_git_service") as mock_get: svc = AsyncMock() svc.get_workspace = AsyncMock(return_value="/tmp/ws") @@ -354,6 +369,11 @@ async def test_branches_with_remote(git_client: dict) -> None: headers=_HDR, ) assert response.status_code == HTTPStatus.OK + names = {b["name"]: b for b in response.json()["branches"]} + assert names["feature/y"]["is_remote"] is True + assert "origin/feature/y" not in names + assert "HEAD" not in names + svc.prune_remote_best_effort.assert_awaited_once_with("/tmp/ws") @pytest.mark.asyncio @@ -361,7 +381,7 @@ async def test_branches_skips_empty_lines(git_client: dict) -> None: """Line 246: empty line in branch output triggers continue.""" branch_result = MagicMock() # Embed an empty line between two branches. - branch_result.stdout = "main|abc\n\nfeature/x|def\n" + branch_result.stdout = "refs/heads/main|abc\n\nrefs/heads/feature/x|def\n" with patch("roboco.api.routes.git.get_git_service") as mock_get: svc = AsyncMock() svc.get_workspace = AsyncMock(return_value="/tmp/ws")