Files
roboco/tests/unit/gateway/test_audit_on_rejection.py
T
312ec990dd fix: prod triage 2026-07-08 — MCP auth residue, gateway envelopes, verb-loop cap, A2A interjection, manual spawn UX (#334)
* fix(auth): pass agent UUID to CLI-arg MCP servers (optimal/docs/search)

The container token is HMAC-signed over the agent UUID (#314), but the
optimal/docs/search MCP servers received the slug as their CLI arg and
sent X-Agent-ID=<slug>, so every research/RAG/docs call 401ed with
signature mismatch under enforced auth. Pass the already-computed
agent_uuid in the three args lists instead.

* fix(gateway): include remediate in gateway.rejected audit details

Conventions-gate rejections carry the offending file:line listing only
in the envelope's remediate field, which the audit row dropped -- ops
logs showed just the violation count with no way to see what blocked.

* fix(gateway): return envelope on do/commit git failure

A GitError from the commit verb propagated to the generic middleware
handler, so agents got a raw error blob with no remediate/next. Catch
it and return an error envelope; 'no changes added to commit' with an
explicit files list now names the mismatch and the omit-files fallback.

* fix(agent-sdk): absolute rejection cap breaks slow-drip verb loops

The verb circuit breaker only counted rejections inside a 60s sliding
window, so an agent retrying i_am_done every 3-4 minutes looped for 30+
minutes without tripping it. Add a session-scoped cumulative per-(verb,
task) cap at 3x the windowed limit that trips regardless of pacing.

* feat(a2a): CEO chime-in interjects into the viewed conversation

Previously reply_as_ceo re-homed the message into a canonical CEO<->target
conversation with no panel surface, so a chime-in reported success but was
invisible and only opportunistically delivered. interject_as_ceo now inserts
the message into the conversation being viewed (from_agent=ceo, directed via
an @target content prefix), bumps that conversation's counters with the
unread ping keyed to the addressed participant, and both participants see it
in transcript and read_a2a.

* feat(panel): manual spawn carries task + message, surfaces refusals

The agent detail page spawned with no request body (task/message impossible),
the spawn button could double-fire (2.5ms double-POST seen live), and refusal
reasons never reached the UI: readiness refusals were generic 500s and the
already-running no-op looked like success. Detail page now uses
SpawnAgentDialog, a synchronous ref guard blocks re-entry, AgentReadinessError
maps to 409 with its reason shown, already_running is signalled and toasted,
and a task_id builds a task-aware prompt instructing the claim (task_id alone
never did), with the CEO's message appended as a note.

* test(panel): align a2a page test with the interjection footer copy

The chime-in rebuild changed the composer footer; the page-level test
asserting the old copy was outside the rebuild's scoped vitest run.

* fix(api): commit the request DB session before the response is sent

FastAPI unwinds yield-dependencies after the response bytes go out, so
get_db's post-yield commit raced the client's next request -- a verb
could return ok while its claim/status write was still uncommitted (the
e2e ok-without-effect flake family), and a failed commit was silently
lost behind an already-sent 200. DbCommitMiddleware (innermost, pure
ASGI) commits the session stashed by get_db_committed before forwarding
http.response.start; commit failure now surfaces as a 5xx. get_db is
untouched for its direct non-request callers.

* fix(db): invalidate, not rollback, the session on request cancellation

With the commit moved into the send path, the flow-verb timeout can
cancel mid-commit; rolling back then issues another command over an
asyncpg connection stranded mid-wire-protocol, and the poisoned
connection segfaults uvloop/asyncpg when a later checkout recycles it
(3/3 identical CI faulthandler dumps). On CancelledError discard the
connection via session.invalidate() -- SQLAlchemy's documented handling
for a timeout during commit -- and keep rollback for plain exceptions.

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-08 10:41:02 +02:00

240 lines
8.4 KiB
Python

"""Every Envelope rejection from a Choreographer verb writes an audit row.
Choreographer takes an ``audit`` dependency but historically never invoked
it. The result: every rejection envelope (invalid_state, not_authorized,
tracing_gap, not_found) silently disappeared. With no forensic trail, a
stuck flow had no breadcrumbs.
These tests pin the ``gateway.rejected`` audit-write behavior across the
range of rejection-returning verbs, including:
- not_authorized rejections (PM cannot execute code, role-typed claim)
- invalid_state rejections (no active task, expected status mismatch)
- tracing_gap rejections (missing notes, missing journal entries)
- not_found rejections (unknown task id)
The audit call is fire-and-forget; an exception inside ``log_event`` must
not propagate or alter the envelope returned to the agent.
"""
from __future__ import annotations
from datetime import UTC, datetime
from typing import Any
from unittest.mock import AsyncMock, MagicMock
from uuid import uuid4
import pytest
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
from roboco.services.gateway.envelope import Envelope
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)
repo = base["evidence_repo"]
for method in (
"list_unread_a2a",
"list_unread_mentions",
"list_pending_notifications",
"task_metadata_gaps",
"recent_team_activity",
"blockers_in_lane",
"journal_highlights_for_task",
):
getattr(repo, method).return_value = []
# C8: default-fresh journal:decision so PM-decision gate passes.
# Tests that exercise the gate boundary stub their own value.
# The check matches MagicMock and AsyncMock (the two default sentinel
# types pytest's unittest.mock leaves on un-stubbed return_values).
_ldef = base["journal"].latest_decision_at.return_value
if type(_ldef).__name__ in ("MagicMock", "AsyncMock"):
base["journal"].latest_decision_at.return_value = datetime.now(UTC)
return ChoreographerDeps(**base)
# ---------------------------------------------------------------------------
# Primary acceptance test: PM cannot claim a code task — not_authorized path
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_pm_cannot_execute_code_writes_audit_row() -> None:
"""A cell_pm calling i_will_work_on on a code task must:
1. Return an Envelope with error == 'not_authorized'
2. Write a gateway.rejected audit event with verb + reason details
"""
aid = uuid4()
tid = uuid4()
code_task = MagicMock(
id=tid,
status="pending",
assigned_to=aid,
task_type="code",
priority=1,
parent_task_id=None,
sequence=0,
team="backend",
)
task_svc = AsyncMock()
task_svc.get.return_value = code_task
task_svc.agent_for.return_value = MagicMock(role="cell_pm", team="backend")
task_svc.list_in_progress_for_agent.return_value = []
task_svc.list_paused_for_agent.return_value = []
task_svc.get_subtasks.return_value = []
audit_svc = AsyncMock()
deps = _make_deps(task=task_svc, audit=audit_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(aid, tid, plan="x")
assert env.error == "not_authorized"
audit_svc.log_event.assert_awaited()
args = audit_svc.log_event.await_args
assert args.kwargs["event_type"] == "gateway.rejected"
assert args.kwargs["details"]["verb"] == "i_will_work_on"
assert args.kwargs["details"]["reason"] == "not_authorized"
# ---------------------------------------------------------------------------
# not_found path: unknown task id
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_unknown_task_writes_audit_row() -> None:
"""not_found rejection (unknown task id) is audited."""
aid = uuid4()
tid = uuid4()
task_svc = AsyncMock()
task_svc.get.return_value = None
audit_svc = AsyncMock()
deps = _make_deps(task=task_svc, audit=audit_svc)
c = Choreographer(deps)
env = await c.i_am_done(aid, tid, notes="something")
assert env.error == "not_found"
audit_svc.log_event.assert_awaited()
args = audit_svc.log_event.await_args
assert args.kwargs["event_type"] == "gateway.rejected"
assert args.kwargs["details"]["verb"] == "i_am_done"
assert args.kwargs["details"]["reason"] == "not_found"
# ---------------------------------------------------------------------------
# Happy path must NOT write an audit row.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_successful_verb_does_not_write_audit_row() -> None:
"""Successful (non-error) Envelope must not emit gateway.rejected audit."""
aid = uuid4()
task_svc = AsyncMock()
task_svc.list_assigned_for_agent.return_value = []
task_svc.list_paused_for_agent.return_value = []
audit_svc = AsyncMock()
deps = _make_deps(task=task_svc, audit=audit_svc)
c = Choreographer(deps)
env = await c.give_me_work(aid)
# No rejection, so no audit row.
assert env.error is None
audit_svc.log_event.assert_not_awaited()
# ---------------------------------------------------------------------------
# Audit failure must NOT block the verb (best-effort rule).
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_audit_log_event_failure_does_not_propagate() -> None:
"""If log_event raises, the verb still returns the rejection envelope."""
aid = uuid4()
tid = uuid4()
task_svc = AsyncMock()
# Unknown task id triggers not_found rejection on i_am_done.
task_svc.get.return_value = None
audit_svc = AsyncMock()
audit_svc.log_event.side_effect = RuntimeError("audit DB down")
deps = _make_deps(task=task_svc, audit=audit_svc)
c = Choreographer(deps)
# Must not raise; the rejection envelope should still come back.
env = await c.i_am_done(aid, tid, notes="x")
assert env.error == "not_found"
audit_svc.log_event.assert_awaited()
# ---------------------------------------------------------------------------
# remediate must ride along into the audit row — it's the only place a
# conventions-gate rejection's file:line violation listing lives.
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_rejection_remediate_lands_in_audit_details() -> None:
"""A rejection's `remediate` hint is copied into the audit row's details.
Without this, an operator reading `gateway.rejected` audit rows for a
conventions-gate rejection sees only the summary message — the
actionable detail lives solely in `remediate`.
"""
aid = uuid4()
tid = uuid4()
code_task = MagicMock(
id=tid,
status="pending",
assigned_to=aid,
task_type="code",
priority=1,
parent_task_id=None,
sequence=0,
team="backend",
)
task_svc = AsyncMock()
task_svc.get.return_value = code_task
task_svc.agent_for.return_value = MagicMock(role="cell_pm", team="backend")
task_svc.list_in_progress_for_agent.return_value = []
task_svc.list_paused_for_agent.return_value = []
task_svc.get_subtasks.return_value = []
audit_svc = AsyncMock()
deps = _make_deps(task=task_svc, audit=audit_svc)
c = Choreographer(deps)
env = await c.i_will_work_on(aid, tid, plan="x")
assert env.error == "not_authorized"
assert env.remediate
args = audit_svc.log_event.await_args
assert args.kwargs["details"]["remediate"] == env.remediate
@pytest.mark.asyncio
async def test_rejection_without_remediate_omits_audit_key() -> None:
"""A rejection with no remediate must not add a null key to the row."""
aid = uuid4()
tid = uuid4()
audit_svc = AsyncMock()
deps = _make_deps(audit=audit_svc)
c = Choreographer(deps)
env = Envelope(error="not_found", message="bare rejection, no remediate")
await c._emit_rejection(env, agent_id=aid, task_id=tid, verb="test_verb")
args = audit_svc.log_event.await_args
assert "remediate" not in args.kwargs["details"]