Files
roboco/tests/unit/api/test_correlation_id.py
T
7901ea419e Retire channels/sessions/messages; A2A becomes primary agent comms (#306)
* feat(a2a): deliver latest incoming message preview into the claim briefing

list_unread_a2a now carries last_message_preview (the latest message from the
OTHER agent, never the agent's own reply), fetched via a correlated subquery in
the same query — no N+1 on the per-verb briefing path.

* feat(a2a): read_a2a verb delivers unread message bodies to the agent

A2AService.get_unread_messages returns the caller's unread INCOMING messages
(never its own sends), marking exactly those rows read atomically so a message
arriving mid-call is preserved. Wired as the read_a2a content verb (route +
do_server tool + granted to every delivery role) — the content-bearing read the
A2A inbox lacked (read_messages only zeroed the counter).

* docs(rag): document read_a2a as the A2A content-read path

* fix(task): backlog activation no longer requires a discussion session

Removes the SessionTaskTable gate in activate() (and its dangling log field),
deletes _inherit_parent_session + its create() call, and drops the now-unused
SessionTaskTable import. Coordination rides task state; the session subsystem is
being retired. Tests updated to the new (no-session) behavior.

* fix(orchestrator): drop session sweep from _run_sweep

Removes the messaging import + sweep_timed_out_sessions call. That import sat
outside the try/except, so once messaging.py is deleted it would have killed the
entire sweep cascade (budget kill-switch, token rollups, retention, image prune,
superseded-PR reconcile). Notification sweep + all maintenance sweeps unchanged.

* release-manager --no-tags read-clone fix

* test: update evidence_repo unit test for a2a last_message_preview

* refactor(gateway): drop session propagation on delegate

Removes propagate_sessions_to_subtask from delegate(), the ChoreographerDeps
messaging field + property, and the ChoreographerDeps messaging arg in deps.py
(ContentActions messaging + import stay until the verbs are removed). Deletes the
propagation test; strips the now-invalid messaging kwarg from ChoreographerDeps
test builders.

* refactor(gateway): remove say/open_session/link_session/channels verbs

Removes the four channel/session verbs across content_actions (impls +
ContentActionsDeps.messaging), do_server (tools + registry), role_config (grants
+ _CHANNEL_DISCOVERY), do.py (routes), schemas/v1/do.py (request models), and
deps.py (MessagingService import + construction). Regenerates the prompt verb
tables. dm/notify/read_messages/read_a2a stay. Tests deleted/updated accordingly.

* uv.lock Upgrade

* refactor: remove conversation RAG indexing; Secretary announces via notification

Drops the CONVERSATIONS index (index_conversation, ConversationsIndexPlugin,
IndexType.CONVERSATIONS enum, IndexConversationParams, mentor.py type-label, the
messaging index hook) and its chunk-table manifest entries. The Secretary's
ANNOUNCE/RELAY_MESSAGE now fan out a BROADCAST notification to every agent's
inbox (NotificationService.broadcast) instead of posting to a dead channel.

* fix(panel): label RAG health error lines by subsystem

A red llm_error (e.g. the glm-5.2:cloud weekly-limit 429) rendered under
the 'Embedding: ok' header with no label, reading as an embedding failure.
Prefix each error line with LLM / Embedding / Vector store.

* refactor: remove channel/message reads from metrics, dashboard, git, events

MetricsService drops get_communication_volume + the MessageTable
message-count in get_agent_metrics (and the now-dead messages_sent_week
field). DashboardService drops get_channel_feeds/_compute_channel_status
and the message read in get_recent_activity (task activity kept);
get_auditor_metrics no longer reports communication_volume.
GitService's two primary-session-id helpers always return None now
(callers already treat None as "no primary session"). events/handlers.py
drops the SESSION_CLOSED/SESSION_TIMEOUT subscriptions + the
handle_session_boundary handler.

Forced follow-on: api/routes/dashboard.py + api/schemas/dashboard.py
dropped the now-dangling live_feeds/ChannelFeed surface and the
/metrics/communication route, which wrapped the removed service calls
directly (mypy would otherwise fail on the missing attributes).

* refactor: delete MessagingService + channel seeding

Edited db/__init__.py and services/__init__.py first (drop the unconditional
Channel/Group/Message/Session table + MessagingService re-exports), then
deleted services/messaging.py, then trimmed db/seed.py to only create_agents
(create_channels/create_channel_memberships/create_initial_messages gone).

Forced expansion: api/routes/{channels,groups,sessions,messages}.py import
roboco.services.messaging directly (not through the package __init__), as
does api/routes/tasks.py (the session-links embed on GET /tasks/{id} and the
GET /{id}/sessions route). Deleting messaging.py without addressing these
breaks `import roboco.api.app` immediately, since app.py eagerly imports all
route modules at startup. Since the 4 CRUD route files are 100%
MessagingService-backed with zero independent logic (and are wholesale
deletes in the plan's later API-routes task anyway), deleted them now +
unmounted from app.py/routes/__init__.py; tasks.py got the same surgical
trim its later task already specified (drop session-links embed +
TaskSessionLinkResponse/TaskResponse.sessions). This pulls a slice of that
later work forward — the routes/schemas for channels/groups/sessions/messages
still need their own pass, but their messaging-coupled parts are gone.

Verified with a full-suite collection sweep (12010 tests collected, zero
import errors) beyond the directly touched test dirs, given the expanded
blast radius.

* refactor: remove channel/session/message models, tables, and channel policy

Models: deleted channel.py/group.py/session.py/messaging.py wholesale
(zero external consumers besides the models/__init__.py re-export).
message.py surgically trimmed: removed MessageCreate (dead) and MessageEdit
(never instantiated; ExtractedMessage.edit_history retyped to
list[dict[str, Any]] to match how it's actually persisted — confirmed
ExtractedMessage was never written to any DB table, so MessageTable's
removal carries no functional risk to the kept extraction pipeline).
base.py: removed SessionStatus + ChannelType, kept MessageType. Also
removed the confirmed-dead channels_read/channels_write fields from
models/agent.py:AgentPermissions and models/dashboard.py:ChannelFeedData.

db/tables.py: deleted ChannelTable/GroupTable/SessionTable/SessionTaskTable/
MessageTable, TaskTable.session_links, and JournalEntryTable.session_id —
cascaded through models/journal.py, services/journal.py, and
api/schemas+routes/journals.py (22 plumbing sites).

foundation/policy/communications.py: removed the ChannelSpec/CHANNELS
catalog + TEAM_SCOPED_ROLES/_CELL_*/_AUDITOR_ONLY helpers, kept the
notification policy (Priority/parse_priority/NOTIFY_SENDER_ROLES/
ACK_REQUIRED_BY_TYPE). enforcement/channel_access.py deleted (confirmed
fully dead in production). agents_config.py: removed CHANNEL_ACCESS
(kept A2A_ALLOWED_PAIRS). seeds/initial_data.py: removed
DEFAULT_CHANNELS/CHANNEL_MEMBERSHIPS/AUDITOR_SILENT_ACCESS + the
never-consumed INITIAL_MESSAGES. config.py: removed
session_idle_timeout_seconds (zero consumers). exceptions.py: removed
dead ChannelError/ChannelAccessDeniedError/SessionClosedError.

Forced expansion beyond the original file list — ChannelType cascaded
into a live, mounted surface the plan didn't trace: agents_config.
CHANNEL_ACCESS -> services/permissions.py's channel-RBAC methods (not
models/permissions.py, which turned out to have no channel code at all)
-> two real endpoints in api/routes/stream.py (GET /permissions,
GET /permissions/channel/{name}) and two dependency factories in
api/deps.py. Removed the channel methods + fields, deleted the
channel-specific stream.py endpoint, deleted require_channel_read/write.
Also deleted api/schemas/{channels,sessions}.py (hard dependency on the
removed enums; already fully dead after the Task 10 route deletions) and
api/schemas/messages.py (a TYPE_CHECKING-only import of the deleted
MessageTable; likewise already fully dead) + its dedicated test file.

Test updates: test_permissions.py -14 channel tests (matches the planned
count exactly), test_communications.py / test_communications_consumers.py
split to keep only notification-policy coverage, test_exceptions.py -9,
test_deps.py -4, plus the journal/stream/foundation-smoke fallout. Also
fixed a pre-existing (Task 7) broken assertion in
test_foundation_phase3_smoke.py that inspected a `say()` method already
removed from ContentActions.

Verified: full-suite collection (11961 tests, zero import errors) and a
complete test run (11567 passed, 394 skipped, 0 failed) in addition to
the targeted suites.

* migration: drop channels/groups/sessions/session_tasks/messages + enum types

alembic/versions/060_drop_messaging.py: drop_column journal_entries.
session_id (sidesteps hardcoding the FK constraint name — verified
empirically against a live migrated DB that it's actually
fk_journal_entries_session_id_sessions, but drop_column doesn't care
either way); drop_table in FK order (messages -> session_tasks ->
sessions -> groups -> channels); DROP TABLE IF EXISTS chunks_conversations
(runtime-provisioned, not alembic-managed, would otherwise orphan); DROP
TYPE IF EXISTS for messagetype/sessionstatus/sessionscope/channeltype
(messagetype's Python enum stays for ExtractedMessage, but the DB type
had zero live columns left once MessageTable was dropped in the prior
commit). downgrade() raises NotImplementedError — one-way removal.

Pruned scripts/reset_runtime_state.sql + .sh: removed the DELETE/COUNT
lines for messages/session_tasks/sessions/groups/channels and the
groups.active_session_id reset block.

Verified end-to-end against a scratch Postgres DB: full migration chain
001->060 applies cleanly, alembic heads shows a single head, all 6 dropped
tables + 4 enum types + the journal_entries.session_id column are
confirmed gone, journal_entries keeps only its journal_id/task_id FKs,
downgrade correctly raises NotImplementedError without corrupting DB
state, and the pruned reset_runtime_state.sql runs clean (no errors)
against a fully-migrated DB.

* refactor(api): remove channel/session/message routes + WS streams

Most of this task's file list was already forced through in earlier
commits (routes/{channels,groups,sessions,messages}.py + app.py/__init__.py
unmounting in the MessagingService-deletion commit; tasks.py's
session-links embed + GET /{id}/sessions + schemas/tasks.py's
TaskResponse.sessions in that same commit; deps.py's require_channel_read/
write + schemas/{channels,sessions}.py in the models/tables commit). This
closes out what was left:

- api/websocket.py: deleted the channel_stream + session_stream routes,
  ConnectionManager's channel_connections/session_connections dicts,
  connect_channel/connect_session, broadcast_to_channel/broadcast_to_session,
  get_channel_subscriber_count, and their cleanup lines in disconnect().
  Agent streams, notification streams, and the operator system stream are
  untouched.
- api/websocket_bridge.py: deleted _handle_session_event +
  _handle_message_event and their SESSION_CREATED/SESSION_CLOSED/
  SESSION_TIMEOUT/MESSAGE_SENT subscriptions. The A2A live-view, rate-limit,
  usage, agent-lifecycle, and notification bridges are untouched.
- api/schemas/websocket.py: removed NewMessageBroadcast, WSMessageNew,
  WSMessageEdit, WSMessageDelete, WSSessionClosed — kept the WSMessage base
  class (still subclassed by the kept WSAgentStream/WSNotification) plus
  those two.
- api/schemas/groups.py: deleted (already fully orphaned since routes/
  groups.py was removed; its GroupResponse/GroupDetailResponse had zero
  consumers).

Updated the 5 websocket test files accordingly (removed the channel/
session-specific tests + fixed imports); test_websocket_bridge.py's
registration-coverage test dropped the SESSION_*/MESSAGE_SENT assertions.

Verified: full-suite collection (11943 tests, zero import errors) and a
complete test run (11549 passed, 394 skipped, 0 failed).

* docs: retire channels/sessions/messages from agent-facing docs + CLAUDE.md

Rewrites docs/rag (RAG-indexed) + docs/map + CLAUDE.md to reflect A2A (dm +
read_a2a) as primary agent comms; deletes the channel docs, splits messaging-tools
+ messaging-notification (renamed notification.md), swaps the WS worked example to
A2A_MESSAGE_SENT. _complete_map.md still needs regeneration (generated file).

* refactor(panel): remove Communications surface (channels/sessions)

Deletes the /communications routes, message components, task-detail Sessions tab,
use-channels + channel/session WS hooks, and the channels/sessions/messages/groups
api clients; prunes the Channel/Session/Message/Group types + mock data. (Auditor
live-feeds + dashboard.ts dead-route cleanup is a follow-up.)

* refactor(panel): drop auditor channel-feed + dead communication-metric route

* docs(map): regenerate _complete_map from updated slices

* fix(a2a): reduce get_unread_messages complexity below xenon C + stale comments

Extract the per-conversation unread-counter recompute into _reset_unread_counter
(the CI quality gate flagged get_unread_messages as rank C). Also drop the deleted
open_session from a content_actions comment and reword an evidence_repo docstring
that cited the removed messaging._notify_mentions.

---------

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

276 lines
9.7 KiB
Python

"""End-to-end correlation_id propagation: header -> envelope -> audit row.
Audit F20 surfaced that ``X-Correlation-ID`` was bound to structlog by the
``CorrelationIdMiddleware`` but never travelled past the API boundary:
* The MCP shims (``flow_server`` / ``do_server``) didn't forward it, so
every MCP -> API hop got a fresh server-generated UUID.
* The Envelope returned to the agent had no slot to carry the id back.
* The ``audit_log`` rows the choreographer writes had no correlation_id
field, so post-mortem joins across logs and audit trail were impossible.
These tests pin the contract:
1. Envelope holds an optional ``correlation_id`` and round-trips it via
``as_dict()``.
2. The v1 flow route reads ``request.state.correlation_id`` (set by
``CorrelationIdMiddleware``) and stamps it onto the envelope before
returning.
3. Both MCP shims attach an ``X-Correlation-ID`` header on every POST,
mirroring how they attach ``X-Agent-ID`` / ``X-Agent-Role``.
4. The choreographer's ``_emit_rejection`` audit writer pulls the
correlation_id from the structlog contextvars (where the middleware
binds it) and stuffs it into the audit row's ``details`` dict.
"""
from __future__ import annotations
import asyncio
import importlib
import json
import tempfile
from pathlib import Path
from typing import TYPE_CHECKING, Any
from unittest.mock import AsyncMock, MagicMock, patch
from uuid import UUID
import pytest
import structlog
from fastapi import FastAPI
from fastapi.testclient import TestClient
from roboco.api.deps import get_choreographer
from roboco.api.middleware import CorrelationIdMiddleware
from roboco.api.routes.v1.flow_dev import router as flow_dev_router
from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps
from roboco.services.gateway.envelope import Envelope
if TYPE_CHECKING:
from types import ModuleType
_HTTP_200 = 200
_DEV_AGENT_HEADERS = {
"X-Agent-ID": "00000000-0000-0000-0000-000000000001",
"X-Agent-Role": "developer",
}
# --- Envelope ----------------------------------------------------------------
def test_envelope_ok_carries_correlation_id_when_stamped() -> None:
"""correlation_id is set post-construction by the transport layer."""
env = Envelope.ok(status="idle", next="call i_am_idle()")
env.correlation_id = "test-id-123"
assert env.correlation_id == "test-id-123"
assert env.as_dict()["correlation_id"] == "test-id-123"
def test_envelope_error_carries_correlation_id_when_stamped() -> None:
env = Envelope.invalid_state(message="bad state", remediate="do X first")
env.correlation_id = "abc"
assert env.correlation_id == "abc"
assert env.as_dict()["correlation_id"] == "abc"
def test_envelope_correlation_id_defaults_to_none() -> None:
env = Envelope.ok(status="idle", next="call i_am_idle()")
assert env.correlation_id is None
# When None we still emit the key so consumers don't have to special-case.
assert env.as_dict()["correlation_id"] is None
# --- Route -> Envelope wiring ------------------------------------------------
def _build_app() -> tuple[FastAPI, MagicMock]:
app = FastAPI()
app.add_middleware(CorrelationIdMiddleware)
app.include_router(flow_dev_router)
mock_chore = MagicMock()
mock_envelope = Envelope.ok(status="idle", next="...")
mock_chore.give_me_work = AsyncMock(return_value=mock_envelope)
app.dependency_overrides[get_choreographer] = lambda: mock_chore
return app, mock_chore
def test_route_stamps_request_correlation_id_onto_envelope() -> None:
app, _ = _build_app()
client = TestClient(app)
r = client.post(
"/api/v1/flow/developer/give_me_work",
json={},
headers={**_DEV_AGENT_HEADERS, "X-Correlation-ID": "trace-xyz"},
)
assert r.status_code == _HTTP_200
assert r.json()["correlation_id"] == "trace-xyz"
# Middleware also echoes the header back, so ops can grep for it.
assert r.headers["X-Correlation-ID"] == "trace-xyz"
def test_route_stamps_generated_correlation_id_when_header_missing() -> None:
app, _ = _build_app()
client = TestClient(app)
r = client.post(
"/api/v1/flow/developer/give_me_work",
json={},
headers=_DEV_AGENT_HEADERS,
)
assert r.status_code == _HTTP_200
body_id = r.json()["correlation_id"]
header_id = r.headers["X-Correlation-ID"]
# Middleware generates a UUID and binds it. The route must read the
# SAME id back from request.state and stamp it onto the envelope.
assert body_id is not None
assert body_id == header_id
# --- MCP shims ---------------------------------------------------------------
def _reload_mcp_module(monkeypatch: pytest.MonkeyPatch, dotted: str) -> ModuleType:
"""Set the env vars MCP servers expect at import-time and reload the module.
Both servers read AGENT_ID / AGENT_ROLE / ORCHESTRATOR_URL once at
import; we have to re-import after monkey-patching so the test sees
the patched values. The reload itself is the lazy import — keeping
importlib at the top-level keeps PLC0415 happy.
Also writes a stub manifest file and points the MCP server at it,
since both servers now refuse to register any tools without one.
"""
monkeypatch.setenv("ROBOCO_AGENT_ID", "00000000-0000-0000-0000-000000000001")
monkeypatch.setenv("ROBOCO_AGENT_ROLE", "developer")
monkeypatch.setenv("ROBOCO_ORCHESTRATOR_URL", "http://test-orchestrator:8000")
manifest_path = Path(tempfile.mkdtemp()) / "tool-manifest.json"
manifest_path.write_text(
json.dumps(
{
"agent_id": "00000000-0000-0000-0000-000000000001",
"role": "developer",
"team": "backend",
"workspace_path": "/tmp/test",
"flow_tools": [
"give_me_work",
"i_will_work_on",
"open_pr",
"i_am_done",
"i_am_blocked",
"unclaim",
"resume",
"i_am_idle",
],
"do_tools": ["commit", "note", "dm", "evidence"],
"read_tools": [],
"write_tools": [],
"bash_allowed": True,
"subagent_allowed": False,
"subagent_model": None,
"env": {},
}
)
)
monkeypatch.setenv("ROBOCO_TOOL_MANIFEST_PATH", str(manifest_path))
module = importlib.import_module(dotted)
return importlib.reload(module)
@pytest.fixture
def flow_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType:
return _reload_mcp_module(monkeypatch, "roboco.mcp.flow_server")
@pytest.fixture
def do_module(monkeypatch: pytest.MonkeyPatch) -> ModuleType:
return _reload_mcp_module(monkeypatch, "roboco.mcp.do_server")
def _fake_client(payload: dict[str, Any]) -> MagicMock:
fake_response = MagicMock()
fake_response.json.return_value = payload
fake_client = MagicMock()
fake_client.__enter__ = MagicMock(return_value=fake_client)
fake_client.__exit__ = MagicMock(return_value=False)
fake_client.post.return_value = fake_response
return fake_client
def test_flow_server_attaches_correlation_id_header(
flow_module: ModuleType,
) -> None:
fake = _fake_client({"status": "idle"})
with patch("httpx.Client", return_value=fake):
flow_module.give_me_work()
_args, kwargs = fake.post.call_args
headers = kwargs["headers"]
assert headers["X-Agent-ID"] == "00000000-0000-0000-0000-000000000001"
assert headers["X-Agent-Role"] == "developer"
assert "X-Correlation-ID" in headers
assert headers["X-Correlation-ID"] # non-empty
def test_flow_server_generates_unique_correlation_id_per_call(
flow_module: ModuleType,
) -> None:
fake = _fake_client({"status": "idle"})
with patch("httpx.Client", return_value=fake):
flow_module.give_me_work()
flow_module.give_me_work()
first = fake.post.call_args_list[0].kwargs["headers"]["X-Correlation-ID"]
second = fake.post.call_args_list[1].kwargs["headers"]["X-Correlation-ID"]
assert first != second
def test_do_server_attaches_correlation_id_header(
do_module: ModuleType,
) -> None:
fake = _fake_client({"status": "noted"})
with patch("httpx.Client", return_value=fake):
do_module.note("hi")
_args, kwargs = fake.post.call_args
headers = kwargs["headers"]
assert headers["X-Agent-ID"] == "00000000-0000-0000-0000-000000000001"
assert headers["X-Agent-Role"] == "developer"
assert "X-Correlation-ID" in headers
assert headers["X-Correlation-ID"]
# --- Audit-row stash ---------------------------------------------------------
def test_choreographer_emit_rejection_includes_correlation_id_in_details() -> None:
"""Audit row's `details` dict carries correlation_id from contextvars."""
audit = MagicMock()
audit.log_event = AsyncMock()
deps = ChoreographerDeps(
task=MagicMock(),
work_session=MagicMock(),
git=MagicMock(),
a2a=MagicMock(),
journal=MagicMock(),
audit=audit,
evidence_repo=MagicMock(),
)
chore = Choreographer(deps)
bad_env = Envelope.invalid_state(message="oops", remediate="do X")
structlog.contextvars.clear_contextvars()
structlog.contextvars.bind_contextvars(correlation_id="cid-9")
try:
asyncio.run(
chore._emit_rejection(
bad_env,
agent_id=UUID("00000000-0000-0000-0000-000000000001"),
task_id=None,
verb="i_will_work_on",
)
)
finally:
structlog.contextvars.clear_contextvars()
audit.log_event.assert_awaited_once()
kwargs = audit.log_event.await_args.kwargs
assert kwargs["details"]["correlation_id"] == "cid-9"