mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* fix(git): don't delete a branch that still has open dependent PRs
Root cause of the run-zombifying "integration branch gone from origin" wedge.
_delete_remote_branch_best_effort deleted a merged PR's head branch
unconditionally, so:
- merging a cell->root PR deleted the cell branch while a sibling leaf PR was
still targeting it as base, and
- the CEO's root->master merge deleted the feature/main_pm/{root} integration
branch.
The dependent PRs lost their base, every later git op against the vanished
branch failed, and the task zombified (a51c3d31 only made the post-merge sync
non-fatal; this removes the cause).
The remote-branch delete chokepoint (the single path all merge/close/cancel
deletions funnel through) now first checks _branch_has_open_dependents: any OPEN
PR targeting the branch as its base marks it an active integration target and
preserves it. Fails safe (any error => keep the branch; cleanup is best-effort,
stranding is not). True leaf branches with no open dependents are still cleaned
up. Adds 6 unit tests for the guard + the probe.
* fix(git): recover a drifted shared clone on resume instead of BRANCH_MISMATCH
A dev/documenter/QA clone is shared across that agent's tasks. On a
respawn/resume it can sit on a sibling task's branch, or a re-provisioned clone
can lack the task branch as a local ref (commits only on origin). The
fresh-claim path git-resets the clone clean, but resume deliberately
short-circuits before it (_dev_reentry), so the agent's next commit hit
_assert_on_task_branch's BRANCH_MISMATCH, failed, and the task wedged in a
blocked respawn loop (the documenter that could never land its doc commit).
_assert_on_task_branch now recovers instead of only rejecting: fetch + checkout
the task branch (recreating a missing local ref from origin via `git branch
<b> origin/<b>`), and raise only when the switch genuinely can't happen
(uncommitted changes block it). Never discards work — checkout, not reset — so
a resumed agent's unpushed commits are preserved. Updates the RAG troubleshooting
+ developer docs to describe the auto-recovery. Adds 5 unit tests.
* fix(runtime): re-adopt running agent containers on restart (no double-spawn)
An orchestrator restart loses the in-memory _instances registry while the agent
containers keep running. The reaper already had a Docker-liveness fallback
(_assignee_container_running), but the spawn gate (_is_agent_active) did not, so
right after a restart it saw a live agent as inactive and could launch a second
container onto work the forgotten-but-running one was already doing.
start() now calls _readopt_running_agents() after _reconcile_orphan_claims_on_startup
and before the dispatcher/reaper loops launch: it probes each known agent slug's
container (AGENT_IMAGES, reusing _inspect_container_state — the same docker
inspect the reaper uses) and registers a minimal AgentInstance(state=ACTIVE) for
any that is running and not already tracked. Inert when nothing runs (cold start
unchanged); best-effort (a probe error leaves that slot for the reaper's own
fallback). This is the gateway-health spec's Task 4 / the orchestrator-state
spec's Phase 3 (_instances reconcile). Adds 4 unit tests.
* fix(git): treat an already-merged PR as idempotent success on merge
A merge PUT against an already-merged PR returns the same 405 as a genuine
"not mergeable" conflict, so _merge_with_retry raised MergeConflictError and the
completion path tried to rebase / close-superseded / escalate a PR that had
already landed (a prior cycle, a sibling, or the CEO merged it) — the
cell_pm_complete block<->unblock respawn loop.
_merge_with_retry now disambiguates before raising: a new _pr_is_merged probe
(GET the PR, check merged==true) returns success on an already-merged PR so
completion proceeds idempotently; a genuinely-unmerged 405 still raises the
conflict. Best-effort probe (False on any error → falls through to the existing
conflict handling). Adds 4 unit tests.
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
115 lines
4.2 KiB
Python
115 lines
4.2 KiB
Python
"""On resume, the branch-mismatch chokepoint recovers the clone instead of
|
|
hard-failing.
|
|
|
|
A dev/documenter/QA clone is shared across tasks; on a respawn/resume it can sit
|
|
on a sibling task's branch, or a re-provisioned clone can lack the task branch as
|
|
a local ref (commits only on origin). `_assert_on_task_branch` used to raise
|
|
BRANCH_MISMATCH in that state, so the agent's next commit failed and the task
|
|
wedged in a blocked respawn loop (the documented resume deadlock — e.g. the
|
|
documenter PR #102 case). It now fetches + checks out the task branch (recreating
|
|
a missing local ref from origin) and only raises if it genuinely cannot switch
|
|
(uncommitted changes block it). It NEVER discards local commits — checkout, not
|
|
reset.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
from unittest.mock import AsyncMock, MagicMock
|
|
|
|
import pytest
|
|
from roboco.services.base import ValidationError
|
|
from roboco.services.git import GitService
|
|
|
|
|
|
def _service() -> GitService:
|
|
session = MagicMock()
|
|
session.execute = AsyncMock(return_value=None)
|
|
return GitService(session)
|
|
|
|
|
|
def _bind(svc: GitService, name: str, value: object) -> None:
|
|
object.__setattr__(svc, name, value)
|
|
|
|
|
|
def _run_git_mock(*, local_ref_rc: int = 0, checkout_rc: int = 0) -> AsyncMock:
|
|
async def _run(_workspace: Path, args: list[str], **_kw: object) -> MagicMock:
|
|
if args[:2] == ["rev-parse", "--verify"]:
|
|
return MagicMock(returncode=local_ref_rc, stdout="")
|
|
if args[0] == "checkout":
|
|
return MagicMock(returncode=checkout_rc, stdout="")
|
|
return MagicMock(returncode=0, stdout="")
|
|
|
|
return AsyncMock(side_effect=_run)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_noop_when_already_on_task_branch() -> None:
|
|
svc = _service()
|
|
_bind(svc, "get_current_branch", AsyncMock(return_value="feature/main_pm/abc"))
|
|
run = _run_git_mock()
|
|
_bind(svc, "_run_git", run)
|
|
await svc._assert_on_task_branch(Path("/ws"), "feature/main_pm/abc")
|
|
run.assert_not_awaited() # already on it → no git work, no raise
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_noop_when_task_branch_none() -> None:
|
|
svc = _service()
|
|
gcb = AsyncMock(return_value="whatever")
|
|
_bind(svc, "get_current_branch", gcb)
|
|
await svc._assert_on_task_branch(Path("/ws"), None)
|
|
gcb.assert_not_awaited()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_recovers_by_checkout_when_local_ref_present() -> None:
|
|
svc = _service()
|
|
_bind(
|
|
svc,
|
|
"get_current_branch",
|
|
AsyncMock(side_effect=["feature/other--leaf", "feature/main_pm/abc"]),
|
|
)
|
|
_bind(svc, "_token_for_workspace", AsyncMock(return_value="tok"))
|
|
run = _run_git_mock(local_ref_rc=0, checkout_rc=0)
|
|
_bind(svc, "_run_git", run)
|
|
|
|
await svc._assert_on_task_branch(Path("/ws"), "feature/main_pm/abc")
|
|
|
|
cmds = [c.args[1] for c in run.await_args_list]
|
|
assert ["checkout", "feature/main_pm/abc"] in cmds
|
|
# local ref present → no recovery fetch/branch-create
|
|
assert not any(c[0] == "fetch" for c in cmds)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_recovers_missing_local_ref_from_origin() -> None:
|
|
svc = _service()
|
|
_bind(
|
|
svc,
|
|
"get_current_branch",
|
|
AsyncMock(side_effect=["feature/other--leaf", "feature/main_pm/abc"]),
|
|
)
|
|
_bind(svc, "_token_for_workspace", AsyncMock(return_value="tok"))
|
|
run = _run_git_mock(local_ref_rc=1, checkout_rc=0) # local ref missing
|
|
_bind(svc, "_run_git", run)
|
|
|
|
await svc._assert_on_task_branch(Path("/ws"), "feature/main_pm/abc")
|
|
|
|
cmds = [c.args[1] for c in run.await_args_list]
|
|
assert ["fetch", "origin", "feature/main_pm/abc"] in cmds
|
|
assert ["branch", "feature/main_pm/abc", "origin/feature/main_pm/abc"] in cmds
|
|
assert ["checkout", "feature/main_pm/abc"] in cmds
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_raises_when_cannot_switch() -> None:
|
|
svc = _service()
|
|
_bind(svc, "get_current_branch", AsyncMock(return_value="feature/other--leaf"))
|
|
_bind(svc, "_token_for_workspace", AsyncMock(return_value="tok"))
|
|
run = _run_git_mock(local_ref_rc=0, checkout_rc=1) # checkout fails (dirty tree)
|
|
_bind(svc, "_run_git", run)
|
|
|
|
with pytest.raises(ValidationError, match="BRANCH_MISMATCH"):
|
|
await svc._assert_on_task_branch(Path("/ws"), "feature/main_pm/abc")
|