mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(run-hardening): don't respawn-loop when a merge target branch is gone from origin
When an integration (cell/root) branch is deleted from origin — e.g. after a sibling cell->root merge, stranding a late straggler leaf — pr_merge's post-merge _sync_target_branch ran 'git fetch origin <branch>' (check=True) and raised 'couldn't find remote ref'. That surfaced as a retryable SERVICE_ERROR, so complete() re-blocked the task and respawn-looped the PM on an already-landed merge (observed live on cell branches feature/backend/31ae12fc--0e49e04e and 7aeee245--dcfe9fc2, blocking be-pm's complete() 5+ cycles). pr_merge reaches the post-merge sync only after the authoritative GitHub merge has already succeeded (_merge_with_retry raises otherwise), so refreshing the local workspace copy of the target branch is cosmetic. Route it through a new _sync_target_branch_best_effort that logs and returns None instead of raising. merge_pull_request (CEO path) keeps the strict sync — its target is the default branch, which always exists on origin.
This commit is contained in:
+33
-1
@@ -2530,6 +2530,33 @@ class GitService(BaseService):
|
||||
log_result = await self._run_git(workspace, ["log", "-1", "--format=%H"])
|
||||
return log_result.stdout.strip()
|
||||
|
||||
async def _sync_target_branch_best_effort(
|
||||
self, workspace: Path, target_branch: str, git_token: str
|
||||
) -> str | None:
|
||||
"""Post-merge local sync of ``target_branch`` — best-effort, never raises.
|
||||
|
||||
Callers reach this only AFTER the authoritative GitHub merge has already
|
||||
landed (``merge_pull_request`` / ``pr_merge`` raise if it did not), so
|
||||
refreshing the local workspace copy of the target branch is cosmetic.
|
||||
If the branch is gone from origin — e.g. an integration (cell/root)
|
||||
branch deleted after a sibling cell→root merge, stranding a late
|
||||
straggler leaf — ``_sync_target_branch`` raises ``fetch origin <branch>
|
||||
— couldn't find remote ref``. Letting that propagate turned an
|
||||
already-completed merge into a retryable ``SERVICE_ERROR`` that re-blocked
|
||||
the task and respawn-looped the PM. Swallow + log instead: the merge is
|
||||
done; the local sync is not worth failing on.
|
||||
"""
|
||||
try:
|
||||
return await self._sync_target_branch(workspace, target_branch, git_token)
|
||||
except GitError as exc:
|
||||
self.log.warning(
|
||||
"post-merge target-branch sync failed; merge already landed,"
|
||||
" continuing without local sync",
|
||||
target_branch=target_branch,
|
||||
error=str(exc),
|
||||
)
|
||||
return None
|
||||
|
||||
async def _delete_remote_branch_best_effort(
|
||||
self, owner: str, repo: str, branch: str, git_token: str
|
||||
) -> None:
|
||||
@@ -2687,6 +2714,9 @@ class GitService(BaseService):
|
||||
await self._delete_pr_branch_best_effort(owner, repo, pr_number, git_token)
|
||||
|
||||
target_branch = await self._project_default_branch(project_slug)
|
||||
# Default branch always exists on origin, so the plain sync is correct
|
||||
# here. The best-effort variant guards the agent-facing pr_merge path,
|
||||
# whose target can be an integration branch deleted from origin.
|
||||
merge_commit = await self._sync_target_branch(
|
||||
workspace, target_branch, git_token
|
||||
)
|
||||
@@ -3317,7 +3347,9 @@ class GitService(BaseService):
|
||||
)
|
||||
)
|
||||
await self._delete_pr_branch_best_effort(owner, repo, pr_number, git_token)
|
||||
merge_commit = await self._sync_target_branch(workspace, target, git_token)
|
||||
merge_commit = await self._sync_target_branch_best_effort(
|
||||
workspace, target, git_token
|
||||
)
|
||||
|
||||
if task.work_session_id:
|
||||
ws_service = get_work_session_service(self.session)
|
||||
|
||||
@@ -885,3 +885,47 @@ async def test_sync_target_branch_raises_when_origin_also_missing() -> None:
|
||||
await svc._sync_target_branch(
|
||||
Path("/tmp/ws"), "feature/backend/parent", "token"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _sync_target_branch_best_effort — post-merge local sync must never re-block
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_target_branch_best_effort_swallows_missing_branch() -> None:
|
||||
"""A post-merge local sync of a target branch gone from origin must NOT raise.
|
||||
|
||||
``pr_merge`` / ``merge_pull_request`` reach the post-merge sync only after the
|
||||
authoritative GitHub merge already succeeded, so updating the local workspace
|
||||
copy of the (possibly-deleted) target branch is cosmetic. Letting it raise
|
||||
re-blocks the just-merged task and respawn-loops the PM — the live failure
|
||||
where an integration branch deleted from origin made ``complete()`` loop on
|
||||
``fetch origin <cell-branch> — fatal: couldn't find remote ref``.
|
||||
"""
|
||||
svc = _service()
|
||||
_bind(
|
||||
svc,
|
||||
"_sync_target_branch",
|
||||
AsyncMock(
|
||||
side_effect=GitCommandError(
|
||||
"fetch origin feature/backend/31ae12fc--0e49e04e",
|
||||
"fatal: couldn't find remote ref feature/backend/31ae12fc--0e49e04e",
|
||||
)
|
||||
),
|
||||
)
|
||||
result = await svc._sync_target_branch_best_effort(
|
||||
Path("/tmp/ws"), "feature/backend/31ae12fc--0e49e04e", "token"
|
||||
)
|
||||
assert result is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_target_branch_best_effort_returns_sha_on_success() -> None:
|
||||
"""On success it passes the synced tip sha straight through."""
|
||||
svc = _service()
|
||||
_bind(svc, "_sync_target_branch", AsyncMock(return_value="merged-tip-sha"))
|
||||
result = await svc._sync_target_branch_best_effort(
|
||||
Path("/tmp/ws"), "feature/backend/parent", "token"
|
||||
)
|
||||
assert result == "merged-tip-sha"
|
||||
|
||||
Reference in New Issue
Block a user