Files
roboco/tests/unit/services/test_workspace_dep_install.py
T
303c2db289 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>
2026-06-11 18:16:20 +02:00

309 lines
10 KiB
Python

"""Unit tests for the post-clone dev-dependency install (issue #10).
Per-agent workspace clones never had the project's dev dependencies
installed, so the `make quality` gate (ruff/mypy/pytest for Python, the TS
toolchain for the panel) was missing and devs re-downloaded tooling per
task. `WorkspaceService.install_dev_deps` now runs `uv sync` / `pnpm install`
after cloning, idempotently (skipped when lockfiles are unchanged).
These tests cover the pure detection/digest helpers and the install method's
ecosystem detection, idempotency, and best-effort failure handling. They run
without a DB or a real git remote.
"""
from __future__ import annotations
import subprocess
from typing import TYPE_CHECKING
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from roboco.services.workspace import (
_DEP_INSTALL_MARKER,
WorkspaceService,
_detect_dep_commands,
_lockfile_digest,
)
if TYPE_CHECKING:
from pathlib import Path
# Two installs expected when the lockfile changes between calls (named to
# satisfy ruff PLR2004 — magic-value comparison).
_EXPECTED_RERUN_INSTALLS = 2
def _service() -> WorkspaceService:
"""Build a WorkspaceService over a MagicMock session."""
session = MagicMock()
session.execute = AsyncMock()
return WorkspaceService(session)
def _make_workspace(tmp_path: Path) -> Path:
"""A workspace dir with a `.git/` so the marker has somewhere to live."""
workspace = tmp_path / "roboco" / "backend" / "be-dev-1"
(workspace / ".git").mkdir(parents=True)
return workspace
# ---------------------------------------------------------------------------
# _detect_dep_commands
# ---------------------------------------------------------------------------
def test_detect_python_project(tmp_path: Path) -> None:
"""A `pyproject.toml` yields a `uv sync` command."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
commands = _detect_dep_commands(ws)
assert commands == [("uv sync --extra dev", ["uv", "sync", "--extra", "dev"])]
def test_detect_pnpm_project(tmp_path: Path) -> None:
"""A `pnpm-lock.yaml` yields a frozen-lockfile pnpm install."""
ws = _make_workspace(tmp_path)
(ws / "package.json").write_text("{}")
(ws / "pnpm-lock.yaml").write_text("lockfileVersion: 9\n")
commands = _detect_dep_commands(ws)
assert commands == [("pnpm install", ["pnpm", "install", "--frozen-lockfile"])]
def test_detect_npm_ci_when_package_lock(tmp_path: Path) -> None:
"""`package-lock.json` (no pnpm lock) yields `npm ci`."""
ws = _make_workspace(tmp_path)
(ws / "package.json").write_text("{}")
(ws / "package-lock.json").write_text("{}")
commands = _detect_dep_commands(ws)
assert commands == [("npm ci", ["npm", "ci"])]
def test_detect_npm_install_bare_package_json(tmp_path: Path) -> None:
"""A bare `package.json` (no lockfile) falls back to `npm install`."""
ws = _make_workspace(tmp_path)
(ws / "package.json").write_text("{}")
commands = _detect_dep_commands(ws)
assert commands == [("npm install", ["npm", "install"])]
def test_detect_monorepo_both_ecosystems(tmp_path: Path) -> None:
"""A Python + pnpm monorepo yields both install commands."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
(ws / "package.json").write_text("{}")
(ws / "pnpm-lock.yaml").write_text("lockfileVersion: 9\n")
commands = _detect_dep_commands(ws)
assert ("uv sync --extra dev", ["uv", "sync", "--extra", "dev"]) in commands
assert ("pnpm install", ["pnpm", "install", "--frozen-lockfile"]) in commands
def test_detect_nothing_to_install(tmp_path: Path) -> None:
"""A repo with no recognized manifest yields no commands."""
ws = _make_workspace(tmp_path)
assert _detect_dep_commands(ws) == []
# ---------------------------------------------------------------------------
# _lockfile_digest
# ---------------------------------------------------------------------------
def test_lockfile_digest_none_when_no_lockfiles(tmp_path: Path) -> None:
"""No manifests → None (nothing to hash, nothing to install)."""
ws = _make_workspace(tmp_path)
assert _lockfile_digest(ws) is None
def test_lockfile_digest_changes_with_content(tmp_path: Path) -> None:
"""Editing a lockfile changes the digest (so a re-install is triggered)."""
ws = _make_workspace(tmp_path)
lock = ws / "uv.lock"
lock.write_text("a = 1\n")
digest_a = _lockfile_digest(ws)
lock.write_text("a = 2\n")
digest_b = _lockfile_digest(ws)
assert digest_a is not None
assert digest_b is not None
assert digest_a != digest_b
# ---------------------------------------------------------------------------
# install_dev_deps
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_install_runs_detected_command(tmp_path: Path) -> None:
"""A Python workspace runs `uv sync` and writes the digest marker."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
(ws / "uv.lock").write_text("version = 1\n")
svc = _service()
captured: list[list[str]] = []
def _fake_run(argv: list[str], **_kw: object) -> subprocess.CompletedProcess[str]:
captured.append(argv)
return subprocess.CompletedProcess(argv, returncode=0, stdout="", stderr="")
with (
patch("roboco.services.workspace.subprocess.run", side_effect=_fake_run),
patch("roboco.services.workspace._ensure_agent_owned"),
):
ran = await svc.install_dev_deps(ws)
assert ran is True
assert ["uv", "sync", "--extra", "dev"] in captured
assert (ws / _DEP_INSTALL_MARKER).is_file()
@pytest.mark.asyncio
async def test_install_idempotent_on_unchanged_lockfiles(tmp_path: Path) -> None:
"""A second call with the same lockfiles is a no-op (cache hit)."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
(ws / "uv.lock").write_text("version = 1\n")
svc = _service()
run_count = 0
def _fake_run(argv: list[str], **_kw: object) -> subprocess.CompletedProcess[str]:
nonlocal run_count
run_count += 1
return subprocess.CompletedProcess(argv, returncode=0, stdout="", stderr="")
with (
patch("roboco.services.workspace.subprocess.run", side_effect=_fake_run),
patch("roboco.services.workspace._ensure_agent_owned"),
):
first = await svc.install_dev_deps(ws)
second = await svc.install_dev_deps(ws)
assert first is True
assert second is False
assert run_count == 1
@pytest.mark.asyncio
async def test_install_reruns_when_lockfile_changes(tmp_path: Path) -> None:
"""Changing the lockfile invalidates the marker and re-installs."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
lock = ws / "uv.lock"
lock.write_text("version = 1\n")
svc = _service()
run_count = 0
def _fake_run(argv: list[str], **_kw: object) -> subprocess.CompletedProcess[str]:
nonlocal run_count
run_count += 1
return subprocess.CompletedProcess(argv, returncode=0, stdout="", stderr="")
with (
patch("roboco.services.workspace.subprocess.run", side_effect=_fake_run),
patch("roboco.services.workspace._ensure_agent_owned"),
):
await svc.install_dev_deps(ws)
lock.write_text("version = 2\n")
await svc.install_dev_deps(ws)
assert run_count == _EXPECTED_RERUN_INSTALLS
@pytest.mark.asyncio
async def test_install_failure_is_best_effort(tmp_path: Path) -> None:
"""A non-zero install exit logs but does NOT raise, and writes no marker."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
(ws / "uv.lock").write_text("version = 1\n")
svc = _service()
def _fake_run(argv: list[str], **_kw: object) -> subprocess.CompletedProcess[str]:
return subprocess.CompletedProcess(argv, returncode=1, stdout="", stderr="boom")
with (
patch("roboco.services.workspace.subprocess.run", side_effect=_fake_run),
patch("roboco.services.workspace._ensure_agent_owned"),
):
ran = await svc.install_dev_deps(ws)
assert ran is False
# No marker on failure → next call retries.
assert not (ws / _DEP_INSTALL_MARKER).is_file()
@pytest.mark.asyncio
async def test_install_missing_tool_does_not_raise(tmp_path: Path) -> None:
"""A missing `uv`/`pnpm` on the host is swallowed (FileNotFoundError)."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
(ws / "uv.lock").write_text("version = 1\n")
svc = _service()
def _fake_run(*_a: object, **_kw: object) -> subprocess.CompletedProcess[str]:
raise FileNotFoundError("uv not found")
with (
patch("roboco.services.workspace.subprocess.run", side_effect=_fake_run),
patch("roboco.services.workspace._ensure_agent_owned"),
):
ran = await svc.install_dev_deps(ws)
assert ran is False
@pytest.mark.asyncio
async def test_install_skipped_when_disabled(tmp_path: Path) -> None:
"""`workspace_install_dev_deps=False` short-circuits before any subprocess."""
ws = _make_workspace(tmp_path)
(ws / "pyproject.toml").write_text("[project]\nname = 'x'\n")
(ws / "uv.lock").write_text("version = 1\n")
svc = _service()
with (
patch(
"roboco.services.workspace.settings.workspace_install_dev_deps",
False,
),
patch("roboco.services.workspace.subprocess.run") as run_mock,
patch("roboco.services.workspace._ensure_agent_owned"),
):
ran = await svc.install_dev_deps(ws)
assert ran is False
run_mock.assert_not_called()
@pytest.mark.asyncio
async def test_install_noop_when_no_manifest(tmp_path: Path) -> None:
"""A workspace with no recognized manifest installs nothing."""
ws = _make_workspace(tmp_path)
svc = _service()
with (
patch("roboco.services.workspace.subprocess.run") as run_mock,
patch("roboco.services.workspace._ensure_agent_owned"),
):
ran = await svc.install_dev_deps(ws)
assert ran is False
run_mock.assert_not_called()