mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
Fix: rate limit real probe (#110)
* 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>
This commit is contained in:
@@ -15,6 +15,7 @@ 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,
|
||||
@@ -237,6 +238,59 @@ async def test_handle_agent_event_broadcasts() -> None:
|
||||
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
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -267,6 +321,8 @@ def test_register_websocket_bridge_handlers_subscribes_all_event_types() -> None
|
||||
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
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
"""Operator/system WebSocket stream — ConnectionManager + the /ws/system endpoint.
|
||||
|
||||
The system stream carries system-wide events (rate limits) to the panel with no
|
||||
per-agent keying. These tests exercise the manager's system-connection
|
||||
bookkeeping and the endpoint's connect → ping/pong → disconnect lifecycle
|
||||
against mock sockets (no real app/lifespan/Redis).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
from fastapi import WebSocketDisconnect
|
||||
from roboco.api.websocket import ConnectionManager, manager, system_stream
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_system_accepts_and_tracks() -> None:
|
||||
mgr = ConnectionManager()
|
||||
ws = MagicMock()
|
||||
ws.accept = AsyncMock()
|
||||
|
||||
await mgr.connect_system(ws)
|
||||
|
||||
ws.accept.assert_awaited_once()
|
||||
assert ws in mgr.system_connections
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_broadcast_system_sends_to_every_connection() -> None:
|
||||
mgr = ConnectionManager()
|
||||
ws1, ws2 = MagicMock(), MagicMock()
|
||||
ws1.send_text = AsyncMock()
|
||||
ws2.send_text = AsyncMock()
|
||||
mgr.system_connections = {ws1, ws2}
|
||||
|
||||
await mgr.broadcast_system({"type": "RATE_LIMIT_HIT", "provider": "anthropic"})
|
||||
|
||||
ws1.send_text.assert_awaited_once()
|
||||
ws2.send_text.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_broadcast_system_is_noop_when_empty() -> None:
|
||||
mgr = ConnectionManager()
|
||||
# No subscribers — must not raise.
|
||||
await mgr.broadcast_system({"type": "x"})
|
||||
|
||||
|
||||
def test_disconnect_removes_from_system() -> None:
|
||||
mgr = ConnectionManager()
|
||||
ws = MagicMock()
|
||||
mgr.system_connections.add(ws)
|
||||
|
||||
mgr.disconnect(ws)
|
||||
|
||||
assert ws not in mgr.system_connections
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_system_stream_connect_ping_disconnect() -> None:
|
||||
"""Endpoint sends 'connected', answers ping with pong, cleans up on close."""
|
||||
ws = MagicMock()
|
||||
ws.accept = AsyncMock()
|
||||
ws.send_json = AsyncMock()
|
||||
ws.send_text = AsyncMock()
|
||||
# One ping, then the client disconnects.
|
||||
ws.receive_text = AsyncMock(side_effect=["ping", WebSocketDisconnect()])
|
||||
|
||||
await system_stream(ws)
|
||||
|
||||
ws.accept.assert_awaited_once()
|
||||
ws.send_json.assert_awaited_once_with({"type": "connected"})
|
||||
ws.send_text.assert_awaited_with("pong")
|
||||
# Disconnect handler removed the socket from the global manager.
|
||||
assert ws not in manager.system_connections
|
||||
@@ -1,17 +1,16 @@
|
||||
"""Unit tests for the rate-limited path in Choreographer.i_am_blocked.
|
||||
|
||||
Acceptance criteria verified here:
|
||||
- AC1: i_am_blocked(reason='rate_limited') calls RateLimitStateTracker.activate()
|
||||
and stores affected agent IDs; all active agents on the rate-limited
|
||||
provider are subsequently marked waiting-long.
|
||||
- AC3: POST /v1/i_am_blocked with reason='rate_limited' does NOT transition
|
||||
the task to 'blocked'; the task remains in its current status
|
||||
(in_progress) and the calling agent is parked via
|
||||
mark_waiting_long(waiting_for='rate_limit_lifted').
|
||||
- AC4: mark_waiting_long is called for every orchestrator-tracked active agent
|
||||
sharing the affected provider — call count equals active agent count.
|
||||
- AC5: A RATE_LIMIT_HIT event is published to the StreamEventBus with fields
|
||||
provider, affectedAgents, retryAfterSeconds, and timestamp.
|
||||
Behaviours verified here:
|
||||
- i_am_blocked(reason='rate_limited') calls RateLimitStateTracker.activate()
|
||||
and stores affected agent IDs; all active agents on the rate-limited
|
||||
provider are subsequently marked waiting-long.
|
||||
- POST /v1/i_am_blocked with reason='rate_limited' does NOT transition the
|
||||
task to 'blocked'; the task remains in its current status (in_progress) and
|
||||
the calling agent is parked via mark_waiting_long(waiting_for='rate_limit_lifted').
|
||||
- mark_waiting_long is called for every orchestrator-tracked active agent
|
||||
sharing the affected provider — call count equals active agent count.
|
||||
- A RATE_LIMIT_HIT event is published to the StreamEventBus with fields
|
||||
provider, affectedAgents, retryAfterSeconds, and timestamp.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -116,7 +115,7 @@ def _make_deps(
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AC3: Task stays in in_progress, agent parked via mark_waiting_long
|
||||
# Task stays in in_progress, agent parked via mark_waiting_long
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -191,7 +190,7 @@ class TestRateLimitedDoesNotBlockTask:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AC4: mark_waiting_long called for every active agent on affected provider
|
||||
# mark_waiting_long called for every active agent on affected provider
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -261,7 +260,7 @@ class TestMarkWaitingLongCallCount:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AC5: RATE_LIMIT_HIT event published with correct payload structure
|
||||
# RATE_LIMIT_HIT event published with correct payload structure
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -382,7 +381,7 @@ class TestRateLimitHitEventPublished:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# AC1: RateLimitStateTracker.activate() called on rate_limited path
|
||||
# RateLimitStateTracker.activate() called on rate_limited path
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_TRACKER_PATCH = "roboco.services.gateway.rate_limit_tracker.RateLimitStateTracker"
|
||||
|
||||
@@ -0,0 +1,138 @@
|
||||
"""Rate-limit recovery probe — real per-provider liveness check.
|
||||
|
||||
``_do_probe`` replaced a time-based stub that always returned True. It now
|
||||
makes a free, unmetered call (Anthropic ``GET /v1/models`` / Ollama
|
||||
``GET /api/tags``) and treats any non-429 response as the rate limit having
|
||||
lifted. These tests pin that contract: target resolution per provider, the
|
||||
429-vs-not decision, network-error → stay-parked, and the un-probeable
|
||||
fallback to time-expiry optimism.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from roboco.config import settings
|
||||
from roboco.runtime.orchestrator import (
|
||||
_HTTP_TOO_MANY_REQUESTS,
|
||||
AgentOrchestrator,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Iterator
|
||||
|
||||
_HTTP_OK = 200
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def orch() -> AgentOrchestrator:
|
||||
return AgentOrchestrator.__new__(AgentOrchestrator)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def with_anthropic_key() -> Iterator[None]:
|
||||
original = settings.anthropic_api_key
|
||||
settings.anthropic_api_key = "sk-test-key"
|
||||
yield
|
||||
settings.anthropic_api_key = original
|
||||
|
||||
|
||||
def _fake_async_client(
|
||||
*, status_code: int | None = None, raise_exc: Exception | None = None
|
||||
) -> MagicMock:
|
||||
"""Patch target for ``httpx.AsyncClient`` — async ctx mgr whose get() responds."""
|
||||
response = MagicMock()
|
||||
response.status_code = status_code
|
||||
client = MagicMock()
|
||||
client.__aenter__ = AsyncMock(return_value=client)
|
||||
client.__aexit__ = AsyncMock(return_value=False)
|
||||
client.get = (
|
||||
AsyncMock(side_effect=raise_exc)
|
||||
if raise_exc is not None
|
||||
else AsyncMock(return_value=response)
|
||||
)
|
||||
return MagicMock(return_value=client)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _probe_target — provider → (url, headers)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("with_anthropic_key")
|
||||
def test_target_anthropic_with_key() -> None:
|
||||
url, headers = AgentOrchestrator._probe_target("anthropic")
|
||||
assert url is not None
|
||||
assert url.endswith("/v1/models")
|
||||
assert headers["x-api-key"] == "sk-test-key"
|
||||
assert "anthropic-version" in headers
|
||||
|
||||
|
||||
def test_target_anthropic_without_key() -> None:
|
||||
original = settings.anthropic_api_key
|
||||
settings.anthropic_api_key = None
|
||||
try:
|
||||
url, headers = AgentOrchestrator._probe_target("anthropic")
|
||||
finally:
|
||||
settings.anthropic_api_key = original
|
||||
assert url is None
|
||||
assert headers == {}
|
||||
|
||||
|
||||
def test_target_ollama_uses_tags_endpoint() -> None:
|
||||
url, _headers = AgentOrchestrator._probe_target("ollama_cloud")
|
||||
assert url is not None
|
||||
assert url.endswith("/api/tags")
|
||||
|
||||
|
||||
def test_target_unknown_provider_is_unprobeable() -> None:
|
||||
assert AgentOrchestrator._probe_target("mystery") == (None, {})
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _do_probe — the liveness decision
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("with_anthropic_key")
|
||||
async def test_probe_anthropic_ok_is_lifted(orch: AgentOrchestrator) -> None:
|
||||
fake = _fake_async_client(status_code=_HTTP_OK)
|
||||
with patch("roboco.runtime.orchestrator.httpx.AsyncClient", fake):
|
||||
assert await orch._do_probe("anthropic") is True
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("with_anthropic_key")
|
||||
async def test_probe_anthropic_429_stays_limited(orch: AgentOrchestrator) -> None:
|
||||
fake = _fake_async_client(status_code=_HTTP_TOO_MANY_REQUESTS)
|
||||
with patch("roboco.runtime.orchestrator.httpx.AsyncClient", fake):
|
||||
assert await orch._do_probe("anthropic") is False
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("with_anthropic_key")
|
||||
async def test_probe_network_error_stays_parked(orch: AgentOrchestrator) -> None:
|
||||
fake = _fake_async_client(raise_exc=httpx.ConnectError("boom"))
|
||||
with patch("roboco.runtime.orchestrator.httpx.AsyncClient", fake):
|
||||
assert await orch._do_probe("anthropic") is False
|
||||
|
||||
|
||||
async def test_probe_ollama_ok_is_lifted(orch: AgentOrchestrator) -> None:
|
||||
fake = _fake_async_client(status_code=_HTTP_OK)
|
||||
with patch("roboco.runtime.orchestrator.httpx.AsyncClient", fake):
|
||||
assert await orch._do_probe("ollama_cloud") is True
|
||||
|
||||
|
||||
async def test_probe_unprobeable_falls_back_to_optimism(
|
||||
orch: AgentOrchestrator,
|
||||
) -> None:
|
||||
"""No key / unknown provider → trust the elapsed retry_after window, no HTTP."""
|
||||
original = settings.anthropic_api_key
|
||||
settings.anthropic_api_key = None
|
||||
try:
|
||||
with patch("roboco.runtime.orchestrator.httpx.AsyncClient") as client_cls:
|
||||
assert await orch._do_probe("anthropic") is True
|
||||
client_cls.assert_not_called()
|
||||
finally:
|
||||
settings.anthropic_api_key = original
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Unit tests for the rate-limit sweeper probe loop (AC4, AC8).
|
||||
"""Unit tests for the rate-limit sweeper probe loop.
|
||||
|
||||
Tests cover:
|
||||
- probe-success path: tracker.clear() + resolve_wait + RATE_LIMIT_LIFTED event
|
||||
@@ -23,6 +23,9 @@ from roboco.models.runtime import WaitingRecord
|
||||
from roboco.runtime.orchestrator import AgentOrchestrator
|
||||
from roboco.services.gateway.rate_limit_tracker import RateLimitStateTracker
|
||||
|
||||
_HTTP_OK = 200
|
||||
_HTTP_NOT_FOUND = 404
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -45,7 +48,9 @@ def _make_redis_mock(initial_store: dict[str, Any] | None = None) -> AsyncMock:
|
||||
return 1 if store.pop(key, None) is not None else 0
|
||||
|
||||
async def _scan(
|
||||
_cursor: int, match: str = "*", count: int = 100 # noqa: ARG001
|
||||
_cursor: int,
|
||||
match: str = "*",
|
||||
count: int = 100, # noqa: ARG001
|
||||
) -> tuple[int, list[bytes]]:
|
||||
# Simple in-memory scan: return all matching keys in one shot
|
||||
matches = [k.encode() for k in store if fnmatch.fnmatch(k, match)]
|
||||
@@ -60,6 +65,10 @@ def _make_redis_mock(initial_store: dict[str, Any] | None = None) -> AsyncMock:
|
||||
mock.delete = AsyncMock(side_effect=_delete)
|
||||
mock.scan = AsyncMock(side_effect=_scan)
|
||||
mock.aclose = AsyncMock(side_effect=_aclose)
|
||||
# Support `async with redis.from_url(...) as r:` — the client returns
|
||||
# itself on enter so the configured side-effects are what the caller uses.
|
||||
mock.__aenter__ = AsyncMock(return_value=mock)
|
||||
mock.__aexit__ = AsyncMock(return_value=False)
|
||||
mock._store = store
|
||||
return mock
|
||||
|
||||
@@ -115,7 +124,7 @@ def _waiting_record(
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests: probe-success path (AC4)
|
||||
# Tests: probe-success path
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -268,7 +277,7 @@ class TestProbeSuccessPath:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests: probe-failure path (AC4)
|
||||
# Tests: probe-failure path
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -315,7 +324,7 @@ class TestProbeFailurePath:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests: CEO notification threshold (AC8)
|
||||
# Tests: CEO notification threshold
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -477,7 +486,7 @@ class TestListRateLimitedProviders:
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests: GET /api/system/rate-limits endpoint schema (AC9)
|
||||
# Tests: GET /api/system/rate-limits endpoint schema
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -498,16 +507,17 @@ class TestRateLimitsEndpoint:
|
||||
) as client:
|
||||
resp = await client.get("/api/system/rate-limits")
|
||||
|
||||
assert resp.status_code == 200 # noqa: PLR2004
|
||||
assert resp.json() == []
|
||||
assert resp.status_code == _HTTP_OK
|
||||
assert resp.json() == {"entries": []}
|
||||
|
||||
async def test_returns_provider_state_when_rate_limited(self) -> None:
|
||||
app = create_app()
|
||||
|
||||
retry_after = 60.0
|
||||
state = {
|
||||
"rate_limited": True,
|
||||
"activated_at": "2026-06-11T00:00:00+00:00",
|
||||
"retry_after": 60.0,
|
||||
"retry_after": retry_after,
|
||||
"affected_agents": ["be-dev-1"],
|
||||
"probe_failures": 3,
|
||||
}
|
||||
@@ -523,14 +533,16 @@ class TestRateLimitsEndpoint:
|
||||
) as client:
|
||||
resp = await client.get("/api/system/rate-limits")
|
||||
|
||||
assert resp.status_code == 200 # noqa: PLR2004
|
||||
data = resp.json()
|
||||
assert len(data) == 1
|
||||
entry = data[0]
|
||||
assert resp.status_code == _HTTP_OK
|
||||
entries = resp.json()["entries"]
|
||||
assert len(entries) == 1
|
||||
entry = entries[0]
|
||||
# Panel-shaped, camelCase fields (not the raw Redis state).
|
||||
assert entry["provider"] == "anthropic"
|
||||
assert entry["rate_limited"] is True
|
||||
assert entry["probe_failures"] == 3 # noqa: PLR2004
|
||||
assert entry["retry_after"] == 60.0 # noqa: PLR2004
|
||||
assert entry["affectedAgents"] == ["be-dev-1"]
|
||||
assert entry["hitAt"] == "2026-06-11T00:00:00+00:00"
|
||||
assert entry["retryAfterSeconds"] == retry_after
|
||||
assert entry["resumeAt"] == "2026-06-11T00:01:00+00:00"
|
||||
|
||||
async def test_endpoint_not_404(self) -> None:
|
||||
"""The endpoint must be registered in app.py — no 404."""
|
||||
@@ -547,4 +559,4 @@ class TestRateLimitsEndpoint:
|
||||
) as client:
|
||||
resp = await client.get("/api/system/rate-limits")
|
||||
|
||||
assert resp.status_code != 404 # noqa: PLR2004
|
||||
assert resp.status_code != _HTTP_NOT_FOUND
|
||||
|
||||
@@ -637,7 +637,12 @@ async def test_confirm_live_draft_main_pm_route_assigns_main_pm(
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_confirm_live_draft_product_routes_to_main_pm(db_session: Any) -> None:
|
||||
"""A product-scoped live draft is a board-led coordination root (Main PM)."""
|
||||
"""A product-scoped draft via the "Approve & Start" path is a Main-PM root.
|
||||
|
||||
The board path (the ``route="board"`` default) keeps the root at
|
||||
``team=board`` until the CEO approves; the Main-PM path is selected
|
||||
explicitly with ``route="main_pm"``.
|
||||
"""
|
||||
_project_id, ceo_id = await _seed_project_and_ceo(db_session)
|
||||
product_id = uuid4()
|
||||
product = ProductTable(
|
||||
@@ -656,7 +661,9 @@ async def test_confirm_live_draft_product_routes_to_main_pm(db_session: Any) ->
|
||||
"acceptance_criteria": ["works end to end"],
|
||||
"team": "backend",
|
||||
}
|
||||
task_id = await service.confirm_live_draft(draft, ceo_id, product_id=product_id)
|
||||
task_id = await service.confirm_live_draft(
|
||||
draft, ceo_id, product_id=product_id, route="main_pm"
|
||||
)
|
||||
row = await db_session.get(TaskTable, task_id)
|
||||
assert row.team == Team.MAIN_PM
|
||||
assert row.product_id == product_id
|
||||
|
||||
@@ -9,15 +9,11 @@ visible to a fresh instance.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
from roboco.services.gateway.rate_limit_tracker import RateLimitStateTracker
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -93,9 +89,10 @@ class TestActivateAndRead:
|
||||
async def test_activate_stores_retry_after(self) -> None:
|
||||
mock = _make_redis_mock()
|
||||
tracker = _make_tracker(redis_mock=mock)
|
||||
await tracker.activate(retry_after=30.0)
|
||||
retry_after = 30.0
|
||||
await tracker.activate(retry_after=retry_after)
|
||||
state = await tracker.get_state()
|
||||
assert state["retry_after"] == 30.0
|
||||
assert state["retry_after"] == retry_after
|
||||
|
||||
async def test_activate_stores_affected_agents(self) -> None:
|
||||
mock = _make_redis_mock()
|
||||
@@ -132,10 +129,10 @@ class TestProbeFailures:
|
||||
mock = _make_redis_mock()
|
||||
tracker = _make_tracker(redis_mock=mock)
|
||||
await tracker.activate()
|
||||
await tracker.increment_probe_failures()
|
||||
await tracker.increment_probe_failures()
|
||||
count = await tracker.increment_probe_failures()
|
||||
assert count == 3
|
||||
increments = 3
|
||||
for _ in range(increments):
|
||||
count = await tracker.increment_probe_failures()
|
||||
assert count == increments
|
||||
|
||||
async def test_reset_sets_zero(self) -> None:
|
||||
mock = _make_redis_mock()
|
||||
@@ -152,10 +149,10 @@ class TestProbeFailures:
|
||||
# Tests: cross-reconnection persistence
|
||||
# ---------------------------------------------------------------------------
|
||||
#
|
||||
# AC2: "State persists across client reconnection: a test writes state via
|
||||
# State persists across client reconnection: a test writes state via
|
||||
# activate(), creates a new RateLimitStateTracker instance pointing at the
|
||||
# same Redis URL, calls is_rate_limited() and get_state() and gets back the
|
||||
# same values — proving state survives a process restart."
|
||||
# same values — proving state survives a process restart.
|
||||
#
|
||||
# We simulate this by sharing the same backing dict between two mock Redis
|
||||
# clients — one injected into the first tracker and one injected into the
|
||||
@@ -188,9 +185,10 @@ class TestStatePersistsAcrossReconnection:
|
||||
async def test_get_state_survives_reconnection(self) -> None:
|
||||
shared_store: dict[str, Any] = {}
|
||||
|
||||
retry_after = 45.0
|
||||
mock_a = _make_redis_mock(initial_store=shared_store)
|
||||
tracker_a = _make_tracker(provider="anthropic", redis_mock=mock_a)
|
||||
await tracker_a.activate(retry_after=45.0, affected_agents=["be-dev-2"])
|
||||
await tracker_a.activate(retry_after=retry_after, affected_agents=["be-dev-2"])
|
||||
|
||||
mock_b = _make_redis_mock(initial_store=mock_a._store)
|
||||
tracker_b = RateLimitStateTracker(
|
||||
@@ -200,7 +198,7 @@ class TestStatePersistsAcrossReconnection:
|
||||
|
||||
state = await tracker_b.get_state()
|
||||
assert state["rate_limited"] is True
|
||||
assert state["retry_after"] == 45.0
|
||||
assert state["retry_after"] == retry_after
|
||||
assert state["affected_agents"] == ["be-dev-2"]
|
||||
|
||||
async def test_clear_via_first_instance_visible_to_second(self) -> None:
|
||||
|
||||
@@ -59,7 +59,7 @@ def test_detect_python_project(tmp_path: Path) -> None:
|
||||
|
||||
commands = _detect_dep_commands(ws)
|
||||
|
||||
assert commands == [("uv sync", ["uv", "sync"])]
|
||||
assert commands == [("uv sync --extra dev", ["uv", "sync", "--extra", "dev"])]
|
||||
|
||||
|
||||
def test_detect_pnpm_project(tmp_path: Path) -> None:
|
||||
@@ -103,7 +103,7 @@ def test_detect_monorepo_both_ecosystems(tmp_path: Path) -> None:
|
||||
|
||||
commands = _detect_dep_commands(ws)
|
||||
|
||||
assert ("uv sync", ["uv", "sync"]) in commands
|
||||
assert ("uv sync --extra dev", ["uv", "sync", "--extra", "dev"]) in commands
|
||||
assert ("pnpm install", ["pnpm", "install", "--frozen-lockfile"]) in commands
|
||||
|
||||
|
||||
@@ -167,7 +167,7 @@ async def test_install_runs_detected_command(tmp_path: Path) -> None:
|
||||
ran = await svc.install_dev_deps(ws)
|
||||
|
||||
assert ran is True
|
||||
assert ["uv", "sync"] in captured
|
||||
assert ["uv", "sync", "--extra", "dev"] in captured
|
||||
assert (ws / _DEP_INSTALL_MARKER).is_file()
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user