diff --git a/CHANGELOG.md b/CHANGELOG.md index a6f8e7e2..23d34630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- **A Main PM blocking its own coordination root no longer hands the whole root to the Board (a respawn catch-22).** Root cause: the generic escalation chain points `main-pm → product-owner`, and `i_am_blocked` / escalate REASSIGNS the task to that chain target. The board-advisory guard that refuses such a hand-off only covered descendant cell tasks (it required `parent_task_id`), so a top-level Main-PM **coordination root** slipped through and the entire root was reassigned to the Product Owner and marked blocked. A Board role has no `unblock` verb at all (only notify / note / triage / i_am_idle) and the unblock gate is assignee-only, so it could neither resolve the blocker nor hand it off — it just spam-notified the CEO while the blocked-task dispatcher respawned it every tick (one live incident burned an estimated 6400+ tool calls on a single root). Fixed at both layers: the escalation / reassign / revival guard now also refuses a Board owner for a `main_pm` coordination task (root or MegaTask root-subtask) and diverts it to the pool for a role-matched (Main-PM) re-claim — the upstream cure — via a single shared `_board_cannot_own` predicate; and, as a defense-in-depth backstop, the orchestrator's blocker dispatcher no longer treats a Board role as a blocker resolver (it returns no resolver, so a mis-owned blocked task is skipped rather than respawned onto a role that physically cannot act). + +- **A racing state change mid-verb no longer crashes a PM into a respawn loop.** The gateway's verb runner guards the *initial* task/agent against `None`, but its composed atomic actions reassign the working task from each step (`i_will_plan` runs claim → set_plan → start). When a concurrent agent transitioned the row between the verb's precondition gate and execution — e.g. a racing `i_am_blocked` moved a coordination root from `needs_revision` to `blocked` — `claim()` found no valid transition and returned `None`, then the next step dereferenced `None.id` and crashed with the opaque `'NoneType' object has no attribute 'id'`, surfaced to the agent as a cryptic "verb runner failed" so the PM respawn-looped on the wedged root. The runner now re-checks after *each* composed action and fails fast with an actionable `INVALID_STATE` that tells the agent the row changed under it and to re-fetch and re-issue its verb (the savepoint rolls the partial sequence back). + +- **A completed task no longer wedges when its branch is missing from a re-provisioned clone.** Push-by-name (the fix that decoupled the push from the workspace checkout) still requires the named task branch to exist as a *local* ref — but a developer's shared clone can be freshly re-provisioned (the per-task workspace-collision recovery re-clones it), leaving the task branch absent locally even though its commits are safely on `origin` and the clone is parked on a different task's branch. `git push origin ` then died with the cryptic `src refspec does not match any` and the task blocked-looped at `i_am_done`. The push now recovers a missing local ref from `origin/` first (a clean no-op when the work is already on origin); if the branch exists on neither the clone nor origin the commits are genuinely gone from this clone, so it fails loud with a recoverable "unclaim the task and re-claim it to rebuild the branch, then replay your commits" instruction instead of the raw refspec error. + - **The orchestrator's own recovery actions now actually run.** Its background dispatcher made internal HTTP calls to its own API without an agent identity, so every self-`PATCH` to a task — auto-blocking a task with missing prerequisites, auto-resuming a PM's paused parent, auto-recovering a stale-blocked parent, annotating an SLA breach — was rejected with `401 Missing X-Agent-ID` and silently dropped. The visible effect was paused/blocked parent tasks staying wedged and their dependent work stranded (with the dispatcher logging a "respawning assignee" loop). Header propagation was inconsistent across the orchestrator's separate HTTP-client call-sites — only the main dispatch loop sent the identity. The system identity is now hoisted into one shared constant and applied to every API-facing dispatcher client (the external provider-recovery probe is intentionally excluded); the `system` role holds the permission required for the audited status-override path those routes use. - **A developer's completed work no longer silently fails to reach GitHub ("No commits between").** A developer's single git clone is shared across all of their tasks, so by the time a task's PR is opened the clone has usually moved on to a *later* task's branch. The push at the QA-submission / `open_pr` boundary, and the PR's head branch, were both taken from the clone's *current* checkout — so the push was rejected (the workspace was parked on another task's branch) and the locally-committed work never reached `origin`, leaving the task branch empty and `open_pr` failing with GitHub's "No commits between" 422. The work was on disk and correct, just never pushed. Both the push and the PR head now operate on the task's recorded branch **by name**, independent of the checkout (`push(branch=…)` targets the named ref; the PR head is the task's `branch_name`). Work committed on any of a shared clone's task branches now pushes and opens its PR correctly. diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index ef9f266e..38ea828b 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -8974,14 +8974,29 @@ Never `commit`, never write code, never run `git`. PMs coordinate. orchestrator respawns it forever (a livelock — a task escalated to Main PM kept respawning the ex-assignee cell PM, which could not author the note). So whenever the blocked task carries an assignee that is a - PM or board role, dispatch THAT assignee. Only a task with no PM/board - assignee (e.g. still held by the dev who raised i_am_blocked) falls back - to the cell PM for its team. + PM role, dispatch THAT assignee. Only a task with no PM assignee + (e.g. still held by the dev who raised i_am_blocked) falls back to the + cell PM for its team. + + A BOARD/advisory assignee (product-owner / head-marketing) is the one + case we must NOT dispatch: a board role has no ``unblock`` verb at all + — its only moves are notify/note/triage/i_am_idle — so dispatching it + to "resolve" a blocker is a futile catch-22. It cannot unblock, cannot + hand the task off (the assignee-only gate also forbids any PM from + unblocking a task it does not own), and so it spam-notifies the CEO and + the orchestrator respawns it forever (observed: 6400+ tool calls burned + on a single delivery root mis-assigned to product-owner). Return None so + the blocker dispatch SKIPS it — the task is mis-owned and must be + re-routed / surfaced to the CEO out-of-band, never auto-respawned onto a + role that physically cannot act. (The upstream cure is to never assign a + board role as the owner of an executable delivery/coordination root.) """ assignee_uuid = task.get("assigned_to") or task.get("claimed_by") if assignee_uuid: assignee_slug = self._resolve_agent_slug(str(assignee_uuid)) - if assignee_slug in self._PM_AGENTS or assignee_slug in self._BOARD_AGENTS: + if assignee_slug in self._BOARD_AGENTS: + return None + if assignee_slug in self._PM_AGENTS: return assignee_slug team = task.get("team") if team not in ("backend", "frontend", "ux_ui"): diff --git a/roboco/services/gateway/choreographer/_verb_runner.py b/roboco/services/gateway/choreographer/_verb_runner.py index 528119d5..e824710d 100644 --- a/roboco/services/gateway/choreographer/_verb_runner.py +++ b/roboco/services/gateway/choreographer/_verb_runner.py @@ -67,7 +67,33 @@ class VerbRunner: for side_effect_name in intent.pre_side_effects: await self._dispatch_side_effect(side_effect_name, task, agent) async with self.task_service.session.begin_nested(): - for action_name in intent.composes: + for position, action_name in enumerate(intent.composes): + # A composed atomic action (claim/set_plan/start) returns None when + # its source-status check fails — which happens mid-sequence when a + # CONCURRENT agent transitions the row between the verb's precondition + # gate and this execution (e.g. i_will_plan's gate saw `needs_revision` + # but a racing i_am_blocked moved it to `blocked`, so claim() found no + # valid transition and returned None). The NEXT action would then + # dereference None.id and crash with the cryptic "'NoneType' object has + # no attribute 'id'" — the entry guard above only covers the INITIAL + # task. Fail loud + clean before the next dispatch so the choreographer + # surfaces an actionable rejection and the agent re-fetches, not + # respawn-loops. + # + # Only an INTERMEDIATE None is fatal here. A None from the LAST + # composed action is the verb's own result: it flows out as the + # runner's return value so the caller's existing `if task is None` + # handler can surface the verb-specific message (e.g. "start + # failed for task ...", or the board verb's decline envelope) — + # preserving that contract instead of masking it as a crash. + if position > 0 and task is None: + raise ValueError( + f"INVALID_STATE: a composed action before '{action_name}' in " + f"'{intent_name}' returned no task — its source status was " + "invalid, most likely because a concurrent transition changed " + "the task between the precondition gate and execution. " + "Re-fetch with evidence(task_id) and re-issue your verb." + ) task = await self._dispatch_atomic(action_name, task, agent, context) for side_effect_name in intent.side_effects: await self._dispatch_side_effect(side_effect_name, task, agent) diff --git a/roboco/services/git.py b/roboco/services/git.py index d464eebb..fb75ba3a 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -1150,6 +1150,66 @@ class GitService(BaseService): workspace = await self.get_workspace(data.project_slug, agent_id) await self.checkout(workspace, data.branch) + async def _ensure_pushable_branch( + self, workspace: Path, branch: str, token: str | None + ) -> None: + """Make sure ``branch`` exists as a local ref before a push-by-name. + + Push-by-name (``git push origin ``) decouples the push from the + workspace checkout, but it still requires the named ref to exist LOCALLY. + A dev's clone is shared across tasks and may be freshly re-provisioned + (the per-task workspace-collision recovery re-clones it), so by push time + the task branch can be ABSENT as a local ref even though its commits are + already safe on origin — the workspace is parked on a different task's + branch. ``git push`` then dies with the cryptic ``src refspec + does not match any`` and the task wedges in a blocked respawn loop. + + Recover idempotently: if the local ref is missing, fetch ``origin + `` and recreate the local tracking ref so the subsequent + push-by-name is a clean no-op. If origin has no such branch either, the + commits are genuinely not in this clone — fail loud with a recoverable + instruction instead of the raw refspec error. + """ + local = await self._run_git( + workspace, + ["rev-parse", "--verify", "--quiet", f"refs/heads/{branch}"], + check=False, + ) + if local.returncode == 0: + return + # Local ref missing — try to recover it from origin (commits often + # already pushed in a prior cycle / clone). + await self._run_git( + workspace, + ["fetch", "origin", branch], + token=token, + check=False, + timeout=_network_git_timeout(), + ) + remote = await self._run_git( + workspace, + ["rev-parse", "--verify", "--quiet", f"refs/remotes/origin/{branch}"], + check=False, + ) + if remote.returncode == 0: + # Recreate the local ref from origin so push-by-name is a no-op + # instead of a refspec error. The local ref is known-absent here, so + # this only ever creates (never clobbers local-only commits). + await self._run_git( + workspace, + ["branch", branch, f"origin/{branch}"], + check=False, + ) + return + raise GitCommandError( + "push", + f"the task branch '{branch}' does not exist in this workspace and " + "is not on origin — your commits are not in this clone (it was " + "likely re-provisioned after a reassignment). unclaim the task and " + "re-claim it to rebuild the branch, then replay your commits via " + "commit(...).", + ) + async def push( self, workspace: Path, force: bool = False, branch: str | None = None ) -> tuple[str, int]: @@ -1165,6 +1225,10 @@ class GitService(BaseService): if branch is None: branch = await self.get_current_branch(workspace) token = await self._token_for_workspace(workspace) + # The named ref must exist locally for push-by-name; recover it from + # origin if a re-provisioned/shared clone is missing it (else the push + # dies on "src refspec ... does not match any" and the task wedges). + await self._ensure_pushable_branch(workspace, branch, token) count_result = await self._run_git( workspace, diff --git a/roboco/services/task.py b/roboco/services/task.py index fe26bd50..6695879b 100644 --- a/roboco/services/task.py +++ b/roboco/services/task.py @@ -140,6 +140,43 @@ def _is_cell_team_task(task: TaskTable) -> bool: return str(team_value) in _CELL_TEAMS +def _is_coordination_task(task: TaskTable) -> bool: + """True for a Main-PM-owned coordination task (delivery root or batch + root-subtask). + + A board/advisory role can no more OWN a coordination task than a cell task: + it has no verb to delegate, submit, unblock, or complete it. The escalation + chain points main-pm at product-owner, so a Main PM's ``i_am_blocked`` / + escalate on its own coordination root used to reassign the WHOLE root to the + board — which can only notify / triage / i_am_idle. The blocker dispatcher + then respawned that board role forever to "resolve" a blocker it physically + cannot unblock (a catch-22 that burned thousands of tool calls on a single + root). Keyed on the ``main_pm`` team, which is set on every coordination root + and MegaTask root-subtask, so it holds whether the task is a top-level root + or parented under an umbrella. The two predicates above both require a + descendant (``parent_task_id`` set), so a top-level coordination root slipped + through them — this closes that gap. + """ + team_value = getattr(task.team, "value", task.team) + return str(team_value) == Team.MAIN_PM.value + + +def _board_cannot_own(task: TaskTable) -> bool: + """True when a board/advisory role must NOT become the owner of ``task``. + + The single invariant behind the escalation / reassign / revival board guards: + a board role (product-owner / head-marketing / auditor) has no verb to build, + document, delegate, submit, unblock, or complete delivery work. It covers + cell-executed descendants, a cell's own coordination/planning descendants, + AND Main-PM coordination roots — every task shape a board role cannot drive. + """ + return ( + _is_descendant_executable_task(task) + or _is_cell_team_task(task) + or _is_coordination_task(task) + ) + + # Notes fields (dev_notes, qa_notes, quick_context) are append-only — # every revision cycle adds more. Cap total size so a task that cycles # dozens of times doesn't grow into megabytes. When we exceed the cap, @@ -4389,16 +4426,21 @@ class TaskService(BaseService): and the orchestrator re-spawns them. Without this, escalation loses the dev's identity permanently. - Invariant: a descendant executable task (code / documentation / - design) is NEVER assigned to a board/advisory role (they cannot own - cell-executed work). Such an escalation is diverted to a pool release so - a role-matched cell agent reclaims it. Enforced here — the single write - primitive — so both the gateway ``escalate`` verb and the HTTP escalate - route are covered. + Invariant: a board/advisory role is NEVER assigned a task it cannot own + — a descendant executable task (code / documentation / design), a cell's + own coordination/planning descendant, OR a Main-PM coordination root + (see ``_board_cannot_own``). They have no verb to build, delegate, + unblock, or complete such work. The escalation chain points main-pm at + product-owner, so without the coordination-root arm a Main PM's + ``i_am_blocked`` on its own root reassigned the whole root to the board, + which then respawn-looped on a blocker it could not resolve. Such an + escalation is diverted to a pool release so a role-matched agent reclaims + it. Enforced here — the single write primitive — so both the gateway + ``escalate`` verb and the HTTP escalate route are covered. """ - if ( - _is_descendant_executable_task(task) or _is_cell_team_task(task) - ) and await self._is_board_advisory_agent(target_agent_id): + if _board_cannot_own(task) and await self._is_board_advisory_agent( + target_agent_id + ): await self._release_code_task_to_pool( task=task, escalator_slug=escalator_slug, @@ -5100,9 +5142,7 @@ class TaskService(BaseService): """ owner = cast("Any", task.claimed_by or task.assigned_to) needs_rehome = owner is None or await self._is_board_advisory_agent(owner) - if needs_rehome and ( - _is_descendant_executable_task(task) or _is_cell_team_task(task) - ): + if needs_rehome and _board_cannot_own(task): await self._divert_owned_task_to_pool( task, note=( @@ -6723,7 +6763,7 @@ class TaskService(BaseService): # guard never fires for them. if ( new_assignee is not None - and (_is_descendant_executable_task(task) or _is_cell_team_task(task)) + and _board_cannot_own(task) and await self._is_board_advisory_agent(new_assignee) ): await self._divert_owned_task_to_pool( @@ -6769,9 +6809,9 @@ class TaskService(BaseService): return None # Invariant backstop (mirrors `reassign`): an active claim must not be # handed to a board/advisory role on a cell task — divert to the pool. - if ( - _is_descendant_executable_task(task) or _is_cell_team_task(task) - ) and await self._is_board_advisory_agent(new_assignee): + if _board_cannot_own(task) and await self._is_board_advisory_agent( + new_assignee + ): await self._divert_owned_task_to_pool( task, note=( @@ -7262,9 +7302,10 @@ class TaskService(BaseService): task, note=( f"\n\n[ESCALATION REDIRECTED] {escalator_slug} escalated this" - f" executable task toward {blocked_target_slug} (a board/advisory" - f" role that cannot own cell-executed work). Released to the pool" - f" for a role-matched claim instead." + f" task toward {blocked_target_slug} (a board/advisory role that" + f" cannot own delivery / coordination work — no build, delegate," + f" unblock, or complete verb). Released to the pool for a" + f" role-matched claim instead." f"\nReason: {reason}" ), ) diff --git a/tests/unit/gateway/test_verb_runner.py b/tests/unit/gateway/test_verb_runner.py index fea3693d..292d49f5 100644 --- a/tests/unit/gateway/test_verb_runner.py +++ b/tests/unit/gateway/test_verb_runner.py @@ -72,6 +72,42 @@ async def test_runner_rejects_none_task_or_agent() -> None: await runner.run_intent("i_will_plan", task, None, ctx) +@pytest.mark.asyncio +async def test_runner_rejects_none_returned_mid_composition() -> None: + """A composed action returning None mid-sequence fails loud, not a crash. + + Observed in prod: i_will_plan on a task a concurrent agent had just moved to + `blocked` — claim() returned None (no valid transition), then + _do_set_plan(None, ...) crashed with "'NoneType' object has no attribute + 'id'". The choreographer surfaced it as a cryptic "verb runner failed" and + the PM respawn-looped. The entry guard only covers the INITIAL task, so the + loop body must re-check after each composed action. + """ + task_svc = AsyncMock() + # __aexit__ must return falsy so the savepoint context does not SUPPRESS the + # ValueError raised inside it (real SQLAlchemy begin_nested re-raises + rolls back). + task_svc.session.begin_nested = MagicMock( + return_value=MagicMock( + __aenter__=AsyncMock(), __aexit__=AsyncMock(return_value=False) + ) + ) + # claim() returns None — its source status was invalid (concurrent change). + task_svc.claim = AsyncMock(return_value=None) + task_svc.set_plan = AsyncMock() + task_svc.start = AsyncMock() + runner = VerbRunner(task_service=task_svc, git_service=AsyncMock()) + + task = MagicMock(id=uuid4(), status="needs_revision", plan="p", commits=[]) + agent = MagicMock(id=uuid4(), role="main_pm") + ctx = spec.Context(plan="my plan") + + with pytest.raises(ValueError, match="INVALID_STATE"): + await runner.run_intent("i_will_plan", task, agent, ctx) + # The downstream composed actions must NOT run on a None task. + task_svc.set_plan.assert_not_called() + task_svc.start.assert_not_called() + + @pytest.mark.asyncio async def test_runner_runs_side_effects_after_db_commit() -> None: """For open_pr: composes is empty; side_effects (push_branch, create_pr) run.""" diff --git a/tests/unit/runtime/test_blocker_and_claimed_dispatch.py b/tests/unit/runtime/test_blocker_and_claimed_dispatch.py index d8d20d7f..dd970d5e 100644 --- a/tests/unit/runtime/test_blocker_and_claimed_dispatch.py +++ b/tests/unit/runtime/test_blocker_and_claimed_dispatch.py @@ -49,14 +49,30 @@ def test_blocked_task_assigned_to_main_pm_dispatches_main_pm() -> None: assert orch._blocker_resolver_slug(task) == "main-pm" -def test_blocked_task_assigned_to_board_dispatches_board() -> None: +def test_blocked_task_assigned_to_board_is_not_dispatched() -> None: + # A board/advisory role (product-owner / head-marketing) has NO unblock + # verb — dispatching it to resolve a blocker is a futile catch-22 (it can + # only notify/triage, so it spam-notifies the CEO and respawns forever). + # The resolver must be None so the blocker dispatch SKIPS it; the task is + # mis-owned and must be re-routed / surfaced to the CEO out-of-band. orch = _orch() task: dict[str, Any] = { "id": "t1", "team": "backend", "assigned_to": AGENT_UUIDS["product-owner"], } - assert orch._blocker_resolver_slug(task) == "product-owner" + assert orch._blocker_resolver_slug(task) is None + + +def test_blocked_task_assigned_to_head_marketing_is_not_dispatched() -> None: + # Same catch-22 guard for the other board role. + orch = _orch() + task: dict[str, Any] = { + "id": "t1", + "team": "backend", + "assigned_to": AGENT_UUIDS["head-marketing"], + } + assert orch._blocker_resolver_slug(task) is None def test_blocked_task_held_by_dev_falls_back_to_cell_pm() -> None: diff --git a/tests/unit/services/test_escalation_board_guard.py b/tests/unit/services/test_escalation_board_guard.py index b04a04ba..cb53d0f6 100644 --- a/tests/unit/services/test_escalation_board_guard.py +++ b/tests/unit/services/test_escalation_board_guard.py @@ -20,7 +20,9 @@ import pytest from roboco.models.base import AgentRole, TaskStatus, TaskType, Team from roboco.services.task import ( TaskService, + _board_cannot_own, _is_cell_team_task, + _is_coordination_task, _is_descendant_executable_task, ) @@ -116,11 +118,83 @@ def test_documentation_task_type_as_raw_string_is_flagged() -> None: assert _is_descendant_executable_task(task) is True +# --------------------------------------------------------------------------- +# _is_coordination_task (pure) — Main-PM coordination roots / root-subtasks +# --------------------------------------------------------------------------- + + +def test_main_pm_root_is_coordination_task() -> None: + # A top-level delivery coordination root (no parent, main_pm team). The two + # descendant predicates miss it (they require parent_task_id); this catches it. + task = MagicMock(parent_task_id=None, team=Team.MAIN_PM) + assert _is_coordination_task(task) is True + assert _board_cannot_own(task) is True + + +def test_main_pm_root_subtask_is_coordination_task() -> None: + # A MegaTask root-subtask is parented under the umbrella but still main_pm. + task = MagicMock(parent_task_id=uuid4(), team=Team.MAIN_PM) + assert _is_coordination_task(task) is True + + +def test_main_pm_team_as_raw_string_is_coordination_task() -> None: + task = MagicMock(parent_task_id=None, team="main_pm") + assert _is_coordination_task(task) is True + + +def test_board_root_is_not_coordination_task() -> None: + # A board/product root (e.g. a product root the PO reviews) is board-ownable. + task = MagicMock(parent_task_id=None, team=Team.BOARD) + assert _is_coordination_task(task) is False + assert _board_cannot_own(task) is False + + +def test_cell_root_is_not_coordination_task() -> None: + task = MagicMock(parent_task_id=None, team=Team.FRONTEND) + assert _is_coordination_task(task) is False + + # --------------------------------------------------------------------------- # apply_escalation board-role divert # --------------------------------------------------------------------------- +@pytest.mark.asyncio +async def test_apply_escalation_diverts_main_pm_coordination_root_from_board() -> None: + # The confirmed catch-22: a Main PM's i_am_blocked on its own coordination + # ROOT escalated up the chain to product-owner (a board role). The root is + # neither a descendant nor a cell task, so the old guard missed it and the + # whole root was reassigned to the board, which respawn-looped on a blocker it + # could not unblock. It must now divert to the pool instead. + svc = _service() + target_id = uuid4() + task = MagicMock( + id=uuid4(), + parent_task_id=None, + team=Team.MAIN_PM, + task_type=TaskType.CODE, + assigned_to=uuid4(), + blocker_raised_by=None, + status=TaskStatus.IN_PROGRESS, + ) + _bind(svc, "_is_board_advisory_agent", AsyncMock(return_value=True)) + release_mock = AsyncMock() + _bind(svc, "_release_code_task_to_pool", release_mock) + + await svc.apply_escalation( + task=task, + target_agent_id=target_id, + escalator_slug="main-pm", + target_slug="product-owner", + reason="root blocked: branch behind master", + ) + + # Diverted — NOT blocked-and-reassigned onto the board. + release_mock.assert_awaited_once() + assert task.status == TaskStatus.IN_PROGRESS + assert task.assigned_to != target_id + + @pytest.mark.asyncio async def test_apply_escalation_diverts_descendant_code_to_board() -> None: svc = _service() diff --git a/tests/unit/services/test_git.py b/tests/unit/services/test_git.py index f9f7173e..ce838eb7 100644 --- a/tests/unit/services/test_git.py +++ b/tests/unit/services/test_git.py @@ -186,6 +186,67 @@ async def test_push_targets_explicit_branch_not_current_checkout() -> None: assert "feature/frontend/OTHER" not in push_args +@pytest.mark.asyncio +async def test_push_recovers_missing_local_branch_from_origin() -> None: + """A push-by-name on a re-provisioned/shared clone missing the local ref + recovers it from origin instead of dying on "src refspec ... does not + match any". + + The branch's commits are already on origin (pushed in a prior cycle/clone), + so after recreating the local tracking ref the push is a clean no-op. + """ + svc = _service() + _bind(svc, "_token_for_workspace", AsyncMock(return_value=None)) + calls: list[list[str]] = [] + + async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock: + calls.append(args) + res = MagicMock() + # Local ref MISSING; origin HAS it. + if args[:2] == ["rev-parse", "--verify"]: + is_local = any(a.startswith("refs/heads/") for a in args) + res.returncode = 1 if is_local else 0 + res.stdout = "" + return res + res.returncode = 0 + res.stdout = "0" if args[:2] == ["rev-list", "--count"] else "" + return res + + _bind(svc, "_run_git", AsyncMock(side_effect=_run_git)) + + branch, _pushed = await svc.push(Path("/tmp/ws"), branch="feature/backend/TASK") + + assert branch == "feature/backend/TASK" + # It fetched origin and recreated the local ref before pushing. + assert ["fetch", "origin", "feature/backend/TASK"] in calls + assert ["branch", "feature/backend/TASK", "origin/feature/backend/TASK"] in calls + assert any(a and a[0] == "push" for a in calls) + + +@pytest.mark.asyncio +async def test_push_fails_loud_when_branch_absent_local_and_origin() -> None: + """When the named branch is in neither the local clone nor origin, the work + is genuinely lost from this clone — fail with a recoverable instruction, not + the raw "src refspec does not match any".""" + svc = _service() + _bind(svc, "_token_for_workspace", AsyncMock(return_value=None)) + + async def _run_git(_ws: object, args: list[str], **_kw: object) -> MagicMock: + res = MagicMock() + if args[:2] == ["rev-parse", "--verify"]: + res.returncode = 1 # absent both locally and on origin + res.stdout = "" + return res + res.returncode = 0 + res.stdout = "" + return res + + _bind(svc, "_run_git", AsyncMock(side_effect=_run_git)) + + with pytest.raises(GitCommandError, match="unclaim the task and"): + await svc.push(Path("/tmp/ws"), branch="feature/backend/GONE") + + @pytest.mark.asyncio async def test_pr_head_is_task_branch_not_current() -> None: """The PR head is the task's recorded branch, not the workspace checkout."""