mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(gateway): Wave 2 pre-gateway parity — structured note, sub_tasks, channels
Three Wave 2 gaps from the 2026-05-11 pre-gateway parity design:
G4 — note() decision/reflect now require structured fields at the gateway
(pre-gateway `Field(...)` parity). Returns `incomplete_input` envelope
with field-by-field hints when any required field is missing.
- decision: context (str), options (list[{name,pros,cons}] min len 2),
chosen (str), rationale (str). `consequences` and `next_steps` are
now list[str] (was str). Renderer emits each option as a "### Name
+ Pros / Cons" block instead of a bullet — matches the pre-gateway
DecisionOption sub-shape exposed in `roboco/mcp/schemas/__init__.py`
at `254cc93`.
- reflect: what_done, what_learned, what_struggled (each non-empty
str). next_steps stays optional.
- Bumped tests/unit/gateway/test_content_actions.py with explicit
pass-with-N-options coverage (≥2 floor; 3-option case green).
G5 — i_will_plan now persists sub_tasks alongside approach / risks /
open_questions / technical_considerations. The Plan tab's Sub-Tasks
section was empty because the verb didn't accept the field. Choreographer
server-assigns id + order to each sub_task (pre-gateway build_plan_data
parity) and normalizes every list entry to the EXACT shape
`panel/src/types/index.ts::TaskPlan` consumes:
- SubTask: {id, title, description, completed:false, order,
estimated_hours:null, notes:null}
- Risk: {description, mitigation, severity:null} — accepts the
{risk, mitigation} pre-gateway shape too
- OpenQuestion: {question, answer:null, answered_by:null,
answered_at:null} — accepts a bare string fallback
The normalization lives in three small module-level helpers
(_normalize_sub_task / _normalize_risk / _normalize_open_question)
called from _build_panel_shaped_plan, keeping i_will_plan's branch
count under PLR0912.
G6 — new `channels()` verb returns the agent's readable + writable
channel slugs from foundation.policy.communications. Stops invented
slugs ("backend-dev", "backend") that we kept seeing in smoke runs.
Added to every role's manifest including auditor (read-only access).
Wired through:
- roboco/api/schemas/v2/do.py — list-typed consequences/next_steps,
dict-typed options, ChannelsRequest
- roboco/api/schemas/v2/flow.py — IWillPlanRequest.sub_tasks
- roboco/api/routes/v2/do.py — /channels endpoint
- roboco/api/routes/v2/flow_*.py — pass sub_tasks through
- roboco/services/gateway/content_actions.py — channels() method;
_check_scope_required_fields enforces decision/reflect structure;
_render_option_block emits per-option markdown blocks
- roboco/services/gateway/choreographer/_impl.py — _build_panel_shaped_plan
helper used by i_will_plan
- roboco/services/gateway/role_config.py — _CHANNEL_DISCOVERY tuple
on every role
- roboco/mcp/do_server.py — channels() tool + note() signature with
options as list[dict[str,str]]
- roboco/mcp/flow_server.py — i_will_plan signature with sub_tasks
Frontend: no code change. panel/src/types/index.ts already declares
the exact shape we now write; panel/src/components/tasks/task-detail/
{tab-plan,tab-progress,tab-sessions,tab-notes}.tsx already reads it.
The empty panels we observed were a backend write-side problem, not
a frontend read-side problem — Wave 1 + Wave 2 close it.
Quality: ruff + mypy clean. 505 unit tests pass (added 2 new tests on
decision-scope requirements, updated 3 existing tests to fit the
pre-gateway-parity contract).
Spec ref: docs/superpowers/specs/2026-05-11-pre-gateway-parity-design.md
This commit is contained in:
@@ -221,7 +221,12 @@ async def test_commit_allows_documenter_role() -> None:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_note_reflect_scope_succeeds() -> None:
|
||||
"""scope='reflect' is valid; journal.write_entry is called."""
|
||||
"""scope='reflect' is valid; journal.write_entry is called.
|
||||
|
||||
Pre-gateway parity: reflect requires what_done / what_learned /
|
||||
what_struggled (each a non-empty string). The gateway returns
|
||||
`incomplete_input` if any is missing.
|
||||
"""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
@@ -240,6 +245,11 @@ async def test_note_reflect_scope_succeeds() -> None:
|
||||
text="Reflected on approach: went with async generator pattern.",
|
||||
scope="reflect",
|
||||
task_id=task_id,
|
||||
structured={
|
||||
"what_done": "Shipped the async generator pattern in service.py:120-180",
|
||||
"what_learned": "asyncio.shield wraps cancellation correctly here",
|
||||
"what_struggled": "Initially missed the cleanup race; commits 3a4f1 fix it",
|
||||
},
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
@@ -250,6 +260,119 @@ async def test_note_reflect_scope_succeeds() -> None:
|
||||
assert call_kwargs["scope"] == "reflect"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_note_reflect_missing_required_fields_returns_incomplete_input() -> None:
|
||||
"""Pre-gateway parity: reflect without structured fields fails fast."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get_active_task_for_agent.return_value = None
|
||||
task_svc.get.return_value = MagicMock(
|
||||
id=task_id, assigned_to=agent_id, status="in_progress"
|
||||
)
|
||||
journal_svc = AsyncMock()
|
||||
|
||||
deps = _make_deps(task=task_svc, journal=journal_svc)
|
||||
ca = ContentActions(deps)
|
||||
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="bare reflect with no structured fields",
|
||||
scope="reflect",
|
||||
task_id=task_id,
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] == "incomplete_input"
|
||||
assert {"what_done", "what_learned", "what_struggled"}.issubset(set(body["missing"]))
|
||||
journal_svc.write_entry.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_note_decision_requires_options_and_more() -> None:
|
||||
"""Pre-gateway parity: decision requires context/options(>=2)/chosen/rationale."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get_active_task_for_agent.return_value = None
|
||||
task_svc.get.return_value = MagicMock(
|
||||
id=task_id, assigned_to=agent_id, status="in_progress"
|
||||
)
|
||||
journal_svc = AsyncMock()
|
||||
|
||||
deps = _make_deps(task=task_svc, journal=journal_svc)
|
||||
ca = ContentActions(deps)
|
||||
|
||||
# Missing everything structured → incomplete_input listing all required.
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="bare decision",
|
||||
scope="decision",
|
||||
task_id=task_id,
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "incomplete_input"
|
||||
assert {"context", "options", "chosen", "rationale"}.issubset(set(body["missing"]))
|
||||
|
||||
# Single option still fails (min 2).
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="decision with one option",
|
||||
scope="decision",
|
||||
task_id=task_id,
|
||||
structured={
|
||||
"context": "needed a queue",
|
||||
"options": [{"name": "redis", "pros": "fast", "cons": "ephemeral"}],
|
||||
"chosen": "redis",
|
||||
"rationale": "speed beats durability for this case",
|
||||
},
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "incomplete_input"
|
||||
assert "options" in body["missing"]
|
||||
|
||||
# Two options + all required → success.
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="real decision",
|
||||
scope="decision",
|
||||
task_id=task_id,
|
||||
structured={
|
||||
"context": "needed a queue",
|
||||
"options": [
|
||||
{"name": "redis", "pros": "fast", "cons": "ephemeral"},
|
||||
{"name": "postgres", "pros": "durable", "cons": "slower writes"},
|
||||
],
|
||||
"chosen": "redis",
|
||||
"rationale": "speed beats durability for ephemeral work",
|
||||
},
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] is None
|
||||
assert body["status"] == "noted"
|
||||
|
||||
# Three+ options also pass — 2 is the floor, not the ceiling.
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="three-way decision",
|
||||
scope="decision",
|
||||
task_id=task_id,
|
||||
structured={
|
||||
"context": "queue tech choice",
|
||||
"options": [
|
||||
{"name": "redis", "pros": "fast", "cons": "ephemeral"},
|
||||
{"name": "postgres", "pros": "durable", "cons": "slower"},
|
||||
{"name": "rabbitmq", "pros": "ordered", "cons": "ops overhead"},
|
||||
],
|
||||
"chosen": "rabbitmq",
|
||||
"rationale": "ordering matters more than raw speed here",
|
||||
},
|
||||
)
|
||||
body = env.as_dict()
|
||||
assert body["error"] is None
|
||||
assert body["status"] == "noted"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_note_invalid_scope_returns_invalid_state() -> None:
|
||||
"""Unknown scope yields invalid_state with valid-scope hint."""
|
||||
@@ -283,6 +406,15 @@ async def test_note_auto_fills_task_id_from_active_task() -> None:
|
||||
agent_id=agent_id,
|
||||
text="Decided to use UUIDs instead of integer PKs for portability.",
|
||||
scope="decision",
|
||||
structured={
|
||||
"context": "Choosing primary-key strategy for the new tables",
|
||||
"options": [
|
||||
{"name": "int", "pros": "compact", "cons": "leaks volume"},
|
||||
{"name": "uuid", "pros": "portable", "cons": "wider rows"},
|
||||
],
|
||||
"chosen": "uuid",
|
||||
"rationale": "portability matters more than 8 bytes/row",
|
||||
},
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
|
||||
@@ -133,10 +133,13 @@ async def test_note_with_task_id_allows_when_assignee() -> None:
|
||||
deps = _make_deps(task=task_svc, journal=journal_svc)
|
||||
ca = ContentActions(deps)
|
||||
|
||||
# Uses scope='note' (no structured-field requirement) — the test
|
||||
# exercises the ownership gate, not the journal-shape gate (which is
|
||||
# covered separately in test_content_actions.py).
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="Reflecting on my own task",
|
||||
scope="reflect",
|
||||
text="Working on my own task",
|
||||
scope="note",
|
||||
task_id=task_id,
|
||||
)
|
||||
assert env.error is None
|
||||
@@ -155,8 +158,8 @@ async def test_note_without_task_id_skips_ownership_check() -> None:
|
||||
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="A general reflection note with enough length",
|
||||
scope="reflect",
|
||||
text="A general note with enough length",
|
||||
scope="note",
|
||||
)
|
||||
assert env.error is None
|
||||
journal_svc.write_entry.assert_awaited_once()
|
||||
|
||||
Reference in New Issue
Block a user