Redesign registration to make orchestration simpler

engine/tests/agents/test_orchestrator_pdf_comment.py

@@ -1,11 +1,10 @@
 """
-Orchestrator ``delegate_pdf_review`` contract test.
+PDF-review delegate contract test.
 
-The real orchestrator delegates PDF-review requests via a pydantic-ai tool
-output. Exercising the full ``agent.run(...)`` call would hit the LLM and
-requires building a real ``RunContext`` — so instead this test invokes
-``delegate_pdf_review`` directly with a minimal ``deps`` stand-in. That's
-enough to verify the wire contract the orchestrator produces:
+The orchestrator routes PDF-review requests to ``PdfReviewAgent.orchestrate``
+(the orchestrator merely selects the delegate; the review logic lives on the
+agent). Exercising the agent directly avoids the LLM routing call and verifies
+the wire contract the delegate produces for a plain prose-review request:
 
 * it returns an ``EditPlanResponse``;
 * with exactly one step;
@@ -16,13 +15,11 @@ enough to verify the wire contract the orchestrator produces:
 
 from __future__ import annotations
 
-from dataclasses import dataclass
-from types import SimpleNamespace
 from unittest.mock import AsyncMock, patch
 
 import pytest
 
-from stirling.agents import OrchestratorAgent
+from stirling.agents import PdfReviewAgent
 from stirling.contracts import AiFile, OrchestratorRequest
 from stirling.contracts.pdf_edit import EditPlanResponse
 from stirling.models import FileId
@@ -30,27 +27,28 @@ from stirling.models.agent_tool_models import AgentToolId, PdfCommentAgentParams
 from stirling.services.runtime import AppRuntime
 
 
-@dataclass(frozen=True)
-class _FakeDeps:
-    request: OrchestratorRequest
-
-
 @pytest.mark.anyio
-async def test_delegate_pdf_review_wires_prompt_to_tool_step(runtime: AppRuntime) -> None:
-    orchestrator = OrchestratorAgent(runtime)
+async def test_pdf_review_wires_prompt_to_tool_step(runtime: AppRuntime) -> None:
+    review_agent = PdfReviewAgent(runtime)
     request = OrchestratorRequest(
         user_message="please add review comments flagging ambiguous dates",
         files=[AiFile(id=FileId("contract-id"), name="contract.pdf")],
     )
-    ctx = SimpleNamespace(deps=_FakeDeps(request=request))
 
-    # PdfReviewAgent now classifies math intent locally via a tiny LLM. Stub it
-    # to false so this test stays focused on the prose-review wire contract.
-    with patch(
-        "stirling.agents.pdf_review.MathIntentClassifier.classify",
-        new=AsyncMock(return_value=False),
+    # PdfReviewAgent classifies math and contradiction intent locally via tiny
+    # LLMs. Stub both to false so this test stays focused on the prose-review
+    # wire contract.
+    with (
+        patch(
+            "stirling.agents.pdf_review.MathIntentClassifier.classify",
+            new=AsyncMock(return_value=False),
+        ),
+        patch(
+            "stirling.agents.pdf_review.ContradictionIntentClassifier.classify",
+            new=AsyncMock(return_value=False),
+        ),
     ):
-        response = await orchestrator.delegate_pdf_review(ctx)  # type: ignore[arg-type]
+        response = await review_agent.orchestrate(request)
 
     assert isinstance(response, EditPlanResponse)
     assert len(response.steps) == 1

engine/tests/agents/test_orchestrator_routing.py

@@ -0,0 +1,134 @@
+"""Behavioural lock: a scripted top-level tool call reaches the right delegate.
+
+No real LLM — a :class:`FunctionModel` scripts the exact tool call the orchestrator
+would have received, and we assert which delegate handled it. Built on the real
+descriptor list (via ``build_descriptors``) with each agent's ``orchestrate``
+swapped for a recording spy, so the test stays honest to whatever agents are
+actually registered.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Callable
+from dataclasses import replace
+
+import pytest
+from pydantic_ai.messages import ModelMessage, ModelResponse, ToolCallPart
+from pydantic_ai.models.function import AgentInfo, FunctionModel
+
+from stirling.agents import OrchestratorAgent, build_descriptors
+from stirling.agents._registry import AgentDescriptor, OrchestratorRoute, RegisterableAgent
+from stirling.contracts import (
+    ConvertMarkdownResponse,
+    EditCannotDoResponse,
+    EditPlanResponse,
+    OrchestratorRequest,
+    OrchestratorResponse,
+    PdfQuestionNotFoundResponse,
+    SupportedCapability,
+)
+from stirling.services.runtime import AppRuntime
+
+_REACHED: list[SupportedCapability] = []
+
+
+class _SpyAgent(RegisterableAgent):
+    """Stand-in agent that reproduces a real delegate's tool surface but records
+    the reach and returns a fixed sentinel instead of doing work."""
+
+    def __init__(
+        self,
+        capability: SupportedCapability,
+        tool_name: str,
+        response: OrchestratorResponse,
+    ) -> None:
+        self._capability = capability
+        self._tool_name = tool_name
+        self._response = response
+
+    def describe(self) -> AgentDescriptor:
+        return AgentDescriptor(
+            orchestrator=OrchestratorRoute(
+                capability=self._capability,
+                tool_name=self._tool_name,
+                tool_description=f"spy for {self._tool_name}",
+                orchestrate=self._orchestrate,
+            ),
+        )
+
+    async def _orchestrate(self, _request: OrchestratorRequest) -> OrchestratorResponse:
+        _REACHED.append(self._capability)
+        return self._response
+
+
+def _spies() -> list[RegisterableAgent]:
+    return [
+        _SpyAgent(SupportedCapability.PDF_EDIT, "delegate_pdf_edit", EditCannotDoResponse(reason="spy")),
+        _SpyAgent(
+            SupportedCapability.PDF_QUESTION,
+            "delegate_pdf_question",
+            PdfQuestionNotFoundResponse(reason="spy"),
+        ),
+        _SpyAgent(SupportedCapability.PDF_REVIEW, "delegate_pdf_review", EditPlanResponse(summary="", steps=[])),
+    ]
+
+
+def _script(tool_name: str) -> Callable[[list[ModelMessage], AgentInfo], ModelResponse]:
+    def call(_messages: list[ModelMessage], _info: AgentInfo) -> ModelResponse:
+        return ModelResponse(parts=[ToolCallPart(tool_name=tool_name, args={})])
+
+    return call
+
+
+async def _route(runtime: AppRuntime, tool_name: str) -> OrchestratorResponse:
+    _REACHED.clear()
+    scripted = replace(runtime, fast_model=FunctionModel(_script(tool_name)))
+    orchestrator = OrchestratorAgent(scripted, build_descriptors(_spies()))
+    return await orchestrator.handle(OrchestratorRequest(user_message="x"))
+
+
+@pytest.mark.anyio
+async def test_delegate_pdf_edit_reaches_edit_delegate(runtime: AppRuntime) -> None:
+    response = await _route(runtime, "delegate_pdf_edit")
+    assert _REACHED == [SupportedCapability.PDF_EDIT]
+    assert isinstance(response, EditCannotDoResponse)
+
+
+@pytest.mark.anyio
+async def test_delegate_pdf_question_reaches_question_delegate(runtime: AppRuntime) -> None:
+    response = await _route(runtime, "delegate_pdf_question")
+    assert _REACHED == [SupportedCapability.PDF_QUESTION]
+    assert isinstance(response, PdfQuestionNotFoundResponse)
+
+
+@pytest.mark.anyio
+async def test_delegate_pdf_review_reaches_review_delegate(runtime: AppRuntime) -> None:
+    response = await _route(runtime, "delegate_pdf_review")
+    assert _REACHED == [SupportedCapability.PDF_REVIEW]
+    assert isinstance(response, EditPlanResponse)
+
+
+@pytest.mark.anyio
+async def test_delegate_pdf_ingest_returns_convert_markdown(runtime: AppRuntime) -> None:
+    # pdf_ingest is the canned descriptor appended by build_descriptors — no agent,
+    # no reach recorded, just a deterministic convert response.
+    response = await _route(runtime, "delegate_pdf_ingest")
+    assert _REACHED == []
+    assert isinstance(response, ConvertMarkdownResponse)
+
+
+@pytest.mark.anyio
+async def test_resume_dispatches_to_matching_delegate(runtime: AppRuntime) -> None:
+    _REACHED.clear()
+    orchestrator = OrchestratorAgent(runtime, build_descriptors(_spies()))
+    await orchestrator.handle(OrchestratorRequest(user_message="x", resume_with=SupportedCapability.PDF_REVIEW))
+    assert _REACHED == [SupportedCapability.PDF_REVIEW]
+
+
+@pytest.mark.anyio
+async def test_resume_with_non_resumable_capability_raises(runtime: AppRuntime) -> None:
+    orchestrator = OrchestratorAgent(runtime, build_descriptors(_spies()))
+    with pytest.raises(ValueError, match="Cannot resume"):
+        await orchestrator.handle(
+            OrchestratorRequest(user_message="x", resume_with=SupportedCapability.PDF_TO_MARKDOWN)
+        )

