Adam Moussa
75f1dc6f75
Gap-audit findings: - G1 (blocks-feature): make_cross_reviewer_invoker / make_fast_coder_invoker did 'from models import' without putting the orchestrator root on sys.path. The run-team serve daemon only bootstraps agent-team/, so on the live box every GPT-4.1 plan review hit ModuleNotFoundError -> review_plan's blanket except silently fail-closed to REQUEST_CHANGES (GPT-4.1 never actually ran). Both in-process invokers now call invoker_multi._ensure_orchestrator_on_path() before the deferred import. WS1 introduced this when it swapped the review default from the subprocess invoker to in-process. - G4 (degrades): the handbook context_provider was wired into the planner only; default_clarify_node_factory now accepts + forwards it, and run-team wires it into build_clarify_node too, so clarifying questions are handbook-aware. Tests: +2 regression tests (path-bootstrap, clarifier threading); _FakeCoordinator gains build_clarify_node. 1142 passed, ruff clean.
220 lines
8 KiB
Python
220 lines
8 KiB
Python
"""Activation-wiring tests (integration branch): prove the WS seams that the
|
|
``serve`` path flips ON are actually wired, without a live Slack socket.
|
|
|
|
Covers:
|
|
* run-team ``_build_context_provider`` returns the handbook loader (WS5 / D10).
|
|
* run-team ``_build_coordinator`` threads that context_provider into the planner
|
|
node factory.
|
|
* run-team ``_build_coordinator`` sets a ``/new-task`` callback that starts a
|
|
task on the SAME coordinator with transport_name="slack" (WS2).
|
|
* ``default_slack_listener_factory`` forwards ``new_task_callback`` to the
|
|
SlackListener (WS2).
|
|
* ``Coordinator`` stores ``new_task_callback`` / ``set_new_task_callback`` and
|
|
forwards it when it builds the default listener.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import importlib.util
|
|
import sys
|
|
from pathlib import Path
|
|
from types import SimpleNamespace
|
|
from typing import Any
|
|
from unittest.mock import MagicMock
|
|
|
|
|
|
_AGENT_TEAM_DIR = Path(__file__).resolve().parents[1]
|
|
if str(_AGENT_TEAM_DIR) not in sys.path:
|
|
sys.path.insert(0, str(_AGENT_TEAM_DIR))
|
|
|
|
|
|
def _load_run_team():
|
|
"""Import run-team.py (hyphenated, so loaded by path) as a module."""
|
|
cli_path = _AGENT_TEAM_DIR / "run-team.py"
|
|
spec = importlib.util.spec_from_file_location("run_team_cli", cli_path)
|
|
assert spec is not None and spec.loader is not None
|
|
module = importlib.util.module_from_spec(spec)
|
|
spec.loader.exec_module(module)
|
|
return module
|
|
|
|
|
|
def _dry_args(tmp_path: Path) -> SimpleNamespace:
|
|
return SimpleNamespace(
|
|
db=str(tmp_path / "agent_team.sqlite"),
|
|
transport="slack",
|
|
dry_run=True,
|
|
)
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# WS5: context_provider = handbook loader, threaded into the plan node
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_build_context_provider_returns_handbook_loader() -> None:
|
|
cli = _load_run_team()
|
|
provider = cli._build_context_provider()
|
|
assert callable(provider)
|
|
# Zero-arg and returns a string (fail-safe: "" when no handbook dir).
|
|
result = provider()
|
|
assert isinstance(result, str)
|
|
|
|
|
|
def test_build_coordinator_threads_context_provider_into_plan_node(
|
|
tmp_path: Path, monkeypatch: Any
|
|
) -> None:
|
|
cli = _load_run_team()
|
|
from agent_team import coordinator as coord_mod
|
|
|
|
captured: dict[str, Any] = {}
|
|
|
|
def _spy_plan_factory(context_provider: Any = None):
|
|
captured["context_provider"] = context_provider
|
|
return lambda state: state
|
|
|
|
monkeypatch.setattr(coord_mod, "default_plan_node_factory", _spy_plan_factory)
|
|
|
|
coordinator = cli._build_coordinator(_dry_args(tmp_path))
|
|
# The plan node factory the coordinator holds is the run-team lambda; calling
|
|
# it must invoke default_plan_node_factory WITH a non-None context_provider.
|
|
coordinator._build_plan_node()
|
|
assert "context_provider" in captured
|
|
assert callable(captured["context_provider"])
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# WS2: /new-task callback wired to this coordinator's start_task
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_build_coordinator_wires_new_task_callback_to_start_task(
|
|
tmp_path: Path, monkeypatch: Any
|
|
) -> None:
|
|
cli = _load_run_team()
|
|
coordinator = cli._build_coordinator(_dry_args(tmp_path))
|
|
|
|
# Replace start_task so we can observe the callback routing without running
|
|
# the real graph.
|
|
calls: dict[str, Any] = {}
|
|
|
|
def _fake_start_task(*, task_text: str, transport_name: str) -> str:
|
|
calls["task_text"] = task_text
|
|
calls["transport_name"] = transport_name
|
|
return "thread-xyz"
|
|
|
|
monkeypatch.setattr(coordinator, "start_task", _fake_start_task)
|
|
|
|
cb = coordinator._new_task_callback
|
|
assert cb is not None
|
|
thread_id = cb("fix the flaky test", "slack")
|
|
assert thread_id == "thread-xyz"
|
|
assert calls == {"task_text": "fix the flaky test", "transport_name": "slack"}
|
|
|
|
|
|
def test_set_new_task_callback_overrides() -> None:
|
|
from agent_team.coordinator import Coordinator
|
|
from agent_team.transport.base import Transport
|
|
|
|
class _T(Transport):
|
|
def post_question(self, **kw: Any) -> str: # type: ignore[override]
|
|
return "q"
|
|
|
|
def parse_answer(self, raw: Any): # type: ignore[override]
|
|
raise NotImplementedError
|
|
|
|
coord = Coordinator(db_path=":memory:", transport=_T())
|
|
assert coord._new_task_callback is None
|
|
sentinel = lambda t, s: "tid" # noqa: E731
|
|
coord.set_new_task_callback(sentinel)
|
|
assert coord._new_task_callback is sentinel
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# WS2: factory + coordinator forward new_task_callback to the SlackListener
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_slack_listener_factory_forwards_new_task_callback(
|
|
tmp_path: Path, monkeypatch: Any
|
|
) -> None:
|
|
import agent_team.coordinator as coord_mod
|
|
|
|
captured: dict[str, Any] = {}
|
|
|
|
class _FakeListener:
|
|
def __init__(self, *args: Any, **kwargs: Any) -> None:
|
|
captured["new_task_callback"] = kwargs.get("new_task_callback")
|
|
|
|
# Patch the lazily-imported SlackListener symbol.
|
|
import agent_team.transport.slack_listener as sl_mod
|
|
|
|
monkeypatch.setattr(sl_mod, "SlackListener", _FakeListener)
|
|
|
|
sentinel = lambda t, s: "tid" # noqa: E731
|
|
coord_mod.default_slack_listener_factory(
|
|
transport=MagicMock(),
|
|
db_path=tmp_path / "x.sqlite",
|
|
enqueue_resume=lambda _x: None,
|
|
new_task_callback=sentinel,
|
|
)
|
|
assert captured["new_task_callback"] is sentinel
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# WS5 (G4): context_provider is threaded into the CLARIFIER too, not just plan
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_build_coordinator_threads_context_provider_into_clarify_node(
|
|
tmp_path: Path, monkeypatch: Any
|
|
) -> None:
|
|
cli = _load_run_team()
|
|
from agent_team import coordinator as coord_mod
|
|
|
|
captured: dict[str, Any] = {}
|
|
|
|
def _spy_clarify_factory(context_provider: Any = None):
|
|
captured["context_provider"] = context_provider
|
|
return lambda state: state
|
|
|
|
monkeypatch.setattr(coord_mod, "default_clarify_node_factory", _spy_clarify_factory)
|
|
|
|
coordinator = cli._build_coordinator(_dry_args(tmp_path))
|
|
coordinator._build_clarify_node()
|
|
assert "context_provider" in captured
|
|
assert callable(captured["context_provider"])
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# WS1 (G1): the in-process cross-reviewer bootstraps the orchestrator root onto
|
|
# sys.path before importing `models` (else the daemon silently REQUEST_CHANGES).
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_cross_reviewer_invoker_bootstraps_orchestrator_path(monkeypatch: Any) -> None:
|
|
import types
|
|
|
|
from agent_team.invoker_multi import _ensure_orchestrator_on_path # noqa: F401
|
|
from agent_team.nodes.review_loop_llm import make_cross_reviewer_invoker
|
|
|
|
# The orchestrator root is parents[2] of invoker_multi.py.
|
|
import agent_team.invoker_multi as im
|
|
|
|
root = str(Path(im.__file__).resolve().parents[2])
|
|
|
|
# Simulate the daemon: root NOT on sys.path. Inject a fake `models` so the
|
|
# deferred import resolves without real provider keys — the point is to
|
|
# prove the bootstrap runs (root re-added) BEFORE the import.
|
|
monkeypatch.setattr(sys, "path", [p for p in sys.path if p != root])
|
|
fake_models = types.ModuleType("models")
|
|
fake_reviewer = MagicMock()
|
|
fake_reviewer.invoke.return_value = MagicMock(content="APPROVE")
|
|
fake_models.get_cross_reviewer = lambda: fake_reviewer # type: ignore[attr-defined]
|
|
monkeypatch.setitem(sys.modules, "models", fake_models)
|
|
|
|
invoker = make_cross_reviewer_invoker()
|
|
out = invoker("review this plan")
|
|
assert out == "APPROVE"
|
|
assert root in sys.path, (
|
|
"invoker must bootstrap the orchestrator root onto sys.path"
|
|
)
|