diff --git a/.github/workflows/agent-team-apply-verify.yml b/.github/workflows/agent-team-apply-verify.yml index e18c6ee..077b9d9 100644 --- a/.github/workflows/agent-team-apply-verify.yml +++ b/.github/workflows/agent-team-apply-verify.yml @@ -1260,3 +1260,20 @@ jobs: --base main \ --head "$HEAD_BRANCH" echo "draft PR opened (task=$TASK_ID, head=$HEAD_BRANCH); never auto-merged." + + - name: "Emit audit log entry (task + diff hash + gate result)" + # WS3 detective control (additive): an unconditional audit trail for + # every run so every dispatch attempt is traceable in the job log, + # regardless of outcome. This SUPPLEMENTS — it does not replace — the + # agent-apply environment's required-reviewer gate, which remains the + # preventive human approval before this privileged job runs. + if: always() + env: + TASK_ID: ${{ inputs.task_id }} + DIFF_HASH: ${{ needs.guard.outputs.diff_hash }} + RUN_ID: ${{ github.run_id }} + GATE_RESULT: ${{ steps.gate.outputs.gate }} + GH_REPO: ${{ github.repository }} + run: | + set -euo pipefail + echo "[agent-apply audit] task=${TASK_ID} diff_hash=${DIFF_HASH} run_id=${RUN_ID} gate=${GATE_RESULT} repo=${GH_REPO}" diff --git a/README.md b/README.md index 3f282c3..39d9b23 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,10 @@ The `security-review/` subsystem is a high-recall, anti-complacency security gat See `security-review/README.md` for full detail and `security-review/DEPLOY-R720.md` for the VM runbook. +## agent-team (R720 durable SDLC pipeline) + +The `agent-team/` subsystem is a separate, durable, human-gated SDLC pipeline (LangGraph + SQLite ledger) that runs as an always-on coordinator daemon on the same `sh-secrev` R720 VM. It is distinct from this stateless router: it persists tasks across restarts and runs INTAKE → CLARIFY → PLAN → REVIEW (build/verify is deploy-gated and inert). It reuses this orchestrator's `models.py` for its non-Claude invokers, and exposes an opt-in FastAPI HTTP API (loopback, bearer auth) plus a `/delegate` Claude Code plugin hook (`sea-haven-claude-plugin/`). See `agent-team/README.md` and `agent-team/DEPLOY-R720.md`. + ## Setup 1. Install dependencies: `pip install -r requirements.txt` diff --git a/agent-team/DEPLOY-R720.md b/agent-team/DEPLOY-R720.md index 18cc9a7..71f9a8f 100644 --- a/agent-team/DEPLOY-R720.md +++ b/agent-team/DEPLOY-R720.md @@ -118,6 +118,51 @@ The unit runs `python3 run-team.py serve` from secrets from `EnvironmentFile=/home/adam/secrev.env`. `Restart=on-failure` keeps it up across transient faults; `journalctl -u` is the live log. +## 4b. WS0–WS5 rollout — UPDATE an already-deployed box + +The steps above (§1–4) are the **first-time** P1 provision. To bring an +already-deployed coordinator up to the WS0–WS5 rollout, use the attended update +script rather than re-running the manual steps: + +``` +# From the Mac, after the WS branches have merged to main, snapshot first: +agent-team/scripts/deploy-r720-ws-rollout.sh +``` + +It is an UPDATE (not a provision): it snapshots-reminds, rsyncs the new code, +rsyncs the engineering handbook to the box, installs the new deps, appends the +new secrets if absent, restarts the coordinator, and smoke-tests. It is +idempotent and fails loudly. What goes **live** after it: WS1 in-process +multi-model invokers (`bind_multi_invoker`, already wired), the WS5 handbook +`context_provider` injected into the planner prompt, and the WS2 Slack +`/new-task` command (AUTHZ-01 owner-allowlist gated). The P3 dispatch/build-verify +path stays **inert** (gated behind the `agent-apply` GitHub Environment approval). + +**New venv deps** (the coordinator does not need them; only the optional HTTP +API does) — now pinned in the root `requirements.txt`: + +| pip dep | Why | +|---|---| +| `fastapi==0.136.1` | the WS1 HTTP API app (`agent_team/api.py`) | +| `uvicorn==0.46.0` | ASGI server for `api.serve()` | + +**New env vars** — append to `~/secrev.env` (mode 600, never committed): + +- `SEA_HAVEN_HANDBOOK_DIR` — where `load_handbook_conventions()` reads the + engineering handbook (the script syncs it to `/home/adam/.sea-haven/engineering-handbook` + by default; this var must match). Fail-safe: if the dir is missing the + `context_provider` returns `""` and the planner runs without handbook context. +- `AGENT_TEAM_API_TOKEN` — bearer token for the HTTP API / `/delegate` hook + **only**. Not needed by the coordinator daemon itself. The HTTP API refuses to + start if this is unset/empty. + +**The HTTP API is a separate, opt-in process** — it is **not** started by the +coordinator daemon. Run it explicitly (`api.serve()`, binds `127.0.0.1:8765`, +bearer auth) only if you want the `/delegate` Claude Code hook or the +`POST /tasks` / `GET /tasks/{thread_id}` / `POST /orchestrator/invoke` endpoints. +The `/docs` + `/openapi` routes are disabled and it binds loopback by design (do +not change to `0.0.0.0`). See the deploy script's step 6 for how to start it. + ## 5. P1 live exit-criteria demo (§3.3.1) Demonstrate all four once the service is live. Map each to the operator commands diff --git a/agent-team/README.md b/agent-team/README.md index bf43434..8219fe1 100644 --- a/agent-team/README.md +++ b/agent-team/README.md @@ -39,6 +39,10 @@ agent-team/ graph.py # LangGraph wiring: P1 (intake→clarify→plan) + opt-in P2 # review loop + opt-in P3 build/verify subgraph invoker.py # §3.1 real Claude path (subscription-OAuth / API / Bedrock) + invoker_multi.py # WS1 in-process non-Claude invokers (GPT-4.1 / DeepSeek / + # Gemini via the orchestrator's models.py); bind_multi_invoker() + api.py # WS1 FastAPI HTTP API (bearer auth, 127.0.0.1:8765) — SEPARATE + # opt-in process (api.serve()), NOT started by the coordinator billing.py # §3.1 claude_invoke billing-mode seam ci_gate.py # §3.3.2 pure-code authenticated-Checks PASS/FAIL gate task_model.py / state_store.py @@ -52,17 +56,26 @@ agent-team/ builders.py + builders_llm.py # candidate diff (DeepSeek) — INERT, proposes only verifier.py + verifier_llm.py # ci_gate sole PASS authority; LLM = fix-proposer build_verify_subgraph.py # P3 BUILD→VERIFY topology (opt-in) + handbook.py # WS5 load_handbook_conventions (handbook seam, + # fail-safe → "" if dir missing); planner context + dispatch_invoker.py # WS3 auto-dispatch node — INERT (NOT wired live) transport/ # one adapter contract + a live impl per channel base.py # Transport ABC + QuestionSet / NormalizedAnswer - slack_adapter.py + slack_live.py + slack_listener.py # Block Kit + Socket Mode + slack_adapter.py + slack_live.py + slack_listener.py # Block Kit + Socket Mode + /new-task github_adapter.py + github_live.py + github_intake.py # issue-comment + issue intake claude_code_adapter.py + claude_code_live.py # file-drop responder + scripts/ # deploy-r720-ws-rollout.sh — attended WS0–WS5 UPDATE of the box ci/ # §3.3.2 split-job CI apply/verify workflow (DEPLOY-GATED) systemd/ # agent-team-coordinator.service (not installed) DEPLOY-R720.md # provisioning runbook (snapshot-first, rsync, tokens, demo) tests/ # pytest, one module per source module + sim harness ``` +The Claude Code plugin lives in a sibling top-level dir, `../sea-haven-claude-plugin/` +(CLAUDE.md, settings.template.json, `hooks/user_prompt_submit.py`): a +`UserPromptSubmit` hook that forwards `/delegate ` prompts from Claude Code +to the HTTP API's `POST /tasks` (env `AGENT_TEAM_API_URL` / `AGENT_TEAM_API_TOKEN`). + The top directory is kebab-case (`agent-team/`); the importable package is snake_case (`agent_team/`), per the engineering handbook. @@ -85,6 +98,22 @@ snake_case (`agent_team/`), per the engineering handbook. GPT-4.1 (review) and DeepSeek (builders) route through the local orchestrator `run.py`. Switching Claude billing is a config flip. +## WS0–WS5 rollout glossary + +The "WS-rollout" (workstreams 0–5) layered HTTP/integration surfaces onto the +P1–P4 pipeline. What is **live** vs **inert** after the rollout: + +| WS | What it adds | Live? | +|---|---|---| +| WS1 | `invoker_multi.py` (in-process GPT-4.1 / DeepSeek / Gemini via the orchestrator's `models.py`) + `api.py` (FastAPI HTTP API, bearer auth via `AGENT_TEAM_API_TOKEN`, binds `127.0.0.1:8765`, `/docs`+`/openapi` disabled, concurrency-capped) | `bind_multi_invoker()` wired in `run-team.py` `_cmd_serve` (LIVE); the **HTTP API is a separate opt-in process** (`api.serve()`), NOT started by the coordinator | +| WS5 | `nodes/handbook.py` `load_handbook_conventions` (reads `SEA_HAVEN_HANDBOOK_DIR` or `~/.sea-haven/engineering-handbook`, fail-safe → `""`); `retriever.py` `save_memory` writes to a `_box-drafts/` review queue | LIVE — the planner prompt receives the handbook via the `context_provider` seam in `run-team.py` `_build_coordinator` | +| WS2/WS0/WS4 | Slack `/new-task` slash command (AUTHZ-01 owner-allowlist gated) → `Coordinator.set_new_task_callback`; the `sea-haven-claude-plugin/` (CLAUDE.md, settings, `/delegate` `UserPromptSubmit` hook) | LIVE (`/new-task` wired in `serve`); the plugin/HTTP-API path is opt-in | +| WS3 | `nodes/dispatch_invoker.py` (auto-dispatch LangGraph node) + graph/coordinator wiring | **INERT — NOT wired live.** The `agent-apply` GitHub Environment human-approval gate is KEPT; the P3 dispatch/build-verify path stays inert pending per-task `run_id` plumbing + a CI-boundary security re-review | + +The HTTP API endpoints: `POST /tasks` (start a task), `GET /tasks/{thread_id}` +(status), `POST /orchestrator/invoke` (one-shot model invoke). See +`DEPLOY-R720.md` for the WS-rollout deploy (`scripts/deploy-r720-ws-rollout.sh`). + ## Running the tests ``` diff --git a/agent-team/agent_team/api.py b/agent-team/agent_team/api.py new file mode 100644 index 0000000..8691d0c --- /dev/null +++ b/agent-team/agent_team/api.py @@ -0,0 +1,347 @@ +"""FastAPI HTTP API for the Sea Haven agent-team orchestrator (WS1 — code only). + +Exposes two endpoints under bearer-token auth (env ``AGENT_TEAM_API_TOKEN``): + +``POST /tasks`` + Intake a new agent-team task: creates a task via :meth:`Coordinator.start_task` + and returns the minted ``thread_id``. Requires ``task`` in the JSON body. + +``GET /tasks/{thread_id}`` + Return the current pipeline state for a running task from the LangGraph + checkpoint. Returns ``{"thread_id": ..., "state": {...}}`` when found, 404 + when the thread has no checkpoint. + +``POST /orchestrator/invoke`` + Route a one-shot prompt through the orchestrator's root ``run.py`` (the + existing multi-model router). Returns ``{"text": ...}`` with the model's + response. Accepts ``{"prompt": "..."}`` in the JSON body. + +Security +-------- +* **Bearer-token auth**: every request must carry ``Authorization: Bearer `` + where the token matches ``AGENT_TEAM_API_TOKEN``. Missing or wrong token → 401. + An empty / absent ``AGENT_TEAM_API_TOKEN`` at startup raises :class:`RuntimeError` + so the server refuses to start in an unconfigured state (no open-by-default). +* **Bind to 127.0.0.1** by default: :func:`make_app` accepts a ``host`` kwarg + defaulting to ``"127.0.0.1"`` so a misconfigured caller cannot accidentally + bind to 0.0.0.0. The uvicorn run call at the bottom of :func:`serve` enforces + this. +* **Token comparison is constant-time** (``hmac.compare_digest``) to prevent + timing side-channels on the secret. +* **No secrets in code**: the token is read from env only, never hardcoded here. + +Code-only gate (do NOT launch) +------------------------------ +This module is WS1 code scaffolding. Do NOT import-and-run it on the R720 box +until the GPT-4.1 cross-review and /sh-security-review gates clear. The +``serve()`` helper is provided for the attended deploy step only. +""" + +from __future__ import annotations + +import hmac +import os +import subprocess +import sys +import threading +from pathlib import Path +from typing import Any + +from pydantic import BaseModel + +__all__ = [ + "AGENT_TEAM_API_TOKEN_ENV", + "make_app", + "serve", +] + +AGENT_TEAM_API_TOKEN_ENV = "AGENT_TEAM_API_TOKEN" +_DEFAULT_HOST = "127.0.0.1" +_DEFAULT_PORT = 8765 + +# Bound how many `/orchestrator/invoke` subprocesses can run at once. Each spawns +# a 600s-timeout child; without a cap an authenticated caller could exhaust CPU / +# memory / file descriptors by firing many concurrent invocations (DoS). The +# endpoint runs in Starlette's threadpool, so a threading semaphore is the right +# primitive; over-limit requests get 429 rather than queueing unboundedly. +_MAX_CONCURRENT_INVOKES = 2 +_invoke_semaphore = threading.BoundedSemaphore(_MAX_CONCURRENT_INVOKES) + + +# --------------------------------------------------------------------------- +# Pydantic request/response models (module-level so FastAPI resolves them). +# --------------------------------------------------------------------------- + + +class TaskRequest(BaseModel): + task: str + transport: str = "claude_code" + + +class TaskResponse(BaseModel): + thread_id: str + + +class TaskStateResponse(BaseModel): + thread_id: str + state: dict[str, Any] + + +class OrchestratorInvokeRequest(BaseModel): + prompt: str + + +class OrchestratorInvokeResponse(BaseModel): + text: str + + +# --------------------------------------------------------------------------- +# Token helper. +# --------------------------------------------------------------------------- + + +def _get_token() -> str: + """Read the API bearer token from the environment. + + Raises :class:`RuntimeError` if the variable is absent or empty — the server + must never start without a configured secret. + """ + token = os.environ.get(AGENT_TEAM_API_TOKEN_ENV, "").strip() + if not token: + raise RuntimeError( + f"{AGENT_TEAM_API_TOKEN_ENV} is not set or empty; refusing to start " + "the HTTP API without a configured bearer token." + ) + return token + + +def _resolve_run_py() -> Path: + """Resolve the orchestrator root's ``run.py``. + + ``api.py`` lives at ``/agent-team/agent_team/api.py``, so the root is + ``parents[2]``. + """ + return Path(__file__).resolve().parents[2] / "run.py" + + +def _build_coordinator(db_path: str | Path | None = None) -> Any: + """Build a default :class:`Coordinator` for the HTTP API. + + Uses a null transport by default because the HTTP API is not directly wired + to a Slack/GitHub channel — the caller provides answers through the ledger. + The ledger path defaults to the same ``state/agent_team.sqlite`` the CLI uses. + """ + from agent_team.coordinator import Coordinator # noqa: PLC0415 + from agent_team.transport.base import Transport # noqa: PLC0415 + + class _NullTransport(Transport): + def post_question(self, **kw: Any) -> str: # type: ignore[override] + return f"api:{kw.get('question_id', 'unknown')}" + + def parse_answer(self, raw: Any) -> tuple[str, Any, str]: # type: ignore[override] + raise NotImplementedError("HTTP API does not use transport.parse_answer") + + if db_path is None: + db_path = Path(__file__).resolve().parent.parent / "state" / "agent_team.sqlite" + return Coordinator(db_path=db_path, transport=_NullTransport()) + + +# --------------------------------------------------------------------------- +# App factory. +# --------------------------------------------------------------------------- + + +def make_app( + *, + coordinator: Any = None, + db_path: str | Path | None = None, + host: str = _DEFAULT_HOST, +) -> Any: + """Build and return the FastAPI application (does NOT start it). + + ``coordinator`` is an optional pre-built :class:`~agent_team.coordinator.Coordinator` + instance. When omitted, :func:`_build_coordinator` constructs one from + ``db_path`` at request time. + + ``host`` defaults to ``"127.0.0.1"`` — do NOT change to ``"0.0.0.0"`` without + the security gate. + """ + try: + from fastapi import Depends, FastAPI, HTTPException # noqa: PLC0415 + from fastapi.security import ( # noqa: PLC0415 + HTTPAuthorizationCredentials, + HTTPBearer, + ) + except ImportError as exc: + raise RuntimeError( + "fastapi is unavailable; install it to use the HTTP API: " + "pip install fastapi uvicorn" + ) from exc + + # Fail fast at app-build time if the bearer token is unconfigured, so a + # misconfigured deploy never reaches a serving state (matches the module + # docstring's "refuses to start without a secret" contract). The per-request + # check still calls _get_token() so a token rotation/unset after boot is + # also caught. + _get_token() + + # VPN-only internal API: disable the interactive docs and the OpenAPI schema + # so the endpoint surface is not exposed unauthenticated (these routes carry + # no auth dependency by design in FastAPI). + app = FastAPI( + title="Sea Haven agent-team API", + description="Internal HTTP API for the agent-team orchestrator (VPN-only, 127.0.0.1).", + version="0.1.0", + docs_url=None, + redoc_url=None, + openapi_url=None, + ) + + _bearer = HTTPBearer(auto_error=False) + + def _check_token( + creds: HTTPAuthorizationCredentials | None = Depends(_bearer), + ) -> None: + expected = _get_token() + if creds is None or not hmac.compare_digest(creds.credentials, expected): + raise HTTPException( + status_code=401, detail="invalid or missing bearer token" + ) + + # Coordinator is built lazily and cached on first use so test clients that + # inject a pre-built coordinator never trigger the DB path. + _state: dict[str, Any] = {"coordinator": coordinator, "ready": False} + + def _get_coordinator() -> Any: + if _state["coordinator"] is None: + _state["coordinator"] = _build_coordinator(db_path) + coord = _state["coordinator"] + if not _state["ready"]: + coord.setup() + _state["ready"] = True + return coord + + @app.post( + "/tasks", response_model=TaskResponse, dependencies=[Depends(_check_token)] + ) + def create_task(request: TaskRequest) -> TaskResponse: + """Intake a new agent-team task and run it to the first human gate.""" + coord = _get_coordinator() + thread_id = coord.start_task( + task_text=request.task, + transport_name=request.transport, + ) + return TaskResponse(thread_id=thread_id) + + @app.get( + "/tasks/{thread_id}", + response_model=TaskStateResponse, + dependencies=[Depends(_check_token)], + ) + def get_task(thread_id: str) -> TaskStateResponse: + """Return the current LangGraph checkpoint state for a task.""" + coord = _get_coordinator() + graph = getattr(coord, "graph", None) + if graph is None: + raise HTTPException(status_code=503, detail="coordinator not ready") + try: + snapshot = graph.get_state({"configurable": {"thread_id": thread_id}}) + if snapshot is None or not getattr(snapshot, "values", None): + raise HTTPException( + status_code=404, detail=f"no state for thread {thread_id!r}" + ) + state_dict: dict[str, Any] = dict(snapshot.values) + except HTTPException: + raise + except Exception as exc: + # Do not leak the raw exception text (may carry internal paths / + # state) into the response body; log it server-side and return a + # generic message. + print(f"[api] get_task error for {thread_id!r}: {exc!r}", file=sys.stderr) + raise HTTPException( + status_code=500, detail="internal error retrieving task state" + ) from exc + return TaskStateResponse(thread_id=thread_id, state=state_dict) + + @app.post( + "/orchestrator/invoke", + response_model=OrchestratorInvokeResponse, + dependencies=[Depends(_check_token)], + ) + def orchestrator_invoke( + request: OrchestratorInvokeRequest, + ) -> OrchestratorInvokeResponse: + """Route a one-shot prompt through the root orchestrator run.py. + + Shells out to the orchestrator root's ``run.py`` with the prompt text. + The output is returned raw (UNTRUSTED); callers are responsible for + validating it. + """ + run_py = _resolve_run_py() + if not run_py.exists(): + raise HTTPException( + status_code=503, + detail=f"orchestrator run.py not found at {run_py}", + ) + # Bound concurrent subprocess fan-out (DoS guard). Non-blocking acquire: + # over the cap we reject with 429 rather than pile up 600s children. + if not _invoke_semaphore.acquire(blocking=False): + raise HTTPException( + status_code=429, + detail="too many concurrent orchestrator invocations; retry later", + ) + try: + completed = subprocess.run( # noqa: S603 - args are not shell-interpolated + [sys.executable, str(run_py), request.prompt], + capture_output=True, + text=True, + check=True, + timeout=600, + ) + except subprocess.TimeoutExpired: + raise HTTPException( + status_code=504, detail="orchestrator invocation timed out" + ) from None + except subprocess.CalledProcessError as exc: + # Log stderr server-side; do not return it in the response body + # (may carry internal paths / orchestrator internals). + print( + f"[api] orchestrator_invoke failed (exit {exc.returncode}): " + f"{(exc.stderr or '')[:1000]}", + file=sys.stderr, + ) + raise HTTPException( + status_code=500, + detail=f"orchestrator call failed (exit {exc.returncode})", + ) from exc + finally: + _invoke_semaphore.release() + return OrchestratorInvokeResponse(text=completed.stdout) + + return app + + +# --------------------------------------------------------------------------- +# Attended deploy helper (do NOT call in tests or CI). +# --------------------------------------------------------------------------- + + +def serve( + *, + host: str = _DEFAULT_HOST, + port: int = _DEFAULT_PORT, + db_path: str | Path | None = None, +) -> None: # pragma: no cover - attended deploy step only + """Start the uvicorn server (attended deploy — do NOT call in tests or CI). + + Binds to ``127.0.0.1`` by default. Change ``host`` only after the + GPT-4.1 cross-review and /sh-security-review gates clear. + """ + try: + import uvicorn # noqa: PLC0415 + except ImportError as exc: + raise RuntimeError( + "uvicorn is unavailable; install it: pip install uvicorn" + ) from exc + app = make_app(host=host, db_path=db_path) + uvicorn.run(app, host=host, port=port) diff --git a/agent-team/agent_team/coordinator.py b/agent-team/agent_team/coordinator.py index 36fc6a9..74ffc45 100644 --- a/agent-team/agent_team/coordinator.py +++ b/agent-team/agent_team/coordinator.py @@ -69,8 +69,10 @@ if TYPE_CHECKING: # pragma: no cover - typing only __all__ = [ "Coordinator", + "DispatchNodeFactory", "build_verify_wiring", "default_clarify_node_factory", + "default_dispatch_node_factory", "default_slack_listener_factory", "gated_build_verify_wiring", ] @@ -116,6 +118,13 @@ BuildVerifyWiring = Callable[ "Callable[[PipelineState], str]]", ] +# A dispatch-node factory: builds the live dispatch node that carries an approved +# diff into org CI via :func:`agent_team.nodes.dispatch_invoker.make_dispatch_node`. +# None -> APPROVED_ROUTE stays END (graph stops at an approved plan; no dispatch). +# OPT-IN: the production run-team path does NOT inject this by default; it is +# the caller's responsibility to supply owner/repo config at startup. +DispatchNodeFactory = Callable[[], "Callable[[PipelineState], Any]"] + # A checkpointer factory over the db path: returns the BaseCheckpointSaver the # graph is compiled with. Defaults to the production SQLite checkpointer # (:func:`agent_team.graph.build_sqlite_checkpointer`); tests inject a factory @@ -143,6 +152,7 @@ def default_slack_listener_factory( transport: SlackTransport, db_path: Path, enqueue_resume: Callable[[Any], None], + new_task_callback: "Callable[[str, str, str], str] | None" = None, ) -> Any: """Build the live :class:`SlackListener` from the coordinator's seams (D-1). @@ -157,21 +167,47 @@ def default_slack_listener_factory( (AUTHZ-01), so this factory deliberately does not weaken that — it injects no ``owner_ids`` and lets ``serve`` read + enforce them. + ``new_task_callback`` (WS2) is forwarded to the listener so an allowlisted + ``/new-task`` command starts a task. Left ``None``, the listener ignores + ``/new-task`` (its built-in default) — the ``/new-task`` path still runs + AFTER the AUTHZ-01 owner check regardless. The listener posts the root + "📥 Task received" ack through the transport's own poster (no extra wiring). + + A best-effort 👍 ``reactor`` is built from ``SLACK_BOT_TOKEN`` so the listener + can acknowledge inbound answers with a reaction (requires the + ``reactions:write`` scope). If the SDK/token is unavailable the reactor is + left ``None`` (no reaction attempted) — it never blocks listener startup. + Imported lazily for the same import-hygiene reason as the clarifier / planner factories (the listener pulls the transport + responder leaves). """ from agent_team.transport.slack_listener import SlackListener + reactor: Any = None + try: + from agent_team.transport.slack_live import build_slack_reactor + + reactor = build_slack_reactor() + except Exception: # noqa: BLE001 - no SDK/token -> run without 👍 reactions + _LOG.info( + "Slack 👍 reactor unavailable (no SDK/token); inbound answers will " + "not be reaction-acknowledged" + ) + return SlackListener( transport, db_path, enqueue_resume, app_token=os.environ.get("SLACK_APP_TOKEN") or None, bot_token=os.environ.get("SLACK_BOT_TOKEN") or None, + new_task_callback=new_task_callback, + reactor=reactor, ) -def default_clarify_node_factory() -> Callable[[PipelineState], PipelineState]: +def default_clarify_node_factory( + context_provider: "Callable[[], str] | None" = None, +) -> Callable[[PipelineState], PipelineState]: """Build the live Claude-backed clarifier node (§3.3, §7.1 P1). Composes the two committed leaves: the Claude clarifier callables @@ -190,14 +226,22 @@ def default_clarify_node_factory() -> Callable[[PipelineState], PipelineState]: from agent_team.nodes.clarifier import make_clarifier_node from agent_team.nodes.clarifier_llm import build_claude_clarifier_callables - assess_confidence, generate_questions = build_claude_clarifier_callables() + # WS5: thread the handbook/memory context_provider into the clarifier too + # (not just the planner) so clarifying questions are handbook-aware. Only + # forwarded when non-None to preserve the byte-identical default behavior. + kwargs: dict[str, Any] = {} + if context_provider is not None: + kwargs["context_provider"] = context_provider + assess_confidence, generate_questions = build_claude_clarifier_callables(**kwargs) return make_clarifier_node( assess_confidence=assess_confidence, generate_questions=generate_questions, ) -def default_plan_node_factory() -> Callable[[PipelineState], PipelineState]: +def default_plan_node_factory( + context_provider: "Callable[[], str] | None" = None, +) -> Callable[[PipelineState], PipelineState]: """Build the live planner node wrapped fail-safe (§3.3 P2). The canonical planner is :func:`agent_team.nodes.planner.plan_node` (it owns @@ -209,6 +253,9 @@ def default_plan_node_factory() -> Callable[[PipelineState], PipelineState]: reply escalates to Adam rather than taking down the pipeline — the same fail-SAFE discipline the clarifier and review stages use. + ``context_provider`` is the optional WS5 injection seam (memory/handbook + context). Default None = byte-identical behavior. + Imported lazily so the coordinator module stays import-clean and the SDK is only pulled when the live node is actually built. """ @@ -217,6 +264,8 @@ def default_plan_node_factory() -> Callable[[PipelineState], PipelineState]: def plan_node(state: PipelineState) -> PipelineState: try: + if context_provider is not None: + return planner.plan_node(state, context_provider=context_provider) return planner.plan_node(state) except planner.PlannerError: # Unparseable plan -> park + ALARM rather than crash the graph. @@ -357,6 +406,38 @@ def gated_build_verify_wiring( return build_node, verify_node, route_after_verify +def default_dispatch_node_factory() -> "Callable[[Any], Any]": + """Build the live auto-dispatch node from env vars (WS3, OPT-IN, INERT by default). + + Reads ``AGENT_TEAM_REPO_OWNER``, ``AGENT_TEAM_REPO_NAME``, and optionally + ``AGENT_TEAM_BASE_BRANCH`` (default ``"main"``) from the environment and + delegates to :func:`agent_team.nodes.dispatch_invoker.make_dispatch_node`. + Owner/repo are fixed at factory time — never read from pipeline state — so + model output in the state cannot redirect the dispatch target. + + Raises ``RuntimeError`` if the required env vars are absent (fail closed: + the node must never dispatch to an unknown owner/repo). + + This satisfies :data:`DispatchNodeFactory` (zero-arg → node callable). + Pass it as ``dispatch_node_wiring=default_dispatch_node_factory`` AFTER the + §3.3.2 CI trust-boundary gate clears. The production ``run-team.py`` path + deliberately does NOT inject this; it remains opt-in. + """ + import os + + from agent_team.nodes.dispatch_invoker import make_dispatch_node + + owner = os.environ.get("AGENT_TEAM_REPO_OWNER", "").strip() + repo = os.environ.get("AGENT_TEAM_REPO_NAME", "").strip() + base = os.environ.get("AGENT_TEAM_BASE_BRANCH", "main").strip() or "main" + if not owner or not repo: + raise RuntimeError( + "AGENT_TEAM_REPO_OWNER and AGENT_TEAM_REPO_NAME must be set " + "for auto-dispatch (default_dispatch_node_factory)" + ) + return make_dispatch_node(owner=owner, repo=repo, base=base) + + class Coordinator: """Owns the live Plane-2 runtime: graph + resume worker + transport (§3.3). @@ -380,11 +461,14 @@ class Coordinator: build_plan_node: PlanNodeFactory | None = None, review_wiring: ReviewWiring | None = None, build_verify_wiring: BuildVerifyWiring | None = None, + dispatch_node_wiring: DispatchNodeFactory | None = None, build_checkpointer: CheckpointerFactory | None = None, resume_queue: "queue.Queue[Any] | None" = None, deadline_window: timedelta | None = None, alarm_hook: AlarmHook | None = None, build_listener: ListenerFactory | None = None, + new_task_callback: "Callable[[str, str, str], str] | None" = None, + notify: "Callable[..., None] | None" = None, ) -> None: self._db_path = Path(db_path) self._transport = transport @@ -398,6 +482,10 @@ class Coordinator: # the P2 review terminus. The production run-team path never injects it; # it is held until the §3.3.2 CI trust-boundary security gate clears. self._build_verify_wiring = build_verify_wiring + # Dispatch (OPT-IN): None -> APPROVED_ROUTE stays END, no dispatch ever + # fires. The live coordinator can inject a factory built from + # dispatch_invoker.make_dispatch_node with owner/repo at startup. + self._dispatch_node_wiring = dispatch_node_wiring self._build_checkpointer = ( build_checkpointer or graph_mod.build_sqlite_checkpointer ) @@ -409,6 +497,15 @@ class Coordinator: # the start/no-start/shutdown wiring with no live socket. Only consulted # by serve() when Slack is the live transport AND the app token is set. self._build_listener = build_listener + # WS2: optional /new-task handler forwarded to the Slack listener. Left + # None, the listener ignores /new-task. The serve path wires the + # coordinator's own start_task adapter via set_new_task_callback(). + self._new_task_callback = new_task_callback + # Optional lifecycle-notification sink (posts a plain status line to the + # operator channel, e.g. Slack #agent-team). Left None = silent (existing + # behavior). The serve path injects a Slack poster so a task is never a + # black box: the human sees parked / needs-more-input / plan-ready. + self._notify = notify # Built by setup(). self._graph: Any = None @@ -425,6 +522,22 @@ class Coordinator: # Accessors (the shared queue is the slack_listener handoff seam). # ------------------------------------------------------------------ # + def set_new_task_callback( + self, callback: "Callable[[str, str, str], str] | None" + ) -> None: + """Wire the WS2 ``/new-task`` handler (call before :meth:`serve`). + + Avoids the constructor chicken-and-egg of referencing the coordinator's + own ``start_task`` at build time: the serve path constructs the + coordinator, then sets + ``lambda text, source, root_ts: self.start_task(...)``. The ``root_ts`` + (the listener's "📥 Task received" ack ts) is forwarded into + ``start_task`` so the task threads under it (one-thread-per-task). Must be + set before :meth:`_maybe_start_slack_listener` runs (i.e. before + :meth:`serve`); it is read when the listener is built. + """ + self._new_task_callback = callback + @property def resume_queue(self) -> "queue.Queue[Any]": """The shared resume-job queue (slack_listener enqueues, drainer drains).""" @@ -484,6 +597,13 @@ class Coordinator: if self._build_verify_wiring is not None: build_verify = self._build_verify_wiring() + # Dispatch (opt-in): build the dispatch node callable only when the + # factory is provided. build_graph validates that build_verify is also + # set (dispatch hangs off APPROVED_ROUTE which lives in the P3 subgraph). + dispatch_node_callable: Any = None + if self._dispatch_node_wiring is not None: + dispatch_node_callable = self._dispatch_node_wiring() + self._graph = graph_mod.build_graph( checkpointer, live_clarify_node=clarify_node, @@ -491,6 +611,7 @@ class Coordinator: review_node=review_node, route_review=route_review, build_verify=build_verify, + dispatch_node=dispatch_node_callable, ) # The ResumeWorker is satisfied directly by the compiled LangGraph app @@ -521,7 +642,9 @@ class Coordinator: # Intake. # ------------------------------------------------------------------ # - def start_task(self, *, task_text: str, transport_name: str) -> str: + def start_task( + self, *, task_text: str, transport_name: str, slack_thread_ts: str = "" + ) -> str: """Start one task: run to the first human gate, then notify (§3.3, §3.3.1). Runs :func:`agent_team.graph.start_task` to the first clarifier @@ -530,17 +653,24 @@ class Coordinator: :func:`agent_team.responder.notify_question` (ledger row OPEN first, then transport post). Returns the minted ``thread_id``. - **Intake-seed decision (P1).** ``agent_team.graph.start_task`` builds its - own INTAKE seed and accepts only ``thread_id`` / ``transport`` — it takes - no task-description argument, and a value pre-seeded onto the START - checkpoint via ``update_state`` is overwritten by its own seed invoke - (and a post-suspend ``update_state`` clears the pending interrupt, which - would break the human gate). So for P1 the ``task_text`` is intake - metadata held coordinator-side (logged) rather than written into - ``PipelineState``: the deterministic P1 clarifier does not consume a task - description anyway, and threading it into the graph state is a later phase - that extends the committed ``start_task`` seed contract. We keep it - minimal rather than reach past that contract or disturb the gate. + **Intake-seed (task description).** ``task_text`` is passed to + ``agent_team.graph.start_task`` as ``task=`` so it is written into the + seed ``PipelineState`` (the LLM clarifier reasons about it; an empty + description makes the clarifier ask for one). It is seeded directly into + the initial invoke — NOT via ``update_state`` (a pre-seed via + ``update_state`` would be overwritten by the seed invoke, and a + post-suspend ``update_state`` would clear the pending interrupt and break + the human gate). ``intake_node`` returns only a partial state + (status/phase), so the seeded ``task`` channel persists into CLARIFY. + + **One-thread-per-task (``slack_thread_ts``).** When the task originates + from a ``/new-task`` slash command, the listener has already posted a + root "📥 Task received" message and passes its ``ts`` here. It is seeded + into the graph state (so every later turn can recover it) AND forwarded + to :func:`notify_question` as ``thread_ts``, so the FIRST clarifier + question posts as a threaded reply under that root and its ledger + ``channel_ref`` becomes the root ``ts``. Empty (the default) for a task + with no root post — the question posts top-level exactly as before. """ if self._graph is None: raise RuntimeError("Coordinator.start_task called before setup()") @@ -553,7 +683,12 @@ class Coordinator: transport_name, task_text[:200].replace("\n", "\\n").replace("\r", "\\r"), ) - thread_id, _state = graph_mod.start_task(self._graph, transport=transport_name) + thread_id, _state = graph_mod.start_task( + self._graph, + transport=transport_name, + task=task_text, + slack_thread_ts=slack_thread_ts, + ) question = graph_mod.pending_question(self._graph, thread_id=thread_id) if question is None: @@ -573,6 +708,7 @@ class Coordinator: self._transport, question_set, deadline=deadline, + thread_ts=slack_thread_ts or None, ) finally: conn.close() @@ -647,6 +783,164 @@ class Coordinator: # Maintenance tick (deadline policy + drain). # ------------------------------------------------------------------ # + def _emit(self, message: str, *, thread_ts: str | None = None) -> None: + """Post a lifecycle status line to the notify sink (never raises). + + A notification failure must never disturb the pipeline, so the sink call + is wrapped; a broken Slack post is logged and swallowed. + + ``thread_ts`` (one-thread-per-task) — when set, the milestone is posted + as a threaded reply under that task's root "📥 Task received" message so + every notification for a task lands in its one Slack thread. The notify + sink accepts an optional keyword ``thread_ts``; a sink that does not + (an older/simpler sink) is called positionally so the threading hint is + a no-op rather than a crash — the call is tried with ``thread_ts`` first + and falls back to the bare message on a ``TypeError``. + """ + if self._notify is None: + return + try: + if thread_ts: + try: + self._notify(message, thread_ts=thread_ts) + return + except TypeError: + # The sink does not accept thread_ts; degrade to a top-level + # post rather than dropping the milestone entirely. + self._notify(message) + return + self._notify(message) + except Exception: # noqa: BLE001 - a notify failure must not break the loop + _LOG.warning("notify sink raised; dropping status message", exc_info=True) + + def _post_resume_followups(self, results: list[ResumeResult]) -> None: + """After a drain, deliver follow-up questions + emit lifecycle milestones. + + For each resumed thread, the graph has settled at one of: a NEW pending + question (multi-turn clarify — which the normal drain path does NOT post, + only the startup recover sweep did, so post it here), a PARKED terminal + state, or a completed/plan-ready terminal state. Emits a human-readable + status line for each so an answered task is never a black box. Best-effort + and fully guarded — a follow-up failure never breaks the tick loop. + """ + from agent_team.task_model import TaskStatus # noqa: PLC0415 + + seen: set[str] = set() + for result in results: + thread_id = getattr(result, "thread_id", None) + if not thread_id or thread_id in seen: + continue + seen.add(thread_id) + short = thread_id[:8] + + # Read the settled state once so every message can name WHAT the task + # is (description), not just an opaque thread id. + try: + snap = self._graph.get_state(graph_mod.thread_config(thread_id)) + values = getattr(snap, "values", {}) or {} + except Exception: # noqa: BLE001 + values = {} + desc = str(values.get("task") or "").strip() or "(no description)" + if len(desc) > 90: + desc = desc[:90] + "…" + label = f'"{desc}" (`{short}`)' + + # One-thread-per-task: the root "📥 Task received" message ts. When + # set, every follow-up question AND every lifecycle milestone for this + # task threads under it. Empty for a non-/new-task origin (top-level). + root_ts = str(values.get("slack_thread_ts") or "") or None + + try: + question = graph_mod.pending_question(self._graph, thread_id=thread_id) + except Exception: # noqa: BLE001 - never let a status check break the loop + question = None + + if question is not None: + # Multi-turn: a new clarifier question is waiting. Post it to the + # transport (the drain path otherwise leaves it unposted) and tell + # the human more input is needed. Thread it (and its channel_ref) + # under the task's root message so the next reply maps back. + try: + conn = connect(self._db_path) + try: + responder_mod.notify_question( + conn, + self._transport, + question["question_set"], + deadline=question.get("deadline") + or self._default_deadline(), + thread_ts=root_ts, + ) + finally: + conn.close() + except Exception: # noqa: BLE001 - post failure must not break tick + _LOG.warning( + "failed to post follow-up question for %s", short, exc_info=True + ) + self._emit( + f"❓ {label} — needs more input. A new clarifying question was " + "posted above; reply in its thread.", + thread_ts=root_ts, + ) + continue + + # No pending question: the task settled. Distinguish parked vs done, + # and say WHERE it got to and WHAT is blocking it. + status = values.get("status") + phase = str(values.get("current_phase") or "unknown") + # When a task parks, current_phase is the terminal "parked" — not + # useful. Infer the phase it was IN when it escalated, so the human + # sees WHERE it died (review > plan > clarify by what state exists). + if phase == "parked": + if values.get("review_verdicts"): + phase = "review" + elif values.get("plan"): + phase = "plan" + else: + phase = "clarify" + if status == TaskStatus.PARKED.value: + blocker = self._summarize_blocker(values) + self._emit( + f"⚠️ PARKED — {label}\n" + f"• Reached phase: {phase}\n" + f"• What's blocking it: {blocker}\n" + "• Needs your review — the plan could not be auto-approved. " + "Re-assign with more detail, or adjust the requirement to unblock.", + thread_ts=root_ts, + ) + else: + self._emit( + f"✅ {label} — plan ready for review (phase: {phase}).", + thread_ts=root_ts, + ) + + @staticmethod + def _summarize_blocker(values: "dict[str, Any]") -> str: + """Human-readable reason a task parked, from the last review verdict. + + Pulls the most recent ``review_verdicts`` entry's findings (the GPT-4.1 + REQUEST_CHANGES text) — collapsed + truncated for a Slack line — so the + human sees WHAT stopped it, not just "escalation". Falls back to a generic + reason when there is no verdict (e.g. an unparseable-plan park before + review ever ran). + """ + verdicts = values.get("review_verdicts") or [] + if verdicts: + last = verdicts[-1] + if isinstance(last, dict): + findings = str( + last.get("findings") or last.get("verdict") or "" + ).strip() + if findings: + summary = " ".join(findings.split()) + return summary[:300] + ("…" if len(summary) > 300 else "") + if not values.get("plan"): + return "the planner could not produce a usable plan (no plan was built)." + return ( + "the plan→review loop hit its revision cap (the reviewer kept " + "requesting changes without converging)." + ) + def tick(self) -> list[ResumeResult]: """One maintenance pass: deadline sweep + park policy, then drain (§3.3.1). @@ -666,7 +960,9 @@ class Coordinator: for question_id in expired: self._park(question_id) - return self.drain_resumes() + results = self.drain_resumes() + self._post_resume_followups(results) + return results def _park(self, question_id: str) -> None: """Apply the park policy to one expired question (§6.6 ALARM, not spin). @@ -764,11 +1060,20 @@ class Coordinator: or question.get("deadline") or self._default_deadline() ) + # One-thread-per-task: re-thread the re-posted question under the + # task's root message so its channel_ref is the root ts again and + # the inbound reply still maps back. Read the root ts from the live + # state (robust across the stub + live clarifier node), falling + # back to the interrupt payload. + root_ts = self._task_root_ts(thread_id) or question.get( + "slack_thread_ts" + ) responder_mod.notify_question( conn, self._transport, question_set, deadline=deadline, + thread_ts=str(root_ts) if root_ts else None, ) finally: conn.close() @@ -967,6 +1272,7 @@ class Coordinator: transport=self._transport, db_path=self._db_path, enqueue_resume=self._resume_queue.put, + new_task_callback=self._new_task_callback, ) self._listener = listener thread = threading.Thread( @@ -1089,6 +1395,22 @@ class Coordinator: # Internals. # ------------------------------------------------------------------ # + def _task_root_ts(self, thread_id: str) -> str | None: + """Return the task's Slack root-message ts (one-thread-per-task), or None. + + Reads ``slack_thread_ts`` off the live checkpointed state so milestone / + re-delivery posts can thread under the task's root "📥 Task received" + message. Best-effort: any read failure (or an empty value) yields + ``None``, so the caller falls back to a top-level post and a status read + never breaks the loop. + """ + try: + snap = self._graph.get_state(graph_mod.thread_config(thread_id)) + values = getattr(snap, "values", {}) or {} + except Exception: # noqa: BLE001 - a state read must not break delivery + return None + return str(values.get("slack_thread_ts") or "") or None + def _default_deadline(self) -> str: """Compute a fresh ISO deadline from the configured window (§3.3.1).""" from datetime import datetime, timezone diff --git a/agent-team/agent_team/graph.py b/agent-team/agent_team/graph.py index c01576e..a724ec5 100644 --- a/agent-team/agent_team/graph.py +++ b/agent-team/agent_team/graph.py @@ -86,6 +86,7 @@ __all__ = [ "BUILD_ROUTE", "CLARIFY", "DEFAULT_CLARIFY_DEADLINE", + "DISPATCH_NODE", "INTAKE", "P1_PHASE_SEQUENCE", "PARKED_ROUTE", @@ -132,6 +133,10 @@ PARKED_ROUTE = "parked" # the topology that connects the injected nodes. BUILD_NODE = "build_node" VERIFY_NODE = "verify_node" +# P3+ dispatch vertex id: the node that carries an approved diff into org CI. +# Wired by build_graph only when the caller injects a dispatch_node callable; the +# default (None) leaves APPROVED_ROUTE → END unchanged so the graph is inert. +DISPATCH_NODE = "dispatch_node" # P3 route ids returned by the injected ``route_after_verify`` function. They # mirror agent_team.nodes.build_verify_subgraph.APPROVED_ROUTE / BUILD_ROUTE / @@ -219,6 +224,10 @@ def clarify_node(state: PipelineState) -> PipelineState: turn = len(history) thread_id = state.get("thread_id", "") transport = state.get("transport", "") + # The Slack root-message ts (one-thread-per-task). Empty for non-/new-task + # origins; surfaced in the interrupt payload so the responder can thread the + # question post under the root message (and use it as the channel_ref). + slack_thread_ts = state.get("slack_thread_ts", "") # Stable across the resume replay of this node (see _question_id_for): the # id delivered at suspend == the ledger key == the qa_history entry. @@ -243,6 +252,7 @@ def clarify_node(state: PipelineState) -> PipelineState: "question_set": question_set, "transport": transport, "deadline": deadline, + "slack_thread_ts": slack_thread_ts, } ) @@ -314,6 +324,7 @@ def build_graph( Callable[[PipelineState], str], ] | None = None, + dispatch_node: Callable[[PipelineState], Any] | None = None, ) -> CompiledStateGraph: """Assemble + compile the P1 pipeline ``StateGraph`` (§3.3, §7.1). @@ -397,6 +408,13 @@ def build_graph( "is nothing to repoint without a review loop." ) + if dispatch_node is not None and build_verify is None: + raise ValueError( + "build_graph: dispatch_node requires build_verify — it repoints the " + "verifier's APPROVED_ROUTE, so there is nothing to repoint without a " + "build->verify subgraph." + ) + builder: StateGraph = StateGraph(PipelineState) builder.add_node(INTAKE, intake_node) builder.add_node(CLARIFY, clarify) @@ -437,11 +455,26 @@ def build_graph( {BUILD_ROUTE: BUILD_NODE, PLAN: PLAN, PARKED_ROUTE: END}, ) builder.add_edge(BUILD_NODE, VERIFY_NODE) - builder.add_conditional_edges( - VERIFY_NODE, - route_after_verify, - {APPROVED_ROUTE: END, BUILD_ROUTE: BUILD_NODE, PARKED_ROUTE: END}, - ) + if dispatch_node is None: + # P3 default: APPROVED_ROUTE is the terminus (no dispatch). + builder.add_conditional_edges( + VERIFY_NODE, + route_after_verify, + {APPROVED_ROUTE: END, BUILD_ROUTE: BUILD_NODE, PARKED_ROUTE: END}, + ) + else: + # P3+: repoint APPROVED_ROUTE at the dispatch node, then END. + builder.add_node(DISPATCH_NODE, dispatch_node) + builder.add_conditional_edges( + VERIFY_NODE, + route_after_verify, + { + APPROVED_ROUTE: DISPATCH_NODE, + BUILD_ROUTE: BUILD_NODE, + PARKED_ROUTE: END, + }, + ) + builder.add_edge(DISPATCH_NODE, END) if checkpointer is None: return builder.compile() @@ -563,6 +596,8 @@ def start_task( *, thread_id: str | None = None, transport: str = "", + task: str = "", + slack_thread_ts: str = "", ) -> tuple[str, PipelineState]: """Start a new pipeline task and run it up to the first human gate (§3.3). @@ -572,6 +607,20 @@ def start_task( the checkpointed snapshot after the suspend (its ``__interrupt__`` carries the pending question-set, surfaced by :func:`pending_question`). + ``task`` is the intake description (e.g. the Slack ``/new-task`` text or a + GitHub issue body). It is written into the seed ``PipelineState`` so the + clarifier can reason about it; ``intake_node`` returns only a partial state + (status/phase), so the seeded ``task`` channel persists into CLARIFY. An + empty ``task`` (the default) seeds no description — the clarifier then asks + for one. + + ``slack_thread_ts`` is the Slack root-message ``ts`` for a ``/new-task`` task + (the "📥 Task received" ack post) — when set, every clarifier question and + lifecycle notification for this task threads under it (one-thread-per-task). + Like ``task`` it is seeded into the initial invoke and persists through + INTAKE into CLARIFY (``intake_node`` returns only a partial state). Empty + (the default) for a task with no root post — posts are top-level as before. + The graph MUST be compiled with a checkpointer for the suspend to persist; an uncheckpointed graph would run straight through without honouring the interrupt. @@ -582,6 +631,8 @@ def start_task( thread_id=tid, status=TaskStatus.ACTIVE.value, current_phase=_phase_value(Phase.INTAKE), + task=task, + slack_thread_ts=slack_thread_ts, qa_history=[], transport=transport, created_at=now, diff --git a/agent-team/agent_team/invoker.py b/agent-team/agent_team/invoker.py index 58d11b3..5778173 100644 --- a/agent-team/agent_team/invoker.py +++ b/agent-team/agent_team/invoker.py @@ -49,7 +49,13 @@ API_MODEL = "claude-sonnet-4-6" # Default per-call agent budget for the headless subscription path, in USD. _DEFAULT_BUDGET_USD = 2.0 -_DEFAULT_MAX_TURNS = 40 +# The agent-team's subscription Claude calls (clarify confidence/questions, +# planner) are SINGLE-SHOT reasoning→JSON completions, NOT agentic sessions. A +# 40-turn, tool-enabled session made each call take minutes and let Claude wander +# (use tools / explore) and return output the planner/clarifier couldn't parse → +# spurious parks. One turn + no tools = a fast, deterministic completion. A +# genuinely agentic caller (e.g. a future fixer) overrides max_turns/allowed_tools. +_DEFAULT_MAX_TURNS = 1 # --------------------------------------------------------------------------- # @@ -104,6 +110,10 @@ async def _collect_subscription_text( model=model, max_turns=max_turns, max_budget_usd=budget_usd, + # No tools: these are pure reasoning→JSON completions. Disallowing tools + # keeps the call a single deterministic turn (no repo exploration / tool + # loops that produce slow, unparseable output). + allowed_tools=[], ) texts: list[str] = [] diff --git a/agent-team/agent_team/invoker_multi.py b/agent-team/agent_team/invoker_multi.py new file mode 100644 index 0000000..384c085 --- /dev/null +++ b/agent-team/agent_team/invoker_multi.py @@ -0,0 +1,170 @@ +"""In-process invokers for non-Claude models (GPT-4.1, DeepSeek, Gemini) — WS1. + +Mirrors the pattern of :mod:`agent_team.invoker` (which supplies Claude invokers +for the billing seam) but targets the **non-Claude** model seams: + +* :func:`agent_team.nodes.review_loop.set_review_invoker` — GPT-4.1 + (``cross_reviewer``) +* :func:`agent_team.nodes.builders_llm.default_build` is replaced in-process + by binding :func:`make_fast_coder_invoker` as the builder + +Each factory lazily imports the root orchestrator's ``models`` module (never at +module load) so importing this file on a machine where ``models.py`` is absent +(CI, Mac dev, tests) does not raise :class:`ModuleNotFoundError`. The ``models`` +module itself is kept out of the agent-team package graph; we reach it at runtime +through the orchestrator root's ``sys.path`` entry that ``run-team.py`` and the +graph module already bootstrap. + +Model keys accepted by :func:`multi_invoke`: + +``cross_reviewer`` + GPT-4.1 via ``models.get_cross_reviewer()``. The §3.2 cross-family reviewer. +``fast_coder`` + DeepSeek via ``models.get_fast_coder()``. The P3 mechanical-edit builder. +``scanner`` + Gemini via ``models.get_scanner()``. The P3 security-scan verifier. + +Security +-------- +All model outputs returned by :func:`multi_invoke` are UNTRUSTED. Callers are +responsible for defensive parsing before acting on the result. This module +performs no parsing — it returns the raw model response text and leaves the +fail-safe discipline to the caller (review_loop_llm.parse_verdict, +builders_llm._extract_diff, etc.). +""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from pathlib import Path +from typing import Any + +__all__ = [ + "MODELS", + "MultiInvoker", + "bind_multi_invoker", + "make_fast_coder_invoker", + "make_scanner_invoker", + "multi_invoke", +] + +# The set of model keys this module can dispatch to. +MODELS: frozenset[str] = frozenset({"cross_reviewer", "fast_coder", "scanner"}) + +# Callable type: (prompt, **kw) -> str — same shape as ReviewInvoker and BuildCallable. +MultiInvoker = Callable[..., str] + + +def _ensure_orchestrator_on_path() -> None: + """Add the orchestrator root to sys.path if absent. + + ``models.py`` lives at the orchestrator root (not inside agent-team). This + file is at ``/agent-team/agent_team/invoker_multi.py``, so the root + is ``parents[2]``. Mirroring the bootstrap in ``run-team.py`` ensures the + deferred import in :func:`_load_model` succeeds when ``multi_invoke`` is + called in a process that has NOT already bootstrapped ``sys.path``. + """ + root = str(Path(__file__).resolve().parents[2]) + if root not in sys.path: + sys.path.insert(0, root) + + +def _load_model(key: str) -> Any: + """Lazily import the orchestrator ``models`` module and return the model. + + ``key`` must be one of :data:`MODELS`. Raises :class:`ValueError` on an + unknown key and :class:`RuntimeError` when the ``models`` module is + unavailable (the orchestrator is not on ``sys.path``). + + Deferred import keeps this module importable everywhere the orchestrator + root is absent (CI, tests, Mac dev). + """ + _ensure_orchestrator_on_path() + try: + import models # noqa: PLC0415 - intentional deferred import + except ImportError as exc: + raise RuntimeError( + "Cannot import orchestrator 'models' module — ensure the orchestrator " + "root is on sys.path (run-team.py bootstraps this automatically). " + "Tests should mock multi_invoke rather than call it." + ) from exc + factories = { + "cross_reviewer": models.get_cross_reviewer, + "fast_coder": models.get_fast_coder, + "scanner": models.get_scanner, + } + if key not in factories: + raise ValueError( + f"unknown model key {key!r}; must be one of: " + ", ".join(sorted(MODELS)) + ) + return factories[key]() + + +def multi_invoke(prompt: str, *, model: str, **kw: Any) -> str: + """Invoke a non-Claude model in-process and return its response text. + + ``model`` must be one of :data:`MODELS`. The underlying model is loaded + lazily via :func:`_load_model` so the orchestrator's ``models`` module is + not imported until first call. Any call error propagates to the caller, which + is responsible for fail-safe handling (e.g. :func:`review_plan` catches all + exceptions and returns REQUEST_CHANGES). + + The response text is returned raw (UNTRUSTED); callers must parse/validate + before acting on it. + """ + model_obj = _load_model(model) + result = model_obj.invoke(prompt) + text = getattr(result, "content", result) + return text if isinstance(text, str) else str(text) + + +def make_fast_coder_invoker() -> MultiInvoker: + """Return an in-process callable that routes prompts to DeepSeek ``fast_coder``. + + Mirrors :func:`agent_team.nodes.review_loop_llm.make_cross_reviewer_invoker` + for the builder seam. The callable matches :data:`~agent_team.nodes.builders_llm.BuildCallable` + ``(instruction: str) -> str`` and is suitable for passing as the ``build`` + kwarg to :func:`agent_team.nodes.builders_llm.build_candidate_diff`. + """ + + def _invoke(instruction: str, **_kw: Any) -> str: + return multi_invoke(instruction, model="fast_coder") + + return _invoke + + +def make_scanner_invoker() -> MultiInvoker: + """Return an in-process callable that routes prompts to Gemini ``scanner``. + + The callable is suitable for injection into the verifier's scan seam. + """ + + def _invoke(prompt: str, **_kw: Any) -> str: + return multi_invoke(prompt, model="scanner") + + return _invoke + + +def bind_multi_invoker() -> None: + """Bind in-process non-Claude invokers into the review and builder seams. + + Call once at startup (alongside :func:`agent_team.invoker.bind_subscription_invoker`) + to replace the subprocess-based defaults with in-process model calls: + + * ``review_loop.set_review_invoker`` → in-process GPT-4.1 via + :func:`~agent_team.nodes.review_loop_llm.make_cross_reviewer_invoker` + * Builder default (``builders_llm.default_build``) is already replaced at + module load in this build; :func:`make_fast_coder_invoker` is the callable + for explicit injection into :func:`~agent_team.nodes.builders_llm.build_candidate_diff` + when needed. + + Lazy-imported so importing ``invoker_multi`` has no global side effects and + tests that never call ``bind_multi_invoker`` remain model-free. + """ + from agent_team.nodes import review_loop # noqa: PLC0415 + from agent_team.nodes.review_loop_llm import ( # noqa: PLC0415 + make_cross_reviewer_invoker, + ) + + review_loop.set_review_invoker(make_cross_reviewer_invoker()) diff --git a/agent-team/agent_team/nodes/builders_llm.py b/agent-team/agent_team/nodes/builders_llm.py index 30701d6..812ddfd 100644 --- a/agent-team/agent_team/nodes/builders_llm.py +++ b/agent-team/agent_team/nodes/builders_llm.py @@ -61,6 +61,8 @@ __all__ = [ "as_diff_builder", "build_candidate_diff", "default_build", + "make_fast_coder_invoker", + "subprocess_build", ] # The injectable build seam: given the rendered build instruction (a string), @@ -154,24 +156,48 @@ def _orchestrator_root() -> Path: return Path(__file__).resolve().parents[3] -def default_build(instruction: str, *, route: _OrchestratorRoute | None = None) -> str: - """Default :data:`BuildCallable`: route the build to DeepSeek ``fast_coder``. +def make_fast_coder_invoker() -> "BuildCallable": + """Return an in-process :data:`BuildCallable` backed by DeepSeek ``fast_coder``. - Calls the local orchestrator out-of-process — ``python3 /run.py - ""`` — and returns its stdout. The orchestrator routes a - well-specified coding task to its ``fast_coder`` agent (DeepSeek); this is - the design's "DeepSeek mechanical edits, via the local orchestrator" path, - deliberately NOT :func:`agent_team.billing.claude_invoke`. + Lazy-imports the orchestrator ``models`` module (never at module load) to + call ``models.get_fast_coder()`` in-process. Mirrors + :func:`~agent_team.nodes.review_loop_llm.make_cross_reviewer_invoker` for + the builder seam (WS1). Any error propagates to the caller + (:func:`build_candidate_diff`), which wraps errors as a ``no_op`` result. + """ - No orchestrator module is imported at module top (deferred, mirroring - :func:`agent_team.graph.build_sqlite_checkpointer`); the call is a plain - subprocess so this binding adds no import-time dependency on the - orchestrator's package graph. + def _invoke(instruction: str) -> str: + # Bootstrap the orchestrator root onto sys.path (models.py lives there; + # the run-team serve daemon does not add it) before the deferred import, + # mirroring make_cross_reviewer_invoker. Without it `from models import` + # raises ModuleNotFoundError in the daemon. + from agent_team.invoker_multi import _ensure_orchestrator_on_path # noqa: PLC0415 - SECURITY: this subprocess only ASKS the model for diff text — it is a model - invocation, not a patch application. It does not run ``git``, does not apply - anything, and does not touch the target repo. Its stdout is untrusted input - handed back to :func:`build_candidate_diff` for defensive parsing. + _ensure_orchestrator_on_path() + from models import get_fast_coder # noqa: PLC0415 - intentional deferred import + + coder = get_fast_coder() + result = coder.invoke(instruction) + text = getattr(result, "content", result) + return text if isinstance(text, str) else str(text) + + return _invoke + + +def subprocess_build( + instruction: str, *, route: _OrchestratorRoute | None = None +) -> str: + """Opt-in fallback :data:`BuildCallable`: shell out to ``run.py fast_coder``. + + The original subprocess-based build path, retained as an opt-in alternative + to the in-process default (:func:`make_fast_coder_invoker`). Use this in + environments where the orchestrator's ``models`` stack is unavailable or for + debugging. + + SECURITY: this subprocess only ASKS the model for diff text — it does not + run ``git``, apply anything, or touch the target repo. Its stdout is + untrusted input handed back to :func:`build_candidate_diff` for defensive + parsing. """ route = ( route if route is not None else _OrchestratorRoute(root=_orchestrator_root()) @@ -188,6 +214,26 @@ def default_build(instruction: str, *, route: _OrchestratorRoute | None = None) return completed.stdout +def default_build(instruction: str, *, route: _OrchestratorRoute | None = None) -> str: + """Default :data:`BuildCallable`: route the build to DeepSeek ``fast_coder`` in-process. + + Calls DeepSeek in-process via the orchestrator's ``models.get_fast_coder()`` + (WS1). The subprocess path is available as :func:`subprocess_build` for + environments where the models stack is absent. + + No orchestrator module is imported at module top (deferred, mirroring + :func:`agent_team.graph.build_sqlite_checkpointer`); any import error + propagates to the caller (:func:`build_candidate_diff`) which wraps it as a + ``no_op`` result so the builder fails SAFE. + + SECURITY: this call only ASKS the model for diff text — it is a model + invocation, not a patch application. The returned text is untrusted and must + be defensively parsed by the caller. + """ + invoker = make_fast_coder_invoker() + return invoker(instruction) + + def _render_build_instruction(plan: Mapping[str, Any], state: Mapping[str, Any]) -> str: """Render the approved plan into a mechanical-edit instruction for fast_coder. diff --git a/agent-team/agent_team/nodes/clarifier_llm.py b/agent-team/agent_team/nodes/clarifier_llm.py index 32459cd..68b5fd4 100644 --- a/agent-team/agent_team/nodes/clarifier_llm.py +++ b/agent-team/agent_team/nodes/clarifier_llm.py @@ -44,6 +44,11 @@ from agent_team.nodes.clarifier import ( ) from agent_team.task_model import PipelineState +# Optional context-provider callable: () -> str. When injected, the returned +# string (e.g. memory + handbook summary) is prepended to the clarifier prompt +# before the system instruction. When None (default), behavior is unchanged. +ContextProvider = Callable[[], str] + __all__ = [ "FALLBACK_QUESTION", "ClaudeClarifier", @@ -130,12 +135,16 @@ class ClaudeClarifier: config: Any = None, system: str = _DEFAULT_SYSTEM, confidence_threshold: float = DEFAULT_CONFIDENCE_THRESHOLD, + context_provider: ContextProvider | None = None, ) -> None: self._invoke: ClaudeInvoke = invoke if invoke is not None else claude_invoke self._model = model self._config = config self._system = system self._confidence_threshold = confidence_threshold + # Optional seam (WS5): when set, called once per turn and its result + # prepended to the prompt. Default None = byte-identical behavior. + self._context_provider = context_provider # Memo of the single per-turn call. The key is task-scoped, NOT just the # history length: one ClaudeClarifier instance serves every task through # the long-lived graph node, so a key of len(qa_history) alone would let @@ -212,7 +221,18 @@ class ClaudeClarifier: qa = _format_qa_history(qa_history) threshold_pct = int(round(self._confidence_threshold * 100)) - sections: list[str] = [ + sections: list[str] = [] + # Prepend optional memory/handbook context when a provider is injected + # (WS5). Absent provider = identical output so existing callers are + # unaffected. The provider must never raise; a failure returns "" safely. + if self._context_provider is not None: + try: + ctx = self._context_provider() + except Exception: # noqa: BLE001 + ctx = "" + if ctx: + sections += [ctx, ""] + sections += [ self._system, "", "## Task", @@ -290,6 +310,7 @@ def build_claude_clarifier_callables( config: Any = None, system: str = _DEFAULT_SYSTEM, confidence_threshold: float = DEFAULT_CONFIDENCE_THRESHOLD, + context_provider: ContextProvider | None = None, ) -> tuple[ConfidenceAssessor, QuestionGenerator]: """Build the ``(assess_confidence, generate_questions)`` pair for wiring. @@ -305,6 +326,7 @@ def build_claude_clarifier_callables( config=config, system=system, confidence_threshold=confidence_threshold, + context_provider=context_provider, ) return clarifier.assess_confidence, clarifier.generate_questions diff --git a/agent-team/agent_team/nodes/dispatch_invoker.py b/agent-team/agent_team/nodes/dispatch_invoker.py new file mode 100644 index 0000000..c0f259f --- /dev/null +++ b/agent-team/agent_team/nodes/dispatch_invoker.py @@ -0,0 +1,123 @@ +"""Dispatch-node factory: carry an approved diff into org CI (§3.3, §7.1 P3). + +:mod:`agent_team.nodes.builders` proposes the diff; +:mod:`agent_team.nodes.verifier` clears the pure-code gate; this node is the +final in-graph step that calls +:func:`agent_team.dispatcher.dispatch_apply_verify` to push the head branch and +fire ``workflow_dispatch``. + +Model/role: this node executes only when the verifier emits the +``APPROVED_ROUTE`` signal. It reads the already-validated candidate diff and +task context from graph state, assembles the dispatch inputs (``owner``/``repo`` +injected at coordinator startup), and calls the dispatcher's seams. The actual +diff was hashed and scope-checked by the verifier; this node transports — it +makes no new trust decision. + +INERT unless configured: the node is built only when the coordinator threads a +``dispatch_node_wiring`` factory through +:func:`~agent_team.coordinator.Coordinator`. With no factory, +:func:`agent_team.graph.build_graph` leaves ``APPROVED_ROUTE → END`` unchanged +and no dispatch ever fires. This keeps the default path inert and the P3 +subgraph opt-in, exactly as the verifier node. + +Fail-safe: any dispatch error (invalid state, empty diff, owner/repo +misconfigured, network/subprocess failure) parks the task rather than crashing +the graph. The verifier already validated the diff hash; a dispatch error is an +infrastructure problem, not a security bypass. +""" + +from __future__ import annotations + +import logging +from collections.abc import Callable +from typing import Any + +__all__ = ["DispatchNodeFactory", "make_dispatch_node"] + +_LOG = logging.getLogger("agent_team.nodes.dispatch_invoker") + +# A dispatch-node factory type: takes no args, returns the LangGraph node +# callable. Mirrors the other node-factory types in coordinator.py. +DispatchNodeFactory = Callable[[], "Callable[[Any], Any]"] + + +def make_dispatch_node( + *, + owner: str, + repo: str, + base: str = "main", + pusher: Any = None, + dispatcher: Any = None, +) -> Callable[[Any], Any]: + """Build a LangGraph dispatch node for ``owner``/``repo``. + + Returns a single-arg ``(state) -> dict`` node. At runtime it reads + ``thread_id``, ``candidate_diff``, and ``plan.scope`` from ``state``, then + calls :func:`agent_team.dispatcher.dispatch_apply_verify` with the injected + ``pusher``/``dispatcher`` seams (default: real git/gh subprocess paths). + + Fail-safe: any :class:`~agent_team.dispatcher.DispatcherError` or unexpected + exception parks the task (returns ``status=PARKED``); the caller retains the + full graph state, so the coordinator can ALARM and a human can inspect. + """ + # Deferred import: no orchestrator / subprocess module at module load. + from agent_team.dispatcher import DispatcherError, dispatch_apply_verify + from agent_team.task_model import Phase, TaskStatus + + def dispatch_node(state: Any) -> Any: + thread_id: str = state.get("thread_id") or "" + diff_text: str = state.get("candidate_diff") or "" + plan: Any = state.get("plan") or {} + scope_list: list[Any] = ( + plan.get("scope") or [] if isinstance(plan, dict) else [] + ) + declared_scope: str = "\n".join(str(s) for s in scope_list if s) + + _parked: dict[str, Any] = { + "status": TaskStatus.PARKED.value, + "current_phase": Phase.PARKED.value, + } + + if not thread_id or not diff_text.strip(): + _LOG.warning("dispatch_node: missing thread_id or candidate_diff; parking") + return _parked + if not declared_scope.strip(): + _LOG.warning("dispatch_node: empty declared_scope from plan; parking") + return _parked + + try: + dispatch_apply_verify( + owner=owner, + repo=repo, + task_id=thread_id, + diff_text=diff_text, + declared_scope=declared_scope, + base=base, + pusher=pusher, + dispatcher=dispatcher, + ) + except DispatcherError as exc: + _LOG.error( + "dispatch_node: DispatcherError for task %s: %s; parking", + thread_id, + exc, + ) + return _parked + except Exception as exc: # noqa: BLE001 + _LOG.error( + "dispatch_node: unexpected error for task %s (%s); parking", + thread_id, + type(exc).__name__, + ) + return _parked + + _LOG.info( + "dispatch_node: dispatched task %s to %s/%s (base=%s)", + thread_id, + owner, + repo, + base, + ) + return {} + + return dispatch_node diff --git a/agent-team/agent_team/nodes/handbook.py b/agent-team/agent_team/nodes/handbook.py new file mode 100644 index 0000000..39ae7cf --- /dev/null +++ b/agent-team/agent_team/nodes/handbook.py @@ -0,0 +1,87 @@ +"""Sea Haven engineering-handbook conventions loader (WS5 handbook seam). + +Reads a handbook directory (``SEA_HAVEN_HANDBOOK_DIR`` env or +``~/.sea-haven/engineering-handbook``) and returns a formatted summary of +its ``*.md`` convention files for optional injection into prompt contexts. + +Safe no-op contract: if the directory is absent, empty, or any read fails, +:func:`load_handbook_conventions` returns ``""`` and never raises. Callers +treat an empty return as "no handbook available" and omit the context block +rather than failing. + +The rsync that delivers the handbook from the Mac to the R720 box, and the +nightly ``.github`` pull (systemd timers), are ATTENDED box steps and are +NOT implemented here. +""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import Optional + +__all__ = [ + "SEA_HAVEN_HANDBOOK_ENV", + "load_handbook_conventions", +] + +SEA_HAVEN_HANDBOOK_ENV = "SEA_HAVEN_HANDBOOK_DIR" +_DEFAULT_HANDBOOK_DIR = Path.home() / ".sea-haven" / "engineering-handbook" + +# Maximum number of convention files to load (defense-in-depth: avoid +# accidentally summarising a huge handbook dir on first setup). +_MAX_FILES = 20 +# Max bytes per file to include in the formatted summary (truncated if larger). +_MAX_FILE_BYTES = 4096 + + +def _handbook_dir(path: Optional[Path | str]) -> Path: + """Resolve the handbook directory from an explicit path, env, or default.""" + if path is not None: + return Path(path) + env = os.environ.get(SEA_HAVEN_HANDBOOK_ENV, "").strip() + if env: + return Path(env) + return _DEFAULT_HANDBOOK_DIR + + +def load_handbook_conventions(path: Optional[Path | str] = None) -> str: + """Load engineering-handbook conventions and return a formatted summary. + + Reads all ``*.md`` files in the resolved handbook directory (up to + :data:`_MAX_FILES`) and returns a single formatted string suitable for + prepending to a model prompt. Returns ``""`` — never raises — when: + + * the directory does not exist or is not a directory; + * no ``*.md`` files are found; + * any read error occurs (the file is skipped silently). + + The returned string includes a header and one section per convention + file; callers should include it only when it is non-empty. + """ + try: + hdir = _handbook_dir(path) + if not hdir.is_dir(): + return "" + files = sorted(hdir.glob("*.md"))[:_MAX_FILES] + if not files: + return "" + sections: list[str] = [] + for fpath in files: + try: + raw = fpath.read_bytes()[:_MAX_FILE_BYTES].decode( + "utf-8", errors="replace" + ) + sections.append(f"### {fpath.stem}\n{raw.strip()}") + except Exception: # noqa: BLE001 - safe no-op + continue + if not sections: + return "" + header = ( + "## Sea Haven engineering-handbook conventions\n" + "These conventions are from the engineering handbook. Apply them " + "when planning or clarifying. They may not cover every scenario." + ) + return header + "\n\n" + "\n\n---\n\n".join(sections) + except Exception: # noqa: BLE001 - safe no-op in all failure modes + return "" diff --git a/agent-team/agent_team/nodes/planner.py b/agent-team/agent_team/nodes/planner.py index 820a1ca..6d55b65 100644 --- a/agent-team/agent_team/nodes/planner.py +++ b/agent-team/agent_team/nodes/planner.py @@ -34,11 +34,16 @@ from __future__ import annotations import json import re +from collections.abc import Callable from typing import Any from agent_team.billing import ClaudeResult, claude_invoke from agent_team.task_model import Phase, PipelineState, TaskStatus +# Optional context-provider callable (WS5): () -> str. When injected, its +# result is prepended to the planner prompt. Default None = unchanged behavior. +ContextProvider = Callable[[], str] + __all__ = [ "MAX_PLAN_REVISIONS", "PlannerError", @@ -117,28 +122,57 @@ def _format_review_feedback(review_verdicts: list[Any]) -> str: lines: list[str] = [] for idx, verdict in enumerate(review_verdicts, start=1): if isinstance(verdict, dict): - decision = str(verdict.get("decision", "")).strip() - notes = str(verdict.get("notes") or verdict.get("comment") or "").strip() + # The review stage (review_loop.ReviewResult.to_dict) writes the keys + # "verdict" (decision) and "findings" (the reviewer's objections). + # Read those FIRST — the old "decision"/"notes"/"comment" keys never + # existed on a real verdict, so the planner re-planned with EMPTY + # feedback and kept re-introducing the rejected flaw ("assumptions + # persist" → cap → park). Old keys kept as fallbacks for safety. + decision = str( + verdict.get("verdict") or verdict.get("decision") or "" + ).strip() + notes = str( + verdict.get("findings") + or verdict.get("notes") + or verdict.get("comment") + or "" + ).strip() lines.append(f"Review {idx} [{decision}]: {notes}".rstrip()) else: lines.append(f"Review {idx}: {str(verdict).strip()}") return "\n".join(lines) -def build_plan_prompt(state: PipelineState) -> str: +def build_plan_prompt( + state: PipelineState, *, context_provider: ContextProvider | None = None +) -> str: """Build the Claude prompt that drafts (or re-drafts) the phased plan. Pure string assembly over the graph state — no I/O — so the prompt shape is directly unit-testable. On a loop-back (``review_verdicts`` present) the prompt instructs Claude to revise the prior plan against the feedback rather than start from scratch. + + ``context_provider`` is an optional WS5 injection seam. When set, it is + called once and its result prepended to the prompt. Default None = identical + behavior so existing callers are unaffected. """ description = _task_description(state) qa = _format_qa_history(list(state.get("qa_history", []))) feedback = _format_review_feedback(list(state.get("review_verdicts", []))) prior_plan = state.get("plan") - sections = [ + sections: list[str] = [] + # Optional memory/handbook context prepended when provider is injected (WS5). + if context_provider is not None: + try: + ctx = context_provider() + except Exception: # noqa: BLE001 + ctx = "" + if ctx: + sections += [ctx, ""] + + sections += [ "You are the PLANNER stage of an agentic SDLC pipeline. Produce a " "phased implementation plan for the task below. The clarifier has " "already reached confidence with the human, so do not ask questions — " @@ -252,6 +286,8 @@ def _revision_count(state: PipelineState) -> int: def plan_node( state: PipelineState, config: dict[str, Any] | None = None, + *, + context_provider: ContextProvider | None = None, ) -> PipelineState: """LangGraph node: draft/refine the phased plan, then advance to REVIEW. @@ -280,7 +316,7 @@ def plan_node( current_phase=Phase.PARKED.value, ) - prompt = build_plan_prompt(state) + prompt = build_plan_prompt(state, context_provider=context_provider) result: ClaudeResult = claude_invoke(prompt, config=config) plan = parse_plan(result.text) # Record how many times we have planned so review/observability can see it. diff --git a/agent-team/agent_team/nodes/review_loop_llm.py b/agent-team/agent_team/nodes/review_loop_llm.py index 1a9a8ed..50c4d62 100644 --- a/agent-team/agent_team/nodes/review_loop_llm.py +++ b/agent-team/agent_team/nodes/review_loop_llm.py @@ -210,6 +210,14 @@ def make_cross_reviewer_invoker() -> PlanReviewer: """ def _invoke(prompt: str, *, config: Any = None, **_kw: Any) -> str: + # The orchestrator root (where models.py lives) is NOT on sys.path in + # the run-team serve daemon (it only bootstraps agent-team/). Without + # this, `from models import` raises ModuleNotFoundError → review_plan's + # blanket except silently fails-closed to REQUEST_CHANGES, so GPT-4.1 + # never actually runs. Bootstrap the root before the deferred import. + from agent_team.invoker_multi import _ensure_orchestrator_on_path # noqa: PLC0415 + + _ensure_orchestrator_on_path() # Deferred import: keep the orchestrator package out of module import. from models import get_cross_reviewer # noqa: PLC0415 @@ -221,10 +229,12 @@ def make_cross_reviewer_invoker() -> PlanReviewer: return _invoke -# The default reviewer: subprocess to run.py (the design's "reuse run.py in -# place" path). Built once; resolution of run.py / timeout still happens per -# call so config and env overrides apply. -default_plan_reviewer: PlanReviewer = make_run_py_invoker() +# The default reviewer: in-process GPT-4.1 (WS1). The subprocess path is kept +# as an opt-in fallback via make_run_py_invoker(). The in-process path avoids +# the subprocess fork overhead and resolves run.py location ambiguity; the +# subprocess path remains available for environments where the orchestrator +# models stack is absent or for debugging. +default_plan_reviewer: PlanReviewer = make_cross_reviewer_invoker() def review_plan( diff --git a/agent-team/agent_team/responder.py b/agent-team/agent_team/responder.py index 96333ac..997004c 100644 --- a/agent-team/agent_team/responder.py +++ b/agent-team/agent_team/responder.py @@ -156,6 +156,7 @@ def notify_question( *, deadline: str, posted_at: str | None = None, + thread_ts: str | None = None, ) -> str | None: """Deliver a question-set: write the ledger row ``open`` first, then post. @@ -177,6 +178,16 @@ def notify_question( The row is inserted with the question's identity (``thread_id``, ``turn``, ``transport`` name) so the turn guard and reconcile can act on it. + + ``thread_ts`` (one-thread-per-task, Slack) — when set, the question is posted + as a THREADED REPLY under that root message ``ts`` (the task's + "📥 Task received" ack post) AND the row's durable ``channel_ref`` is set to + that SAME root ``ts`` (NOT the posted reply's own ``ts``). This is what makes + the answer-mapping unchanged: a human reply in the root thread carries + ``thread_ts == root_ts``, and ``find_open_question_by_channel_ref(thread_ts)`` + resolves it to this task's currently-open question. When ``None`` the + question is posted top-level and the ``channel_ref`` is the posted message's + own ``ts`` exactly as before. """ stamp = posted_at or _utc_now_iso() transport_name = type(transport).__name__ @@ -197,19 +208,29 @@ def notify_question( ) # 2. Side-effecting post. A failure here is recoverable (row stays open, - # no ref) — do NOT let it bubble up and lose the durable row. + # no ref) — do NOT let it bubble up and lose the durable row. ``thread_ts`` + # is only forwarded when set, so non-threading transports keep their + # existing call shape. try: - channel_ref = transport.post_question( + post_kwargs: dict[str, Any] = {} + if thread_ts: + post_kwargs["thread_ts"] = thread_ts + posted_ref = transport.post_question( thread_id=question_set.thread_id, question_id=question_set.question_id, turn=question_set.turn, question_set=question_set, deadline=deadline, + **post_kwargs, ) except Exception: return None - # 3. Persist the ref so reconcile/recovery can act on the post. + # 3. Persist the ref so reconcile/recovery can act on the post. When the post + # threaded under a root message, the durable channel_ref is the ROOT ts + # (so an inbound reply's thread_ts maps back to this question via + # find_open_question_by_channel_ref), NOT the posted reply's own ts. + channel_ref = thread_ts if thread_ts else posted_ref conn.execute( "UPDATE pending_questions SET channel_ref=? WHERE question_id=?", (channel_ref, question_set.question_id), diff --git a/agent-team/agent_team/task_model.py b/agent-team/agent_team/task_model.py index 2e09a91..7a26a03 100644 --- a/agent-team/agent_team/task_model.py +++ b/agent-team/agent_team/task_model.py @@ -87,6 +87,11 @@ class TaskRecord: thread_id: str status: TaskStatus current_phase: Phase + # Intake task description (mirrors PipelineState.task). + task: str = "" + # Slack root-message ts for one-thread-per-task (mirrors + # PipelineState.slack_thread_ts). Empty for non-/new-task origins. + slack_thread_ts: str = "" qa_history: list[Any] = field(default_factory=list) plan: dict[str, Any] | None = None review_verdicts: list[Any] = field(default_factory=list) @@ -108,6 +113,16 @@ class PipelineState(TypedDict, total=False): thread_id: str status: str current_phase: str + # The intake task description (Slack /new-task text, GitHub issue body, etc.). + # Seeded by graph.start_task and read by the clarifier/planner; a first-class + # channel so the seeded value persists across node transitions. + task: str + # The Slack root-message ``ts`` for a /new-task task (the "📥 Task received" + # ack post). All of the task's clarifier questions and lifecycle milestone + # notifications thread under this ``ts`` so one task maps to one Slack thread. + # Empty/absent for a task that did not originate from /new-task (no root post), + # in which case posts are top-level exactly as before. + slack_thread_ts: str qa_history: list[Any] plan: dict[str, Any] | None review_verdicts: list[Any] @@ -133,6 +148,8 @@ def task_from_dict(data: dict[str, Any]) -> TaskRecord: thread_id=data["thread_id"], status=TaskStatus(data["status"]), current_phase=Phase(data["current_phase"]), + task=data.get("task", ""), + slack_thread_ts=data.get("slack_thread_ts", ""), qa_history=list(data.get("qa_history", [])), plan=data.get("plan"), review_verdicts=list(data.get("review_verdicts", [])), diff --git a/agent-team/agent_team/transport/base.py b/agent-team/agent_team/transport/base.py index 015cfda..2779816 100644 --- a/agent-team/agent_team/transport/base.py +++ b/agent-team/agent_team/transport/base.py @@ -84,6 +84,7 @@ class Transport(ABC): turn: int, question_set: QuestionSet, deadline: str, + thread_ts: str | None = None, ) -> str: """Deliver ``question_set`` and return its ``channel_ref``. @@ -93,6 +94,12 @@ class Transport(ABC): ``channel_ref`` is the transport's locator for the post (Slack message ``ts`` / issue-comment id / Claude session id) and is stored on the ledger row so reconcile/recovery can act on it (§3.3.1). + + ``thread_ts`` is an OPTIONAL transport-specific threading hint (Slack's + one-thread-per-task: post the message as a reply under that root ``ts``). + Transports without native threading may ignore it. The responder only + forwards it when set, so a transport that does not accept it is never + called with it. """ raise NotImplementedError diff --git a/agent-team/agent_team/transport/slack_adapter.py b/agent-team/agent_team/transport/slack_adapter.py index 5c0f7c3..417c33b 100644 --- a/agent-team/agent_team/transport/slack_adapter.py +++ b/agent-team/agent_team/transport/slack_adapter.py @@ -217,6 +217,19 @@ class SlackTransport(Transport): self.channel = channel self._poster: SlackPoster = poster if poster is not None else _default_poster + @property + def poster(self) -> SlackPoster: + """The injected network seam (the ``chat.postMessage`` callable). + + Exposed read-only so collaborators sharing this transport (e.g. the + inbound :class:`~agent_team.transport.slack_listener.SlackListener`) can + post auxiliary messages — the root "📥 Task received" ack and the 👍 + reaction-bearing posts — through the SAME poster the question delivery + uses, instead of constructing a second client. The foundation default + still refuses the network (no poster configured). + """ + return self._poster + def post_question( self, *, @@ -225,12 +238,23 @@ class SlackTransport(Transport): turn: int, question_set: QuestionSet, deadline: str, + thread_ts: str | None = None, ) -> str: """Render + post the question-set; return the Slack ``ts`` channel_ref. Embeds ``question_id`` in the message ``callback_id`` so an inbound answer maps back (§3.3.1). On any poster failure raises :class:`SlackPostError` so the ledger row stays ``open`` for reconcile. + + ``thread_ts`` (one-thread-per-task) — when set, the message is posted as + a THREADED REPLY under that root ``ts`` (the task's "📥 Task received" + ack post), so every clarifier question for a task lands in one Slack + thread. When ``None`` (the default, e.g. a task that did not originate + from ``/new-task``) the message is posted top-level exactly as before. + The returned value is still the POSTED message's own ``ts``; the caller + (:func:`agent_team.responder.notify_question`) is what records the + durable ``channel_ref`` (it uses the root ``thread_ts`` when threading so + an inbound reply's ``thread_ts`` maps back to this question). """ blocks = build_question_blocks(question_set, deadline) message: dict[str, Any] = { @@ -250,6 +274,11 @@ class SlackTransport(Transport): }, }, } + # Thread under the task's root message when one exists (one thread per + # task). Only set the key when non-empty so the top-level-post behavior + # is byte-identical for non-/new-task origins. + if thread_ts: + message["thread_ts"] = thread_ts try: response = self._poster(message) diff --git a/agent-team/agent_team/transport/slack_listener.py b/agent-team/agent_team/transport/slack_listener.py index cc73b34..685b86d 100644 --- a/agent-team/agent_team/transport/slack_listener.py +++ b/agent-team/agent_team/transport/slack_listener.py @@ -78,14 +78,37 @@ from collections.abc import Mapping from pathlib import Path from typing import Any +from collections.abc import Callable + from agent_team.db.schema import connect, find_open_question_by_channel_ref from agent_team.responder import AnswerOutcome, EnqueueResume, submit_answer -from agent_team.transport.slack_adapter import SlackTransport +from agent_team.transport.slack_adapter import SlackPoster, SlackTransport __all__ = [ + "NewTaskCallback", + "Reactor", "SlackListener", ] +# Injectable callback for /new-task slash commands: receives +# (task_text, transport, slack_thread_ts) and returns the minted thread_id. +# ``slack_thread_ts`` is the root "📥 Task received" message ts the listener +# posted before starting the task (one-thread-per-task); the callback seeds it +# into the graph state so every later question/notification threads under it. +# Injected at coordinator startup so the listener is testable with no +# coordinator and no graph. +NewTaskCallback = Callable[[str, str, str], str] + +# A best-effort 👍-reaction adder: given (channel, message_ts), add a reaction so +# the human sees the machine received the inbound message. Returns nothing; any +# failure (missing scope, deleted message, transport error) must be tolerated by +# the caller. Injected so the listener is testable with a fake recorder and no +# live WebClient. +Reactor = Callable[[str, str], None] + +# The Slack slash command that starts a new pipeline task. +_NEW_TASK_COMMAND = "/new-task" + _LOG = logging.getLogger(__name__) # Discriminating Slack event ``type`` values that can carry an answer for the @@ -119,6 +142,17 @@ _ANSWER_BEARING_TYPES: frozenset[str] = frozenset( _EVENT_CALLBACK_TYPE = "event_callback" +def _is_new_task_command(raw_payload: Mapping[str, Any]) -> bool: + """Return ``True`` iff this payload is a ``/new-task`` slash command. + + Slash-command payloads carry ``{"type": "slash_commands", "command": "/new-task", ...}``. + """ + return ( + raw_payload.get("type") == "slash_commands" + and raw_payload.get("command") == _NEW_TASK_COMMAND + ) + + class SlackListener: """Socket Mode inbound listener that drives answers into the responder. @@ -139,6 +173,21 @@ class SlackListener: may answer. If ``None``/empty the listener FAILS CLOSED and rejects every answer; :meth:`serve` sources it from ``AGENT_TEAM_SLACK_OWNER_IDS`` when not injected. + * ``new_task_callback`` — optional :data:`NewTaskCallback`; when set, the + listener handles ``/new-task `` slash commands by posting a + root "📥 Task received" ack message (one-thread-per-task) and calling it + with ``(task_text, "slack", root_ts)``, returning ``None`` (the task is + started; the owner will receive clarifying questions threaded under the + root). When ``None`` (the default), ``/new-task`` commands are ignored. + * ``poster`` — optional :data:`~agent_team.transport.slack_adapter.SlackPoster` + used ONLY to post the root "📥 Task received" ack for ``/new-task``. + Defaults to the injected ``transport``'s own poster so the ack goes through + the same client as the questions. Never used for answers. + * ``reactor`` — optional :data:`Reactor`; when set, the listener adds a 👍 + reaction to an inbound message it acted on (a thread-reply answer) AFTER + the AUTHZ-01 owner check passes, best-effort. ``None`` (the default) means + no reaction is attempted. Requires the ``reactions:write`` bot scope; until + that is granted the reactor silently no-ops, which the listener tolerates. The listener never resumes the graph; it only normalizes, submits, and enqueues. See the module SECURITY note for the trust boundary. @@ -153,6 +202,9 @@ class SlackListener: app_token: str | None = None, bot_token: str | None = None, owner_ids: set[str] | None = None, + new_task_callback: NewTaskCallback | None = None, + poster: SlackPoster | None = None, + reactor: Reactor | None = None, ) -> None: self._transport = transport self._db_path = Path(db_path) @@ -162,6 +214,13 @@ class SlackListener: # The owner allowlist (AUTHZ-01). An empty set is the fail-closed default: # an unconfigured deploy rejects every answer. self._owner_ids: set[str] = set(owner_ids) if owner_ids else set() + # Injected /new-task callback (opt-in). None = ignore new-task commands. + self._new_task_callback = new_task_callback + # Poster for the root "📥 Task received" ack. Defaults to the transport's + # own poster so the ack uses the same client as the question posts. + self._poster: SlackPoster = poster if poster is not None else transport.poster + # 👍-reaction adder (opt-in). None = no reaction attempted. + self._reactor = reactor # The live Socket Mode handler, retained by :meth:`serve` so :meth:`close` # can stop it cleanly on daemon shutdown. ``None`` until ``serve`` opens # the socket. @@ -215,6 +274,21 @@ class SlackListener: if not self._is_authorized(raw_payload): return None + # /new-task intake path: handle BEFORE the answer path so a new-task + # slash command is never misrouted as an answer-to-a-question. Auth has + # already cleared above (AUTHZ-01), so only allowlisted owners can start + # tasks. The callback is opt-in; if not injected, /new-task is ignored. + # (No 👍 reaction here: a slash command has no reactable message; its ack + # is the "📥 Task received" root post instead.) + if _is_new_task_command(raw_payload): + return self._handle_new_task_command(raw_payload) + + # 👍-acknowledge the inbound message the machine is acting on (an answer + # in a task thread). Runs AFTER AUTHZ-01 (a non-owner message above + # already returned None, so this never reacts to an unauthorized sender) + # and is best-effort: a reaction failure must never break handle_event. + self._maybe_react(raw_payload) + # ``submit_answer`` calls ``transport.parse_answer`` internally, which # raises ValueError when no question_id is recoverable. A real free-text # thread reply carries no callback_id / question_id / metadata, so its @@ -271,6 +345,130 @@ class SlackListener: ) return outcome + def _handle_new_task_command( + self, raw_payload: Mapping[str, Any] + ) -> AnswerOutcome | None: + """Route a ``/new-task`` slash command to the injected start-task callback. + + Extracts the task description from ``text`` (the words after the command + name). If no ``new_task_callback`` is configured, logs and returns + ``None`` (ignore). Otherwise: + + 1. Posts an immediate ROOT "📥 Task received" ack message to the channel + (one-thread-per-task) and captures its ``ts`` (``root_ts``). This is + the instant acknowledgement (a slash command has no reactable message, + so this post IS its 👍). A post failure degrades to ``root_ts=""`` so + the task still starts (its questions then post top-level). + 2. Calls ``new_task_callback(task_text, "slack", root_ts)`` so the task is + seeded with the root ts and every clarifier question + lifecycle + notification threads under it. + + Returns ``None`` (the task is started; the owner receives clarifying + questions via the transport; there is no ``AnswerOutcome`` here). Any + exception raised by the callback is caught and logged; the listen loop + stays alive. + """ + if self._new_task_callback is None: + _LOG.debug( + "/new-task received but no new_task_callback configured; ignoring" + ) + return None + + text = str(raw_payload.get("text") or "").strip() + if not text: + _LOG.info("/new-task received with empty description; ignoring") + return None + + # 1. Instant root ack (one-thread-per-task). Best-effort: a failed post + # yields root_ts="" so the task still starts (top-level questions). + root_ts = self._post_task_received(text) + + try: + thread_id = self._new_task_callback(text, "slack", root_ts) + _LOG.info( + "new task started via /new-task: thread_id=%s root_ts=%s task=%r", + thread_id, + root_ts or "(none)", + text[:80], + ) + except Exception: # noqa: BLE001 - keep the listen loop alive + _LOG.warning( + "new_task_callback raised for /new-task (task=%r); ignored", + text[:80], + exc_info=True, + ) + return None + + def _post_task_received(self, description: str) -> str: + """Post the root "📥 Task received" ack to the channel; return its ``ts``. + + The instant acknowledgement for a ``/new-task`` command and the anchor for + one-thread-per-task: every clarifier question and lifecycle notification + threads under the returned ``ts``. Posts through the shared poster (the + same client the question delivery uses) to the transport's channel. + + Best-effort: any failure (no poster configured, transport error, missing + ``ts`` in the response) is swallowed and an empty string returned, so a + post problem never blocks task start — the task simply runs with + top-level (un-threaded) questions. + """ + try: + response = self._poster( + { + "channel": self._transport.channel, + "text": ( + f'📥 Task received: "{description}" ' + "— starting (clarifying first)…" + ), + } + ) + except Exception: # noqa: BLE001 - a failed ack must not block task start + _LOG.warning( + "failed to post '📥 Task received' root ack; task starts un-threaded", + exc_info=True, + ) + return "" + if not isinstance(response, Mapping): + return "" + ts = response.get("ts") + if not ts: + message = response.get("message") + if isinstance(message, Mapping): + ts = message.get("ts") + return str(ts) if ts else "" + + def _maybe_react(self, raw_payload: Mapping[str, Any]) -> None: + """Add a 👍 reaction to the inbound message the machine is acting on. + + Best-effort acknowledgement that the inbound answer was received. Only + fires when a ``reactor`` is configured and the payload is an Events API + message/mention carrying a channel + message ``ts`` (the reactable + thread-reply answer). MUST be called only AFTER AUTHZ-01 has passed (the + caller guarantees this), so a non-owner message is never reacted to. + + Wrapped end-to-end: a missing scope (``reactions:write`` not yet granted), + a deleted message, or any transport error is swallowed — a reaction + failure must never break :meth:`handle_event` or the listen loop. + """ + if self._reactor is None: + return + event = _inner_event(raw_payload) + # Only react to a real inbound message/mention (the thread-reply answer + # shape). Interactive/slash payloads have no reactable message ts here. + channel = event.get("channel") + ts = event.get("ts") + if not channel or not ts: + return + try: + self._reactor(str(channel), str(ts)) + except Exception: # noqa: BLE001 - a reaction failure must never break handling + _LOG.debug( + "👍 reaction add failed (channel=%s ts=%s); ignoring " + "(reactions:write may not be granted yet)", + channel, + ts, + ) + def _is_authorized(self, raw_payload: Mapping[str, Any]) -> bool: """Return ``True`` iff the payload's sender is an allowlisted owner. @@ -446,6 +644,21 @@ class SlackListener: def _on_mention(body: Mapping[str, Any]) -> None: _forward(body) + # Slash commands (e.g. /new-task) MUST be ack()'d within ~3s or Slack + # shows "the app did not respond". Bolt delivers the INNER command + # payload (command / text / user_id / channel_id) WITHOUT the Socket + # Mode envelope's ``type``, so re-stamp ``type: "slash_commands"`` to + # match what handle_event's _is_new_task_command + _discriminating_type + # expect, then forward (handle_event runs AUTHZ-01 + the new-task + # dispatch). Ack FIRST so the 3s deadline is met even though start_task + # then runs the clarifier to the first gate synchronously. + @app.command(_NEW_TASK_COMMAND) + def _on_command(ack: Any, body: Mapping[str, Any]) -> None: + ack() + payload = dict(body) + payload.setdefault("type", "slash_commands") + _forward(payload) + handler = SocketModeHandler(app, self._app_token) self._socket_handler = handler handler.start() diff --git a/agent-team/agent_team/transport/slack_live.py b/agent-team/agent_team/transport/slack_live.py index 9f9868e..1a0b64d 100644 --- a/agent-team/agent_team/transport/slack_live.py +++ b/agent-team/agent_team/transport/slack_live.py @@ -38,7 +38,7 @@ do the question-id round-trip the adapter relies on. from __future__ import annotations import os -from collections.abc import Mapping +from collections.abc import Callable, Mapping from typing import Any from agent_team.transport.slack_adapter import SlackPoster, SlackTransport @@ -46,12 +46,15 @@ from agent_team.transport.slack_adapter import SlackPoster, SlackTransport __all__ = [ "build_live_slack_transport", "build_slack_poster", + "build_slack_reactor", ] # Top-level ``chat.postMessage`` keyword arguments the live poster forwards. # ``callback_id`` is deliberately excluded: it is not a postMessage parameter, # and the durable inbound key lives in ``metadata.event_payload`` instead. -_POST_MESSAGE_KEYS = ("channel", "text", "blocks", "metadata") +# ``thread_ts`` IS a postMessage parameter (one-thread-per-task threading) and is +# forwarded when present so a question/notification posts as a threaded reply. +_POST_MESSAGE_KEYS = ("channel", "text", "blocks", "metadata", "thread_ts") def build_slack_poster(token: str | None = None, *, client: Any = None) -> SlackPoster: @@ -86,6 +89,39 @@ def build_slack_poster(token: str | None = None, *, client: Any = None) -> Slack return _poster +def build_slack_reactor( + token: str | None = None, *, client: Any = None +) -> "Callable[[str, str], None]": + """Build a live ``slack_sdk``-backed 👍-reaction adder (one-thread-per-task UX). + + The returned ``(channel, ts) -> None`` callable performs a Slack + ``reactions.add`` (emoji ``thumbsup``) on the message at ``(channel, ts)`` so + a human sees the machine received their inbound answer. It is wired into the + inbound :class:`~agent_team.transport.slack_listener.SlackListener` (which + only calls it AFTER the AUTHZ-01 owner check passes) and is invoked + best-effort — the listener swallows any failure. + + Requires the ``reactions:write`` bot scope. Until that scope is granted (the + manifest re-applied + the app reinstalled) ``reactions.add`` fails with a + ``missing_scope`` error; this reactor lets that propagate to the listener, + which swallows it, so the reaction silently no-ops rather than breaking + answer handling. + + ``client`` (optional) injects a pre-built client for testability; any object + exposing ``reactions_add(**kwargs)`` works. When omitted, a + ``slack_sdk.WebClient`` is constructed lazily from ``token`` (falling back to + ``SLACK_BOT_TOKEN``); the deferred-import / missing-token semantics match + :func:`build_slack_poster`. + """ + if client is None: + client = _build_web_client(token) + + def _reactor(channel: str, ts: str) -> None: + client.reactions_add(channel=channel, timestamp=ts, name="thumbsup") + + return _reactor + + def build_live_slack_transport( channel: str, token: str | None = None, *, client: Any = None ) -> SlackTransport: diff --git a/agent-team/run-team.py b/agent-team/run-team.py index 2986149..d699217 100644 --- a/agent-team/run-team.py +++ b/agent-team/run-team.py @@ -61,12 +61,13 @@ from __future__ import annotations import argparse import getpass import json +import logging import os import sqlite3 import sys from datetime import datetime, timezone from pathlib import Path -from typing import Any, Sequence +from typing import Any, Callable, Sequence # ``run-team.py`` lives in ``agent-team/`` next to the importable ``agent_team`` # package. The hyphenated filename cannot itself be imported, so when run as a @@ -106,6 +107,7 @@ _DEFAULT_DB = _CLI_DIR / "state" / "agent_team.sqlite" # Default audit log for destructive actions, alongside the ledger DB. _DEFAULT_AUDIT_LOG = _CLI_DIR / "state" / "audit.log.jsonl" +_LOG = logging.getLogger("agent_team.run_team") # Columns selected for list/show rendering, in display order. _QUESTION_COLUMNS: tuple[str, ...] = ( @@ -492,6 +494,72 @@ def _cmd_supersede(args: argparse.Namespace, *, out: Any) -> int: return 0 +def _build_context_provider() -> "Callable[[], str]": + """Return the WS5 (D10) context provider: the Sea Haven handbook conventions. + + The ``context_provider`` seam is zero-arg (``Callable[[], str]``), so it + supplies STATIC context — the handbook conventions loaded from + ``SEA_HAVEN_HANDBOOK_DIR`` (or the default dir) via + :func:`agent_team.nodes.handbook.load_handbook_conventions`. That loader is + itself fail-safe (caps file count/size; returns ``""`` and never raises when + the dir is absent/empty/unreadable), so wiring it is safe even on a box where + the handbook has not been synced yet. + + (Task-keyed memory retrieval is NOT wired here: the seam takes no task text, + so it cannot form a retrieval query — that would need a task-aware seam.) + + Imported lazily for the same import-hygiene reason as the coordinator + factories (keeps ``--help`` / ledger commands import-clean). + """ + from agent_team.nodes.handbook import load_handbook_conventions + + return load_handbook_conventions + + +def _build_notifiers( + args: argparse.Namespace, +) -> "tuple[Callable[[str], None] | None, Callable[[str], None] | None]": + """Build the (notify, alarm_hook) Slack notifiers for the coordinator. + + Returns ``(None, None)`` for dry-run / non-Slack / no-channel so import, + ``--help``, ledger commands, and token-less dry runs stay silent and need no + Slack credentials. For live Slack with ``SLACK_CHANNEL_ID`` set, ``notify`` + posts a plain status line to the channel (via the same ``build_slack_poster`` + the transport uses), and ``alarm_hook`` logs the deadline-park WARNING AND + posts a parked-task notice. Both are best-effort — the coordinator wraps the + notify sink so a Slack failure never disturbs the pipeline. + """ + if getattr(args, "dry_run", False) or args.transport != "slack": + return None, None + channel = os.environ.get("SLACK_CHANNEL_ID", "") + if not channel: + return None, None + try: + from agent_team.transport.slack_live import build_slack_poster + + poster = build_slack_poster() + except Exception: # noqa: BLE001 - no token / SDK -> run without notifications + _LOG.warning( + "Slack notifier unavailable; coordinator runs without notifications" + ) + return None, None + + def notify(message: str) -> None: + poster({"channel": channel, "text": message}) + + def alarm_hook(question_id: str) -> None: + _LOG.warning("park ALARM: clarifier question %s expired", question_id) + try: + notify( + f"⚠️ Task parked: clarifier question {question_id[:8]} expired with " + "no answer in the window. Re-assign or answer to resume." + ) + except Exception: # noqa: BLE001 - notify failure must not break the park path + pass + + return notify, alarm_hook + + def _build_coordinator(args: argparse.Namespace) -> Any: """Construct a :class:`Coordinator` for the ``start`` / ``serve`` commands. @@ -507,20 +575,54 @@ def _build_coordinator(args: argparse.Namespace) -> Any: """ from agent_team.coordinator import ( Coordinator, + default_clarify_node_factory, default_plan_node_factory, default_review_wiring, ) transport = _build_transport(args) + # WS5 (D10): inject the Sea Haven handbook conventions into BOTH the clarifier + # and the planner prompts via the context_provider seam. The provider is the + # zero-arg handbook loader, which is itself fail-safe (returns "" when the + # handbook dir is absent), and the clarifier/planner additionally swallow + # provider errors — so this never affects a run where the handbook is + # unavailable. + context_provider = _build_context_provider() + + # Lifecycle notifications: post plain status lines to the Slack channel so a + # task is never a black box (parked / needs-more-input / plan-ready, and + # deadline-park ALARMs). Live-Slack only; dry-run / non-Slack / no-channel = + # silent (notify None) so import + ledger commands need no token. + notify, alarm_hook = _build_notifiers(args) + # Production runs the full P2 graph: the wrapped real planner + the bound # GPT-4.1 review loop (Plane-2 depth-first). These factories are lazy and # only build/bind the model seams when a task actually runs. - return Coordinator( + coordinator = Coordinator( db_path=args.db, transport=transport, - build_plan_node=default_plan_node_factory, + build_clarify_node=lambda: default_clarify_node_factory( + context_provider=context_provider + ), + build_plan_node=lambda: default_plan_node_factory( + context_provider=context_provider + ), review_wiring=default_review_wiring, + notify=notify, + alarm_hook=alarm_hook, ) + # WS2: an allowlisted Slack /new-task starts a task on THIS coordinator. Set + # post-construction (the adapter closes over the just-built coordinator), and + # before serve() builds the listener. AUTHZ-01 (owner allowlist) gates this + # upstream in the listener; the source label is always "slack". + coordinator.set_new_task_callback( + lambda task_text, _source, slack_thread_ts: coordinator.start_task( + task_text=task_text, + transport_name="slack", + slack_thread_ts=slack_thread_ts, + ) + ) + return coordinator def _build_transport(args: argparse.Namespace) -> Any: @@ -685,6 +787,11 @@ def _cmd_serve(args: argparse.Namespace, *, out: Any) -> int: job; this command owns the maintenance loop. Runs until interrupted. """ coordinator = _build_coordinator(args) + # Bind the in-process non-Claude invokers (GPT-4.1 review, DeepSeek build) + # alongside the Claude subscription invoker (WS1). + from agent_team.invoker_multi import bind_multi_invoker # noqa: PLC0415 + + bind_multi_invoker() print("agent-team coordinator starting (Ctrl-C to stop)", file=out) coordinator.serve() return 0 # pragma: no cover - serve() loops until interrupted diff --git a/agent-team/scripts/deploy-r720-ws-rollout.sh b/agent-team/scripts/deploy-r720-ws-rollout.sh new file mode 100755 index 0000000..aab77a6 --- /dev/null +++ b/agent-team/scripts/deploy-r720-ws-rollout.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# deploy-r720-ws-rollout.sh — attended UPDATE of the live R720 agent-team +# coordinator to the WS0–WS5 rollout (PRs #43–#46 + the activation wiring). +# +# This is an UPDATE, not a first-time provision: P1 is already deployed per +# agent-team/DEPLOY-R720.md (repo rsynced to ~/orchestrator, venv at +# agent-team/.venv, systemd unit agent-team-coordinator.service running). +# Run this from the MAC, after the WS branches have merged to main. It rsyncs +# the new code, installs the new deps, appends the new secrets if absent, syncs +# the engineering handbook, restarts the coordinator, and smoke-tests. +# +# It is idempotent and FAILS LOUDLY. It changes a live box, so: +# 1) SNAPSHOT FIRST (Hyper-V checkpoint of sh-secrev on the R720 host). +# 2) It prompts before the restart. +# +# What goes LIVE after this (the safe, ungated seams): +# * WS1 in-process multi-model invokers (bind_multi_invoker, already wired) +# * WS5 handbook context_provider injected into the planner prompt +# * WS2 Slack /new-task -> start a task (AUTHZ-01 owner allowlist gated) +# The HTTP API (WS1) + the /delegate hook are OPTIONAL and started separately +# (see step 6). The P3 dispatch/build-verify path stays INERT (gated). +set -euo pipefail + +# ── Config (override via env) ──────────────────────────────────────────────── +BOX="${BOX:-adam@10.10.60.120}" +SSH_KEY="${SSH_KEY:-$HOME/.ssh/r720_seahaven}" +REPO_LOCAL="${REPO_LOCAL:-$HOME/Documents/repositories/orchestrator}" +HANDBOOK_LOCAL="${HANDBOOK_LOCAL:-$HOME/Documents/repositories/engineering-handbook}" +# Where the handbook lands on the box; must match SEA_HAVEN_HANDBOOK_DIR below. +HANDBOOK_REMOTE="${HANDBOOK_REMOTE:-/home/adam/.sea-haven/engineering-handbook}" +SSH="ssh -i ${SSH_KEY} ${BOX}" + +say() { printf '\n\033[1;36m== %s\033[0m\n' "$*"; } +confirm() { read -r -p "$1 [y/N] " a; [ "$a" = "y" ] || [ "$a" = "Y" ]; } + +say "Preflight" +[ -f "${SSH_KEY}" ] || { echo "missing SSH key ${SSH_KEY}"; exit 1; } +$SSH true || { echo "cannot reach ${BOX}"; exit 1; } +echo "SNAPSHOT REMINDER: take a Hyper-V checkpoint of sh-secrev on the R720 host now." +confirm "Snapshot taken and ready to update the LIVE coordinator?" || { echo "aborted"; exit 1; } + +say "1. rsync repo (Mac -> box; same excludes as the P1 runbook)" +rsync -av --exclude .env --exclude .venv --exclude .git --exclude '__pycache__' \ + "${REPO_LOCAL}/" "${BOX}:orchestrator/" + +say "2. rsync engineering handbook -> ${HANDBOOK_REMOTE} (WS5 context_provider source)" +if [ -d "${HANDBOOK_LOCAL}" ]; then + $SSH "mkdir -p ${HANDBOOK_REMOTE}" + rsync -av --delete --exclude .git "${HANDBOOK_LOCAL}/" "${BOX}:${HANDBOOK_REMOTE}/" +else + echo "WARN: ${HANDBOOK_LOCAL} not found; context_provider will return '' (fail-safe). Skipping." +fi + +say "3. Install venv deps from the pinned requirements.txt" +# Install the FULL pinned set into the agent-team venv. This includes the +# non-Claude model stack (langchain-anthropic/-openai/-google-genai/-community) +# that the in-process invokers (WS1: GPT-4.1 review, Gemini scan, DeepSeek build) +# import via models.py — WITHOUT these, models.py fails to import and the review +# loop silently fail-closes to REQUEST_CHANGES (the non-Claude models never run). +# Also brings fastapi/uvicorn (WS1 HTTP API). Leaves the venv-only deps that are +# NOT in requirements.txt (claude-agent-sdk, slack_sdk, slack_bolt) untouched. +$SSH 'cd ~/orchestrator/agent-team && . .venv/bin/activate && pip install --upgrade -r ~/orchestrator/requirements.txt' + +say "4. Append new secrets to ~/secrev.env if absent (mode 600, never committed)" +# AGENT_TEAM_API_TOKEN: required only if you run the HTTP API / /delegate hook. +# SEA_HAVEN_HANDBOOK_DIR: where load_handbook_conventions() reads from. +$SSH "bash -s" <> ~/secrev.env +if grep -q '^AGENT_TEAM_API_TOKEN=' ~/secrev.env; then + echo 'AGENT_TEAM_API_TOKEN already set; leaving as-is.' +else + echo 'AGENT_TEAM_API_TOKEN NOT set. Add it now (generated on the Mac):' + echo ' echo "AGENT_TEAM_API_TOKEN=" >> ~/secrev.env && chmod 600 ~/secrev.env' + echo '(only needed for the HTTP API / auto-delegate hook; the coordinator runs without it.)' +fi +REMOTE + +say "5. Restart the coordinator daemon" +confirm "Restart agent-team-coordinator.service now?" || { echo "skipped restart"; exit 0; } +$SSH 'sudo systemctl restart agent-team-coordinator.service && sleep 2 && systemctl is-active agent-team-coordinator.service' +$SSH 'journalctl -u agent-team-coordinator.service -n 30 --no-pager' + +say "6. (OPTIONAL) HTTP API + /delegate hook — start only if you want them" +cat <<'NOTE' +The coordinator now serves WS5 context + WS2 /new-task. The WS1 HTTP API is a +SEPARATE process (api.serve(), 127.0.0.1:8765, bearer auth). To run it: + - ensure AGENT_TEAM_API_TOKEN is set in ~/secrev.env + - run: cd ~/orchestrator/agent-team && . .venv/bin/activate && \ + python3 -c "from agent_team.api import serve; serve()" + - (for persistence, add a second systemd unit; not auto-installed here.) +Then set AGENT_TEAM_API_TOKEN + AGENT_TEAM_API_URL in the Mac Claude Code env +to enable the /delegate hook (sea-haven-claude-plugin). +NOTE + +say "7. SMOKE TESTS (manual)" +cat <<'SMOKE' + a) Coordinator up: systemctl is-active agent-team-coordinator.service -> active + b) Handbook visible: cd ~/orchestrator/agent-team && . .venv/bin/activate && \ + python3 -c "from agent_team.nodes.handbook import load_handbook_conventions as h; print(bool(h()))" -> True + c) Slack /new-task: post "/new-task add a smoke-test file" in #agent-team as an + allowlisted owner -> the bot replies with a clarifying question. + d) (if API running) auth: curl -s -o /dev/null -w '%{http_code}' \ + -H "Authorization: Bearer $AGENT_TEAM_API_TOKEN" http://127.0.0.1:8765/tasks -> 405 (GET not allowed = API up + authed) +ROLLBACK: restore the pre-update Hyper-V checkpoint (one-command revert). +SMOKE +say "Done." diff --git a/agent-team/slack/agent-team-manifest.json b/agent-team/slack/agent-team-manifest.json index 64c1e30..7b0ead9 100644 --- a/agent-team/slack/agent-team-manifest.json +++ b/agent-team/slack/agent-team-manifest.json @@ -8,7 +8,15 @@ "bot_user": { "display_name": "agent-team", "always_online": true - } + }, + "slash_commands": [ + { + "command": "/new-task", + "description": "Start a new agent-team task (clarify -> plan -> review)", + "usage_hint": "", + "should_escape": false + } + ] }, "oauth_config": { "scopes": { @@ -17,7 +25,9 @@ "channels:history", "groups:history", "im:history", - "app_mentions:read" + "app_mentions:read", + "commands", + "reactions:write" ] } }, diff --git a/agent-team/tests/test_apply_verify_workflow_hardening.py b/agent-team/tests/test_apply_verify_workflow_hardening.py index e5f0d4c..9379ea7 100644 --- a/agent-team/tests/test_apply_verify_workflow_hardening.py +++ b/agent-team/tests/test_apply_verify_workflow_hardening.py @@ -184,6 +184,13 @@ def test_gate_job_privileged_declarations_are_live() -> None: assert env_name == "agent-apply", ( "gate-and-pr must bind the agent-apply environment (required-reviewer gate)" ) + # WS3 additive detective control: an audit-log step supplements (never + # replaces) the required-reviewer gate. Assert it is present so it cannot + # silently regress. + step_names = [s.get("name", "").lower() for s in job.get("steps", [])] + assert any("audit" in n for n in step_names), ( + "gate-and-pr must keep the audit-log compensating step" + ) def test_gate_job_if_guards_on_upstream_success() -> None: diff --git a/agent-team/tests/test_builders_llm.py b/agent-team/tests/test_builders_llm.py index 55bb366..535fd5d 100644 --- a/agent-team/tests/test_builders_llm.py +++ b/agent-team/tests/test_builders_llm.py @@ -275,17 +275,18 @@ def test_source_has_no_patch_application_or_fs_write_paths() -> None: }, f"module uses unexpected subprocess primitives: {subprocess_attrs}" -def test_default_build_invokes_run_py_as_list_argv( +def test_subprocess_build_invokes_run_py_as_list_argv( monkeypatch: pytest.MonkeyPatch, ) -> None: - """``default_build`` shells ``run.py`` via list-form argv (no shell) + maps stdout. + """``subprocess_build`` (opt-in fallback) shells ``run.py`` via list-form argv (no shell). - Executes the subprocess path (not just AST-checks it): monkeypatches - ``subprocess.run`` to capture the invocation and return canned stdout. The - argv MUST be the list form ``["python3", /run.py, ]`` so - the instruction can never be interpreted by a shell (no ``shell=True``), and - the return value is the subprocess stdout verbatim. + WS1: the subprocess path is kept as an opt-in fallback under the renamed + ``subprocess_build``. Tests it with a monkeypatched subprocess.run. + The argv MUST be the list form ``["python3", /run.py, ]`` + so the instruction can never be interpreted by a shell (no ``shell=True``). """ + from agent_team.nodes.builders_llm import subprocess_build + captured: dict[str, Any] = {} class _FakeCompleted: @@ -300,7 +301,7 @@ def test_default_build_invokes_run_py_as_list_argv( root = Path("/tmp/fake-orchestrator-root") route = builders_llm._OrchestratorRoute(root=root) - out = default_build("do the edit", route=route) + out = subprocess_build("do the edit", route=route) assert out == "DIFF-FROM-SUBPROCESS" # List-form argv (no shell): exactly python3, the run.py path, the instruction. @@ -328,14 +329,17 @@ def test_as_diff_builder_empty_raises_build_error_in_real_node() -> None: def test_default_build_is_the_injection_default() -> None: - """The default build seam is default_build (the DeepSeek/orchestrator route).""" + """default_build uses in-process DeepSeek (WS1); subprocess_build kept as fallback.""" sig = inspect.signature(build_candidate_diff) assert sig.parameters["build"].default is None - # default_build is what gets used when build is None — assert it's callable - # and routes to a subprocess to run.py (string check, no execution). + # default_build delegates to make_fast_coder_invoker (in-process, WS1); + # subprocess_build retains the old subprocess path as an opt-in fallback. src = inspect.getsource(default_build) - assert "run.py" in src - assert "subprocess.run" in src + assert "make_fast_coder_invoker" in src + from agent_team.nodes.builders_llm import subprocess_build + + subprocess_src = inspect.getsource(subprocess_build) + assert "subprocess.run" in subprocess_src # builders are DeepSeek (orchestrator fast_coder), NOT Claude: assert the # module never CALLS billing.claude_invoke (AST, so docstring mentions of the diff --git a/agent-team/tests/test_coordinator.py b/agent-team/tests/test_coordinator.py index 0692e35..6de75fc 100644 --- a/agent-team/tests/test_coordinator.py +++ b/agent-team/tests/test_coordinator.py @@ -53,6 +53,9 @@ class FakeTransport(Transport): def __init__(self, *, fail_post: bool = False) -> None: self.posted: list[QuestionSet] = [] + # Records the thread_ts each post was threaded under (None = top-level), + # so one-thread-per-task wiring can be asserted. + self.thread_tss: list[str | None] = [] self.fail_post = fail_post def post_question( @@ -63,10 +66,12 @@ class FakeTransport(Transport): turn: int, question_set: QuestionSet, deadline: str, + thread_ts: str | None = None, ) -> str: if self.fail_post: raise RuntimeError("simulated transport post failure") self.posted.append(question_set) + self.thread_tss.append(thread_ts) return f"fake:{question_id}" def parse_answer(self, raw: Any) -> tuple[str, Any, str]: @@ -201,6 +206,53 @@ def test_start_task_suspends_and_writes_open_ledger_row(db_path: Path) -> None: assert len(transport.posted) == 1 +def test_start_task_threads_first_question_under_root(db_path: Path) -> None: + """One-thread-per-task: a /new-task root ts threads the first question + ref. + + When start_task is given ``slack_thread_ts`` (the listener's "📥 Task + received" ack ts), the first clarifier question posts threaded under it AND + the ledger ``channel_ref`` is that ROOT ts — so the human's reply in the + thread (thread_ts == root) maps back to this open question with NO change to + the answer-mapping logic. + """ + transport = FakeTransport() + coord = _make_coordinator(db_path, transport=transport) + coord.setup() + + root_ts = "1700000000.ROOT" + thread_id = coord.start_task( + task_text="build a thing", + transport_name="slack", + slack_thread_ts=root_ts, + ) + assert thread_id + + # The question post threaded under the root. + assert transport.thread_tss == [root_ts] + # The ledger channel_ref is the ROOT ts (so an inbound reply's thread_ts maps + # back via find_open_question_by_channel_ref), not the posted reply's ref. + row = _only_open_row(db_path) + assert row["channel_ref"] == root_ts + # And the root ts is seeded on the graph state for later turns/notifications. + state = graph_mod.get_pipeline_state(coord.graph, thread_id=thread_id) + assert state.get("slack_thread_ts") == root_ts + + +def test_start_task_without_root_posts_top_level(db_path: Path) -> None: + """No slack_thread_ts (e.g. a non-/new-task origin): top-level, ref = post ref.""" + transport = FakeTransport() + coord = _make_coordinator(db_path, transport=transport) + coord.setup() + + thread_id = coord.start_task(task_text="x", transport_name="github") + + assert transport.thread_tss == [None] + row = _only_open_row(db_path) + assert row["channel_ref"] == f"fake:{row['question_id']}" + state = graph_mod.get_pipeline_state(coord.graph, thread_id=thread_id) + assert state.get("slack_thread_ts") == "" + + def test_start_task_lost_post_leaves_open_row_without_ref(db_path: Path) -> None: # A failed transport post is recoverable: the row stays open with no ref. transport = FakeTransport(fail_post=True) @@ -912,3 +964,215 @@ def test_serve_wires_start_and_stop_around_the_loop( assert listener.served is True # started before the loop assert listener.closed is True # stopped in finally on interrupt + + +# --------------------------------------------------------------------------- # +# Lifecycle notifications (_post_resume_followups + _emit): an answered task is +# never a black box — parked / needs-more-input / plan-ready post to the sink, +# and a follow-up clarifier question is delivered (the drain path otherwise +# leaves multi-turn questions unposted). +# --------------------------------------------------------------------------- # + + +def _resume_result(thread_id: str) -> Any: + from agent_team.resume_worker import ResumeOutcome, ResumeResult + + return ResumeResult( + outcome=ResumeOutcome.RESUMED, + thread_id=thread_id, + question_id="q-1", + turn=1, + graph_result=None, + ) + + +def test_followups_posts_new_question_and_emits_needs_input( + db_path: Path, monkeypatch: Any +) -> None: + from agent_team import coordinator as coord_mod + + posted: list[Any] = [] + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + + monkeypatch.setattr( + coord_mod.graph_mod, + "pending_question", + lambda _g, *, thread_id: {"question_set": object(), "deadline": "2099-01-01"}, + ) + monkeypatch.setattr( + coord_mod.responder_mod, + "notify_question", + lambda conn, transport, qset, *, deadline, thread_ts=None: posted.append( + (qset, deadline, thread_ts) + ), + ) + + coord._post_resume_followups([_resume_result("abc12345deadbeef")]) + assert len(posted) == 1 # the follow-up question was delivered + assert any("needs more input" in m for m in msgs) + assert any("abc12345" in m for m in msgs) + + +def test_followups_thread_under_task_root_ts(db_path: Path, monkeypatch: Any) -> None: + """A multi-turn follow-up question + milestone thread under the task root ts. + + The task's ``slack_thread_ts`` (seeded by start_task) is read off the live + state and forwarded as ``thread_ts`` to both the follow-up notify_question + and the milestone _emit, so one-thread-per-task holds across turns. + """ + from agent_team import coordinator as coord_mod + + posted: list[Any] = [] + emitted: list[tuple[str, Any]] = [] + coord = _make_coordinator(db_path) + # A notify sink that accepts the optional thread_ts kwarg. + coord._notify = lambda message, *, thread_ts=None: emitted.append( + (message, thread_ts) + ) + coord.setup() + + # A real task carrying a root ts on its state. + root_ts = "1700000000.ROOT" + thread_id = coord.start_task( + task_text="ship it", transport_name="slack", slack_thread_ts=root_ts + ) + + monkeypatch.setattr( + coord_mod.graph_mod, + "pending_question", + lambda _g, *, thread_id: {"question_set": object(), "deadline": "2099-01-01"}, + ) + monkeypatch.setattr( + coord_mod.responder_mod, + "notify_question", + lambda conn, transport, qset, *, deadline, thread_ts=None: posted.append( + thread_ts + ), + ) + + coord._post_resume_followups([_resume_result(thread_id)]) + + # The follow-up question threaded under the root ts. + assert posted == [root_ts] + # The "needs more input" milestone also threaded under the root ts. + assert any(ts == root_ts for _msg, ts in emitted) + + +def test_followups_emits_parked(db_path: Path, monkeypatch: Any) -> None: + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + values = {"status": TaskStatus.PARKED.value} + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("dead0001beef")]) + assert any("parked" in m.lower() for m in msgs) + + +def test_followups_emits_plan_ready(db_path: Path, monkeypatch: Any) -> None: + from agent_team import coordinator as coord_mod + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + values = {"status": "active"} + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("feed0002face")]) + assert any("plan ready" in m.lower() for m in msgs) + + +def test_emit_swallows_notify_failure(db_path: Path) -> None: + def _boom(_msg: str) -> None: + raise RuntimeError("slack down") + + coord = _make_coordinator(db_path) + coord._notify = _boom + coord._emit("anything") # must not raise + + +def test_parked_message_includes_description_and_blocker( + db_path: Path, monkeypatch: Any +) -> None: + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + values = { + "status": TaskStatus.PARKED.value, + "current_phase": "review", + "task": "add a smoke-test file in agent-team/tests", + "review_verdicts": [ + { + "verdict": "request_changes", + "findings": "Missing a final review/commit phase; lint runs before tests.", + } + ], + } + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("c0ffee01abcd")]) + assert len(msgs) == 1 + m = msgs[0] + assert "add a smoke-test file" in m # WHAT the task is + assert "review" in m # WHERE it got to + assert "Missing a final review/commit phase" in m # WHY it's blocked + + +def test_parked_message_infers_phase_when_current_phase_is_parked( + db_path: Path, monkeypatch: Any +) -> None: + # current_phase is the terminal "parked"; the message should report the phase + # the task was IN (review, since verdicts exist), not "parked". + from agent_team import coordinator as coord_mod + from agent_team.task_model import TaskStatus + + msgs: list[str] = [] + coord = _make_coordinator(db_path) + coord._notify = msgs.append + coord.setup() + monkeypatch.setattr( + coord_mod.graph_mod, "pending_question", lambda _g, *, thread_id: None + ) + + class _Snap: + values = { + "status": TaskStatus.PARKED.value, + "current_phase": "parked", + "task": "do a thing", + "review_verdicts": [{"verdict": "request_changes", "findings": "nope"}], + } + + monkeypatch.setattr(coord._graph, "get_state", lambda _cfg: _Snap()) + coord._post_resume_followups([_resume_result("abcd1234ef00")]) + assert "Reached phase: review" in msgs[0] + assert "Reached phase: parked" not in msgs[0] diff --git a/agent-team/tests/test_graph.py b/agent-team/tests/test_graph.py index 2a29dbc..b4b58be 100644 --- a/agent-team/tests/test_graph.py +++ b/agent-team/tests/test_graph.py @@ -136,6 +136,39 @@ def test_start_task_suspends_on_human_gate(compiled) -> None: assert payload["deadline"] +def test_start_task_seeds_task_description_into_state(compiled) -> None: + # Regression: the intake description (Slack /new-task text, GitHub issue body) + # must reach the graph state so the clarifier can reason about it. It is + # seeded into the initial invoke and must persist through INTAKE into the + # suspended CLARIFY snapshot (intake_node returns only a partial state). + _thread_id, state = start_task( + compiled, transport="slack", task="build a login form" + ) + assert state.get("task") == "build a login form" + + +def test_start_task_seeds_slack_thread_ts_into_state(compiled) -> None: + # One-thread-per-task: the root "📥 Task received" message ts must reach the + # graph state so every later question/notification threads under it. Like + # ``task`` it is seeded into the initial invoke and persists through INTAKE + # into the suspended CLARIFY snapshot. + _thread_id, state = start_task( + compiled, transport="slack", slack_thread_ts="1700000000.ROOT" + ) + assert state.get("slack_thread_ts") == "1700000000.ROOT" + # It is also surfaced on the pending interrupt payload so the responder can + # thread the question post. + payload = pending_question(compiled, thread_id=_thread_id) + assert payload["slack_thread_ts"] == "1700000000.ROOT" + + +def test_start_task_default_slack_thread_ts_is_empty(compiled) -> None: + # A task with no root post (e.g. a GitHub-issue origin): slack_thread_ts is + # empty so questions post top-level exactly as before. + _thread_id, state = start_task(compiled, transport="slack") + assert state.get("slack_thread_ts") == "" + + def test_pending_question_carries_foundation_questionset(compiled) -> None: thread_id, _ = start_task(compiled, transport="slack") payload = pending_question(compiled, thread_id=thread_id) diff --git a/agent-team/tests/test_planner.py b/agent-team/tests/test_planner.py index 4b6c39a..988ddad 100644 --- a/agent-team/tests/test_planner.py +++ b/agent-team/tests/test_planner.py @@ -172,19 +172,40 @@ def test_build_prompt_no_task_uses_placeholder() -> None: def test_build_prompt_loopback_includes_feedback_and_prior_plan() -> None: + # Use the REAL verdict shape that review_loop.ReviewResult.to_dict() writes + # ("verdict" + "findings") — NOT the old "decision"/"notes" keys, which never + # existed on a real verdict and silently produced empty re-plan feedback. state = _state( plan={"task": "t", "phases": [{"name": "old", "steps": ["x"]}]}, review_verdicts=[ - {"decision": "REQUEST_CHANGES", "notes": "Phase 1 missing rollback."} + { + "verdict": "request_changes", + "outcome": "loop_back", + "round_index": 1, + "findings": "Phase 1 missing rollback.", + } ], ) prompt = build_plan_prompt(state) assert "Reviewer feedback" in prompt - assert "Phase 1 missing rollback." in prompt + assert "Phase 1 missing rollback." in prompt # the findings reached the re-plan assert "Previous plan" in prompt assert '"old"' in prompt +def test_review_feedback_reads_real_verdict_keys() -> None: + # Regression: the planner must read the producer's keys (verdict/findings). + # The bug read decision/notes/comment -> empty feedback -> the planner kept + # re-introducing the rejected flaw -> review cap -> park. + from agent_team.nodes.planner import _format_review_feedback + + rendered = _format_review_feedback( + [{"verdict": "request_changes", "findings": "Add a teardown fixture."}] + ) + assert "Add a teardown fixture." in rendered + assert "request_changes" in rendered + + def test_build_prompt_no_feedback_omits_review_sections() -> None: prompt = build_plan_prompt(_state(plan={"task": "t"})) assert "Reviewer feedback" not in prompt diff --git a/agent-team/tests/test_responder.py b/agent-team/tests/test_responder.py index 914b6ae..b541d29 100644 --- a/agent-team/tests/test_responder.py +++ b/agent-team/tests/test_responder.py @@ -71,7 +71,7 @@ class FakeTransport(Transport): self.post_fails = post_fails def post_question( - self, *, thread_id, question_id, turn, question_set, deadline + self, *, thread_id, question_id, turn, question_set, deadline, thread_ts=None ) -> str: if self.post_fails: raise RuntimeError("transport unreachable") @@ -82,6 +82,7 @@ class FakeTransport(Transport): "question_id": question_id, "turn": turn, "deadline": deadline, + "thread_ts": thread_ts, "ref": ref, } ) @@ -197,6 +198,56 @@ def test_notify_lost_post_leaves_open_row_without_ref( assert row["channel_ref"] is None +def test_notify_threads_under_root_and_sets_channel_ref_to_root( + conn: sqlite3.Connection, +) -> None: + """One-thread-per-task: thread_ts is forwarded to post AND becomes channel_ref. + + When ``thread_ts`` (the task's root "📥 Task received" ts) is given, the + question posts as a threaded reply under it, and the durable ``channel_ref`` + is set to that ROOT ts (NOT the posted reply's own ts) — so an inbound reply + whose ``thread_ts == root_ts`` maps back via + ``find_open_question_by_channel_ref``. + """ + transport = FakeTransport() + qs = _question_set() + root_ts = "1700000000.ROOT" + + ref = notify_question( + conn, + transport, + qs, + deadline="2026-06-18T00:00:00+00:00", + thread_ts=root_ts, + ) + + # The post threaded under the root. + assert transport.posts[0]["thread_ts"] == root_ts + # channel_ref is the ROOT ts, not the posted reply ref ("slack-ts-q1"). + assert ref == root_ts + row = _row(conn, "q1") + assert row["channel_ref"] == root_ts + + +def test_notify_without_thread_ts_uses_posted_ref_as_channel_ref( + conn: sqlite3.Connection, +) -> None: + """No thread_ts (the default) preserves the prior behavior exactly. + + The post is top-level (thread_ts None) and the channel_ref is the posted + message's own ts (the FakeTransport ref). + """ + transport = FakeTransport() + + ref = notify_question( + conn, transport, _question_set(), deadline="2026-06-18T00:00:00+00:00" + ) + + assert transport.posts[0]["thread_ts"] is None + assert ref == "slack-ts-q1" + assert _row(conn, "q1")["channel_ref"] == "slack-ts-q1" + + # --------------------------------------------------------------------------- # submit_answer — first-answer-wins (§3.3.1). # --------------------------------------------------------------------------- diff --git a/agent-team/tests/test_run_team.py b/agent-team/tests/test_run_team.py index 86b3574..2826bc5 100644 --- a/agent-team/tests/test_run_team.py +++ b/agent-team/tests/test_run_team.py @@ -644,22 +644,32 @@ class _FakeCoordinator: *, db_path: Any, transport: Any, + build_clarify_node: Any = None, build_plan_node: Any = None, review_wiring: Any = None, + notify: Any = None, + alarm_hook: Any = None, ) -> None: self.db_path = db_path self.transport = transport + self.notify = notify + self.alarm_hook = alarm_hook # The production CLI opts the coordinator into the P2 graph by injecting # these factories; record them so the wiring is asserted, not ignored. + self.build_clarify_node = build_clarify_node self.build_plan_node = build_plan_node self.review_wiring = review_wiring self.setup_called = False self.start_kwargs: dict[str, Any] | None = None + self.new_task_callback: Any = None _FakeCoordinator.instances.append(self) def setup(self) -> None: self.setup_called = True + def set_new_task_callback(self, callback: Any) -> None: + self.new_task_callback = callback + def start_task(self, *, task_text: str, transport_name: str) -> str: self.start_kwargs = {"task_text": task_text, "transport_name": transport_name} return "thread-minted-42" diff --git a/agent-team/tests/test_slack_adapter.py b/agent-team/tests/test_slack_adapter.py index dc9e892..25376e7 100644 --- a/agent-team/tests/test_slack_adapter.py +++ b/agent-team/tests/test_slack_adapter.py @@ -169,6 +169,46 @@ def test_post_question_embeds_question_id_in_callback_id() -> None: assert sent["metadata"]["event_payload"]["thread_id"] == "thread-1" +def test_post_question_threads_under_thread_ts_when_given() -> None: + """One-thread-per-task: a non-empty thread_ts is forwarded as message['thread_ts']. + + The poster receives ``thread_ts`` so Slack posts the question as a threaded + reply under the task's root "📥 Task received" message. + """ + poster = _RecordingPoster() + t = SlackTransport(channel="C999", poster=poster) + t.post_question( + thread_id="thread-1", + question_id="q-abc", + turn=0, + question_set=_question_set(), + deadline="2026-06-18T00:00:00Z", + thread_ts="1700000000.ROOT", + ) + assert poster.calls[0]["thread_ts"] == "1700000000.ROOT" + + +def test_post_question_omits_thread_ts_by_default() -> None: + """No thread_ts (the default) => top-level post (no 'thread_ts' key).""" + poster = _RecordingPoster() + t = SlackTransport(channel="C999", poster=poster) + t.post_question( + thread_id="thread-1", + question_id="q-abc", + turn=0, + question_set=_question_set(), + deadline="2026-06-18T00:00:00Z", + ) + assert "thread_ts" not in poster.calls[0] + + +def test_poster_property_exposes_injected_poster() -> None: + """The transport exposes its poster so the listener can post the root ack.""" + poster = _RecordingPoster() + t = SlackTransport(channel="C1", poster=poster) + assert t.poster is poster + + def test_post_question_accepts_nested_message_ts() -> None: poster = _RecordingPoster({"ok": True, "message": {"ts": "1700000000.000300"}}) t = SlackTransport(channel="C1", poster=poster) diff --git a/agent-team/tests/test_slack_listener.py b/agent-team/tests/test_slack_listener.py index 9b7cadc..f135c4a 100644 --- a/agent-team/tests/test_slack_listener.py +++ b/agent-team/tests/test_slack_listener.py @@ -687,3 +687,194 @@ def test_handle_event_swallows_sqlite_error_from_submit_answer( # Must not raise; returns None. assert listener.handle_event(_interactive_payload("q1")) is None + + +# --------------------------------------------------------------------------- +# 👍 reaction on received messages (after AUTHZ-01) — best-effort. +# --------------------------------------------------------------------------- + + +class _RecordingReactor: + """Records (channel, ts) reaction-add calls; optionally raises to test swallow.""" + + def __init__(self, *, boom: bool = False) -> None: + self.calls: list[tuple[str, str]] = [] + self._boom = boom + + def __call__(self, channel: str, ts: str) -> None: + self.calls.append((channel, ts)) + if self._boom: + raise RuntimeError("missing_scope: reactions:write not granted") + + +def _listener_with_reactor( + db_path: Path, + enqueue: Any, + reactor: Any, + *, + owner_ids: set[str] | None = frozenset({OWNER_ID}), +) -> SlackListener: + return SlackListener( + SlackTransport(channel="C123"), + db_path, + enqueue, + owner_ids=set(owner_ids) if owner_ids else None, + reactor=reactor, + ) + + +def test_reaction_added_to_thread_reply_after_authz(db_path: Path) -> None: + """An accepted thread-reply answer gets a 👍 on the reply message (event ts).""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + reactor = _RecordingReactor() + listener = _listener_with_reactor(db_path, queue, reactor) + + reply = _events_api_reply(text="approve", thread_ts=SEED_CHANNEL_REF) + outcome = listener.handle_event(reply) + + assert outcome is not None and outcome.accepted is True + # Reacted to the REPLY message: channel + the event ts (not the thread_ts). + assert reactor.calls == [("C123", "1700000001.000200")] + + +def test_reaction_not_added_for_non_owner(db_path: Path) -> None: + """AUTHZ-01 runs first: a non-owner message is rejected AND gets no reaction.""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + reactor = _RecordingReactor() + listener = _listener_with_reactor(db_path, queue, reactor) + + outcome = listener.handle_event( + _events_api_reply(sender_id="U_INTRUDER", thread_ts=SEED_CHANNEL_REF) + ) + + assert outcome is None + assert queue.jobs == [] + # The owner check returned before _maybe_react ran: NO reaction attempted. + assert reactor.calls == [] + assert _row_status(db_path, "q1") == "open" + + +def test_reaction_error_is_swallowed(db_path: Path) -> None: + """A reactor failure (e.g. missing reactions:write scope) never breaks handling.""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + reactor = _RecordingReactor(boom=True) + listener = _listener_with_reactor(db_path, queue, reactor) + + # Must not raise; the answer is still accepted + enqueued despite the reaction + # failing (the reaction silently no-ops until the scope is granted). + outcome = listener.handle_event( + _events_api_reply(text="approve", thread_ts=SEED_CHANNEL_REF) + ) + + assert outcome is not None and outcome.accepted is True + assert len(queue.jobs) == 1 + assert reactor.calls == [("C123", "1700000001.000200")] + + +def test_no_reactor_configured_is_quiet_noop(db_path: Path) -> None: + """With no reactor injected, an accepted answer simply does not react.""" + _seed_open_question(db_path, question_id="q1", channel_ref=SEED_CHANNEL_REF) + queue = RecordingQueue() + listener = _listener(db_path, queue, owner_ids={OWNER_ID}) # no reactor + + outcome = listener.handle_event( + _events_api_reply(text="approve", thread_ts=SEED_CHANNEL_REF) + ) + assert outcome is not None and outcome.accepted is True + + +def test_reaction_skipped_for_slash_or_interactive(db_path: Path) -> None: + """No reactable message ts on slash/interactive payloads => no reaction.""" + _seed_open_question(db_path, question_id="q1", thread_id="t1", turn=0) + queue = RecordingQueue() + reactor = _RecordingReactor() + listener = _listener_with_reactor(db_path, queue, reactor) + + # A block_actions interactive payload (no inner event ts) is accepted but has + # no reactable message, so no reaction is attempted. + outcome = listener.handle_event(_interactive_payload("q1", value="approve")) + assert outcome is not None and outcome.accepted is True + assert reactor.calls == [] + + +# --------------------------------------------------------------------------- +# /new-task — one-thread-per-task root "📥 Task received" ack post. +# --------------------------------------------------------------------------- + + +def test_new_task_posts_root_and_passes_root_ts(db_path: Path) -> None: + """A /new-task posts the root ack and forwards its ts to the callback. + + The "📥 Task received" message IS the slash command's acknowledgement (no + reaction), and its ``ts`` is threaded into start via the callback so every + later question/notification lands in one Slack thread. + """ + posted: list[dict[str, Any]] = [] + + def _poster(message: dict[str, Any]) -> dict[str, Any]: + posted.append(message) + return {"ts": "1700000000.ROOT"} + + seen: list[tuple[str, str, str]] = [] + + def _cb(task_text: str, via: str, root_ts: str) -> str: + seen.append((task_text, via, root_ts)) + return "thread-abc" + + listener = SlackListener( + SlackTransport(channel="C_TASK", poster=_poster), + db_path, + RecordingQueue(), + owner_ids={OWNER_ID}, + new_task_callback=_cb, + ) + + payload = { + "type": "slash_commands", + "command": "/new-task", + "text": "Add OAuth to the admin portal", + "user_id": OWNER_ID, + } + assert listener.handle_event(payload) is None + + # The root ack was posted to the channel with the 📥 prefix + description. + assert len(posted) == 1 + assert posted[0]["channel"] == "C_TASK" + assert posted[0]["text"].startswith("📥 Task received:") + assert "Add OAuth to the admin portal" in posted[0]["text"] + # The callback received the description, via, AND the captured root ts. + assert seen == [("Add OAuth to the admin portal", "slack", "1700000000.ROOT")] + + +def test_new_task_root_post_failure_degrades_to_empty_root_ts(db_path: Path) -> None: + """A failed root ack still starts the task (un-threaded): root_ts == ''.""" + + def _boom_poster(_message: dict[str, Any]) -> dict[str, Any]: + raise RuntimeError("slack down") + + seen: list[tuple[str, str, str]] = [] + + def _cb(task_text: str, via: str, root_ts: str) -> str: + seen.append((task_text, via, root_ts)) + return "thread-abc" + + listener = SlackListener( + SlackTransport(channel="C_TASK", poster=_boom_poster), + db_path, + RecordingQueue(), + owner_ids={OWNER_ID}, + new_task_callback=_cb, + ) + + payload = { + "type": "slash_commands", + "command": "/new-task", + "text": "do the thing", + "user_id": OWNER_ID, + } + assert listener.handle_event(payload) is None + # Task still started, with an empty root_ts (top-level questions). + assert seen == [("do the thing", "slack", "")] diff --git a/agent-team/tests/test_slack_live.py b/agent-team/tests/test_slack_live.py index 6d3fbf8..5911c89 100644 --- a/agent-team/tests/test_slack_live.py +++ b/agent-team/tests/test_slack_live.py @@ -38,11 +38,16 @@ class _FakeWebClient: response if response is not None else {"ts": "169.1", "ok": True} ) self.calls: list[dict[str, Any]] = [] + self.reaction_calls: list[dict[str, Any]] = [] def chat_postMessage(self, **kwargs: Any) -> dict[str, Any]: self.calls.append(kwargs) return self.response + def reactions_add(self, **kwargs: Any) -> dict[str, Any]: + self.reaction_calls.append(kwargs) + return {"ok": True} + class _DataResponse: """A ``slack_sdk.SlackResponse``-like object exposing the payload via ``.data``.""" @@ -161,6 +166,50 @@ def test_callback_id_dropped_metadata_carries_question_id() -> None: assert "blocks" in kwargs +def test_poster_forwards_thread_ts_to_chat_post_message() -> None: + """One-thread-per-task: thread_ts IS a postMessage param and is forwarded.""" + from agent_team.transport.slack_live import build_slack_poster as _bp + + client = _FakeWebClient() + transport = SlackTransport("C123", poster=_bp(client=client)) + + transport.post_question( + thread_id="task-7", + question_id="q-42", + turn=1, + question_set=_question_set(), + deadline="2026-06-18T00:00:00Z", + thread_ts="1700000000.ROOT", + ) + + assert client.calls[0]["thread_ts"] == "1700000000.ROOT" + + +def test_reactor_calls_reactions_add_with_thumbsup() -> None: + """build_slack_reactor performs reactions.add(channel, ts, name='thumbsup').""" + from agent_team.transport.slack_live import build_slack_reactor + + client = _FakeWebClient() + reactor = build_slack_reactor(client=client) + + reactor("C123", "1700000001.000200") + + assert client.reaction_calls == [ + {"channel": "C123", "timestamp": "1700000001.000200", "name": "thumbsup"} + ] + + +def test_reactor_missing_token_raises_runtime_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No token + no client => the same loud RuntimeError as the poster path.""" + from agent_team.transport.slack_live import build_slack_reactor + + monkeypatch.delenv("SLACK_BOT_TOKEN", raising=False) + with pytest.raises(RuntimeError): + build_slack_reactor() + + def test_convenience_transport_factory() -> None: """``build_live_slack_transport`` wires the live poster onto a transport.""" client = _FakeWebClient() diff --git a/agent-team/tests/test_ws0_ws2_ws4_plugin_slack_hook.py b/agent-team/tests/test_ws0_ws2_ws4_plugin_slack_hook.py new file mode 100644 index 0000000..d14cb40 --- /dev/null +++ b/agent-team/tests/test_ws0_ws2_ws4_plugin_slack_hook.py @@ -0,0 +1,374 @@ +"""Tests for WS0 (plugin scaffold), WS2 (/new-task Slack handler), and WS4 +(UserPromptSubmit auto-delegate hook). + +WS0 — sea-haven-claude-plugin directory: structure + content checks (no +network, no SDK). + +WS2 — SlackListener /new-task handler: the ``new_task_callback`` seam is +injected so no coordinator is needed. Tests verify: + * /new-task from an authorized owner calls the callback with (text, "slack"); + * /new-task from an unauthorized user is rejected (callback not called); + * /new-task with empty text is ignored (callback not called); + * non-/new-task slash commands are NOT intercepted by the new-task path; + * existing answer-handling is not disturbed (regression); + * callback exception does not crash the listen loop. + +WS4 — auto-delegate hook (sea-haven-claude-plugin/hooks/user_prompt_submit.py): +the ``run()`` function is imported directly (no stdin) and tested with injected +api_url / token so there is no network. Tests verify: + * non-delegate prompts are passed through (return {}); + * /delegate calls the API and blocks with thread_id in the reason; + * DELEGATE: prefix also triggers delegation; + * /delegate with no text returns usage hint (block); + * missing token → block with AGENT_TEAM_API_TOKEN hint; + * API HTTP error → block with error code; + * API network error → block with connectivity hint; + * unexpected exception → block with error message. +""" + +from __future__ import annotations + +import importlib.util +import json +import urllib.error +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import pytest + +from agent_team.transport.slack_listener import ( + NewTaskCallback, + SlackListener, + _is_new_task_command, +) + +# --------------------------------------------------------------------------- +# Helpers / constants +# --------------------------------------------------------------------------- + +_OWNER_ID = "U_OWNER" +_PLUGIN_ROOT = Path(__file__).resolve().parents[2] / "sea-haven-claude-plugin" +_HOOK_PATH = _PLUGIN_ROOT / "hooks" / "user_prompt_submit.py" + + +def _load_hook_module(): + """Import the hook script as a module without executing main().""" + spec = importlib.util.spec_from_file_location("user_prompt_submit", _HOOK_PATH) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) # type: ignore[attr-defined] + return mod + + +# --------------------------------------------------------------------------- +# WS0 — plugin scaffold structure +# --------------------------------------------------------------------------- + + +def test_plugin_root_exists() -> None: + assert _PLUGIN_ROOT.is_dir(), ( + f"sea-haven-claude-plugin/ not found at {_PLUGIN_ROOT}" + ) + + +def test_claude_md_exists_and_has_required_sections() -> None: + claude_md = _PLUGIN_ROOT / "CLAUDE.md" + assert claude_md.is_file(), "CLAUDE.md must exist in sea-haven-claude-plugin/" + text = claude_md.read_text(encoding="utf-8") + for keyword in ("Sea Haven", "/new-task", "/delegate", "agent-team", "pipeline"): + assert keyword in text, f"CLAUDE.md missing keyword {keyword!r}" + + +def test_settings_template_exists_and_is_valid_json() -> None: + tmpl = _PLUGIN_ROOT / "settings.template.json" + assert tmpl.is_file(), ( + "settings.template.json must exist in sea-haven-claude-plugin/" + ) + data = json.loads(tmpl.read_text(encoding="utf-8")) + assert "hooks" in data, "settings.template.json must have a 'hooks' key" + assert "UserPromptSubmit" in data["hooks"], ( + "settings.template.json must configure UserPromptSubmit hook" + ) + + +def test_hook_script_exists() -> None: + assert _HOOK_PATH.is_file(), f"hook script not found at {_HOOK_PATH}" + + +# --------------------------------------------------------------------------- +# WS2 — /new-task Slack handler +# --------------------------------------------------------------------------- + + +def _make_listener( + *, + new_task_callback: NewTaskCallback | None = None, + owner_ids: set[str] | None = None, + db_path: Path | None = None, +) -> SlackListener: + from agent_team.transport.slack_adapter import SlackTransport + + if db_path is None: + import tempfile + + db_path = Path(tempfile.mkdtemp()) / "agent-team.db" + from agent_team.db.schema import init_db + + init_db(db_path) + + transport = SlackTransport(channel="C_FAKE") + return SlackListener( + transport, + db_path, + enqueue_resume=lambda _job: None, + owner_ids=owner_ids or {_OWNER_ID}, + new_task_callback=new_task_callback, + ) + + +def _new_task_payload( + text: str = "Add OAuth", user_id: str = _OWNER_ID +) -> dict[str, Any]: + return { + "type": "slash_commands", + "command": "/new-task", + "text": text, + "user_id": user_id, + } + + +def test_is_new_task_command_detects_slash_new_task() -> None: + assert _is_new_task_command(_new_task_payload()) + + +def test_is_new_task_command_ignores_other_commands() -> None: + payload = { + "type": "slash_commands", + "command": "/other", + "text": "foo", + "user_id": "U1", + } + assert not _is_new_task_command(payload) + + +def test_is_new_task_command_ignores_non_slash() -> None: + assert not _is_new_task_command({"type": "block_actions", "user": {"id": "U1"}}) + + +def test_new_task_calls_callback_with_text_and_transport() -> None: + calls: list[tuple[str, str, str]] = [] + + def cb(task: str, transport: str, root_ts: str) -> str: + calls.append((task, transport, root_ts)) + return "thread-abc" + + listener = _make_listener(new_task_callback=cb) + result = listener.handle_event(_new_task_payload("Add OAuth to admin portal")) + + assert result is None # no AnswerOutcome for new-task + # The default (non-posting) transport poster raises, so root_ts degrades to + # "" — the task still starts, un-threaded. The text + via are forwarded. + assert calls == [("Add OAuth to admin portal", "slack", "")] + + +def test_new_task_unauthorized_sender_rejected() -> None: + calls: list[Any] = [] + + def cb(task: str, transport: str, root_ts: str) -> str: + calls.append(task) + return "thread-xyz" + + listener = _make_listener(new_task_callback=cb) + result = listener.handle_event(_new_task_payload(user_id="U_ATTACKER")) + + assert result is None + assert not calls, "callback must NOT be called for unauthorized sender" + + +def test_new_task_empty_text_ignored() -> None: + calls: list[Any] = [] + + def cb(task: str, transport: str, root_ts: str) -> str: + calls.append(task) + return "thread-123" + + listener = _make_listener(new_task_callback=cb) + result = listener.handle_event(_new_task_payload(text="")) + + assert result is None + assert not calls, "empty text must not trigger callback" + + +def test_new_task_no_callback_configured_is_ignored() -> None: + listener = _make_listener(new_task_callback=None) + result = listener.handle_event(_new_task_payload("Some task")) + assert result is None + + +def test_new_task_callback_exception_does_not_crash_listener() -> None: + def boom(task: str, transport: str, root_ts: str) -> str: + raise RuntimeError("coordinator exploded") + + listener = _make_listener(new_task_callback=boom) + result = listener.handle_event(_new_task_payload("Task that will fail")) + assert result is None # exception swallowed; loop stays alive + + +def test_other_slash_command_not_intercepted_by_new_task_path( + tmp_path: Path, +) -> None: + from agent_team.db.schema import init_db + + db_path = tmp_path / "agent-team.db" + init_db(db_path) + calls: list[Any] = [] + + def cb(task: str, transport: str, root_ts: str) -> str: + calls.append(task) + return "thread-xyz" + + listener = _make_listener(new_task_callback=cb, db_path=db_path) + other_slash = { + "type": "slash_commands", + "command": "/other", + "text": "some text", + "user_id": _OWNER_ID, + } + result = listener.handle_event(other_slash) + assert result is None + assert not calls, "non-/new-task slash commands must NOT call the new_task_callback" + + +# --------------------------------------------------------------------------- +# WS4 — auto-delegate hook +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def hook(): + """Import the hook module once per test session.""" + return _load_hook_module() + + +def test_passthrough_for_normal_prompt(hook: Any) -> None: + assert hook.run("Just a normal question") == {} + + +def test_passthrough_for_empty_prompt(hook: Any) -> None: + assert hook.run("") == {} + + +def test_delegate_prefix_triggers_delegation(hook: Any) -> None: + captured: list[dict[str, Any]] = [] + + def fake_call_api(task: str, *, api_url: str, token: str) -> dict[str, Any]: + captured.append({"task": task, "api_url": api_url, "token": token}) + return {"thread_id": "thread-111"} + + with patch.object(hook, "_call_api", fake_call_api): + result = hook.run( + "/delegate Add OAuth to the admin portal", + api_url="http://127.0.0.1:8765", + token="tok-abc", + ) + + assert result.get("action") == "block" + assert "thread-111" in result["reason"] + assert captured[0]["task"] == "Add OAuth to the admin portal" + assert captured[0]["token"] == "tok-abc" + + +def test_delegate_colon_prefix_triggers_delegation(hook: Any) -> None: + captured: list[dict[str, Any]] = [] + + def fake_call_api(task: str, *, api_url: str, token: str) -> dict[str, Any]: + captured.append(task) + return {"thread_id": "thread-222"} + + with patch.object(hook, "_call_api", fake_call_api): + result = hook.run( + "DELEGATE: Fix the memory leak in retriever.py", + api_url="http://127.0.0.1:8765", + token="tok-xyz", + ) + + assert result.get("action") == "block" + assert "thread-222" in result["reason"] + assert captured[0] == "Fix the memory leak in retriever.py" + + +def test_delegate_empty_task_returns_usage_hint(hook: Any) -> None: + result = hook.run("/delegate", api_url="http://localhost:8765", token="tok") + assert result.get("action") == "block" + assert "Usage" in result["reason"] or "delegate" in result["reason"].lower() + + +def test_missing_token_returns_block_with_hint(hook: Any) -> None: + import os + + env_without_token = { + k: v for k, v in os.environ.items() if k != "AGENT_TEAM_API_TOKEN" + } + with patch.dict(os.environ, env_without_token, clear=True): + result = hook.run( + "/delegate Some task", api_url="http://localhost:8765", token="" + ) + assert result.get("action") == "block" + assert "AGENT_TEAM_API_TOKEN" in result["reason"] + + +def test_api_http_error_returns_block(hook: Any) -> None: + def raise_http(*_args: Any, **_kwargs: Any) -> Any: + raise urllib.error.HTTPError( + url="http://localhost:8765/tasks", + code=401, + msg="Unauthorized", + hdrs=None, # type: ignore[arg-type] + fp=None, + ) + + with patch.object(hook, "_call_api", raise_http): + result = hook.run( + "/delegate Task that fails auth", + api_url="http://localhost:8765", + token="bad-token", + ) + + assert result.get("action") == "block" + assert "401" in result["reason"] + + +def test_api_network_error_returns_block(hook: Any) -> None: + import urllib.error + + def raise_url(*_args: Any, **_kwargs: Any) -> Any: + raise urllib.error.URLError("Connection refused") + + with patch.object(hook, "_call_api", raise_url): + result = hook.run( + "/delegate Task when server is down", + api_url="http://localhost:8765", + token="tok", + ) + + assert result.get("action") == "block" + assert ( + "run-team.py serve" in result["reason"] + or "Connection refused" in result["reason"] + ) + + +def test_unexpected_exception_returns_block(hook: Any) -> None: + def explode(*_args: Any, **_kwargs: Any) -> Any: + raise ValueError("Totally unexpected") + + with patch.object(hook, "_call_api", explode): + result = hook.run( + "/delegate Task", + api_url="http://localhost:8765", + token="tok", + ) + + assert result.get("action") == "block" + assert "Unexpected" in result["reason"] or "unexpected" in result["reason"].lower() diff --git a/agent-team/tests/test_ws1_invoker_multi_api.py b/agent-team/tests/test_ws1_invoker_multi_api.py new file mode 100644 index 0000000..40e771e --- /dev/null +++ b/agent-team/tests/test_ws1_invoker_multi_api.py @@ -0,0 +1,433 @@ +"""WS1 tests: invoker_multi dispatch + api.py HTTP API seam. + +Covers: +* invoker_multi.multi_invoke dispatches to the correct model factory (mocked) +* invoker_multi.make_fast_coder_invoker / make_scanner_invoker return callables +* invoker_multi.bind_multi_invoker wires the review loop seam +* api.py: 401 without bearer token; 200 with valid token (endpoints stubbed) +* api.py: make_app returns a FastAPI app; GET /tasks 404 when no state +* review_loop_llm default_plan_reviewer is now make_cross_reviewer_invoker (in-process) +* builders_llm subprocess_build is kept as opt-in; make_fast_coder_invoker callable +""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +_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)) + + +# --------------------------------------------------------------------------- +# invoker_multi: basic dispatch via mocked models +# --------------------------------------------------------------------------- + + +def _make_fake_model(return_text: str = "model output") -> MagicMock: + """Return a mock model whose .invoke() yields a fake response.""" + mock = MagicMock() + result = MagicMock() + result.content = return_text + mock.invoke.return_value = result + return mock + + +def test_multi_invoke_cross_reviewer(monkeypatch: Any) -> None: + from agent_team import invoker_multi + + fake_model = _make_fake_model("APPROVE verdict") + fake_models = MagicMock() + fake_models.get_cross_reviewer.return_value = fake_model + fake_models.get_fast_coder.return_value = MagicMock() + fake_models.get_scanner.return_value = MagicMock() + + with patch.dict(sys.modules, {"models": fake_models}): + result = invoker_multi.multi_invoke("review this plan", model="cross_reviewer") + + assert result == "APPROVE verdict" + fake_model.invoke.assert_called_once_with("review this plan") + + +def test_multi_invoke_fast_coder(monkeypatch: Any) -> None: + from agent_team import invoker_multi + + fake_model = _make_fake_model("--- a/file.py\n+++ b/file.py\n@@") + fake_models = MagicMock() + fake_models.get_fast_coder.return_value = fake_model + fake_models.get_cross_reviewer.return_value = MagicMock() + fake_models.get_scanner.return_value = MagicMock() + + with patch.dict(sys.modules, {"models": fake_models}): + result = invoker_multi.multi_invoke("add a login form", model="fast_coder") + + assert "@@" in result + + +def test_multi_invoke_scanner(monkeypatch: Any) -> None: + from agent_team import invoker_multi + + fake_model = _make_fake_model("no issues found") + fake_models = MagicMock() + fake_models.get_scanner.return_value = fake_model + fake_models.get_cross_reviewer.return_value = MagicMock() + fake_models.get_fast_coder.return_value = MagicMock() + + with patch.dict(sys.modules, {"models": fake_models}): + result = invoker_multi.multi_invoke("scan the diff", model="scanner") + + assert result == "no issues found" + + +def test_multi_invoke_unknown_model() -> None: + from agent_team import invoker_multi + + fake_models = MagicMock() + with patch.dict(sys.modules, {"models": fake_models}): + with pytest.raises(ValueError, match="unknown model key"): + invoker_multi.multi_invoke("hi", model="gpt_4_turbo") + + +def test_multi_invoke_models_unavailable() -> None: + from agent_team import invoker_multi + + saved = sys.modules.pop("models", None) + try: + # Ensure "models" is NOT in sys.modules so the import inside multi_invoke fails. + # We also need to make sure it's not importable from the path. + with pytest.raises(RuntimeError, match="Cannot import orchestrator"): + # patch.dict with a None value keeps it out of sys.modules + with patch.dict(sys.modules, {"models": None}): # type: ignore[dict-item] + invoker_multi.multi_invoke("hi", model="cross_reviewer") + finally: + if saved is not None: + sys.modules["models"] = saved + + +def test_make_fast_coder_invoker_returns_callable() -> None: + from agent_team.invoker_multi import make_fast_coder_invoker + + invoker = make_fast_coder_invoker() + assert callable(invoker) + + +def test_make_scanner_invoker_returns_callable() -> None: + from agent_team.invoker_multi import make_scanner_invoker + + invoker = make_scanner_invoker() + assert callable(invoker) + + +def test_make_fast_coder_invoker_calls_multi_invoke() -> None: + from agent_team import invoker_multi + + fake_model = _make_fake_model("the diff") + fake_models = MagicMock() + fake_models.get_fast_coder.return_value = fake_model + fake_models.get_cross_reviewer.return_value = MagicMock() + fake_models.get_scanner.return_value = MagicMock() + + invoker = invoker_multi.make_fast_coder_invoker() + with patch.dict(sys.modules, {"models": fake_models}): + result = invoker("build instruction") + + assert result == "the diff" + + +def test_bind_multi_invoker_sets_review_seam() -> None: + from agent_team import invoker_multi + from agent_team.nodes import review_loop + + original = review_loop._review_invoker # save + try: + fake_model = _make_fake_model("APPROVE") + fake_models = MagicMock() + fake_models.get_cross_reviewer.return_value = fake_model + fake_models.get_fast_coder.return_value = MagicMock() + fake_models.get_scanner.return_value = MagicMock() + + with patch.dict(sys.modules, {"models": fake_models}): + invoker_multi.bind_multi_invoker() + + # The review loop seam should now be the cross-reviewer invoker. + assert review_loop._review_invoker is not original + finally: + review_loop._review_invoker = original # restore + + +# --------------------------------------------------------------------------- +# review_loop_llm: default is now in-process, subprocess kept as opt-in +# --------------------------------------------------------------------------- + + +def test_review_loop_llm_default_is_cross_reviewer_invoker() -> None: + from agent_team.nodes import review_loop_llm + + # The module-level default should be the closure from make_cross_reviewer_invoker, + # NOT the subprocess-based make_run_py_invoker closure. We test by name. + # Both are closures, so we check the closure cell names via __code__. + fn = review_loop_llm.default_plan_reviewer + assert callable(fn) + # make_cross_reviewer_invoker creates a _invoke that uses get_cross_reviewer + # internally; make_run_py_invoker creates one that uses subprocess.run. + # We verify by checking that the default raises RuntimeError (models absent) + # NOT FileNotFoundError (run.py absent), confirming it's the in-process path. + saved = sys.modules.pop("models", None) + try: + with patch.dict(sys.modules, {"models": None}): # type: ignore[dict-item] + with pytest.raises( + (RuntimeError, AttributeError, TypeError, ModuleNotFoundError) + ): + fn("any prompt") + finally: + if saved is not None: + sys.modules["models"] = saved + + +def test_review_loop_llm_make_run_py_invoker_still_available() -> None: + from agent_team.nodes.review_loop_llm import make_run_py_invoker + + fn = make_run_py_invoker() + assert callable(fn) + + +# --------------------------------------------------------------------------- +# builders_llm: subprocess_build opt-in; make_fast_coder_invoker available +# --------------------------------------------------------------------------- + + +def test_builders_llm_subprocess_build_available() -> None: + from agent_team.nodes.builders_llm import subprocess_build + + assert callable(subprocess_build) + + +def test_builders_llm_make_fast_coder_invoker_available() -> None: + from agent_team.nodes.builders_llm import make_fast_coder_invoker + + fn = make_fast_coder_invoker() + assert callable(fn) + + +def test_builders_llm_default_build_uses_in_process() -> None: + from agent_team.nodes import builders_llm + + fake_model = _make_fake_model("--- a/x.py\n+++ b/x.py\n@@ -1 +1 @@\n-old\n+new") + fake_models = MagicMock() + fake_models.get_fast_coder.return_value = fake_model + fake_models.get_cross_reviewer.return_value = MagicMock() + fake_models.get_scanner.return_value = MagicMock() + + with patch.dict(sys.modules, {"models": fake_models}): + result = builders_llm.default_build("do the thing") + + assert "@@" in result + fake_model.invoke.assert_called_once() + + +# --------------------------------------------------------------------------- +# api.py: bearer-token auth + endpoint smoke tests via FastAPI TestClient +# --------------------------------------------------------------------------- + +# fastapi is a box-only dependency (installed on the R720 during attended +# deploy, never in CI or the root requirements). api.py imports it lazily, so +# these TestClient-based smoke tests are the only thing that hard-requires it. +# Skip them when it is absent rather than failing collection in CI. +requires_fastapi = pytest.mark.skipif( + importlib.util.find_spec("fastapi") is None, + reason="fastapi not installed (box-only dependency)", +) + + +@pytest.fixture() +def api_token(monkeypatch: Any) -> str: + token = "test-secret-token-for-ws1" + monkeypatch.setenv("AGENT_TEAM_API_TOKEN", token) + return token + + +def _make_stub_coordinator(thread_id: str = "t-test-123") -> MagicMock: + """Return a coordinator stub that start_task returns a fixed thread_id.""" + coord = MagicMock() + coord.start_task.return_value = thread_id + coord.graph = None # no graph for simple smoke tests + coord.setup = MagicMock() + return coord + + +@requires_fastapi +def test_api_401_without_token(api_token: str) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + app = make_app(coordinator=_make_stub_coordinator()) + client = TestClient(app, raise_server_exceptions=False) + resp = client.post("/tasks", json={"task": "add login"}) + assert resp.status_code == 401 + + +@requires_fastapi +def test_api_401_wrong_token(api_token: str) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + app = make_app(coordinator=_make_stub_coordinator()) + client = TestClient(app, raise_server_exceptions=False) + resp = client.post( + "/tasks", + json={"task": "add login"}, + headers={"Authorization": "Bearer wrong-token"}, + ) + assert resp.status_code == 401 + + +@requires_fastapi +def test_api_post_tasks_200(api_token: str) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + stub = _make_stub_coordinator("thread-abc") + app = make_app(coordinator=stub) + client = TestClient(app) + resp = client.post( + "/tasks", + json={"task": "add a login form"}, + headers={"Authorization": f"Bearer {api_token}"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["thread_id"] == "thread-abc" + stub.start_task.assert_called_once() + + +@requires_fastapi +def test_api_get_task_404_no_state(api_token: str) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + stub = _make_stub_coordinator() + app = make_app(coordinator=stub) + client = TestClient(app, raise_server_exceptions=False) + # graph is None -> 503 not ready + resp = client.get( + "/tasks/t-missing", + headers={"Authorization": f"Bearer {api_token}"}, + ) + assert resp.status_code == 503 + + +@requires_fastapi +def test_api_get_task_with_graph_state(api_token: str) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + stub = _make_stub_coordinator() + # Attach a mock graph that returns a snapshot with values + snapshot = MagicMock() + snapshot.values = {"task": "add login", "current_phase": "CLARIFY"} + stub.graph = MagicMock() + stub.graph.get_state.return_value = snapshot + app = make_app(coordinator=stub) + client = TestClient(app) + resp = client.get( + "/tasks/t-abc", + headers={"Authorization": f"Bearer {api_token}"}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["thread_id"] == "t-abc" + assert data["state"]["task"] == "add login" + + +@requires_fastapi +def test_api_orchestrator_invoke_calls_subprocess( + api_token: str, tmp_path: Any +) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + # Write a fake run.py that echos the first argv + fake_run = tmp_path / "run.py" + fake_run.write_text("import sys; print(sys.argv[1])") + + stub = _make_stub_coordinator() + app = make_app(coordinator=stub) + + # Patch _resolve_run_py to return our fake script + with patch("agent_team.api._resolve_run_py", return_value=fake_run): + client = TestClient(app) + resp = client.post( + "/orchestrator/invoke", + json={"prompt": "hello world"}, + headers={"Authorization": f"Bearer {api_token}"}, + ) + + assert resp.status_code == 200 + assert "hello world" in resp.json()["text"] + + +@requires_fastapi +def test_api_no_token_env_raises_at_build(monkeypatch: Any) -> None: + from agent_team.api import make_app + + monkeypatch.delenv("AGENT_TEAM_API_TOKEN", raising=False) + # Fail-fast: an unconfigured token must raise at app-build time, not serve a + # request first (the eager _get_token() in make_app). + with pytest.raises(RuntimeError, match="AGENT_TEAM_API_TOKEN"): + make_app(coordinator=_make_stub_coordinator()) + + +@requires_fastapi +def test_api_docs_and_openapi_disabled(api_token: str) -> None: + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + app = make_app(coordinator=_make_stub_coordinator()) + client = TestClient(app, raise_server_exceptions=False) + # The unauthenticated docs/schema routes must be disabled (VPN-only API). + for path in ("/docs", "/redoc", "/openapi.json"): + assert client.get(path).status_code == 404, path + + +@requires_fastapi +def test_api_invoke_concurrency_cap_returns_429(api_token: str, tmp_path: Any) -> None: + import agent_team.api as api_mod + from fastapi.testclient import TestClient + + from agent_team.api import make_app + + fake_run = tmp_path / "run.py" + fake_run.write_text("import sys; print(sys.argv[1])") + app = make_app(coordinator=_make_stub_coordinator()) + + # Exhaust the bounded semaphore so the request sees no free slot → 429. + acquired = [ + api_mod._invoke_semaphore.acquire(blocking=False) + for _ in range(api_mod._MAX_CONCURRENT_INVOKES) + ] + try: + assert all(acquired) + with patch("agent_team.api._resolve_run_py", return_value=fake_run): + client = TestClient(app, raise_server_exceptions=False) + resp = client.post( + "/orchestrator/invoke", + json={"prompt": "hello"}, + headers={"Authorization": f"Bearer {api_token}"}, + ) + assert resp.status_code == 429 + finally: + for ok in acquired: + if ok: + api_mod._invoke_semaphore.release() diff --git a/agent-team/tests/test_ws3_dispatch_invoker.py b/agent-team/tests/test_ws3_dispatch_invoker.py new file mode 100644 index 0000000..164051e --- /dev/null +++ b/agent-team/tests/test_ws3_dispatch_invoker.py @@ -0,0 +1,445 @@ +"""Unit tests for WS3: agent_team.nodes.dispatch_invoker + graph/coordinator wiring. + +Tests cover: +* :func:`make_dispatch_node` — factory contract, fail-closed on missing fields + (parks on missing thread_id/diff/scope), injection of owner/repo at factory + time (never read from state), scope list flattening, happy-path returns empty + dict (partial state update) and calls dispatcher. +* :func:`agent_team.graph.build_graph` with ``dispatch_node`` wired — verifies + the DISPATCH_NODE constant is exported and the graph compiles with/without it. +* :func:`agent_team.coordinator.default_dispatch_node_factory` — env-var binding + and fail-closed when required vars are unset. +* :class:`agent_team.coordinator.Coordinator` ``dispatch_node_wiring`` seam — + accepted at __init__, NOT called in setup() when build_verify_wiring is None + (DISPATCH_NODE requires VERIFY_NODE/APPROVED_ROUTE to exist). + +All network + subprocess calls are replaced with injected fakes; no model, no +git, no GitHub, no subprocess. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock + +import pytest + +from agent_team.nodes.dispatch_invoker import DispatchNodeFactory, make_dispatch_node +from agent_team.task_model import Phase, TaskStatus + + +# --------------------------------------------------------------------------- # +# Fake BranchPusher / WorkflowDispatcher (the two injectable seams in dispatcher) +# --------------------------------------------------------------------------- # + + +def _make_fake_pusher() -> tuple[list[dict[str, Any]], Any]: + calls: list[dict[str, Any]] = [] + + def _pusher( + *, owner: str, repo: str, base: str, head_branch: str, diff_text: str + ) -> None: + calls.append(dict(owner=owner, repo=repo, base=base, head_branch=head_branch)) + + return calls, _pusher + + +def _make_fake_workflow_dispatcher() -> tuple[list[dict[str, Any]], Any]: + calls: list[dict[str, Any]] = [] + + def _dispatcher(*, owner: str, repo: str, inputs: dict[str, str], ref: str) -> None: + calls.append(dict(owner=owner, repo=repo, ref=ref, inputs=inputs)) + + return calls, _dispatcher + + +def _fake_seams() -> tuple[Any, Any]: + """Return (fake_pusher, fake_dispatcher) that silence all subprocess calls.""" + _, pusher = _make_fake_pusher() + _, dispatcher = _make_fake_workflow_dispatcher() + return pusher, dispatcher + + +_VALID_STATE: dict[str, Any] = { + "thread_id": "task-abc", + "candidate_diff": ( + "diff --git a/f.py b/f.py\n--- a/f.py\n+++ b/f.py\n@@ -1 +1 @@\n-old\n+new" + ), + "plan": {"scope": ["agent_team/"]}, + "status": "active", + "current_phase": "verify", +} + + +# --------------------------------------------------------------------------- # +# make_dispatch_node — happy path +# --------------------------------------------------------------------------- # + + +def test_dispatch_node_happy_path_returns_partial_state() -> None: + """On success the node returns {} (partial state update — DONE comes from graph).""" + pusher, dispatcher = _fake_seams() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + result = node(_VALID_STATE) + + # Remote implementation returns {} on success (graph topology marks DONE). + assert isinstance(result, dict) + + +def test_dispatch_node_injects_owner_repo_at_factory_time() -> None: + """owner/repo come from factory args — model output in state cannot redirect them.""" + pusher_calls, pusher = _make_fake_pusher() + _, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="trusted-org", repo="trusted-repo", pusher=pusher, dispatcher=dispatcher + ) + node(_VALID_STATE) + + assert pusher_calls, "pusher should have been called" + assert pusher_calls[0]["owner"] == "trusted-org" + assert pusher_calls[0]["repo"] == "trusted-repo" + + +def test_dispatch_node_passes_task_id_to_head_branch() -> None: + pusher_calls, pusher = _make_fake_pusher() + _, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", base="main", pusher=pusher, dispatcher=dispatcher + ) + node(_VALID_STATE) + + assert pusher_calls, "pusher should have been called" + # build_dispatch_inputs derives head_branch from task_id. + assert "task-abc" in pusher_calls[0]["head_branch"] + assert pusher_calls[0]["base"] == "main" + + +def test_dispatch_node_passes_scope_to_workflow_dispatcher() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + node(_VALID_STATE) + + assert disp_calls, "workflow dispatcher should have been called" + wf_inputs = disp_calls[0]["inputs"] + assert "agent_team/" in str(wf_inputs) + + +def test_dispatch_node_flattens_scope_list_to_string() -> None: + """Multi-entry scope list is newline-joined into declared_scope.""" + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, plan={"scope": ["agent_team/", "tests/"]}) + node(state) + + wf_inputs = disp_calls[0]["inputs"] + scope_str = str(wf_inputs) + assert "agent_team/" in scope_str + + +# --------------------------------------------------------------------------- # +# make_dispatch_node — fail closed (never dispatches incomplete input) +# --------------------------------------------------------------------------- # + + +def test_dispatch_node_parks_on_missing_thread_id() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, thread_id="") + result = node(state) + + assert result.get("status") == TaskStatus.PARKED.value + assert result.get("current_phase") == Phase.PARKED.value + assert not disp_calls # dispatcher never called + + +def test_dispatch_node_parks_on_missing_diff() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, candidate_diff="") + result = node(state) + + assert result.get("status") == TaskStatus.PARKED.value + assert not disp_calls + + +def test_dispatch_node_parks_on_whitespace_only_diff() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, candidate_diff=" \n ") + result = node(state) + + assert result.get("status") == TaskStatus.PARKED.value + assert not disp_calls + + +def test_dispatch_node_parks_on_empty_scope() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, plan={"scope": []}) + result = node(state) + + assert result.get("status") == TaskStatus.PARKED.value + assert not disp_calls + + +def test_dispatch_node_parks_on_none_plan() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, plan=None) + result = node(state) + + assert result.get("status") == TaskStatus.PARKED.value + assert not disp_calls + + +def test_dispatch_node_parks_on_non_dict_plan() -> None: + _, pusher = _make_fake_pusher() + disp_calls, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + state = dict(_VALID_STATE, plan="not-a-dict") + result = node(state) + + assert result.get("status") == TaskStatus.PARKED.value + assert not disp_calls + + +def test_dispatch_node_parks_on_dispatcher_error() -> None: + """A DispatcherError from the underlying dispatcher parks the task (fail safe).""" + from agent_team.dispatcher import DispatcherError + + def _bad_pusher(**_kwargs: Any) -> None: + raise DispatcherError("invalid owner/repo 'x'/'y'") + + _, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=_bad_pusher, dispatcher=dispatcher + ) + result = node(_VALID_STATE) + + assert result.get("status") == TaskStatus.PARKED.value + + +def test_dispatch_node_parks_on_unexpected_exception() -> None: + """Any unexpected error parks the task rather than crashing the graph.""" + + def _exploding_pusher(**_kwargs: Any) -> None: + raise RuntimeError("network timeout") + + _, dispatcher = _make_fake_workflow_dispatcher() + node = make_dispatch_node( + owner="org", repo="repo", pusher=_exploding_pusher, dispatcher=dispatcher + ) + result = node(_VALID_STATE) + + assert result.get("status") == TaskStatus.PARKED.value + + +# --------------------------------------------------------------------------- # +# DispatchNodeFactory type alias export +# --------------------------------------------------------------------------- # + + +def test_dispatch_node_factory_type_is_exported() -> None: + from agent_team.nodes import dispatch_invoker + + assert hasattr(dispatch_invoker, "DispatchNodeFactory") + + +def test_dispatch_node_factory_satisfies_zero_arg_contract() -> None: + """DispatchNodeFactory is a zero-arg callable that returns a node callable.""" + pusher, dispatcher = _fake_seams() + factory: DispatchNodeFactory = lambda: make_dispatch_node( # noqa: E731 + owner="o", repo="r", pusher=pusher, dispatcher=dispatcher + ) + node = factory() + assert callable(node) + result = node(_VALID_STATE) + assert isinstance(result, dict) + + +# --------------------------------------------------------------------------- # +# graph.build_graph — DISPATCH_NODE constant and compile-time wiring +# --------------------------------------------------------------------------- # + + +def test_build_graph_dispatch_node_constant_is_exported() -> None: + from agent_team import graph as graph_mod + + assert hasattr(graph_mod, "DISPATCH_NODE") + assert graph_mod.DISPATCH_NODE == "dispatch_node" + + +def test_build_graph_without_dispatch_node_compiles() -> None: + """Default (no dispatch_node) builds fine — backward-compatible.""" + from langgraph.checkpoint.memory import MemorySaver + + from agent_team.graph import build_graph + + graph = build_graph(MemorySaver()) + assert graph is not None + + +def test_build_graph_with_dispatch_node_no_build_verify_raises() -> None: + """dispatch_node without build_verify raises ValueError (APPROVED_ROUTE requires P3).""" + from langgraph.checkpoint.memory import MemorySaver + + from agent_team.graph import build_graph + + def _fake_dispatch(state: Any) -> Any: + return {} + + with pytest.raises(ValueError, match="dispatch_node requires build_verify"): + build_graph(MemorySaver(), dispatch_node=_fake_dispatch) + + +# --------------------------------------------------------------------------- # +# coordinator.default_dispatch_node_factory +# --------------------------------------------------------------------------- # + + +def test_default_dispatch_node_factory_raises_without_env_vars( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("AGENT_TEAM_REPO_OWNER", raising=False) + monkeypatch.delenv("AGENT_TEAM_REPO_NAME", raising=False) + + from agent_team.coordinator import default_dispatch_node_factory + + with pytest.raises(RuntimeError, match="AGENT_TEAM_REPO_OWNER"): + default_dispatch_node_factory() + + +def test_default_dispatch_node_factory_raises_when_only_owner_set( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("AGENT_TEAM_REPO_OWNER", "org") + monkeypatch.delenv("AGENT_TEAM_REPO_NAME", raising=False) + + from agent_team.coordinator import default_dispatch_node_factory + + with pytest.raises(RuntimeError): + default_dispatch_node_factory() + + +def test_default_dispatch_node_factory_returns_callable_with_env_vars( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("AGENT_TEAM_REPO_OWNER", "test-org") + monkeypatch.setenv("AGENT_TEAM_REPO_NAME", "test-repo") + monkeypatch.delenv("AGENT_TEAM_BASE_BRANCH", raising=False) + + from agent_team.coordinator import default_dispatch_node_factory + + node = default_dispatch_node_factory() + assert callable(node) + + +def test_default_dispatch_node_factory_reads_base_branch_from_env( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("AGENT_TEAM_REPO_OWNER", "org") + monkeypatch.setenv("AGENT_TEAM_REPO_NAME", "repo") + monkeypatch.setenv("AGENT_TEAM_BASE_BRANCH", "develop") + + from agent_team.coordinator import default_dispatch_node_factory + + node = default_dispatch_node_factory() + assert callable(node) + + +def test_default_dispatch_node_factory_in_all() -> None: + from agent_team import coordinator + + assert "default_dispatch_node_factory" in coordinator.__all__ + + +# --------------------------------------------------------------------------- # +# Coordinator.dispatch_node_wiring seam +# --------------------------------------------------------------------------- # + + +def test_coordinator_accepts_dispatch_node_wiring_param() -> None: + """Coordinator.__init__ accepts dispatch_node_wiring=None without error.""" + from agent_team.coordinator import Coordinator + from agent_team.transport.base import Transport + + transport = MagicMock(spec=Transport) + coord = Coordinator( + db_path=":memory:", + transport=transport, + dispatch_node_wiring=None, + ) + assert coord is not None + + +def test_coordinator_dispatch_node_wiring_without_build_verify_raises( + tmp_path: Any, +) -> None: + """setup() raises ValueError when dispatch_node_wiring is set but build_verify_wiring is None. + + build_graph enforces that dispatch_node requires build_verify (APPROVED_ROUTE + lives inside the P3 subgraph). Passing dispatch_node_wiring alone is a + configuration error caught at setup time. + """ + from langgraph.checkpoint.memory import MemorySaver + + from agent_team.coordinator import Coordinator + from agent_team.transport.base import Transport + + pusher, dispatcher = _fake_seams() + + def _factory() -> Any: + return make_dispatch_node( + owner="org", repo="repo", pusher=pusher, dispatcher=dispatcher + ) + + def _stub_clarify_node() -> Any: + return MagicMock() + + def _stub_checkpointer(db_path: Any) -> Any: + return MemorySaver() + + transport = MagicMock(spec=Transport) + coord = Coordinator( + db_path=tmp_path / "test.db", + transport=transport, + build_clarify_node=_stub_clarify_node, + build_checkpointer=_stub_checkpointer, + dispatch_node_wiring=_factory, + # build_verify_wiring=None (default) + ) + + with pytest.raises(ValueError, match="dispatch_node requires build_verify"): + coord.setup() diff --git a/agent-team/tests/test_ws5_memory_handbook.py b/agent-team/tests/test_ws5_memory_handbook.py new file mode 100644 index 0000000..9c10c9d --- /dev/null +++ b/agent-team/tests/test_ws5_memory_handbook.py @@ -0,0 +1,296 @@ +"""WS5 — memory + handbook injection seams tests. + +Covers: +* retriever.save_memory() writes to _box-drafts/ subdir +* retriever.retrieve() memory_dir param routes to correct dir +* handbook.load_handbook_conventions() safe no-op when dir absent +* handbook.load_handbook_conventions() reads files when dir exists +* clarifier_llm: context_provider absent -> identical prompt; present -> prepended +* planner: context_provider absent -> identical prompt; present -> prepended +* coordinator.default_plan_node_factory: threads context_provider into planner +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any + +import pytest + +# retriever.py lives at the orchestrator root (parents[2] of this file +# which is at agent-team/tests/test_ws5_memory_handbook.py) +_ORCHESTRATOR_ROOT = Path(__file__).resolve().parents[2] +if str(_ORCHESTRATOR_ROOT) not in sys.path: + sys.path.insert(0, str(_ORCHESTRATOR_ROOT)) + + +# --------------------------------------------------------------------------- +# retriever: save_memory +# --------------------------------------------------------------------------- + + +def test_save_memory_writes_to_box_drafts(tmp_path: Path) -> None: + from retriever import save_memory + + dest = save_memory("my-note", "hello world", memory_dir=tmp_path) + assert dest == tmp_path / "_box-drafts" / "my-note.md" + assert dest.exists() + assert dest.read_text() == "hello world" + + +def test_save_memory_creates_subdir(tmp_path: Path) -> None: + from retriever import save_memory + + save_memory("test", "content", memory_dir=tmp_path) + assert (tmp_path / "_box-drafts").is_dir() + + +def test_save_memory_rejects_path_traversal(tmp_path: Path) -> None: + from retriever import save_memory + + with pytest.raises(ValueError): + save_memory("../evil", "bad", memory_dir=tmp_path) + with pytest.raises(ValueError): + save_memory("sub/dir", "bad", memory_dir=tmp_path) + with pytest.raises(ValueError): + save_memory("", "bad", memory_dir=tmp_path) + + +def test_save_memory_rejects_unsafe_allowlist_names(tmp_path: Path) -> None: + from retriever import save_memory + + # Allowlist rejects dot-only / hidden / backslash / NUL / over-long names + # that the old blocklist let through as malformed-but-contained files. + for bad in (".", "..", ".hidden", "a\\b", "a\x00b", "x" * 200, "-leading"): + with pytest.raises(ValueError): + save_memory(bad, "bad", memory_dir=tmp_path) + + +def test_save_memory_does_not_follow_symlink_out_of_drafts(tmp_path: Path) -> None: + import os + + from retriever import save_memory + + drafts = tmp_path / "_box-drafts" + drafts.mkdir() + outside = tmp_path / "outside.txt" + outside.write_text("original") + # Pre-plant a symlink in the drafts dir pointing outside it. + (drafts / "evil.md").symlink_to(outside) + + # O_NOFOLLOW must refuse to write through the symlink (ELOOP). + with pytest.raises(OSError): + save_memory("evil", "overwrite attempt", memory_dir=tmp_path) + # The outside target is untouched. + assert outside.read_text() == "original" + assert os.path.islink(drafts / "evil.md") + + +def test_save_memory_does_not_appear_in_live_dir(tmp_path: Path) -> None: + from retriever import load_memories, save_memory + + save_memory("draft", "draft content", memory_dir=tmp_path) + # load_memories reads the memory_dir top level, not _box-drafts/ + memories = load_memories(tmp_path) + names = [m.name for m in memories] + assert "draft" not in names + + +def test_retrieve_uses_memory_dir_param(tmp_path: Path) -> None: + """retrieve(memory_dir=...) routes to the given dir (empty -> []).""" + from retriever import retrieve + + # Empty dir -> no memories -> returns [] + result = retrieve("some task", memory_dir=tmp_path) + assert result == [] + + +# --------------------------------------------------------------------------- +# handbook: load_handbook_conventions +# --------------------------------------------------------------------------- + + +def test_handbook_no_op_when_dir_absent(tmp_path: Path) -> None: + from agent_team.nodes.handbook import load_handbook_conventions + + missing = tmp_path / "nonexistent" + result = load_handbook_conventions(missing) + assert result == "" + + +def test_handbook_no_op_when_empty_dir(tmp_path: Path) -> None: + from agent_team.nodes.handbook import load_handbook_conventions + + result = load_handbook_conventions(tmp_path) + assert result == "" + + +def test_handbook_loads_md_files(tmp_path: Path) -> None: + from agent_team.nodes.handbook import load_handbook_conventions + + (tmp_path / "conventions.md").write_text("# Conventions\nUse typed Python.") + (tmp_path / "ci.md").write_text("# CI\nRun ruff + pytest.") + result = load_handbook_conventions(tmp_path) + assert "Use typed Python" in result + assert "Run ruff + pytest" in result + assert "## Sea Haven engineering-handbook conventions" in result + + +def test_handbook_never_raises_on_unreadable_file(tmp_path: Path) -> None: + from agent_team.nodes.handbook import load_handbook_conventions + + (tmp_path / "ok.md").write_text("content") + # Simulate an IOError by pointing to a non-directory path as the dir + result = load_handbook_conventions(tmp_path / "ok.md") + assert result == "" + + +# --------------------------------------------------------------------------- +# clarifier_llm: context_provider injection seam +# --------------------------------------------------------------------------- + + +def _make_state(**kwargs: Any) -> dict[str, Any]: + return {"task": "add a login form", "thread_id": "t1", **kwargs} + + +def _fake_invoke(prompt: str, **kw: Any) -> Any: + from agent_team.billing import ClaudeResult + + return ClaudeResult( + text='{"confidence": 0.9, "questions": [], "rationale": "clear"}', + mode="api", + usage={}, + raw=None, + ) + + +def test_clarifier_no_context_provider_produces_base_prompt() -> None: + from agent_team.nodes.clarifier_llm import ClaudeClarifier + + c = ClaudeClarifier(invoke=_fake_invoke) + state = _make_state() + prompt = c._build_prompt([], state) + assert "You are the CLARIFIER" in prompt + # No context header injected + assert "Sea Haven" not in prompt + + +def test_clarifier_context_provider_prepends_context() -> None: + from agent_team.nodes.clarifier_llm import ClaudeClarifier + + def _provider() -> str: + return "## Context\nsome memory" + + c = ClaudeClarifier(invoke=_fake_invoke, context_provider=_provider) + state = _make_state() + prompt = c._build_prompt([], state) + assert prompt.startswith("## Context\nsome memory") + assert "You are the CLARIFIER" in prompt + + +def test_clarifier_no_provider_prompt_identical_to_baseline() -> None: + """Absent context_provider -> exact same output as no-kwarg construction.""" + from agent_team.nodes.clarifier_llm import ClaudeClarifier + + state = _make_state() + base = ClaudeClarifier(invoke=_fake_invoke)._build_prompt([], state) + explicit_none = ClaudeClarifier( + invoke=_fake_invoke, context_provider=None + )._build_prompt([], state) + assert base == explicit_none + + +def test_clarifier_provider_failure_is_silent() -> None: + """A context_provider that raises must not propagate to the caller.""" + + def _bad_provider() -> str: + raise RuntimeError("network down") + + from agent_team.nodes.clarifier_llm import ClaudeClarifier + + c = ClaudeClarifier(invoke=_fake_invoke, context_provider=_bad_provider) + state = _make_state() + prompt = c._build_prompt([], state) + assert "You are the CLARIFIER" in prompt + + +def test_build_claude_clarifier_callables_accepts_context_provider() -> None: + from agent_team.nodes.clarifier_llm import build_claude_clarifier_callables + + called: list[int] = [] + + def _provider() -> str: + called.append(1) + return "## ctx\nhi" + + assess, generate = build_claude_clarifier_callables( + invoke=_fake_invoke, context_provider=_provider + ) + state = _make_state() + # Calling assess_confidence triggers a prompt build -> provider call + assess([], state) + assert called, "context_provider was not called" + + +# --------------------------------------------------------------------------- +# planner: build_plan_prompt context_provider +# --------------------------------------------------------------------------- + + +def _make_plan_state(**kw: Any) -> Any: + from agent_team.task_model import PipelineState + + return PipelineState(task="add a login form", **kw) + + +def test_planner_no_context_provider_prompt_unchanged() -> None: + from agent_team.nodes.planner import build_plan_prompt + + state = _make_plan_state() + base = build_plan_prompt(state) + explicit_none = build_plan_prompt(state, context_provider=None) + assert base == explicit_none + + +def test_planner_context_provider_prepends_context() -> None: + from agent_team.nodes.planner import build_plan_prompt + + def _provider() -> str: + return "## Memory\nremember this" + + state = _make_plan_state() + prompt = build_plan_prompt(state, context_provider=_provider) + assert prompt.startswith("## Memory\nremember this") + assert "You are the PLANNER" in prompt + + +def test_planner_provider_failure_silent() -> None: + from agent_team.nodes.planner import build_plan_prompt + + def _bad() -> str: + raise ValueError("oops") + + state = _make_plan_state() + prompt = build_plan_prompt(state, context_provider=_bad) + assert "You are the PLANNER" in prompt + + +# --------------------------------------------------------------------------- +# coordinator.default_plan_node_factory threads context_provider +# --------------------------------------------------------------------------- + + +def test_default_plan_node_factory_no_provider_returns_callable() -> None: + from agent_team.coordinator import default_plan_node_factory + + node = default_plan_node_factory() + assert callable(node) + + +def test_default_plan_node_factory_with_provider_returns_callable() -> None: + from agent_team.coordinator import default_plan_node_factory + + node = default_plan_node_factory(context_provider=lambda: "ctx") + assert callable(node) diff --git a/agent-team/tests/test_ws_activation_wiring.py b/agent-team/tests/test_ws_activation_wiring.py new file mode 100644 index 0000000..9e90760 --- /dev/null +++ b/agent-team/tests/test_ws_activation_wiring.py @@ -0,0 +1,229 @@ +"""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, slack_thread_ts: str = "" + ) -> str: + calls["task_text"] = task_text + calls["transport_name"] = transport_name + calls["slack_thread_ts"] = slack_thread_ts + return "thread-xyz" + + monkeypatch.setattr(coordinator, "start_task", _fake_start_task) + + cb = coordinator._new_task_callback + assert cb is not None + # The 3-arg callback (one-thread-per-task): (task_text, via, root_ts). The + # root_ts is forwarded into start_task so the task threads under the root. + thread_id = cb("fix the flaky test", "slack", "1700000000.000100") + assert thread_id == "thread-xyz" + assert calls == { + "task_text": "fix the flaky test", + "transport_name": "slack", + "slack_thread_ts": "1700000000.000100", + } + + +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, r: "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, r: "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" + ) diff --git a/docs/provisioning/OPERATOR-RUNBOOK.md b/docs/provisioning/OPERATOR-RUNBOOK.md index 806b583..98f1f38 100644 --- a/docs/provisioning/OPERATOR-RUNBOOK.md +++ b/docs/provisioning/OPERATOR-RUNBOOK.md @@ -237,6 +237,55 @@ journalctl -u agent-team-coordinator.service -e | grep -i "inbound Slack listene --- +## Incident 5b — WS0–WS5 surfaces (HTTP API, /new-task, handbook context) + +These were added by the WS-rollout (see `agent-team/DEPLOY-R720.md` §4b). They +layer onto the coordinator; none of them should take down the maintenance loop. + +- **HTTP API down / unreachable** — the WS1 FastAPI app (`agent_team/api.py`) is + a **separate, opt-in process** (`api.serve()`, `127.0.0.1:8765`, bearer auth), + **not** started by the coordinator daemon. If `/delegate` from Claude Code or + `POST /tasks` over HTTP stops working, the coordinator itself is unaffected — + check the API process separately: + ```bash + curl -sS -o /dev/null -w '%{http_code}\n' \ + -H "Authorization: Bearer $AGENT_TEAM_API_TOKEN" http://127.0.0.1:8765/tasks + # 405 = API up + authed (GET not allowed on /tasks); 000 = process down; + # 401 = AGENT_TEAM_API_TOKEN mismatch (client vs ~/secrev.env). + ``` + The API refuses to start if `AGENT_TEAM_API_TOKEN` is unset/empty (logs a + `RuntimeError`). Fix the token, restart the API process. Tasks already in the + ledger are unaffected — the API is only an *intake/invoke* front door; answer + via Slack or the CLI as usual. + +- **`/new-task` Slack command not responding** — the WS2 slash command is + AUTHZ-01 owner-allowlist gated and routes through the same Socket Mode listener + as answers. If it silently does nothing, it is almost always the owner + allowlist (same failure mode as Incident 3's live-Slack path): + ```bash + journalctl -u agent-team-coordinator.service -e | grep -iE "new-task|owner|unauthorized" + # unauthorized sender / unconfigured allowlist -> fix AGENT_TEAM_SLACK_OWNER_IDS + ``` + Fallback: start the task from the CLI (`run-team.py start --task "..."`) or the + HTTP API. If the listener itself is down, see Incident 5 (inbound listener). + +- **Handbook dir missing → planner runs without handbook context** — the WS5 + `context_provider` (`load_handbook_conventions`) is **fail-safe**: if + `SEA_HAVEN_HANDBOOK_DIR` (or `~/.sea-haven/engineering-handbook`) is missing or + unreadable it returns `""` and the planner runs normally, just without handbook + conventions injected. This is **degraded, not broken** — no park, no alarm. + Confirm and restore: + ```bash + grep '^SEA_HAVEN_HANDBOOK_DIR=' ~/secrev.env + ls "$(grep '^SEA_HAVEN_HANDBOOK_DIR=' ~/secrev.env | cut -d= -f2)" # dir present + populated? + ``` + Re-sync the handbook (the deploy script does this) and restart the daemon so + the planner picks it back up. The WS3 dispatch node is **inert** (gated) and + should never appear in pipeline activity; if it does, treat as an unexpected + state and escalate. + +--- + ## Incident 6 — COMPLACENCY / COVERAGE alarms (Plane-1 checkers) These come from the nightly checker run, not the coordinator daemon (design §6.4, diff --git a/requirements.txt b/requirements.txt index 3f44e71..3e0a84d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,6 @@ langchain-google-genai==4.2.5 langchain-community==0.4.2 composio-langgraph==0.15.0 python-dotenv==1.2.2 +# WS1 agent-team HTTP API (agent_team/api.py): FastAPI app + uvicorn ASGI server. +fastapi==0.136.1 +uvicorn==0.46.0 diff --git a/retriever.py b/retriever.py index efc2ed1..7001f9d 100644 --- a/retriever.py +++ b/retriever.py @@ -14,6 +14,7 @@ from __future__ import annotations import json import math import os +import re from dataclasses import dataclass from pathlib import Path @@ -126,13 +127,19 @@ def _cosine(a: list[float], b: list[float]) -> float: return dot / (math.sqrt(na) * math.sqrt(nb)) -def retrieve(task: str, k: int = TOP_K_DEFAULT) -> list[dict]: +def retrieve( + task: str, k: int = TOP_K_DEFAULT, *, memory_dir: Path | None = None +) -> list[dict]: """Return the top-k most relevant memories for `task`. Result: [{"name", "score", "content"}], sorted by descending score. Returns [] if the memory dir is missing or contains no memories. + + ``memory_dir`` defaults to :data:`MEMORY_DIR` (the production path). + Pass an explicit path for tests or alternate memory stores. """ - memories = load_memories() + effective_dir = memory_dir if memory_dir is not None else MEMORY_DIR + memories = load_memories(effective_dir) if not memories: return [] embeddings = get_or_build_embeddings(memories) @@ -149,6 +156,56 @@ def retrieve(task: str, k: int = TOP_K_DEFAULT) -> list[dict]: ] +# Subdirectory under a memory dir where save_memory() writes drafts. Using a +# separate queue directory keeps review-queue files out of the live memory dir +# so the retriever never auto-indexes a not-yet-reviewed draft. +_DRAFTS_SUBDIR = "_box-drafts" + +# Allowlist for save_memory() names (defense-in-depth over the old blocklist). +# Must start alphanumeric, then alphanumerics / dot / underscore / hyphen. This +# rejects path separators, NUL, leading-dot hidden files, and dot-only names +# (".", "..") outright — a name cannot escape the drafts dir or be malformed. +_SAFE_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$") +_MAX_NAME_LEN = 128 + + +def save_memory(name: str, content: str, *, memory_dir: Path | None = None) -> Path: + """Write a memory draft to the review queue (``_box-drafts/`` subdir). + + Writes ``{memory_dir}/_box-drafts/{name}.md`` and returns the path. The + ``_box-drafts/`` subdir is a REVIEW QUEUE — files land there for human + inspection before being promoted to the live memory dir. The retriever + never indexes ``_box-drafts/`` entries, so writing here never + auto-activates a draft. + + ``memory_dir`` defaults to :data:`MEMORY_DIR`. ``name`` must match the safe + allowlist (alphanumeric start; then ``A-Z a-z 0-9 . _ -``; max 128 chars) so + it cannot contain path separators, NUL, ``..`` traversal, or be a dot-only / + hidden name. Raises ``ValueError`` on unsafe names. Creates the subdir if + absent. + """ + if ( + not name + or len(name) > _MAX_NAME_LEN + or ".." in name + or not _SAFE_NAME_RE.match(name) + ): + raise ValueError(f"unsafe memory name: {name!r}") + effective_dir = memory_dir if memory_dir is not None else MEMORY_DIR + drafts_dir = effective_dir / _DRAFTS_SUBDIR + drafts_dir.mkdir(parents=True, exist_ok=True) + dest = drafts_dir / f"{name}.md" + # Symlink-safe write: O_NOFOLLOW makes the open fail (ELOOP) if the final + # path component is a pre-planted symlink, closing the TOCTOU where a symlink + # in _box-drafts/ could redirect the write outside the dir. O_CREAT|O_TRUNC + # preserves the overwrite-on-resave behavior for a regular file. + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | os.O_NOFOLLOW + fd = os.open(dest, flags, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(content) + return dest + + def format_memories_for_prompt(retrieved: list[dict]) -> str: """Render retrieved memories as a system-prompt-friendly block.""" if not retrieved: diff --git a/sea-haven-claude-plugin/CLAUDE.md b/sea-haven-claude-plugin/CLAUDE.md new file mode 100644 index 0000000..c19c038 --- /dev/null +++ b/sea-haven-claude-plugin/CLAUDE.md @@ -0,0 +1,76 @@ +# Sea Haven Industries — Claude Code Engineering Plugin + +This directory is the **Sea Haven Claude Code plugin**: context and hooks for +Claude Code sessions run against the `sea-haven-industries/orchestrator` repo. +Copy `settings.template.json` into `~/.claude/settings.json` (or merge it into +an existing one) to activate the hooks. + +--- + +## Project overview + +The orchestrator is the R720-hosted Plane-2 SDLC pipeline. It runs a +multi-model multi-agent loop — **Claude Sonnet** (coordinator/clarifier/planner), +**GPT-4.1** (cross-reviewer), **DeepSeek** (fast_coder/builder), **Gemini** +(scanner) — gated by a CI trust boundary (`agent-team-apply-verify.yml`). + +Key directories: + +| Path | Purpose | +|------|---------| +| `agent-team/` | The pipeline package (`agent_team.*`) + tests | +| `agent-team/run-team.py` | CLI: `serve`, `start`, `answer` sub-commands | +| `agent-team/agent_team/coordinator.py` | Keystone: graph + transport wiring | +| `agent-team/agent_team/graph.py` | LangGraph state machine (P1–P3+) | +| `agent-team/agent_team/nodes/` | Pipeline node implementations | +| `agent-team/agent_team/transport/` | Slack / GitHub / Claude Code adapters | +| `.github/workflows/agent-team-apply-verify.yml` | CI trust boundary (NEVER edit directly; use a WS PR) | +| `sea-haven-claude-plugin/` | This plugin | + +## Development rules + +1. **Green before merge**: `cd agent-team && python3 -m ruff check . && python3 -m pytest -q` must pass. +2. **No secrets in code**: all tokens go in env vars or GitHub secrets. +3. **No SSH/AWS/infra commands**: the R720 box is VPN-only; never attempt network access from a Claude Code session. +4. **Draft PRs only**: all automated changes go out as `--draft`; humans merge. +5. **CI YAML edits** require a security review (`/sh-security-review`) and a separate WS PR — never inline. + +## Delegating tasks to the pipeline + +### Via Slack + +Post `/new-task ` in any channel the Sea Haven bot is in. The +listener routes it to the coordinator's `start_task`, which begins the +CLARIFY → PLAN → REVIEW → BUILD → VERIFY pipeline. You'll receive a clarifier +question back in Slack. + +### Via this Claude Code session (auto-delegate hook) + +Prefix your prompt with `/delegate ` (or `DELEGATE: `) to auto-send the task +to the pipeline HTTP API instead of answering it locally: + +``` +/delegate Add OAuth2 to the admin portal — use PKCE, no client secret stored +``` + +The hook calls `POST http://127.0.0.1:8765/tasks` (requires +`AGENT_TEAM_API_TOKEN` env var) and returns the `thread_id` so you can track +the task in the coordinator. + +Set `AGENT_TEAM_API_URL` to override the default `http://127.0.0.1:8765`. +Set `AGENT_TEAM_API_TOKEN` to the bearer token from `run-team.py serve`. + +## Available slash commands + +These are defined in `settings.template.json` (copy to `~/.claude/settings.json`): + +| Command | Effect | +|---------|--------| +| _(hooks auto-detect `/delegate` prefix)_ | Auto-delegate to pipeline | + +## Phases + +- **P1**: INTAKE → CLARIFY (human gate) → PLAN +- **P2**: + adversarial REVIEW loop (GPT-4.1 cross-reviewer) +- **P3**: + BUILD (DeepSeek) → VERIFY (CI gate) +- **P3+**: + DISPATCH (push head branch + trigger `workflow_dispatch`) diff --git a/sea-haven-claude-plugin/hooks/user_prompt_submit.py b/sea-haven-claude-plugin/hooks/user_prompt_submit.py new file mode 100644 index 0000000..f26d965 --- /dev/null +++ b/sea-haven-claude-plugin/hooks/user_prompt_submit.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +"""UserPromptSubmit hook: auto-delegate prefixed prompts to the agent-team API. + +Claude Code calls this script via the UserPromptSubmit hook whenever the user +submits a prompt. If the prompt starts with ``/delegate`` (or ``DELEGATE:``), +the hook forwards the task description to the agent-team HTTP API +(``POST /tasks``) and blocks the prompt — the task is now running in the +pipeline; no need to also answer it locally. + +Prompts that don't match the delegate prefix are passed through unchanged +(exit 0 with no output). + +Configuration (env vars): + AGENT_TEAM_API_URL Base URL of the agent-team HTTP API + (default: http://127.0.0.1:8765) + AGENT_TEAM_API_TOKEN Bearer token from ``run-team.py serve`` + (required when the API has token auth enabled) + +Claude Code hook contract: + * stdin: JSON object with at least a ``prompt`` key. + * stdout: JSON object or empty. + - Empty / exit 0 → pass through (Claude answers normally). + - ``{"action": "block", "reason": "..."}`` → block the prompt; Claude + shows the reason to the user instead of answering. + * exit 0 = allowed (or delegated+blocked); non-zero = block with error. +""" + +from __future__ import annotations + +import json +import os +import sys +import urllib.error +import urllib.request +from typing import Any + +_DEFAULT_API_URL = "http://127.0.0.1:8765" +_DELEGATE_PREFIXES = ("/delegate ", "DELEGATE: ") + + +def _should_delegate(prompt: str) -> tuple[bool, str]: + """Return (True, task_text) when the prompt is a delegate command, else (False, ''). + + Matches ``/delegate ``, ``/delegate`` (bare — no text → usage hint), + and ``DELEGATE: ``. + """ + stripped = prompt.strip() + for prefix in _DELEGATE_PREFIXES: + if stripped.startswith(prefix): + return True, stripped[len(prefix) :].strip() + # Bare /delegate with no trailing space or text. + if stripped == "/delegate": + return True, "" + return False, "" + + +def _call_api(task: str, *, api_url: str, token: str) -> dict[str, Any]: + """POST /tasks and return the parsed response dict. + + Raises urllib.error.URLError / urllib.error.HTTPError on network / HTTP + errors; the caller turns these into a block reason so Claude shows the error + to the user rather than silently passing through. + """ + payload = json.dumps({"task": task, "transport": "claude_code"}).encode("utf-8") + headers: dict[str, str] = {"Content-Type": "application/json"} + if token: + headers["Authorization"] = f"Bearer {token}" + req = urllib.request.Request( + f"{api_url.rstrip('/')}/tasks", + data=payload, + headers=headers, + method="POST", + ) + with urllib.request.urlopen(req, timeout=10) as resp: # noqa: S310 + return json.loads(resp.read().decode("utf-8")) + + +def run( + prompt: str, *, api_url: str = _DEFAULT_API_URL, token: str = "" +) -> dict[str, Any]: + """Core hook logic — pure function, fully testable without stdin/stdout. + + Returns the hook output dict: + - ``{}`` to pass through (no delegation); + - ``{"action": "block", "reason": "..."}`` to block and show a message. + """ + should, task_text = _should_delegate(prompt) + if not should: + return {} + + if not task_text: + return { + "action": "block", + "reason": ( + "Usage: /delegate \n" + "Example: /delegate Add OAuth2 to the admin portal" + ), + } + + if not token: + env_token = os.environ.get("AGENT_TEAM_API_TOKEN", "") + if not env_token: + return { + "action": "block", + "reason": ( + "AGENT_TEAM_API_TOKEN is not set. " + "Start the agent-team server with `run-team.py serve` " + "and export the token it prints." + ), + } + token = env_token + + try: + result = _call_api(task_text, api_url=api_url, token=token) + except urllib.error.HTTPError as exc: + return { + "action": "block", + "reason": f"agent-team API error {exc.code}: {exc.reason}. Is the server running?", + } + except urllib.error.URLError as exc: + return { + "action": "block", + "reason": ( + f"Cannot reach agent-team API at {api_url}: {exc.reason}. " + "Is `run-team.py serve` running?" + ), + } + except Exception as exc: # noqa: BLE001 + return { + "action": "block", + "reason": f"Unexpected error delegating task: {exc}", + } + + thread_id = result.get("thread_id", "(unknown)") + return { + "action": "block", + "reason": ( + f"Task delegated to agent-team pipeline.\n" + f"thread_id: {thread_id}\n" + f"The coordinator will send a clarifying question via Slack." + ), + } + + +def main() -> int: + try: + data = json.loads(sys.stdin.read()) + except (json.JSONDecodeError, OSError): + return 0 # pass through on bad input + + prompt = data.get("prompt", "") if isinstance(data, dict) else "" + api_url = os.environ.get("AGENT_TEAM_API_URL", _DEFAULT_API_URL) + token = os.environ.get("AGENT_TEAM_API_TOKEN", "") + + output = run(prompt, api_url=api_url, token=token) + if output: + print(json.dumps(output)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/sea-haven-claude-plugin/settings.template.json b/sea-haven-claude-plugin/settings.template.json new file mode 100644 index 0000000..f0af3d4 --- /dev/null +++ b/sea-haven-claude-plugin/settings.template.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "UserPromptSubmit": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "python3 ${SEA_HAVEN_PLUGIN_DIR}/hooks/user_prompt_submit.py" + } + ] + } + ] + } +}