mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(content): obligate role note sections like journals
Completes the note(scope='handoff') write-path (WIP 23e6ee57): every role
with a dedicated note section is now obligated to populate it, the same way
journals are obligated.
Obligations (foundation.policy.tracing):
- DEV_NOTES / PR_REVIEWER_NOTES / QUICK_CONTEXT_MIN_CHARS requirements +
checkers, wired onto i_am_done (dev_notes), delegate (quick_context), and
pr_pass / pr_fail / post_pr_review (pr_reviewer_notes).
- Fixes a latent bug: the docs-notes checker read dev_notes instead of
doc_notes (the documenter's section); the i_documented shim now feeds
doc_notes to match.
Auditor: a session-scoped note obligation on i_am_idle — the auditor owns no
delivery task and has no delivery verb, so it must have recorded an
observation within the window before going idle (JournalService.has_recent_entry).
Write-then-gate: persisted sections (dev_notes / quick_context) are
pre-written by the agent's note(scope='handoff') before the gated verb;
argument-borne sections (doc_notes / pr_reviewer_notes) are checked through a
SimpleNamespace shim, the same pattern qa_notes already uses.
Config: dev/pr_reviewer/quick_context min-chars (40/40/30), panel-tunable.
Plus per-gap remediation hints and full coverage (write-path routing,
ownership, validation->remediation, each obligation, the doc_notes fix).
Full make-quality green: 9777 passed, 95.6% coverage.
This commit is contained in:
@@ -75,7 +75,8 @@ def _ready_task(task_id: Any, agent_id: Any) -> MagicMock:
|
||||
acceptance_criteria_status=[],
|
||||
commits=[{"sha": "deadbeef"}],
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
# i_am_done obligates the developer's dev_notes section (>=40 chars).
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
quick_context=None,
|
||||
)
|
||||
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
"""Tests for the auditor's i_am_idle note obligation (_auditor_note_guard).
|
||||
|
||||
Every role with a dedicated note section is obligated to populate it, like
|
||||
journals. The auditor owns no delivery task and has no delivery verb, so its
|
||||
obligation is session-scoped: it must have recorded an observation within the
|
||||
window before it may go idle. Inert for every other role.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||
|
||||
|
||||
def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
base: dict[str, Any] = {
|
||||
"task": AsyncMock(),
|
||||
"work_session": AsyncMock(),
|
||||
"git": AsyncMock(),
|
||||
"a2a": AsyncMock(),
|
||||
"journal": AsyncMock(),
|
||||
"audit": AsyncMock(),
|
||||
"evidence_repo": AsyncMock(),
|
||||
}
|
||||
base.update(overrides)
|
||||
return ChoreographerDeps(**base)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auditor_idle_blocked_without_recent_observation() -> None:
|
||||
"""An auditor with no recent journal entry is refused idle."""
|
||||
auditor_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.agent_for.return_value = MagicMock(role="auditor", team="board")
|
||||
journal = AsyncMock()
|
||||
journal.has_recent_entry.return_value = False
|
||||
c = Choreographer(_make_deps(task=task_svc, journal=journal))
|
||||
|
||||
guard = await c._auditor_note_guard(auditor_id, briefing={})
|
||||
|
||||
assert guard is not None
|
||||
body = guard.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "auditor_notes" in body["message"]
|
||||
assert "note(scope='reflect'" in body["remediate"]
|
||||
# The window query was actually consulted.
|
||||
journal.has_recent_entry.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auditor_idle_allowed_with_recent_observation() -> None:
|
||||
"""An auditor that recorded an observation recently may idle."""
|
||||
auditor_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.agent_for.return_value = MagicMock(role="auditor", team="board")
|
||||
journal = AsyncMock()
|
||||
journal.has_recent_entry.return_value = True
|
||||
c = Choreographer(_make_deps(task=task_svc, journal=journal))
|
||||
|
||||
guard = await c._auditor_note_guard(auditor_id, briefing={})
|
||||
|
||||
assert guard is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_idle_note_guard_inert_for_non_auditor() -> None:
|
||||
"""A developer never trips the auditor guard (no journal lookup at all)."""
|
||||
dev_id = uuid4()
|
||||
task_svc = AsyncMock()
|
||||
task_svc.agent_for.return_value = MagicMock(role="developer", team="backend")
|
||||
journal = AsyncMock()
|
||||
c = Choreographer(_make_deps(task=task_svc, journal=journal))
|
||||
|
||||
guard = await c._auditor_note_guard(dev_id, briefing={})
|
||||
|
||||
assert guard is None
|
||||
journal.has_recent_entry.assert_not_awaited()
|
||||
@@ -107,6 +107,7 @@ async def test_delegate_blocks_when_parent_assigned_to_other_agent() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=other_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
@@ -131,6 +132,7 @@ async def test_delegate_allows_when_parent_in_progress_and_owned() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
new_task = MagicMock(id=uuid4())
|
||||
task_svc = AsyncMock()
|
||||
@@ -158,6 +160,7 @@ async def test_delegate_blocks_when_subtask_cap_exceeded() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
too_many = [MagicMock(id=uuid4()) for _ in range(13)]
|
||||
task_svc = AsyncMock()
|
||||
@@ -184,6 +187,7 @@ async def test_delegate_allows_when_subtask_cap_within_soft_zone() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
many = [MagicMock(id=uuid4()) for _ in range(10)]
|
||||
new_task = MagicMock(id=uuid4())
|
||||
@@ -211,6 +215,7 @@ async def test_delegate_allows_at_zero_subtasks() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
new_task = MagicMock(id=uuid4())
|
||||
task_svc = AsyncMock()
|
||||
@@ -236,6 +241,7 @@ async def test_delegate_blocks_at_exact_cap_plus_one() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
# Already 12 children — adding the 13th must be blocked.
|
||||
twelve = [MagicMock(id=uuid4()) for _ in range(12)]
|
||||
@@ -250,3 +256,31 @@ async def test_delegate_blocks_at_exact_cap_plus_one() -> None:
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "invalid_state"
|
||||
task_svc.create_subtask.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_delegate_blocks_when_parent_quick_context_empty() -> None:
|
||||
"""delegate obligates the PM's quick_context resumption section on the
|
||||
parent; an empty quick_context (PM never called note(scope='handoff'))
|
||||
fails the tracing gate before any subtask is created."""
|
||||
pm_id = uuid4()
|
||||
parent_id = uuid4()
|
||||
parent = MagicMock(
|
||||
id=parent_id,
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
task_svc.agent_for.return_value = MagicMock(role="cell_pm", team="backend")
|
||||
task_svc.get_subtasks.return_value = []
|
||||
deps = _make_deps(task=task_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.delegate(pm_id, parent_id, _delegate_inputs())
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "tracing_gap"
|
||||
assert "quick_context>=min" in body["missing"]
|
||||
task_svc.create_subtask.assert_not_awaited()
|
||||
|
||||
@@ -428,7 +428,7 @@ async def test_i_am_done_blocks_when_acceptance_criteria_unaddressed() -> None:
|
||||
pr_url="https://x/pr/8",
|
||||
team="backend",
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
@@ -480,7 +480,7 @@ async def test_i_am_done_reflect_note_addresses_acceptance_criteria() -> None:
|
||||
pr_url="https://x/pr/8",
|
||||
team="backend",
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
qa_notes="",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
@@ -534,7 +534,7 @@ async def test_i_am_done_blocks_when_journal_reflect_missing() -> None:
|
||||
pr_url="https://x/pr/8",
|
||||
team="backend",
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
@@ -717,7 +717,7 @@ def _passing_i_am_done_task(agent_id: Any, task_id: Any) -> Any:
|
||||
pr_url="https://x/pr/8",
|
||||
team="backend",
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
qa_notes="",
|
||||
)
|
||||
|
||||
|
||||
@@ -618,6 +618,9 @@ async def test_delegate_parent_no_project_rejected() -> None:
|
||||
project_id=None,
|
||||
product_id=None,
|
||||
title="p",
|
||||
# delegate obligates the PM's quick_context; supply it so the
|
||||
# no-project guard is the load-bearing rejection.
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
|
||||
@@ -672,6 +672,7 @@ async def test_delegate_main_pm_to_cell_pm_creates_subtask() -> None:
|
||||
project_id=project_id,
|
||||
status="in_progress",
|
||||
assigned_to=main_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
new_task = MagicMock(id=uuid4())
|
||||
task_svc = AsyncMock()
|
||||
@@ -713,6 +714,7 @@ async def test_delegate_cell_pm_to_team_dev_creates_subtask() -> None:
|
||||
project_id=project_id,
|
||||
status="in_progress",
|
||||
assigned_to=cell_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
new_task = MagicMock(id=uuid4())
|
||||
task_svc = AsyncMock()
|
||||
@@ -749,6 +751,7 @@ async def test_delegate_main_pm_to_dev_is_rejected() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=main_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
@@ -784,6 +787,7 @@ async def test_delegate_cell_pm_to_other_pm_rejected() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=cell_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
@@ -818,6 +822,7 @@ async def test_delegate_unknown_assignee_returns_invalid_state() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
@@ -852,6 +857,7 @@ async def test_delegate_invalid_team_enum_rejected() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
@@ -1128,6 +1134,7 @@ async def test_delegate_main_pm_to_cell_pm_rejects_code_typed_subtask() -> None:
|
||||
project_id=uuid4(),
|
||||
status="in_progress",
|
||||
assigned_to=main_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = parent
|
||||
@@ -1166,6 +1173,7 @@ async def test_delegate_main_pm_to_cell_pm_accepts_planning_subtask() -> None:
|
||||
project_id=project_id,
|
||||
status="in_progress",
|
||||
assigned_to=main_pm_id,
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
new_task = MagicMock(id=uuid4())
|
||||
task_svc = AsyncMock()
|
||||
|
||||
@@ -97,7 +97,8 @@ async def test_i_am_done_reassigns_task_to_qa_agent() -> None:
|
||||
# Gate Set E requires non-empty commits before submit_qa.
|
||||
commits=[{"sha": "abc"}],
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
# i_am_done obligates the developer's dev_notes section (>=40 chars).
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
)
|
||||
after_verify = MagicMock(
|
||||
**{**initial.__dict__, "status": "verifying", "self_verified": True},
|
||||
@@ -162,7 +163,8 @@ async def test_i_am_done_skips_reassign_when_no_qa_agent() -> None:
|
||||
# Gate Set E requires non-empty commits before submit_qa.
|
||||
commits=[{"sha": "abc"}],
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
# i_am_done obligates the developer's dev_notes section (>=40 chars).
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
)
|
||||
after_verify = MagicMock(
|
||||
**{**initial.__dict__, "status": "verifying", "self_verified": True},
|
||||
|
||||
@@ -92,7 +92,8 @@ def _ready_task(task_id: Any, agent_id: Any) -> MagicMock:
|
||||
],
|
||||
commits=[{"sha": "abc"}],
|
||||
documents=[],
|
||||
dev_notes="",
|
||||
# i_am_done obligates the developer's dev_notes section (>=40 chars).
|
||||
dev_notes="Implemented the change and added tests covering the new path.",
|
||||
)
|
||||
|
||||
|
||||
@@ -265,6 +266,41 @@ async def test_i_am_done_blocks_when_no_progress() -> None:
|
||||
task_svc.submit_qa.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Role note-section obligation: dev_notes must be filled (note(scope='handoff'))
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_i_am_done_blocks_when_dev_notes_empty() -> None:
|
||||
"""i_am_done obligates the developer's dev_notes section; an empty
|
||||
dev_notes (the dev never called note(scope='handoff')) fails the gate."""
|
||||
agent_id = uuid4()
|
||||
task_id = uuid4()
|
||||
t = _ready_task(task_id, agent_id)
|
||||
t.dev_notes = ""
|
||||
task_svc = AsyncMock()
|
||||
task_svc.get.return_value = t
|
||||
task_svc.agent_for.return_value = MagicMock(
|
||||
id=agent_id, role="developer", team="backend", slug=None
|
||||
)
|
||||
journal_svc = AsyncMock()
|
||||
journal_svc.has_reflect_for_task.return_value = True
|
||||
journal_svc.has_decision_for_task.return_value = True
|
||||
journal_svc.latest_decision_at.return_value = datetime.now(UTC)
|
||||
journal_svc.has_learning_for_task.return_value = False
|
||||
journal_svc.has_struggle_for_task.return_value = False
|
||||
deps = _make_deps(task=task_svc, journal=journal_svc)
|
||||
c = Choreographer(deps)
|
||||
|
||||
env = await c.i_am_done(agent_id, task_id, "done")
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "tracing_gap"
|
||||
assert "dev_notes>=min" in body["missing"]
|
||||
assert "scope='handoff'" in body["remediate"]
|
||||
task_svc.submit_qa.assert_not_awaited()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# E.5 happy path: all gates pass → submit_qa runs (NO catch-up)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -62,6 +62,8 @@ def _parent_in_progress(pm_id: Any) -> MagicMock:
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
priority=2,
|
||||
# delegate obligates the PM's quick_context resumption section.
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -48,6 +48,8 @@ def _parent(pm_id: Any, product_id: Any = None, project_id: Any = None) -> Magic
|
||||
product_id=product_id,
|
||||
status="in_progress",
|
||||
assigned_to=pm_id,
|
||||
# delegate obligates the PM's quick_context resumption section.
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
"""Tests for note(scope='handoff') — the role note-section write-path.
|
||||
|
||||
``note()`` only ever wrote the JOURNAL; ``scope='handoff'`` is how an agent
|
||||
authors its dedicated SECTION (dev_notes / quick_context / auditor_notes …)
|
||||
through the ``apply_structured_note`` chokepoint. These cover the routing,
|
||||
ownership, validation→remediation, and journal-trail behaviours.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.foundation.policy.content import ContentValidationError
|
||||
from roboco.services.gateway.content_actions import ContentActions, ContentActionsDeps
|
||||
|
||||
|
||||
def _make_deps(**overrides: object) -> ContentActionsDeps:
|
||||
task = overrides.get("task") or AsyncMock()
|
||||
return ContentActionsDeps(
|
||||
task=task,
|
||||
git=overrides.get("git") or AsyncMock(),
|
||||
messaging=overrides.get("messaging") or AsyncMock(),
|
||||
a2a=overrides.get("a2a") or AsyncMock(),
|
||||
journal=overrides.get("journal") or AsyncMock(),
|
||||
workspace=overrides.get("workspace") or AsyncMock(),
|
||||
notifications=overrides.get("notifications") or AsyncMock(),
|
||||
notification_delivery=overrides.get("notification_delivery") or AsyncMock(),
|
||||
evidence_repo=overrides.get("evidence_repo") or AsyncMock(),
|
||||
)
|
||||
|
||||
|
||||
def _dev_task_svc(task_id: object, role: str = "developer") -> AsyncMock:
|
||||
"""A task service whose active/context task is owned by the caller."""
|
||||
svc = AsyncMock()
|
||||
svc.agent_for.return_value = MagicMock(role=role)
|
||||
svc.get_journal_context_task_for_agent.return_value = MagicMock(id=task_id)
|
||||
svc.record_section_note.return_value = None
|
||||
return svc
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_developer_writes_dev_notes_from_text() -> None:
|
||||
"""A developer handoff routes to the 'developer' content type, defaulting
|
||||
the payload to {'summary': text} when no explicit section is given."""
|
||||
agent_id, task_id = uuid4(), uuid4()
|
||||
svc = _dev_task_svc(task_id)
|
||||
ca = ContentActions(_make_deps(task=svc))
|
||||
|
||||
summary = "Implemented the endpoint and added happy-path tests."
|
||||
env = await ca.note(agent_id=agent_id, text=summary, scope="handoff")
|
||||
|
||||
assert env.as_dict()["error"] is None
|
||||
svc.record_section_note.assert_awaited_once()
|
||||
called_task_id, content_type, payload = svc.record_section_note.call_args.args
|
||||
assert called_task_id == task_id
|
||||
assert content_type == "developer"
|
||||
assert payload == {"summary": summary}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_passes_explicit_section_through() -> None:
|
||||
"""An explicit ``section`` dict is the payload (e.g. PM resumption)."""
|
||||
agent_id, task_id = uuid4(), uuid4()
|
||||
svc = _dev_task_svc(task_id, role="cell_pm")
|
||||
ca = ContentActions(_make_deps(task=svc))
|
||||
|
||||
section = {"done": "Planned the decomposition.", "next": "Cells implement."}
|
||||
env = await ca.note(
|
||||
agent_id=agent_id, text="handoff", scope="handoff", section=section
|
||||
)
|
||||
|
||||
assert env.as_dict()["error"] is None
|
||||
_tid, content_type, payload = svc.record_section_note.call_args.args
|
||||
assert content_type == "resumption"
|
||||
assert payload == section
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_also_writes_journal_trail_entry() -> None:
|
||||
"""The section write drops a journal trail entry so it shows in the log."""
|
||||
agent_id, task_id = uuid4(), uuid4()
|
||||
svc = _dev_task_svc(task_id)
|
||||
journal = AsyncMock()
|
||||
ca = ContentActions(_make_deps(task=svc, journal=journal))
|
||||
|
||||
await ca.note(agent_id=agent_id, text="Did the thing thoroughly.", scope="handoff")
|
||||
|
||||
journal.write_entry.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_role_without_section_is_rejected() -> None:
|
||||
"""A role with no dedicated section (board/advisory) cannot handoff."""
|
||||
agent_id = uuid4()
|
||||
svc = AsyncMock()
|
||||
svc.agent_for.return_value = MagicMock(role="product_owner")
|
||||
ca = ContentActions(_make_deps(task=svc))
|
||||
|
||||
env = await ca.note(agent_id=agent_id, text="observation", scope="handoff")
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "no dedicated note section" in body["message"]
|
||||
svc.record_section_note.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_validation_error_returns_remediation_not_422() -> None:
|
||||
"""A malformed section payload becomes a remediation Envelope, never a 422
|
||||
(a raw 422 would trip the do-server circuit breaker)."""
|
||||
agent_id, task_id = uuid4(), uuid4()
|
||||
svc = _dev_task_svc(task_id, role="auditor")
|
||||
svc.record_section_note.side_effect = ContentValidationError(
|
||||
"severity", "field required"
|
||||
)
|
||||
ca = ContentActions(_make_deps(task=svc))
|
||||
|
||||
env = await ca.note(
|
||||
agent_id=agent_id,
|
||||
text="risk spotted",
|
||||
scope="handoff",
|
||||
section={"summary": "x"},
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "severity" in body["message"]
|
||||
assert "auditor" in body["remediate"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_no_task_to_attach_is_rejected() -> None:
|
||||
"""With no active/context task and no explicit task_id, handoff refuses."""
|
||||
agent_id = uuid4()
|
||||
svc = AsyncMock()
|
||||
svc.agent_for.return_value = MagicMock(role="developer")
|
||||
svc.get_journal_context_task_for_agent.return_value = None
|
||||
ca = ContentActions(_make_deps(task=svc))
|
||||
|
||||
env = await ca.note(agent_id=agent_id, text="orphan note", scope="handoff")
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] == "invalid_state"
|
||||
assert "task_id" in body["remediate"]
|
||||
svc.record_section_note.assert_not_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_explicit_task_not_owned_is_rejected() -> None:
|
||||
"""An explicit task_id the caller does not own is an ownership violation."""
|
||||
agent_id, task_id = uuid4(), uuid4()
|
||||
svc = AsyncMock()
|
||||
svc.agent_for.return_value = MagicMock(role="developer")
|
||||
svc.get.return_value = MagicMock(
|
||||
id=task_id, assigned_to=uuid4(), project_id=uuid4(), product_id=None
|
||||
)
|
||||
ca = ContentActions(_make_deps(task=svc))
|
||||
|
||||
env = await ca.note(
|
||||
agent_id=agent_id, text="poking another task", scope="handoff", task_id=task_id
|
||||
)
|
||||
body = env.as_dict()
|
||||
|
||||
assert body["error"] == "not_authorized"
|
||||
svc.record_section_note.assert_not_awaited()
|
||||
@@ -89,10 +89,16 @@ def _make_deps(**overrides: Any) -> ChoreographerDeps:
|
||||
def _make_task(task_id: Any) -> Any:
|
||||
"""A task stub that the tracing gate accepts as-is.
|
||||
|
||||
`_check_pm_decision_required` only consults the (agent, task) journal
|
||||
lookup — the task object itself is opaque to that check.
|
||||
`_check_pm_decision_required` consults the (agent, task) journal lookup
|
||||
for the decision window and, for ``delegate``, the persisted
|
||||
``quick_context`` resumption section — so the stub carries a substantive
|
||||
quick_context to satisfy that obligation.
|
||||
"""
|
||||
return MagicMock(id=task_id, status="in_progress")
|
||||
return MagicMock(
|
||||
id=task_id,
|
||||
status="in_progress",
|
||||
quick_context="Decomposition planned; cells implement their slice next.",
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
"""The PR reviewer's pr_reviewer_notes section is obligated on its verbs.
|
||||
|
||||
pr_pass / pr_fail (in-path gate) and post_pr_review (inbound external PR) each
|
||||
require a substantive review note. The note is the verb's own argument (not yet
|
||||
persisted), so it is checked through a SimpleNamespace shim against the
|
||||
``pr_reviewer_notes`` field — these cover both the short-circuit (too short) and
|
||||
the pass-through (long enough) at the tracing-gate helpers.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
|
||||
|
||||
|
||||
def _make_choreographer(*, has_learning: bool = True) -> Choreographer:
|
||||
base: dict[str, Any] = {
|
||||
"task": AsyncMock(),
|
||||
"work_session": AsyncMock(),
|
||||
"git": AsyncMock(),
|
||||
"a2a": AsyncMock(),
|
||||
"journal": AsyncMock(),
|
||||
"audit": AsyncMock(),
|
||||
"evidence_repo": AsyncMock(),
|
||||
}
|
||||
base["journal"].has_learning_for_task.return_value = has_learning
|
||||
return Choreographer(ChoreographerDeps(**base))
|
||||
|
||||
|
||||
_LONG = "Reviewed the assembled diff end to end; the seam contract holds."
|
||||
_SHORT = "looks ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gate_tracing_blocks_on_short_notes() -> None:
|
||||
c = _make_choreographer()
|
||||
env = await c._gate_tracing(
|
||||
uuid4(), uuid4(), MagicMock(), "pr_reviewer", "pr_pass", notes=_SHORT
|
||||
)
|
||||
assert env is not None
|
||||
body = env.as_dict()
|
||||
assert body["error"] == "tracing_gap"
|
||||
assert "pr_reviewer_notes>=min" in body["missing"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_gate_tracing_passes_on_substantive_notes() -> None:
|
||||
c = _make_choreographer()
|
||||
env = await c._gate_tracing(
|
||||
uuid4(), uuid4(), MagicMock(), "pr_reviewer", "pr_fail", notes=_LONG
|
||||
)
|
||||
assert env is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_pr_review_tracing_blocks_on_short_body() -> None:
|
||||
c = _make_choreographer()
|
||||
env = await c._pr_review_tracing_gate(
|
||||
uuid4(), uuid4(), MagicMock(), "pr_reviewer", body=_SHORT
|
||||
)
|
||||
assert env is not None
|
||||
assert "pr_reviewer_notes>=min" in env.as_dict()["missing"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_post_pr_review_tracing_passes_on_substantive_body() -> None:
|
||||
c = _make_choreographer()
|
||||
env = await c._pr_review_tracing_gate(
|
||||
uuid4(), uuid4(), MagicMock(), "pr_reviewer", body=_LONG
|
||||
)
|
||||
assert env is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_learning_still_blocks_even_with_long_notes() -> None:
|
||||
"""The journal:learning requirement remains independent of the note."""
|
||||
c = _make_choreographer(has_learning=False)
|
||||
env = await c._gate_tracing(
|
||||
uuid4(), uuid4(), MagicMock(), "pr_reviewer", "pr_pass", notes=_LONG
|
||||
)
|
||||
assert env is not None
|
||||
assert "journal:learning" in env.as_dict()["missing"]
|
||||
Reference in New Issue
Block a user