engine/tests/test_agent_capabilities.py

@@ -0,0 +1,154 @@
+"""Lock the MCP capabilities manifest wire shape.
+
+Exercises the real ``GET /api/v1/agents/capabilities`` endpoint (so it covers the
+actual startup wiring of ``app.state.agent_descriptors``) and pins every
+capability's id, metadata, and Pydantic-derived input schema. The manifest is
+built by flattening each agent's ``describe()`` rows; this suite is the guard
+that the derived manifest never drifts from the published contract.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Iterator
+from dataclasses import dataclass
+from typing import Any
+
+import pytest
+from conftest import build_app_settings
+from fastapi.testclient import TestClient
+from pydantic import BaseModel
+
+from stirling.api import app
+from stirling.config import load_settings
+from stirling.contracts import (
+    AgentDraftRequest,
+    AgentExecutionRequest,
+    AgentRevisionRequest,
+    Evidence,
+    FolioManifest,
+    PdfCommentRequest,
+    PdfEditRequest,
+    PdfQuestionRequest,
+)
+
+
+@pytest.fixture
+def manifest() -> Iterator[dict[str, Any]]:
+    # Force test settings for the lifespan (other suites pop this override, so we
+    # can't rely on a module-level set), then enter the client as a context manager
+    # so the lifespan runs and populates ``app.state.agent_descriptors`` — the
+    # manifest is built from that real startup registration, not a duplicated list.
+    app.dependency_overrides[load_settings] = build_app_settings
+    try:
+        with TestClient(app) as client:
+            response = client.get("/api/v1/agents/capabilities")
+    finally:
+        app.dependency_overrides.pop(load_settings, None)
+    assert response.status_code == 200
+    yield response.json()
+
+
+@dataclass(frozen=True)
+class _Expected:
+    description: str
+    mode: str
+    required_scope: str
+    route: str
+    input_model: type[BaseModel]
+
+
+# Expected per-capability metadata, keyed by id. Order-independent on purpose —
+# Java consumes the manifest as a keyed operation registry, not a sequence.
+_EXPECTED: dict[str, _Expected] = {
+    "pdf-question-answer": _Expected(
+        description="Answer a natural-language question about a PDF document.",
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/pdf-question",
+        input_model=PdfQuestionRequest,
+    ),
+    "pdf-edit-plan": _Expected(
+        description=(
+            "Produce an edit plan (a structured sequence of PDF operations) from a"
+            " natural-language edit request. The plan is executed by Java through the job"
+            " pipeline; this capability does not modify files itself."
+        ),
+        mode="async",
+        required_scope="mcp.tools.write",
+        route="/api/v1/pdf-edit",
+        input_model=PdfEditRequest,
+    ),
+    "agent-draft": _Expected(
+        description=(
+            "Draft a structured agent specification from a free-text description of the task the user wants automated."
+        ),
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/ai/agents/draft",
+        input_model=AgentDraftRequest,
+    ),
+    "agent-revise": _Expected(
+        description="Revise an existing draft agent specification based on user feedback or constraint changes.",
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/ai/agents/revise",
+        input_model=AgentRevisionRequest,
+    ),
+    "math-audit-examine": _Expected(
+        description=(
+            "Examine a folio manifest of financial / numeric documents and surface the"
+            " evidence that needs to be checked for arithmetic consistency."
+        ),
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/ai/math-auditor-agent/examine",
+        input_model=FolioManifest,
+    ),
+    "math-audit-deliberate": _Expected(
+        description=(
+            "Render a deliberated verdict on a single piece of evidence the examine step"
+            " surfaced (does the arithmetic check out, with what caveats)."
+        ),
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/ai/math-auditor-agent/deliberate",
+        input_model=Evidence,
+    ),
+    "pdf-comment-generate": _Expected(
+        description="Generate inline review comments for a PDF document.",
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/pdf-comment/generate",
+        input_model=PdfCommentRequest,
+    ),
+    "agent-next-action": _Expected(
+        description=(
+            "Decide the next execution step for an in-progress agent workflow. Returns a"
+            " ToolCall, Completed, or CannotContinue action."
+        ),
+        mode="sync",
+        required_scope="mcp.tools.read",
+        route="/api/v1/agents/next-action",
+        input_model=AgentExecutionRequest,
+    ),
+}
+
+
+def test_manifest_version(manifest: dict[str, Any]) -> None:
+    assert manifest["version"] == 1
+
+
+def test_manifest_exposes_exactly_the_expected_capabilities(manifest: dict[str, Any]) -> None:
+    ids = {c["id"] for c in manifest["capabilities"]}
+    assert ids == set(_EXPECTED)
+
+
+def test_manifest_capability_metadata_and_schema(manifest: dict[str, Any]) -> None:
+    by_id = {c["id"]: c for c in manifest["capabilities"]}
+    for cap_id, expected in _EXPECTED.items():
+        entry = by_id[cap_id]
+        assert entry["description"] == expected.description, cap_id
+        assert entry["mode"] == expected.mode, cap_id
+        assert entry["required_scope"] == expected.required_scope, cap_id
+        assert entry["route"] == expected.route, cap_id
+        assert entry["input_schema"] == expected.input_model.model_json_schema(), cap_id