feat(gateway): add submit_for_qa verb so devs can open PRs

Gate E made i_am_done strict (requires pr_number set), but the only
verb that opened PRs was i_am_done_with_catchup which lives off the
dev manifest. Devs hit NO_PR with no escape. Adds submit_for_qa as
the explicit push+PR step, leaving i_am_done to do the strict submit.
This commit is contained in:
Renn F
2026-05-03 05:14:07 +02:00
parent 6643b4c375
commit 3d5f14815d
7 changed files with 265 additions and 5 deletions
+6 -4
View File
@@ -21,7 +21,8 @@ You write code; you do not coordinate. If you find yourself thinking "let me als
| `i_will_work_on(task_id, plan=None)` | Claims a `pending`/`needs_revision` task; resumes a `claimed`/`in_progress` task you own. Auto-creates branch on first claim. | Task assigned to you (or unassigned and matches your role/team); for `claimed` resumption, plan and branch must exist. | | `i_will_work_on(task_id, plan=None)` | Claims a `pending`/`needs_revision` task; resumes a `claimed`/`in_progress` task you own. Auto-creates branch on first claim. | Task assigned to you (or unassigned and matches your role/team); for `claimed` resumption, plan and branch must exist. |
| `commit(message)` | Auto-prefixes `[task-id]`; records a progress entry. | Task in `in_progress`; on your branch. | | `commit(message)` | Auto-prefixes `[task-id]`; records a progress entry. | Task in `in_progress`; on your branch. |
| `i_have_committed(message)` | Quick alias for `commit()`. | Same as `commit`. | | `i_have_committed(message)` | Quick alias for `commit()`. | Same as `commit`. |
| `i_am_done(notes)` | Runs verify -> push -> create PR -> submit_for_qa. | At least one commit; progress entry; journal `reflect`; every acceptance criterion addressed. | | `submit_for_qa(task_id)` | Push your branch and open a PR. Run after your last commit, before `i_am_done`. | Task assigned to you; at least one commit; no PR yet. |
| `i_am_done(notes)` | Strict submit for QA. Requires PR already open — run `submit_for_qa` first. | Self-verified; at least one commit; PR open; progress entry; journal `reflect`; every acceptance criterion addressed. |
| `i_am_blocked(reason)` | Records the blocker, escalates to your PM, idles you. | Task is yours and active. | | `i_am_blocked(reason)` | Records the blocker, escalates to your PM, idles you. | Task is yours and active. |
| `note(text, scope?)` | Journal entry (`scope ∈ note|decision|reflect|learning|struggle`). | None. | | `note(text, scope?)` | Journal entry (`scope ∈ note|decision|reflect|learning|struggle`). | None. |
| `say(channel, text)` / `dm(recipient, text, skill?)` | Channel post / direct message. | Channel slug without `#`. | | `say(channel, text)` / `dm(recipient, text, skill?)` | Channel post / direct message. | Channel slug without `#`. |
@@ -36,12 +37,13 @@ You write code; you do not coordinate. If you find yourself thinking "let me als
4. Edit / Write your changes inside the workspace. Run tests via `Bash` if needed. 4. Edit / Write your changes inside the workspace. Run tests via `Bash` if needed.
5. `commit(message)` after each meaningful change. Repeat 4-5 until the criteria are met. 5. `commit(message)` after each meaningful change. Repeat 4-5 until the criteria are met.
6. `note(scope='reflect', text="<what you did + why>")` before submitting. 6. `note(scope='reflect', text="<what you did + why>")` before submitting.
7. `i_am_done(notes)` -> the gateway pushes, opens the PR, submits for QA. Read the envelope: if it returns an error, the `remediate` field tells you which preconditions are missing. 7. `submit_for_qa(task_id="<your-task>")` -> pushes your branch and opens the PR up to your cell PM's branch. The response includes the PR number.
8. After `i_am_done` succeeds you are finished with this task. `i_am_idle()`. Documenter writes docs; PM merges. You will only be respawned on `needs_revision`. 8. `i_am_done(notes)` -> strict submit for QA against the PR you just opened. Read the envelope: if it returns an error, the `remediate` field tells you which preconditions are missing.
9. After `i_am_done` succeeds you are finished with this task. `i_am_idle()`. Documenter writes docs; PM merges. You will only be respawned on `needs_revision`.
## Anti-patterns ## Anti-patterns
- ❌ Calling `i_am_done` without commits / PR-able state / self-verify / progress entry. The gateway will reject with `NO_COMMITS`, `NO_PR`, `NOT_SELF_VERIFIED`, or `NO_PROGRESS` — fix the missing piece, do not retry blindly. - ❌ Calling `i_am_done` without commits / open PR / self-verify / progress entry. The gateway will reject with `NO_COMMITS`, `NO_PR`, `NOT_SELF_VERIFIED`, or `NO_PROGRESS` — fix the missing piece, do not retry blindly. For `NO_PR`, call `submit_for_qa(task_id)` to push and open the PR, then retry `i_am_done`.
- ❌ Editing files outside your assigned task's branch. Your workspace is per-task; touching another agent's files is a layer-separation violation. - ❌ Editing files outside your assigned task's branch. Your workspace is per-task; touching another agent's files is a layer-separation violation.
- ❌ Trying to merge your own PR. Merging is a PM verb — you have no merge tool. If you call `Bash gh pr merge`, the orchestrator denies it. - ❌ Trying to merge your own PR. Merging is a PM verb — you have no merge tool. If you call `Bash gh pr merge`, the orchestrator denies it.
- ❌ Running `Bash git commit` or `Bash git push`. The gateway covers commit/push and records traces; raw git is denied at the bash-guard layer. - ❌ Running `Bash git commit` or `Bash git push`. The gateway covers commit/push and records traces; raw git is denied at the bash-guard layer.
+11
View File
@@ -13,6 +13,7 @@ from roboco.api.schemas.v2.flow import (
IAmIdleRequest, IAmIdleRequest,
IHaveCommittedRequest, IHaveCommittedRequest,
IWillWorkOnRequest, IWillWorkOnRequest,
SubmitForQaRequest,
) )
from roboco.services.gateway.choreographer import Choreographer from roboco.services.gateway.choreographer import Choreographer
@@ -53,6 +54,16 @@ async def i_have_committed(
return env.as_dict() return env.as_dict()
@router.post("/submit_for_qa")
async def submit_for_qa(
body: SubmitForQaRequest,
x_agent_id: _AgentIdHeader,
choreographer: _ChoreographerDep,
) -> dict:
env = await choreographer.submit_for_qa(x_agent_id, body.task_id)
return env.as_dict()
@router.post("/i_am_done") @router.post("/i_am_done")
async def i_am_done( async def i_am_done(
body: IAmDoneRequest, body: IAmDoneRequest,
+4
View File
@@ -18,6 +18,10 @@ class IHaveCommittedRequest(BaseModel):
message: str = Field(..., min_length=1) message: str = Field(..., min_length=1)
class SubmitForQaRequest(BaseModel):
task_id: UUID
class IAmDoneRequest(BaseModel): class IAmDoneRequest(BaseModel):
task_id: UUID task_id: UUID
notes: str = "" notes: str = ""
+7 -1
View File
@@ -73,8 +73,13 @@ def i_have_committed(message: str) -> dict[str, Any]:
return _post(_role_path("i_have_committed"), {"message": message}) return _post(_role_path("i_have_committed"), {"message": message})
def submit_for_qa(task_id: str) -> dict[str, Any]:
"""Push your branch and open a PR. Run after your last commit, before i_am_done."""
return _post(_role_path("submit_for_qa"), {"task_id": task_id})
def i_am_done(task_id: str, notes: str = "") -> dict[str, Any]: def i_am_done(task_id: str, notes: str = "") -> dict[str, Any]:
"""Submit work for QA. Runs verify/push/PR/submit-qa as needed.""" """Submit for QA. Strict — PR must be open (call submit_for_qa first)."""
return _post(_role_path("i_am_done"), {"task_id": task_id, "notes": notes}) return _post(_role_path("i_am_done"), {"task_id": task_id, "notes": notes})
@@ -205,6 +210,7 @@ _TOOLS: dict[str, Any] = {
"give_me_work": give_me_work, "give_me_work": give_me_work,
"i_will_work_on": i_will_work_on, "i_will_work_on": i_will_work_on,
"i_have_committed": i_have_committed, "i_have_committed": i_have_committed,
"submit_for_qa": submit_for_qa,
"i_am_done": i_am_done, "i_am_done": i_am_done,
"i_am_blocked": i_am_blocked, "i_am_blocked": i_am_blocked,
"i_am_idle": i_am_idle, "i_am_idle": i_am_idle,
+61
View File
@@ -340,6 +340,67 @@ class Choreographer:
context_briefing=await self._briefing_for(agent_id, t.id), context_briefing=await self._briefing_for(agent_id, t.id),
) )
async def submit_for_qa(self, agent_id: UUID, task_id: UUID) -> Envelope:
"""Push the dev's branch and open a PR. Does NOT submit for QA itself —
the dev calls ``i_am_done`` after this verb returns success.
Gate E made ``i_am_done`` strict: it requires ``pr_number`` set. The
catch-up shortcut lives off the dev manifest, so before this verb
existed devs had no escape from the NO_PR rejection. ``submit_for_qa``
is the explicit push + open-PR step, leaving ``i_am_done`` to do the
strict submit.
Pre-flight: caller must own the task, have committed at least once,
and not already have a PR open. If a PR is already open, this verb
is idempotent — it points the dev at ``i_am_done``.
"""
t = await self.task.get(task_id)
if t is None:
return Envelope.not_found(message=f"task {task_id} not found")
briefing = await self._briefing_for(agent_id, task_id)
if t.assigned_to != agent_id:
return Envelope.not_authorized(
message=f"task {task_id} is not assigned to you",
remediate="call give_me_work() to find your work",
context_briefing=briefing,
)
if not t.commits:
return Envelope.invalid_state(
message="no commits on this task yet",
remediate=(
"commit at least one change before submitting for QA — "
"call commit(message='<subject>')"
),
context_briefing=briefing,
)
if t.pr_number is not None:
return Envelope.ok(
status=str(t.status),
task_id=str(task_id),
next=(
f"PR #{t.pr_number} already open; call "
f"i_am_done(task_id, notes='...') when self-verified"
),
context_briefing=briefing,
)
await self._touch(task_id)
await self.git.push_branch(t.branch_name)
parent = parent_branch_for(t.branch_name)
pr = await self.git.create_pr(
t.branch_name, parent=parent, is_root_pr=False
)
return Envelope.ok(
status=str(t.status),
task_id=str(task_id),
next=(
f"PR #{pr['pr_number']} opened; call "
f"i_am_done(task_id, notes='...') when self-verified"
),
context_briefing=briefing,
)
async def i_am_done(self, agent_id: UUID, task_id: UUID, notes: str) -> Envelope: async def i_am_done(self, agent_id: UUID, task_id: UUID, notes: str) -> Envelope:
"""Submit work for QA — strict path. """Submit work for QA — strict path.
+1
View File
@@ -26,6 +26,7 @@ _DEV_FLOW = (
"give_me_work", "give_me_work",
"i_will_work_on", "i_will_work_on",
"i_have_committed", "i_have_committed",
"submit_for_qa",
"i_am_done", "i_am_done",
"i_am_blocked", "i_am_blocked",
"i_am_idle", "i_am_idle",
+175
View File
@@ -0,0 +1,175 @@
"""submit_for_qa pushes the current branch and opens a PR.
Gate E (commit c5c2016) made `i_am_done` strict — requires `pr_number` set.
The catch-up flow is off the dev manifest. Devs need an explicit verb that
pushes + opens the PR so a subsequent `i_am_done` can do the strict submit.
"""
from __future__ import annotations
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
def _make_deps(**overrides: AsyncMock) -> ChoreographerDeps:
"""Local dep-builder. Established pattern: per-test-file, not centralized."""
task = overrides.get("task", AsyncMock())
work_session = overrides.get("work_session", AsyncMock())
git = overrides.get("git", AsyncMock())
a2a = overrides.get("a2a", AsyncMock())
journal = overrides.get("journal", AsyncMock())
audit = overrides.get("audit", AsyncMock())
evidence_repo = overrides.get("evidence_repo", AsyncMock())
for method in (
"list_unread_a2a",
"list_unread_mentions",
"list_pending_notifications",
"task_metadata_gaps",
"recent_team_activity",
"blockers_in_lane",
):
getattr(evidence_repo, method).return_value = []
return ChoreographerDeps(
task=task,
work_session=work_session,
git=git,
a2a=a2a,
journal=journal,
audit=audit,
evidence_repo=evidence_repo,
)
@pytest.mark.asyncio
async def test_submit_for_qa_pushes_and_opens_pr() -> None:
aid = uuid4()
tid = uuid4()
t = MagicMock(
id=tid,
status="in_progress",
assigned_to=aid,
plan="x",
commits=[{"sha": "abc"}],
pr_number=None,
branch_name="feature/backend/abc12345",
)
task_svc = AsyncMock()
task_svc.get.return_value = t
git_svc = AsyncMock()
git_svc.push_branch.return_value = ("feature/backend/abc12345", 1)
git_svc.create_pr.return_value = {
"pr_number": 42,
"pr_url": "https://gh/x/42",
"is_root_pr": False,
}
work_session_svc = AsyncMock()
deps = _make_deps(task=task_svc, git=git_svc, work_session=work_session_svc)
c = Choreographer(deps)
env = await c.submit_for_qa(aid, tid)
git_svc.push_branch.assert_awaited()
git_svc.create_pr.assert_awaited()
assert env.error is None
assert env.next is not None
assert "42" in env.next # remediate points to i_am_done with PR ref
@pytest.mark.asyncio
async def test_submit_for_qa_rejects_when_not_assigned() -> None:
aid = uuid4()
other = uuid4()
tid = uuid4()
t = MagicMock(
id=tid,
status="in_progress",
assigned_to=other,
plan="x",
commits=[{"sha": "abc"}],
pr_number=None,
branch_name="feature/backend/abc12345",
)
task_svc = AsyncMock()
task_svc.get.return_value = t
git_svc = AsyncMock()
deps = _make_deps(task=task_svc, git=git_svc)
c = Choreographer(deps)
env = await c.submit_for_qa(aid, tid)
git_svc.push_branch.assert_not_awaited()
git_svc.create_pr.assert_not_awaited()
assert env.error == "not_authorized"
@pytest.mark.asyncio
async def test_submit_for_qa_rejects_when_no_commits() -> None:
aid = uuid4()
tid = uuid4()
t = MagicMock(
id=tid,
status="in_progress",
assigned_to=aid,
plan="x",
commits=[],
pr_number=None,
branch_name="feature/backend/abc12345",
)
task_svc = AsyncMock()
task_svc.get.return_value = t
git_svc = AsyncMock()
deps = _make_deps(task=task_svc, git=git_svc)
c = Choreographer(deps)
env = await c.submit_for_qa(aid, tid)
git_svc.push_branch.assert_not_awaited()
git_svc.create_pr.assert_not_awaited()
assert env.error == "invalid_state"
assert env.remediate is not None
assert "commit" in env.remediate.lower()
@pytest.mark.asyncio
async def test_submit_for_qa_idempotent_when_pr_already_open() -> None:
aid = uuid4()
tid = uuid4()
t = MagicMock(
id=tid,
status="in_progress",
assigned_to=aid,
plan="x",
commits=[{"sha": "abc"}],
pr_number=7,
branch_name="feature/backend/abc12345",
)
task_svc = AsyncMock()
task_svc.get.return_value = t
git_svc = AsyncMock()
deps = _make_deps(task=task_svc, git=git_svc)
c = Choreographer(deps)
env = await c.submit_for_qa(aid, tid)
git_svc.push_branch.assert_not_awaited()
git_svc.create_pr.assert_not_awaited()
assert env.error is None
assert env.next is not None
assert "7" in env.next
@pytest.mark.asyncio
async def test_submit_for_qa_returns_not_found_for_unknown_task() -> None:
aid = uuid4()
tid = uuid4()
task_svc = AsyncMock()
task_svc.get.return_value = None
deps = _make_deps(task=task_svc)
c = Choreographer(deps)
env = await c.submit_for_qa(aid, tid)
assert env.error == "not_found"