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"])
|
log_result = await self._run_git(workspace, ["log", "-1", "--format=%H"])
|
||||||
return log_result.stdout.strip()
|
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(
|
async def _delete_remote_branch_best_effort(
|
||||||
self, owner: str, repo: str, branch: str, git_token: str
|
self, owner: str, repo: str, branch: str, git_token: str
|
||||||
) -> None:
|
) -> None:
|
||||||
@@ -2687,6 +2714,9 @@ class GitService(BaseService):
|
|||||||
await self._delete_pr_branch_best_effort(owner, repo, pr_number, git_token)
|
await self._delete_pr_branch_best_effort(owner, repo, pr_number, git_token)
|
||||||
|
|
||||||
target_branch = await self._project_default_branch(project_slug)
|
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(
|
merge_commit = await self._sync_target_branch(
|
||||||
workspace, target_branch, git_token
|
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)
|
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:
|
if task.work_session_id:
|
||||||
ws_service = get_work_session_service(self.session)
|
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(
|
await svc._sync_target_branch(
|
||||||
Path("/tmp/ws"), "feature/backend/parent", "token"
|
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