From ee743ffc7a1a2cd33c3d1168455fedd5178dc48a Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 3 May 2026 22:49:12 +0200 Subject: [PATCH] fix(git): pass actor_agent_id to pr_merge for workspace resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pr_merge falls back to task.assigned_to for workspace resolution, but that field is None at merge time (submit_qa/pass_qa cleared it during prior transitions). When project.workspace_path is unset, the resolver raises ValidationError 'no workspace configured and no agent_id provided' — surfaces as 500 from cell_pm_complete. Add actor_agent_id parameter (the PM doing the merge) and use it as the primary workspace owner. Falls back to task.assigned_to, then created_by, before raising. cell_pm_complete now threads pm_agent_id through. --- roboco/services/gateway/choreographer.py | 4 +++- roboco/services/git.py | 21 ++++++++++++++++++--- tests/unit/gateway/test_choreographer_pm.py | 4 +++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/roboco/services/gateway/choreographer.py b/roboco/services/gateway/choreographer.py index 8433d283..21e95673 100644 --- a/roboco/services/gateway/choreographer.py +++ b/roboco/services/gateway/choreographer.py @@ -2145,7 +2145,9 @@ class Choreographer: verb="cell_pm_complete", ) target = parent_branch_for(t.branch_name) - merge_result = await self.git.pr_merge(t.pr_number, target=target) + merge_result = await self.git.pr_merge( + t.pr_number, target=target, actor_agent_id=pm_agent_id + ) leaf_parent_id = t.parent_task_id leaf_team = t.team t = await self.task.cell_pm_complete( diff --git a/roboco/services/git.py b/roboco/services/git.py index a64a8665..ffc2518b 100644 --- a/roboco/services/git.py +++ b/roboco/services/git.py @@ -1740,7 +1740,13 @@ class GitService(BaseService): .with_for_update(of=_TaskTable) ) - async def pr_merge(self, pr_number: int, *, target: str) -> dict[str, Any]: + async def pr_merge( + self, + pr_number: int, + *, + target: str, + actor_agent_id: UUID | None = None, + ) -> dict[str, Any]: """Merge PR `pr_number` into `target`. Returns: ``{"merge_commit_sha": str | None}``. Looks up the @@ -1768,9 +1774,18 @@ class GitService(BaseService): if project is None: raise NotFoundError("Project", str(task.project_id)) + # Workspace resolution priority: caller-provided actor (the PM + # doing the merge) > task.assigned_to > created_by. assigned_to + # is often None at merge time because submit_qa / pass_qa cleared + # it during prior transitions; without a fallback the resolver + # raises ValidationError when project.workspace_path is unset. + workspace_agent_id = actor_agent_id or ( + UUID(str(task.assigned_to)) if task.assigned_to else None + ) + if workspace_agent_id is None and task.created_by: + workspace_agent_id = UUID(str(task.created_by)) workspace = await self.get_workspace( - project.slug, - agent_id=UUID(str(task.assigned_to)) if task.assigned_to else None, + project.slug, agent_id=workspace_agent_id ) git_token = await self._get_project_token_or_raise(project.slug) owner, repo = self._parse_github_remote(workspace) diff --git a/tests/unit/gateway/test_choreographer_pm.py b/tests/unit/gateway/test_choreographer_pm.py index 36593649..8321d9ed 100644 --- a/tests/unit/gateway/test_choreographer_pm.py +++ b/tests/unit/gateway/test_choreographer_pm.py @@ -263,7 +263,9 @@ async def test_cell_pm_complete_merges_then_completes() -> None: env = await c.cell_pm_complete(pm_id, task_id, notes="reviewed and approved") assert env.error is None assert env.status == "completed" - git_svc.pr_merge.assert_awaited_once_with(8, target="feature/backend/abc") + git_svc.pr_merge.assert_awaited_once_with( + 8, target="feature/backend/abc", actor_agent_id=pm_id + ) @pytest.mark.asyncio