feat(gateway): wire pr_update verb — ContentActions + route + MCP

ContentActions.pr_update enforces:
- task.pr_number is set (else invalid_state, remediate 'call open_pr')
- at least one of title/body/reviewers is non-None (else invalid_state)
- caller is task assignee OR PM on team (cell_pm same-team / main_pm
  cross-team), else not_authorized
- GitError raised by the underlying service maps to invalid_state with
  the upstream message preserved

The route at POST /api/v2/do/pr_update binds PRUpdateRequest, whose
model_validator returns 422 on all-None bodies so the verb layer
never sees them. The MCP tool registry adds 'pr_update' so manifest-
scoped do-servers can expose it to roles that opt in (next commit).
This commit is contained in:
Renn F
2026-05-14 04:32:46 +02:00
parent b5d3d13346
commit 74b7c39612
5 changed files with 616 additions and 0 deletions
+18
View File
@@ -20,6 +20,7 @@ from roboco.api.schemas.v2.do import (
NotifyRequest,
OpenSessionRequest,
ProgressRequest,
PRUpdateRequest,
SayRequest,
)
from roboco.services.gateway.content_actions import ContentActions
@@ -243,3 +244,20 @@ async def do_channels(
) -> dict:
env = await actions.channels(agent_id=x_agent_id)
return envelope_to_response(env, request)
@router.post("/pr_update")
async def do_pr_update(
request: Request,
body: PRUpdateRequest,
x_agent_id: _AgentIdHeader,
actions: _ContentActionsDep,
) -> dict:
env = await actions.pr_update(
agent_id=x_agent_id,
task_id=body.task_id,
title=body.title,
body=body.body,
reviewers=body.reviewers,
)
return envelope_to_response(env, request)
+38
View File
@@ -332,6 +332,43 @@ def channels() -> dict[str, Any]:
return _post("/api/v2/do/channels", {})
def pr_update(
task_id: str,
title: str | None = None,
body: str | None = None,
reviewers: list[str] | None = None,
) -> dict[str, Any]:
"""Update an existing PR's title, body, and/or requested reviewers.
Use after ``open_pr`` when you need to correct the title/body or
request a reviewer. At least one of ``title``, ``body``, or
``reviewers`` must be provided — passing all three None is rejected
with ``invalid_state`` before any GitHub call.
Args:
task_id: UUID of the task whose PR you're editing.
title: Replacement PR title (omit to leave unchanged).
body: Replacement PR body markdown (omit to leave unchanged).
reviewers: List of agent slugs to request as reviewers (e.g.
``["be-dev-2", "be-qa"]``). The gateway maps slugs to GitHub
usernames where the project records that mapping, otherwise
the slugs are forwarded as-is.
Authorization: caller must be the task's assignee OR a PM on the
task's team (cell_pm same-team, or main_pm cross-team). Anyone else
receives ``not_authorized``.
"""
return _post(
"/api/v2/do/pr_update",
{
"task_id": task_id,
"title": title,
"body": body,
"reviewers": reviewers,
},
)
# ---------- Tool registry ----------
#
# Maps the tool name an agent calls (matches manifest entries and the
@@ -351,6 +388,7 @@ _TOOLS: dict[str, Any] = {
"notify_get": notify_get,
"notify_ack": notify_ack,
"channels": channels,
"pr_update": pr_update,
}
+102
View File
@@ -15,6 +15,7 @@ import re
from dataclasses import dataclass
from typing import TYPE_CHECKING, Any, ClassVar
from roboco.exceptions import GitError
from roboco.foundation.policy import communications as _comms
from roboco.foundation.policy.journaling import Scope as _Scope
from roboco.services.gateway.commit_validator import validate_commit_message
@@ -927,6 +928,107 @@ class ContentActions:
context_briefing={},
)
_PM_ROLES_FOR_PR_UPDATE: ClassVar[frozenset[str]] = frozenset(
{"cell_pm", "main_pm"}
)
async def pr_update(
self,
*,
agent_id: UUID,
task_id: UUID,
title: str | None = None,
body: str | None = None,
reviewers: list[str] | None = None,
) -> Envelope:
"""Update an existing PR's title, body, and/or requested reviewers.
Smoke-5 surfaced this gap: agents who needed to edit a PR's
title/body or assign a reviewer after ``open_pr`` had no verb
for it and got bash-shimmed by the ``gh pr edit`` guard. This
verb is the gateway-native replacement.
Authorization: caller must be the task's ``assigned_to`` OR a
PM on the task's team (cell_pm.team == task.team, or main_pm
which is cross-team).
Preconditions:
- task must exist (else not_found)
- task.pr_number must be set (else invalid_state, remediate
'call open_pr')
- at least one of title/body/reviewers must be non-None (else
invalid_state — schema-level check is the first line of
defense; this guard catches direct gateway calls)
"""
if title is None and body is None and reviewers is None:
return Envelope.invalid_state(
message="no fields to update",
remediate=(
"provide at least one of title, body, or reviewers; "
"passing all None has no effect"
),
context_briefing={},
)
t = await self.task.get(task_id)
if t is None:
return Envelope.not_found(message=f"task {task_id} not found")
if t.pr_number is None:
return Envelope.invalid_state(
message=f"task {task_id} has no PR open",
remediate=(
"call open_pr(task_id) first; pr_update only edits an "
"already-open PR"
),
context_briefing={},
)
agent = await self.task.agent_for(agent_id)
role_str = str(agent.role) if agent is not None else ""
is_assignee = t.assigned_to == agent_id
is_main_pm = role_str == "main_pm"
is_cell_pm_on_team = (
role_str == "cell_pm"
and agent is not None
and agent.team is not None
and t.team is not None
and str(agent.team) == str(t.team)
)
if not (is_assignee or is_main_pm or is_cell_pm_on_team):
return Envelope.not_authorized(
message=(
f"role {role_str!r} is neither the assignee nor a PM on "
f"this task's team; cannot update PR"
),
remediate=(
"only the task's assignee or a PM on the task's team "
"may edit the PR; ask the assignee or your PM to call "
"pr_update instead"
),
context_briefing={},
)
try:
result = await self.git.update_pr_for_task(
task_id,
title=title,
body=body,
reviewers=reviewers,
)
except GitError as exc:
return Envelope.invalid_state(
message=str(exc),
remediate=(
"check the PR number on the task and retry; if the PR "
"was closed externally, the task should be reset"
),
context_briefing={},
)
return Envelope.ok(
status=str(t.status),
task_id=str(task_id),
next="continue working, or i_am_done when ready",
evidence=result,
context_briefing={},
)
async def notify_ack(
self,
*,
@@ -0,0 +1,111 @@
"""Unit tests for POST /api/v2/do/pr_update — route + schema.
Pydantic's model_validator must reject an all-None payload with 422
before ContentActions ever runs; a valid payload must forward title /
body / reviewers verbatim.
"""
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from fastapi import FastAPI
from fastapi.testclient import TestClient
from roboco.api.deps import get_content_actions
from roboco.api.routes.v2.do import router
from roboco.services.gateway.content_actions import ContentActions
_HTTP_200 = 200
_HTTP_422 = 422
_AGENT_ID = str(uuid4())
_TASK_ID = str(uuid4())
_HEADERS = {"X-Agent-ID": _AGENT_ID}
def _make_envelope(payload: dict | None = None) -> MagicMock:
env = MagicMock()
base = {"status": "in_progress", "task_id": _TASK_ID, "next": "continue"}
if payload:
base.update(payload)
env.as_dict.return_value = base
return env
def _build_app(mock_actions: MagicMock) -> FastAPI:
app = FastAPI()
app.include_router(router)
app.dependency_overrides[get_content_actions] = lambda: mock_actions
return app
@pytest.mark.asyncio
async def test_pr_update_all_none_returns_422() -> None:
"""Body with task_id only (no title/body/reviewers) → 422 from validator."""
mock_actions = MagicMock(spec=ContentActions)
mock_actions.pr_update = AsyncMock(return_value=_make_envelope())
client = TestClient(_build_app(mock_actions))
resp = client.post(
"/api/v2/do/pr_update",
json={"task_id": _TASK_ID},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_422
mock_actions.pr_update.assert_not_awaited()
@pytest.mark.asyncio
async def test_pr_update_title_only_forwards_to_content_actions() -> None:
"""Body with title only → 200, forwarded to ContentActions.pr_update."""
mock_actions = MagicMock(spec=ContentActions)
mock_actions.pr_update = AsyncMock(
return_value=_make_envelope({"evidence": {"updated_fields": ["title"]}})
)
client = TestClient(_build_app(mock_actions))
resp = client.post(
"/api/v2/do/pr_update",
json={"task_id": _TASK_ID, "title": "new title"},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200
mock_actions.pr_update.assert_awaited_once()
call_kwargs = mock_actions.pr_update.call_args.kwargs
assert str(call_kwargs["task_id"]) == _TASK_ID
assert call_kwargs["title"] == "new title"
assert call_kwargs["body"] is None
assert call_kwargs["reviewers"] is None
@pytest.mark.asyncio
async def test_pr_update_all_fields_forwarded() -> None:
"""Body with title + body + reviewers → all three forwarded verbatim."""
mock_actions = MagicMock(spec=ContentActions)
mock_actions.pr_update = AsyncMock(
return_value=_make_envelope(
{"evidence": {"updated_fields": ["title", "body", "reviewers"]}}
)
)
client = TestClient(_build_app(mock_actions))
resp = client.post(
"/api/v2/do/pr_update",
json={
"task_id": _TASK_ID,
"title": "t",
"body": "b",
"reviewers": ["be-dev-2", "be-qa"],
},
headers=_HEADERS,
)
assert resp.status_code == _HTTP_200
call_kwargs = mock_actions.pr_update.call_args.kwargs
assert call_kwargs["title"] == "t"
assert call_kwargs["body"] == "b"
assert call_kwargs["reviewers"] == ["be-dev-2", "be-qa"]
+347
View File
@@ -0,0 +1,347 @@
"""Tests for ContentActions.pr_update — gateway verb behavior matrix.
Covers: missing pr_number, all-None fields, non-assignee non-PM rejection,
per-field forwarding, and GitError surfacing.
"""
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock
from uuid import UUID, uuid4
import pytest
from roboco.exceptions import GitError
from roboco.services.gateway.content_actions import ContentActions, ContentActionsDeps
def _make_deps(**overrides: AsyncMock) -> ContentActionsDeps:
"""Wire ContentActionsDeps with mocks; honour caller overrides."""
task = overrides.get("task", AsyncMock())
git = overrides.get("git", AsyncMock())
return ContentActionsDeps(
task=task,
git=git,
messaging=overrides.get("messaging", AsyncMock()),
a2a=overrides.get("a2a", AsyncMock()),
journal=overrides.get("journal", AsyncMock()),
workspace=overrides.get("workspace", AsyncMock()),
notifications=overrides.get("notifications", AsyncMock()),
notification_delivery=overrides.get("notification_delivery", AsyncMock()),
)
def _task(
*,
pr_number: int | None = 7,
assigned_to: UUID,
team: str = "backend",
) -> MagicMock:
return MagicMock(
id=uuid4(),
status="in_progress",
pr_number=pr_number,
pr_url=(
f"https://github.com/acme/repo/pull/{pr_number}"
if pr_number is not None
else None
),
assigned_to=assigned_to,
team=team,
)
def _agent(role: str, team: str | None = None) -> MagicMock:
return MagicMock(id=uuid4(), role=role, team=team)
@pytest.mark.asyncio
async def test_pr_update_missing_pr_number_returns_invalid_state() -> None:
"""task.pr_number is None → invalid_state with remediate 'call open_pr'."""
agent_id = uuid4()
task = _task(pr_number=None, assigned_to=agent_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
deps = _make_deps(task=task_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id, task_id=task.id, title="new", body=None, reviewers=None
)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "open_pr" in body["remediate"]
deps.git.update_pr_for_task.assert_not_awaited()
@pytest.mark.asyncio
async def test_pr_update_all_fields_none_returns_invalid_state() -> None:
"""All of title/body/reviewers None → invalid_state."""
agent_id = uuid4()
task = _task(assigned_to=agent_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
deps = _make_deps(task=task_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id, task_id=task.id, title=None, body=None, reviewers=None
)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "at least one" in body["remediate"]
deps.git.update_pr_for_task.assert_not_awaited()
@pytest.mark.asyncio
async def test_pr_update_non_assignee_non_pm_returns_not_authorized() -> None:
"""A developer who is neither the assignee nor a PM → not_authorized."""
assignee_id = uuid4()
caller_id = uuid4()
task = _task(assigned_to=assignee_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
deps = _make_deps(task=task_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=caller_id,
task_id=task.id,
title="new title",
body=None,
reviewers=None,
)
body = env.as_dict()
assert body["error"] == "not_authorized"
deps.git.update_pr_for_task.assert_not_awaited()
@pytest.mark.asyncio
async def test_pr_update_assignee_title_only_forwarded() -> None:
"""Assignee + title only forwards (title=X, body=None, reviewers=None)."""
agent_id = uuid4()
task = _task(assigned_to=agent_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
git_svc = AsyncMock()
git_svc.update_pr_for_task.return_value = {
"pr_number": 7,
"pr_url": task.pr_url,
"updated_fields": ["title"],
}
deps = _make_deps(task=task_svc, git=git_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id,
task_id=task.id,
title="new title",
body=None,
reviewers=None,
)
body = env.as_dict()
assert body["error"] is None
assert body["evidence"]["updated_fields"] == ["title"]
git_svc.update_pr_for_task.assert_awaited_once()
call_kwargs = git_svc.update_pr_for_task.call_args.kwargs
assert call_kwargs["title"] == "new title"
assert call_kwargs["body"] is None
assert call_kwargs["reviewers"] is None
@pytest.mark.asyncio
async def test_pr_update_reviewers_only_forwarded() -> None:
"""Assignee + reviewers only → forwarded as the only non-None arg."""
agent_id = uuid4()
task = _task(assigned_to=agent_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
git_svc = AsyncMock()
git_svc.update_pr_for_task.return_value = {
"pr_number": 7,
"pr_url": task.pr_url,
"updated_fields": ["reviewers"],
}
deps = _make_deps(task=task_svc, git=git_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id,
task_id=task.id,
title=None,
body=None,
reviewers=["be-dev-2"],
)
body = env.as_dict()
assert body["error"] is None
git_svc.update_pr_for_task.assert_awaited_once()
call_kwargs = git_svc.update_pr_for_task.call_args.kwargs
assert call_kwargs["title"] is None
assert call_kwargs["body"] is None
assert call_kwargs["reviewers"] == ["be-dev-2"]
@pytest.mark.asyncio
async def test_pr_update_all_three_forwarded() -> None:
"""All three fields → all forwarded; updated_fields reflects all three."""
agent_id = uuid4()
task = _task(assigned_to=agent_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
git_svc = AsyncMock()
git_svc.update_pr_for_task.return_value = {
"pr_number": 7,
"pr_url": task.pr_url,
"updated_fields": ["title", "body", "reviewers"],
}
deps = _make_deps(task=task_svc, git=git_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id,
task_id=task.id,
title="t",
body="b",
reviewers=["be-dev-2"],
)
body = env.as_dict()
assert body["error"] is None
call_kwargs = git_svc.update_pr_for_task.call_args.kwargs
assert call_kwargs["title"] == "t"
assert call_kwargs["body"] == "b"
assert call_kwargs["reviewers"] == ["be-dev-2"]
assert set(body["evidence"]["updated_fields"]) == {"title", "body", "reviewers"}
@pytest.mark.asyncio
async def test_pr_update_cell_pm_on_same_team_allowed() -> None:
"""Cell PM whose team == task.team can update the PR (PM authority)."""
pm_id = uuid4()
assignee_id = uuid4()
task = _task(assigned_to=assignee_id, team="backend")
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("cell_pm", "backend")
git_svc = AsyncMock()
git_svc.update_pr_for_task.return_value = {
"pr_number": 7,
"pr_url": task.pr_url,
"updated_fields": ["title"],
}
deps = _make_deps(task=task_svc, git=git_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=pm_id, task_id=task.id, title="t", body=None, reviewers=None
)
body = env.as_dict()
assert body["error"] is None
git_svc.update_pr_for_task.assert_awaited_once()
@pytest.mark.asyncio
async def test_pr_update_cell_pm_on_other_team_rejected() -> None:
"""Cell PM on a different team than the task → not_authorized."""
pm_id = uuid4()
assignee_id = uuid4()
task = _task(assigned_to=assignee_id, team="backend")
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("cell_pm", "frontend")
deps = _make_deps(task=task_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=pm_id, task_id=task.id, title="t", body=None, reviewers=None
)
assert env.as_dict()["error"] == "not_authorized"
deps.git.update_pr_for_task.assert_not_awaited()
@pytest.mark.asyncio
async def test_pr_update_main_pm_any_team_allowed() -> None:
"""Main PM is cross-team and may update any task's PR."""
pm_id = uuid4()
assignee_id = uuid4()
task = _task(assigned_to=assignee_id, team="backend")
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("main_pm", team=None)
git_svc = AsyncMock()
git_svc.update_pr_for_task.return_value = {
"pr_number": 7,
"pr_url": task.pr_url,
"updated_fields": ["title"],
}
deps = _make_deps(task=task_svc, git=git_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=pm_id, task_id=task.id, title="t", body=None, reviewers=None
)
assert env.as_dict()["error"] is None
git_svc.update_pr_for_task.assert_awaited_once()
@pytest.mark.asyncio
async def test_pr_update_task_not_found_returns_not_found() -> None:
"""Unknown task_id → not_found envelope; git layer never invoked."""
agent_id = uuid4()
task_svc = AsyncMock()
task_svc.get.return_value = None
deps = _make_deps(task=task_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id, task_id=uuid4(), title="t", body=None, reviewers=None
)
body = env.as_dict()
assert body["error"] == "not_found"
deps.git.update_pr_for_task.assert_not_awaited()
@pytest.mark.asyncio
async def test_pr_update_git_error_returned_as_invalid_state() -> None:
"""GitService raises GitError → mapped to invalid_state envelope with detail."""
agent_id = uuid4()
task = _task(assigned_to=agent_id)
task_svc = AsyncMock()
task_svc.get.return_value = task
task_svc.agent_for.return_value = _agent("developer", "backend")
git_svc = AsyncMock()
git_svc.update_pr_for_task.side_effect = GitError("PR not found: #7 on acme/repo")
deps = _make_deps(task=task_svc, git=git_svc)
ca = ContentActions(deps)
env = await ca.pr_update(
agent_id=agent_id, task_id=task.id, title="t", body=None, reviewers=None
)
body = env.as_dict()
assert body["error"] == "invalid_state"
assert "PR not found" in body["message"]