mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
* fix(rate-limit): real provider liveness probe instead of time-based stub
The rate-limit recovery sweeper cleared a provider and resumed parked agents
purely on elapsed time — _do_probe was a stub that always returned True once
the retry_after window passed, so it never confirmed the provider had actually
stopped rate-limiting us. Under a sustained limit that resumes agents straight
into another 429, re-parking them: avoidable churn.
Make the probe real. _do_probe now issues a free, unmetered liveness call —
Anthropic GET /v1/models or Ollama GET /api/tags — and treats any non-429
response as the limit having lifted. A 429 keeps the provider parked; a
network error keeps it parked too (retry next sweep). When the provider can't
be probed (no API key, or an unrecognized provider), it falls back to the
prior time-expiry optimism rather than stranding agents. _probe_target keeps
URL/header resolution separate and testable, and _do_probe stays a
monkeypatchable boundary so the existing sweep tests are unaffected.
Also drop two acceptance-criteria-number labels from comments in this file.
* chore(rate-limit): clear merged gate debt in rate-limit tests + deps lint
The rate-limit PR landed with ruff violations the full gate flags but the
authors' runs missed: test_rate_limit_sweep.py was unformatted, and
test_rate_limit_tracker.py had unsorted/unused imports and magic-value
comparisons. Format the sweep test, drop the dead imports, and bind the
magic comparison values to locals. Also strip acceptance-criteria-number
labels from comments/docstrings across the three rate-limit test files
(leaving genuine acceptance_criteria=[...] test data untouched), and add
api/deps.py to the PLC0415 per-file-ignore — it is the DI wiring hub and
defers a couple of service imports to call time to avoid import cycles,
the same rationale already applied to api/routes, runtime, and services.
* fix(rate-limit): resolve redis type errors in RateLimitStateTracker
A cold mypy run (the gate's true state — prior passes were warm-cache only)
flagged four redis-typing errors in rate_limit_tracker.py that the merge
missed: three unused type:ignore[type-arg] on redis.Redis, and an
aclose() the bundled redis type stub doesn't expose.
Drop the now-unused ignores, and close the scan client via
'async with redis.from_url(...) as r:' instead of a finally-block
aclose(). The context manager closes the client on exit using the modern
redis.asyncio API — no deprecated close(), no stub-missing aclose(), no
suppression. Extend the test's redis mock to model the async
context-manager protocol so it returns itself on enter.
* test(prompter): pass route='main_pm' in the product main-PM routing test
Pre-existing master failure, unrelated to the rate-limit work. The test is
named ...product_routes_to_main_pm and asserts team=MAIN_PM, but called
confirm_live_draft without a route, so it got the 'board' default — which
assigns the Product Owner and yields team=BOARD by design (the board-review
path keeps the root at team=board until the CEO approves). The Main-PM path
is selected with route='main_pm', exactly as the sibling
...main_pm_route_assigns_main_pm test does. Add the missing kwarg so the test
verifies the path it names; behaviour under test is unchanged.
* Updated uv.lock
* refactor(complexity): bring all rank-C blocks under the xenon B ceiling
The full quality gate's xenon step (--max-absolute B --max-modules A
--max-average A) failed on eight rank-C blocks plus the extraction module
average — debt the rate-limit and token-analytics merges deferred. Reduce
each by extracting cohesive helpers, behaviour unchanged:
- orchestrator._probe_one_provider: split into _too_early_to_probe,
_on_probe_success, _on_probe_failure, _parked_agents_for.
- rate_limit_tracker.list_rate_limited_providers: extract _read_rate_limited_entry
and a _decode helper.
- trigger_filter.decide_spawn: extract _stale_trigger_decision (drops the
PLR0911 suppression too).
- ollama_embedder (embed_query, _embed_batch_sync, aembed_query,
_embed_batch_async): share _rl_backoff / _map_embed_error / _log_429 /
_sleep_connect_retry / _asleep_connect_retry; remove a dead post-loop guard
in aembed_query.
- mentor._synthesize_answer: extract _select_system_prompt and
_answer_from_response.
- indexes/base.ask: extract the 429-retried LLM call into _ask_llm.
- extraction.__init__: extract _compile_patterns so the module average
lands at rank A.
xenon now exits 0; rate-limit, optimal_brain, extraction, and events suites
all green.
* chore(deps): drop obsolete types-redis stub; honor redis 8.0 inline types
types-redis 4.6 (typed for redis 4.x) shadowed redis 8.0's own inline types,
which both masked real annotation mismatches in stream_bus.py and forced
awkward workarounds elsewhere. The stale stub is why the mypy gate only ever
passed warm-cached: a cold run under the wrong stub disagreed with the code.
Remove types-redis (and its orphaned transitive stubs) so mypy uses redis's
shipped types. That surfaces that xreadgroup/xclaim return bytes-keyed records
while _handle_message is annotated str — the code already decodes bytes
defensively, so this is an annotation gap, not a runtime bug. Make the types
honest: cast each result to its concrete shape and decode the stream name and
message id to str at the dispatch boundary via a _to_str helper.
mypy roboco/ is now clean cold (247 files) against redis's real types; events
suite green.
* Updated uv.lock
* fix(workspace): install the dev extra so agents can run make quality
Agent workspaces were set up with plain `uv sync`, which installs only the
project's default dependency group (pytest) — not the `dev` *extra* where the
gate tools live (ruff, mypy, xenon, radon, vulture, bandit, deptry). So an
agent's .venv had pytest but no linters, and `make quality` died immediately
on `ruff: command not found`. Agents literally could not lint, type-check, or
complexity-check their own work, which is how format/mypy/xenon debt merged
unseen. Sync the `dev` extra (`uv sync --extra dev`) so the workspace gets the
full toolchain the setup's own docstring already promised.
* fix(panel): rate-limit endpoint shape + websocket path
Two panel-facing breakages from the rate-limit rework:
- GET /api/system/rate-limits returned a raw list, but the panel store reads
response.entries — so `r.entries is not iterable` crashed the banner sync on
page load. Return the panel's contract: a { entries: [...] } envelope whose
items are camelCase {provider, affectedAgents, hitAt, resumeAt,
retryAfterSeconds}, derived from the raw Redis state (resumeAt = hitAt +
retryAfter).
- The rate-limit websocket hook passed "/ws/system" while getWebSocketUrl()
already supplies the "/ws" base, producing the doubled "/ws/ws/system" URL.
Pass "/system" to match the agents/channels/notifications hooks.
Note: the backend /ws/system endpoint itself does not yet exist (the rework
shipped the panel hook only); the REST fix keeps the banner correct on load
and reconnect until that endpoint is built.
* test(workspace): assert uv sync installs the dev extra
Follow the workspace setup change: the dependency-install command is now
`uv sync --extra dev` so the agent workspace gets the lint/type/complexity
toolchain. Update the three assertions that pinned the old `uv sync`.
* feat(ws): add /ws/system stream and bridge rate-limit events to the panel
The rate-limit rework shipped the panel's websocket hook but no backend: there
was no /ws/system endpoint and nothing forwarded RATE_LIMIT_HIT/LIFTED to a
socket, so the banner got no live updates.
Build the missing half:
- ConnectionManager grows a system-wide connection set with connect_system /
broadcast_system, and disconnect() now clears it.
- A /ws/system websocket endpoint (operator stream, no per-agent keying) with
the same connected + ping/pong lifecycle as the other streams.
- websocket_bridge subscribes RATE_LIMIT_HIT/LIFTED and forwards each to
broadcast_system tagged with the type the panel switches on. Both events
ride the same StreamEventBus singleton, and the subscriptions register
before start_listening(), so the consumer reads their streams.
Pairs with the panel hook now passing '/system' (getWebSocketUrl supplies the
'/ws' base). Covered by handler, manager, and endpoint-lifecycle tests.
---------
Co-authored-by: Renn F <rennf93@users.noreply.github.com>
334 lines
12 KiB
Python
334 lines
12 KiB
Python
"""websocket_bridge coverage — event handlers + bridge starter.
|
|
|
|
The handlers fan events from the Redis-stream bus to per-recipient WebSocket
|
|
connections. We don't need real Redis or sockets; we patch `manager` and the
|
|
`broadcast_*` helpers so each handler exercises its branches against
|
|
in-memory state.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
from unittest.mock import AsyncMock, patch
|
|
from uuid import uuid4
|
|
|
|
import pytest
|
|
from roboco.api.websocket_bridge import (
|
|
_handle_agent_event,
|
|
_handle_notification_sent,
|
|
_handle_rate_limit_event,
|
|
_handle_session_event,
|
|
register_websocket_bridge_handlers,
|
|
start_websocket_bridge,
|
|
)
|
|
from roboco.models.events import Event, EventType
|
|
|
|
|
|
def _evt(event_type: EventType, data: dict, source_agent: str | None = None) -> Event:
|
|
return Event(type=event_type, data=data, source_agent=source_agent)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _handle_notification_sent
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_notification_sent_skips_when_missing_ids() -> None:
|
|
"""Incomplete event (missing recipient/notification IDs) → log + return."""
|
|
event = _evt(EventType.NOTIFICATION_SENT, {}) # No notification_id/recipient_id
|
|
with patch("roboco.api.websocket_bridge.broadcast_notification") as bcast:
|
|
await _handle_notification_sent(event)
|
|
bcast.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_notification_sent_skips_invalid_uuid() -> None:
|
|
"""Invalid UUID strings → log error + return without broadcasting."""
|
|
event = _evt(
|
|
EventType.NOTIFICATION_SENT,
|
|
{"notification_id": "not-a-uuid", "recipient_id": str(uuid4())},
|
|
)
|
|
with patch("roboco.api.websocket_bridge.broadcast_notification") as bcast:
|
|
await _handle_notification_sent(event)
|
|
bcast.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_notification_sent_skips_when_no_connections() -> None:
|
|
"""Recipient has no WS connections → no broadcast."""
|
|
nid = uuid4()
|
|
rid = uuid4()
|
|
event = _evt(
|
|
EventType.NOTIFICATION_SENT,
|
|
{
|
|
"notification_id": str(nid),
|
|
"recipient_id": str(rid),
|
|
"type": "blocker",
|
|
"subject": "x",
|
|
"priority": "high",
|
|
},
|
|
)
|
|
with (
|
|
patch("roboco.api.websocket_bridge.broadcast_notification") as bcast,
|
|
patch("roboco.api.websocket_bridge.manager") as mgr,
|
|
):
|
|
mgr.notification_connections = {} # No connections for any agent.
|
|
await _handle_notification_sent(event)
|
|
bcast.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_notification_sent_broadcasts_when_connected() -> None:
|
|
"""Recipient has WS connection → broadcast_notification called."""
|
|
nid = uuid4()
|
|
rid = uuid4()
|
|
event = _evt(
|
|
EventType.NOTIFICATION_SENT,
|
|
{
|
|
"notification_id": str(nid),
|
|
"recipient_id": str(rid),
|
|
"type": "qa_ready",
|
|
"subject": "Task ready",
|
|
"priority": "normal",
|
|
},
|
|
)
|
|
bcast = AsyncMock()
|
|
with (
|
|
patch("roboco.api.websocket_bridge.broadcast_notification", bcast),
|
|
patch("roboco.api.websocket_bridge.manager") as mgr,
|
|
):
|
|
mgr.notification_connections = {rid: {"socket-1"}} # Has a connection.
|
|
await _handle_notification_sent(event)
|
|
bcast.assert_awaited_once()
|
|
call_kwargs = bcast.await_args.kwargs
|
|
assert call_kwargs["notification_id"] == nid
|
|
assert call_kwargs["agent_ids"] == [rid]
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_notification_acked_broadcasts_using_agent_id() -> None:
|
|
"""ACKED events carry `agent_id`, not `recipient_id`; the shared handler
|
|
must still forward (to the acking agent) rather than log 'Incomplete
|
|
notification event' on every acknowledgement."""
|
|
nid = uuid4()
|
|
aid = uuid4()
|
|
event = _evt(
|
|
EventType.NOTIFICATION_ACKED,
|
|
{"notification_id": str(nid), "agent_id": str(aid), "ack_type": "read"},
|
|
)
|
|
bcast = AsyncMock()
|
|
with (
|
|
patch("roboco.api.websocket_bridge.broadcast_notification", bcast),
|
|
patch("roboco.api.websocket_bridge.manager") as mgr,
|
|
):
|
|
mgr.notification_connections = {aid: {"socket-1"}}
|
|
await _handle_notification_sent(event)
|
|
bcast.assert_awaited_once()
|
|
call_kwargs = bcast.await_args.kwargs
|
|
assert call_kwargs["notification_id"] == nid
|
|
assert call_kwargs["agent_ids"] == [aid]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _handle_session_event
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_session_event_skips_missing_session_id() -> None:
|
|
event = _evt(EventType.SESSION_CREATED, {})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_to_session = AsyncMock()
|
|
await _handle_session_event(event)
|
|
mgr.broadcast_to_session.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_session_event_skips_invalid_uuid() -> None:
|
|
event = _evt(EventType.SESSION_CREATED, {"session_id": "bad-uuid"})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_to_session = AsyncMock()
|
|
await _handle_session_event(event)
|
|
mgr.broadcast_to_session.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_session_event_skips_when_no_connections() -> None:
|
|
sid = uuid4()
|
|
event = _evt(EventType.SESSION_CREATED, {"session_id": str(sid)})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.session_connections = {}
|
|
mgr.broadcast_to_session = AsyncMock()
|
|
await _handle_session_event(event)
|
|
mgr.broadcast_to_session.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_session_event_broadcasts() -> None:
|
|
sid = uuid4()
|
|
event = _evt(EventType.SESSION_CLOSED, {"session_id": str(sid)})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.session_connections = {sid: {"sock-1"}}
|
|
mgr.broadcast_to_session = AsyncMock()
|
|
await _handle_session_event(event)
|
|
mgr.broadcast_to_session.assert_awaited_once()
|
|
# Payload includes the trailing piece of the event-type ('closed').
|
|
call_args = mgr.broadcast_to_session.await_args
|
|
assert call_args.args[0] == sid
|
|
assert call_args.args[1]["type"] == "session.closed"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _handle_agent_event
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_agent_event_skips_when_no_agent_id() -> None:
|
|
event = _evt(EventType.AGENT_SPAWNED, {})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_to_agent_watchers = AsyncMock()
|
|
await _handle_agent_event(event)
|
|
mgr.broadcast_to_agent_watchers.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_agent_event_skips_invalid_uuid() -> None:
|
|
event = _evt(EventType.AGENT_SPAWNED, {"agent_id": "bad"})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_to_agent_watchers = AsyncMock()
|
|
await _handle_agent_event(event)
|
|
mgr.broadcast_to_agent_watchers.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_agent_event_uses_source_agent_fallback() -> None:
|
|
"""When data has no agent_id, falls back to event.source_agent."""
|
|
aid = uuid4()
|
|
event = _evt(EventType.AGENT_STOPPED, {}, source_agent=str(aid))
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.agent_connections = {aid: {"sock"}}
|
|
mgr.broadcast_to_agent_watchers = AsyncMock()
|
|
await _handle_agent_event(event)
|
|
mgr.broadcast_to_agent_watchers.assert_awaited_once()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_agent_event_skips_when_no_connections() -> None:
|
|
aid = uuid4()
|
|
event = _evt(EventType.AGENT_SPAWNED, {"agent_id": str(aid)})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.agent_connections = {}
|
|
mgr.broadcast_to_agent_watchers = AsyncMock()
|
|
await _handle_agent_event(event)
|
|
mgr.broadcast_to_agent_watchers.assert_not_called()
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_agent_event_broadcasts() -> None:
|
|
aid = uuid4()
|
|
event = _evt(EventType.AGENT_RESUMED, {"agent_id": str(aid)})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.agent_connections = {aid: {"sock"}}
|
|
mgr.broadcast_to_agent_watchers = AsyncMock()
|
|
await _handle_agent_event(event)
|
|
mgr.broadcast_to_agent_watchers.assert_awaited_once()
|
|
call_args = mgr.broadcast_to_agent_watchers.await_args
|
|
assert call_args.args[0] == aid
|
|
assert call_args.args[1]["type"] == "agent.resumed"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _handle_rate_limit_event
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_rate_limit_hit_broadcasts_to_system() -> None:
|
|
"""RATE_LIMIT_HIT → broadcast_system tagged with the type, payload intact."""
|
|
retry_after = 60.0
|
|
event = _evt(
|
|
EventType.RATE_LIMIT_HIT,
|
|
{
|
|
"provider": "anthropic",
|
|
"affectedAgents": ["be-dev-1"],
|
|
"retryAfterSeconds": retry_after,
|
|
"timestamp": "2026-06-11T00:00:00+00:00",
|
|
},
|
|
)
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_system = AsyncMock()
|
|
await _handle_rate_limit_event(event)
|
|
mgr.broadcast_system.assert_awaited_once()
|
|
msg = mgr.broadcast_system.await_args.args[0]
|
|
assert msg["type"] == "RATE_LIMIT_HIT"
|
|
assert msg["provider"] == "anthropic"
|
|
assert msg["affectedAgents"] == ["be-dev-1"]
|
|
assert msg["retryAfterSeconds"] == retry_after
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_rate_limit_lifted_broadcasts_to_system() -> None:
|
|
event = _evt(
|
|
EventType.RATE_LIMIT_LIFTED,
|
|
{"provider": "anthropic", "timestamp": "2026-06-11T00:01:00+00:00"},
|
|
)
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_system = AsyncMock()
|
|
await _handle_rate_limit_event(event)
|
|
msg = mgr.broadcast_system.await_args.args[0]
|
|
assert msg["type"] == "RATE_LIMIT_LIFTED"
|
|
assert msg["provider"] == "anthropic"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_handle_rate_limit_ignores_unrelated_event() -> None:
|
|
"""A non-rate-limit event type is a no-op (defensive guard)."""
|
|
event = _evt(EventType.AGENT_SPAWNED, {"provider": "anthropic"})
|
|
with patch("roboco.api.websocket_bridge.manager") as mgr:
|
|
mgr.broadcast_system = AsyncMock()
|
|
await _handle_rate_limit_event(event)
|
|
mgr.broadcast_system.assert_not_called()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Registration + start
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def test_register_websocket_bridge_handlers_subscribes_all_event_types() -> None:
|
|
"""Registration wires up notification + session + agent event handlers."""
|
|
|
|
class _FakeBus:
|
|
def __init__(self) -> None:
|
|
self.subscribed: list[tuple[EventType, object]] = []
|
|
|
|
def subscribe(self, event_type: EventType, handler: object) -> None:
|
|
self.subscribed.append((event_type, handler))
|
|
|
|
fake = _FakeBus()
|
|
with patch("roboco.api.websocket_bridge.get_event_bus", return_value=fake):
|
|
register_websocket_bridge_handlers()
|
|
types = [t for t, _ in fake.subscribed]
|
|
# All 10 expected event types appear at least once.
|
|
assert EventType.NOTIFICATION_SENT in types
|
|
assert EventType.NOTIFICATION_ACKED in types
|
|
assert EventType.SESSION_CREATED in types
|
|
assert EventType.SESSION_CLOSED in types
|
|
assert EventType.SESSION_TIMEOUT in types
|
|
assert EventType.AGENT_SPAWNED in types
|
|
assert EventType.AGENT_STOPPED in types
|
|
assert EventType.AGENT_WAITING in types
|
|
assert EventType.AGENT_RESUMED in types
|
|
assert EventType.AGENT_ERROR in types
|
|
assert EventType.RATE_LIMIT_HIT in types
|
|
assert EventType.RATE_LIMIT_LIFTED in types
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_start_websocket_bridge_registers_handlers() -> None:
|
|
"""start_websocket_bridge() calls register_websocket_bridge_handlers."""
|
|
with patch("roboco.api.websocket_bridge.register_websocket_bridge_handlers") as reg:
|
|
await start_websocket_bridge()
|
|
reg.assert_called_once()
|