[MINOR] §11: No MCP server configuration or integration

[mvillmow]

Evidence

.claude/settings.json, repo root (no mcp-config)

Description

No Model Context Protocol server configuration is defined. MCP would allow AI agents to query Agamemnon or NATS state directly during development, which would be highly useful for this project.

Part of #92

[mvillmow]

Implementation Plan

Implementation Plan

Objective

Add a minimal, project-scoped MCP (Model Context Protocol) server to ProjectTelemachy so AI agents working in this repo can query live Agamemnon state (agents, team tasks) during development. The server is read-only — it wraps the two existing GET methods on AgamemnonClient (list_agents, get_tasks) and exposes nothing else. NATS event subscription is explicitly deferred: per CLAUDE.md, the NATS subscriber is planned under #92 and not yet wired, so there is nothing live to expose. This closes issue #173 by adding the .mcp.json configuration the audit flagged as missing, plus a small mcp_server.py module backing it. Tests are written against a thin, plan-owned seam (Dispatcher) — not against the MCP SDK's private internals — so the TDD RED → GREEN cycle is executable from a clean checkout without any spike-and-rewrite.

Approach

Decision 1 — Repo-root .mcp.json (project-scoped registration). Evidence: cat .claude/settings.json (71 bytes) holds only {"enabledPlugins": …}; it is per-user state and not the right home for shared MCP wiring. Project-scoped MCP servers are conventionally registered in a repo-root .mcp.json so every contributor cloning the repo picks them up.

Decision 2 — Two tools, both read-only. Evidence: grep -n "list_agents\|get_tasks" src/telemachy/agamemnon_client.py → agamemnon_client.py:184 (async def list_agents) and agamemnon_client.py:274 (async def get_tasks). These are the only client methods that are pure HTTP GET and don't require constructing a workflow. Exposing anything else would either need new client code (out of scope for a config issue) or risk write paths.

Decision 3 — Test against a plan-owned Dispatcher, not the SDK's internals. Evidence: the R2 review's major finding was that the previous test snippet poked server._tool_handlers (private, undocumented). Fix: factor the dispatch logic into a Dispatcher class in mcp_server.py whose public methods (list_tools(), call_tool(name, arguments)) are exactly what the test suite drives. build_server() then registers thin lambdas that delegate to the same Dispatcher. Tests never import mcp.server internals and run cleanly on a fresh checkout — eliminating the spike-before-RED dependency the R2 review flagged.

Decision 4 — Module lives at src/telemachy/mcp_server.py, registered via pyproject.toml [project.scripts] as telemachy-mcp. Evidence: src/telemachy/ is the canonical package; the entry-point pattern mirrors telemachy.cli. A [project.scripts] entry makes telemachy-mcp available on PATH after pixi install, so .mcp.json can spawn it without a brittle Python path. grep -n '\[project.scripts\]' pyproject.toml returns no matches today.

Decision 5 — Add mcp to pixi.toml [pypi-dependencies] only; do NOT add a [project] dependencies key to pyproject.toml. Evidence: pixi.toml:9-17 lists every runtime dep under [pypi-dependencies]; pyproject.toml:1-13 deliberately has no dependencies key. The pixi run license-audit task at pixi.toml:24 cross-checks pixi.toml only — splitting deps across both files breaks that invariant. The license audit must be re-run per CLAUDE.md guardrail.

Decision 6 — Reuse Settings.client_kwargs(). Evidence: src/telemachy/config.py:55 defines def client_kwargs(self) -> AgamemnonClientKwargs for exactly this DRY purpose (verified with grep -n "def client_kwargs" src/telemachy/config.py → line 55, correcting the R2 review's flagged inaccuracy from the prior plan that cited line 64). The MCP server will use the same path, picking up AGAMEMNON_URL, AGAMEMNON_API_KEY, REQUIRE_TLS, etc. from the existing env-var contract.

Decision 7 — One long-lived AgamemnonClient per server lifetime. Evidence: executor.py:424 uses AgamemnonClient as an async context manager around a whole workflow. The MCP server is a long-lived stdio process; opening the httpx.AsyncClient once in _run() and sharing it via the Dispatcher/build_server closure avoids TLS handshake overhead and matches the "long-lived process" mental model the R1 review flagged as P7-preferable.

Decision 8 — Pin mcp >= 1.0,<2.0. Evidence: an unconstrained >=1.0 lets a 2.x release silently land and break stdio wiring. Capping at <2.0 keeps the upgrade explicit; the spike confirms 1.x decorators (@server.list_tools(), @server.call_tool()), but build_server() calls them ONLY to register thin delegates, so even if a 1.x dot-release renames an internal accessor, the Dispatcher (and its tests) are unaffected.

Decision 9 — Capture spike output as a docstring constant. Evidence: the R2 review flagged that step 1's spike result had no documented artifact. Fix: the implementation order step 1 writes the confirmed SDK API surface as a _SDK_API_NOTES module-level string-literal constant in mcp_server.py (a checklist item, see step 1 below), so the spike result lives in source and the implementer cannot forget to record it.

Files to Create

.mcp.json (repo root)

Project-scoped MCP server registration. Spawns the telemachy-mcp console script over stdio. Inherits AGAMEMNON_URL, AGAMEMNON_API_KEY, NATS_URL, REQUIRE_TLS from the user's environment (no values hardcoded — matches CLAUDE.md REQUIRE_TLS=true default).

{
  "mcpServers": {
    "telemachy-agamemnon": {
      "type": "stdio",
      "command": "telemachy-mcp",
      "args": [],
      "env": {}
    }
  }
}

src/telemachy/mcp_server.py

Read-only MCP server. The Dispatcher class is the unit under test — it has no MCP dependency at all; it just routes a (name, arguments) pair to the appropriate AgamemnonClient read call and returns serialised JSON. build_server() is a thin SDK adapter that registers two decorators delegating to the same Dispatcher. This is the plan-owned seam that makes the R2 "tests cannot run before a spike" finding moot.

"""Read-only MCP server exposing Agamemnon state to AI agents.

Exposes two tools (both backed by existing AgamemnonClient GET methods):
- ``agamemnon_list_agents`` — wraps ``AgamemnonClient.list_agents``
- ``agamemnon_list_team_tasks`` — wraps ``AgamemnonClient.get_tasks``

No write endpoints are exposed. See issue #173.
"""

from __future__ import annotations

import asyncio
import json
from dataclasses import dataclass
from typing import Any

from telemachy.agamemnon_client import AgamemnonClient
from telemachy.config import settings

# Recorded in step 1's spike gate (see implementation_order). Updated to match
# the installed `mcp` SDK before `build_server()` is wired in step 4.
_SDK_API_NOTES = """
mcp SDK 1.x API used by build_server():
- mcp.server.Server(name) — server factory
- @server.list_tools() decorator on async () -> list[Tool]
- @server.call_tool() decorator on async (name, arguments) -> list[TextContent]
- mcp.server.stdio.stdio_server() — async context yielding (read, write) streams
- mcp.types.Tool, mcp.types.TextContent — return types
"""


@dataclass
class _ToolDescriptor:
    name: str
    description: str
    input_schema: dict[str, Any]


_TOOLS: list[_ToolDescriptor] = [
    _ToolDescriptor(
        name="agamemnon_list_agents",
        description="List all agents currently known to Agamemnon (read-only).",
        input_schema={"type": "object", "properties": {}, "additionalProperties": False},
    ),
    _ToolDescriptor(
        name="agamemnon_list_team_tasks",
        description="List all tasks for a given Agamemnon team (read-only).",
        input_schema={
            "type": "object",
            "properties": {"team_id": {"type": "string"}},
            "required": ["team_id"],
            "additionalProperties": False,
        },
    ),
]


class UnknownToolError(ValueError):
    """Raised when call_tool() receives a name not in _TOOLS."""


class Dispatcher:
    """Pure read-only routing layer over AgamemnonClient.

    Has NO dependency on the `mcp` SDK so it is unit-testable on a clean
    checkout without any spike. ``build_server()`` adapts it to the SDK.
    """

    def __init__(self, client: AgamemnonClient) -> None:
        self._client = client

    def list_tools(self) -> list[_ToolDescriptor]:
        return list(_TOOLS)

    async def call_tool(self, name: str, arguments: dict[str, Any]) -> str:
        """Return the tool's JSON-serialised result text."""
        if name == "agamemnon_list_agents":
            agents = await self._client.list_agents()
            return json.dumps(agents, indent=2)
        if name == "agamemnon_list_team_tasks":
            team_id = str(arguments["team_id"])
            tasks = await self._client.get_tasks(team_id)
            return json.dumps(tasks, indent=2)
        raise UnknownToolError(f"unknown tool: {name}")


def build_server(dispatcher: Dispatcher) -> Any:
    """Wire the Dispatcher into the mcp SDK. Thin adapter only."""
    from mcp.server import Server
    from mcp.types import TextContent, Tool

    server = Server("telemachy-agamemnon")

    @server.list_tools()
    async def _list_tools() -> list[Tool]:
        return [
            Tool(name=t.name, description=t.description, inputSchema=t.input_schema)
            for t in dispatcher.list_tools()
        ]

    @server.call_tool()
    async def _call_tool(name: str, arguments: dict[str, Any]) -> list[TextContent]:
        text = await dispatcher.call_tool(name, arguments)
        return [TextContent(type="text", text=text)]

    return server


async def _run() -> None:
    from mcp.server.stdio import stdio_server

    async with AgamemnonClient(**settings.client_kwargs()) as client:
        dispatcher = Dispatcher(client)
        server = build_server(dispatcher)
        async with stdio_server() as (read, write):
            await server.run(read, write, server.create_initialization_options())


def main() -> None:
    asyncio.run(_run())


if __name__ == "__main__":
    main()

tests/test_mcp_server.py

Pytest-asyncio tests targeting the Dispatcher seam — zero references to mcp.server internals, so the tests run on a fresh checkout the moment the source file lands. This resolves the R2 major finding directly.

"""Tests for the read-only MCP server (issue #173)."""

from __future__ import annotations

import json
from pathlib import Path
from unittest.mock import AsyncMock

import pytest

from telemachy.mcp_server import Dispatcher, UnknownToolError, _TOOLS


def test_list_tools_exposes_only_read_only_tool_names() -> None:
    dispatcher = Dispatcher(AsyncMock())
    names = {t.name for t in dispatcher.list_tools()}
    assert names == {"agamemnon_list_agents", "agamemnon_list_team_tasks"}


def test_tool_descriptors_have_object_input_schemas() -> None:
    for tool in _TOOLS:
        assert tool.input_schema["type"] == "object"
        assert tool.input_schema["additionalProperties"] is False


@pytest.mark.asyncio
async def test_call_tool_list_agents_delegates_to_client() -> None:
    client = AsyncMock()
    client.list_agents.return_value = [{"id": "a1"}, {"id": "a2"}]
    dispatcher = Dispatcher(client)
    text = await dispatcher.call_tool("agamemnon_list_agents", {})
    assert json.loads(text) == [{"id": "a1"}, {"id": "a2"}]
    client.list_agents.assert_awaited_once_with()


@pytest.mark.asyncio
async def test_call_tool_list_team_tasks_passes_team_id() -> None:
    client = AsyncMock()
    client.get_tasks.return_value = [{"subject": "build"}]
    dispatcher = Dispatcher(client)
    text = await dispatcher.call_tool("agamemnon_list_team_tasks", {"team_id": "t-42"})
    assert json.loads(text) == [{"subject": "build"}]
    client.get_tasks.assert_awaited_once_with("t-42")


@pytest.mark.asyncio
async def test_call_tool_unknown_name_raises() -> None:
    dispatcher = Dispatcher(AsyncMock())
    with pytest.raises(UnknownToolError):
        await dispatcher.call_tool("agamemnon_delete_agent", {})


def test_module_has_no_write_method_references() -> None:
    """Belt-and-suspenders: forbid write methods from appearing in the module text."""
    src = Path("src/telemachy/mcp_server.py").read_text()
    forbidden = [
        "create_agent",
        "start_agent",
        "delete_agent",
        "create_team",
        "create_task",
        "create_tasks",
        "assign_task",
        "teardown",
    ]
    for name in forbidden:
        assert name not in src, f"write-path symbol leaked into MCP server: {name}"


def test_build_server_is_thin_adapter_that_delegates_to_dispatcher() -> None:
    """build_server() must not duplicate dispatcher logic — verify by source inspection.

    The dispatcher is the unit-tested seam; build_server() should only wire
    decorators into the SDK. Keeps the read-only invariant single-sourced.
    """
    src = Path("src/telemachy/mcp_server.py").read_text()
    # build_server must reference dispatcher.list_tools and dispatcher.call_tool
    assert "dispatcher.list_tools()" in src
    assert "dispatcher.call_tool(" in src

docs/mcp.md

A one-page operator guide: what the server exposes, how to enable it (.mcp.json is auto-discovered by Claude Code at repo root), env vars consumed (none new — inherited from Settings), an offline-test recipe (mock Agamemnon with a fixture JSON via a tiny python -m http.server pointed at a directory containing v1/agents), and a Non-goals section listing write endpoints and NATS subscription as explicitly out of scope pending #92.

Files to Modify

pixi.toml

Add mcp to [pypi-dependencies] with an upper bound (Decision 8). All runtime deps live here per pixi.toml:9-17.

[pypi-dependencies]
telemachy         = { path = ".", editable = true }
pydantic          = ">=2.0"
pydantic-settings = ">=2.0"
httpx             = ">=0.27"
pyyaml            = ">=6.0"
rich              = ">=13.0"
typer             = ">=0.12"
mcp               = ">=1.0,<2.0"

pyproject.toml

Add a [project.scripts] table immediately after the [project] block at pyproject.toml:5-8. No [project] dependencies key is added — runtime deps stay in pixi.toml only (Decision 5; preserves the license-audit invariant).

[project.scripts]
telemachy-mcp = "telemachy.mcp_server:main"

.env.example

Append a short note that the MCP server reuses the existing Agamemnon env vars — no new variables introduced.

# === MCP (Model Context Protocol) server ===
# The `telemachy-mcp` console script (registered via .mcp.json) reuses
# AGAMEMNON_URL / AGAMEMNON_API_KEY / REQUIRE_TLS above. No new env vars
# are required. See docs/mcp.md.

justfile

Add an mcp recipe matching the run / plan env-var-forwarding pattern (insert after the plan recipe at the existing # === Workflow Execution === block).

# Run the read-only MCP server over stdio (for local smoke testing)
mcp:
    AGAMEMNON_URL={{AGAMEMNON_URL}} NATS_URL={{NATS_URL}} \
        pixi run telemachy-mcp

README.md

Add a one-line pointer under the existing project overview:

- See [docs/mcp.md](docs/mcp.md) for the read-only MCP server that lets AI
  agents query live Agamemnon state during development.

docs/license-audit.md

Add a row for mcp with its license (confirmed in step 1 spike: the official mcp SDK on PyPI is MIT). Re-run pixi run license-audit to cross-check that pixi.toml still matches.

CHANGELOG.md

Under ## [Unreleased]:

### Added
- Read-only MCP server (`telemachy-mcp`) exposing `agamemnon_list_agents`
  and `agamemnon_list_team_tasks` so AI agents can query Agamemnon state
  during development. Closes #173.

Implementation Order

1. Spike gate + artifact capture (≤30 min). Run pixi add 'mcp>=1.0,<2.0' (which writes pixi.toml and regenerates pixi.lock via pixi install — never hand-edit pixi.lock per CLAUDE.md). Open a REPL: import mcp; from mcp.server import Server; from mcp.server.stdio import stdio_server; from mcp.types import Tool, TextContent. Confirm the @server.list_tools() / @server.call_tool() decorator API and the stdio_server() async context shape. Checklist item: update the _SDK_API_NOTES constant in mcp_server.py with the actual imports/signatures observed. Confirm the SDK's license via pip show mcp | grep -i license and record in docs/license-audit.md. Stop and reconcile if any import path differs.
2. TDD RED. Write tests/test_mcp_server.py from the snippet above (it imports only Dispatcher, UnknownToolError, _TOOLS from telemachy.mcp_server — no SDK internals). Run pixi run pytest tests/test_mcp_server.py. Expect ImportError: cannot import name 'Dispatcher' — a clean, predictable RED.
3. TDD GREEN (Dispatcher first). Implement only the Dispatcher class, _TOOLS, _ToolDescriptor, and UnknownToolError in src/telemachy/mcp_server.py. Re-run the test file; the seven non-source-text tests pass. The source-text invariant test (test_module_has_no_write_method_references) also passes. The build-server adapter test fails until step 4.
4. Wire the SDK adapter. Add build_server(dispatcher), _run(), main() per the snippet. Re-run the suite — the adapter-shape test now passes.
5. Add [project.scripts]. Edit pyproject.toml, then pixi install so telemachy-mcp lands on PATH.
6. Add .mcp.json at repo root per the snippet.
7. Justfile recipe. Add the mcp recipe.
8. Docs + changelog. Create docs/mcp.md, edit README.md, .env.example, CHANGELOG.md, docs/license-audit.md.
9. Lint, format, mypy, tests. just lint && just format --check && just mypy && just test.
10. Manual smoke (offline-friendly). Stand up a fixture HTTP server: mkdir -p /tmp/fakeagm/v1 && printf '{"agents":[]}' > /tmp/fakeagm/v1/agents && (cd /tmp/fakeagm && python -m http.server 8080 &). Then REQUIRE_TLS=false AGAMEMNON_URL=http://localhost:8080 pixi run telemachy-mcp and send tools/list on stdin; verify both tool names appear. This satisfies AC10 without requiring a live Agamemnon, resolving the R2 review's offline-test gap.
11. Open PR following CLAUDE.md guardrails (no direct commit to main, no --admin merge, no --no-verify). PR body documents the read-only scope and links [MINOR] §11: No MCP server configuration or integration #173 + epic Epic: Strict audit 2026-04-28 — ProjectTelemachy #92.

Verification

Each command maps to one acceptance criterion. Every command is runnable on a clean checkout post-implementation — no pre-spike dependency.

# AC1 — MCP config exists at repo root and is valid JSON
test -f .mcp.json && pixi run python -c "import json; json.load(open('.mcp.json'))"

# AC2 — MCP server module + entry point importable
pixi run python -c "from telemachy.mcp_server import Dispatcher, build_server, main"

# AC3 — Read-only invariant: no write-path symbols in the server module
! grep -E 'create_agent|start_agent|delete_agent|create_team|create_task|assign_task|teardown' src/telemachy/mcp_server.py

# AC4 — Unit tests pass (Dispatcher dispatch + read-only-source-text + adapter shape)
pixi run pytest tests/test_mcp_server.py -v

# AC5 — Lint + format + mypy clean
just lint && just format --check && just mypy

# AC6 — Dependency-split invariant: runtime deps remain in pixi.toml only
grep -A2 '^\[project\]' pyproject.toml | grep -i '^dependencies' && exit 1 || echo "OK: no [project] dependencies"

# AC7 — Existing workflow tests unaffected (no regression)
just test

# AC8 — Existing workflow validation still passes
just validate workflows/example.yaml

# AC9 — License audit re-run prints the new mcp dep
pixi run license-audit | grep -i '^  mcp'

# AC10 — Smoke: server speaks MCP stdio against fixture Agamemnon (offline-capable)
mkdir -p /tmp/fakeagm/v1 && printf '{"agents":[]}' > /tmp/fakeagm/v1/agents
(cd /tmp/fakeagm && python -m http.server 8080 &) ; sleep 1
printf '%s\n' '{"jsonrpc":"2.0","id":1,"method":"tools/list"}' | \
    timeout 5 env REQUIRE_TLS=false AGAMEMNON_URL=http://localhost:8080 pixi run telemachy-mcp \
    | grep -E 'agamemnon_list_agents|agamemnon_list_team_tasks'

# AC11 — SDK upper-bound pin present
grep -E '^mcp\s*=\s*">=1\.0,<2\.0"' pixi.toml

Skills Used

• hephaestus:test-driven-development — RED-then-GREEN cycle in steps 2–4; tests target a plan-owned seam so RED is clean.
• hephaestus:verification — every acceptance criterion has a runnable command above, including the new offline smoke and upper-bound-pin checks.
• hephaestus:systematic-debugging — spike gate (step 1) records SDK surface as a source-code constant (_SDK_API_NOTES) so the result is durable.
• From the team knowledge base (Prior Learnings):
  • claude-code-settings-config — choice of repo-root .mcp.json over .claude/settings.json.
  • architecture-rest-api-httpx-integration — reuse AgamemnonClient at the boundary rather than reimplementing HTTP calls.
  • config-loader-schema-validation — route MCP config through the existing Settings.client_kwargs() typed accessor.
  • nats-publish-retry-backoff — Pydantic Settings env-var pattern; informed the decision to defer NATS exposure until the Epic: Strict audit 2026-04-28 — ProjectTelemachy #92 subscriber lands.
  • architecture-crosshost-nats-compose-deployment — .mcp.json env block stays empty so endpoints come from the user's environment, never hardcoded.

Changes from Review

The R2 review issued a NOGO with one major finding and three minor findings. Each is addressed below with a concrete change.

1. Major — "Test code as written accesses server._tool_handlers — a private attribute that may not exist. TDD RED may fail with AttributeError from the test harness itself, not from missing implementation. The plan delivers unrunnable test code."
   Fixed by Decisions 3 and 7 + the new Dispatcher seam. tests/test_mcp_server.py now imports only Dispatcher, UnknownToolError, and _TOOLS from telemachy.mcp_server — zero references to mcp.server internals. The RED in step 2 is now a clean ImportError: cannot import name 'Dispatcher'. Five of the seven tests can be written, run, and reasoned about with no mcp package installed at all. The remaining two (source-text invariant and adapter-shape) read the source file as text. The TDD RED → GREEN cycle is fully executable from a clean checkout.

2. Minor — "config.py line number cited as 64 but is actually 55."
   Fixed. Decision 6 now cites src/telemachy/config.py:55 (confirmed by grep -n "def client_kwargs" src/telemachy/config.py).

3. Minor — "Spike gate output (handler accessor path) has no documented artifact location."
   Fixed by Decision 9. The implementation order step 1 has an explicit checklist item to update the _SDK_API_NOTES module-level constant in mcp_server.py with the observed import paths and signatures. The spike result now lives in source code, not in the implementer's memory.

4. Minor — "NATS state query is deferred (issue says 'Agamemnon or NATS')."
   Acknowledged; defensible scope call retained. Per CLAUDE.md the NATS subscriber is planned under Epic: Strict audit 2026-04-28 — ProjectTelemachy #92 and not yet wired — there is no live NATS state to expose today. The docs/mcp.md Non-goals section now names NATS subscription explicitly as out-of-scope-pending-Epic: Strict audit 2026-04-28 — ProjectTelemachy #92 so future readers see the deferral instead of guessing. No change to scope.

Additional self-reflection items addressed (from my own post-plan critique):

5. Self-reflection — "MCP SDK upper bound unpinned." Added Decision 8 (mcp >= 1.0,<2.0) and AC11 verification.
6. Self-reflection — "AC10 requires live Agamemnon; no offline path." Implementation step 10 and AC10 now use a python -m http.server fixture for an offline-capable smoke test.
7. Self-reflection — "License audit assertion is unverified at plan time." Step 1 now mandates pip show mcp | grep -i license to confirm before editing docs/license-audit.md.

---

Final review verdict (from strict review loop)

Good. The codebase claims are accurate (claims 5 and 6 are correctly about files to be created, not pre-existing). Now let me analyze the R1 plan carefully against what the prior review flagged.

R1 Plan Analysis

Prior Review Major Finding: Unrunnable Test Code (P3/TDD + P7/POLA)

The prior review's major finding was:

"Test code as written accesses server._tool_handlers — a private attribute that may not exist. TDD RED may fail with AttributeError from the test harness itself, not from missing implementation."

R1 response: The plan introduced a Dispatcher class as a plan-owned seam. The new tests/test_mcp_server.py imports only Dispatcher, UnknownToolError, and _TOOLS — zero references to mcp.server internals. The seven tests in the snippet never touch private SDK attributes. RED is now a clean ImportError: cannot import name 'Dispatcher' at step 2, which is exactly the right kind of RED.

Assessment: Major finding is genuinely resolved. The architectural change (Dispatcher class as testable seam, build_server() as thin SDK adapter) eliminates the private-attribute access entirely. The test suite is fully runnable from a clean checkout — no spike required before writing tests. This is a substantive fix, not just an acknowledgment.

Prior Minor Finding 1: config.py line number (64 → 55)

R1 response: Decision 6 now explicitly cites src/telemachy/config.py:55. Verified correct by the Explore agent above.

Assessment: Resolved. ✓

Prior Minor Finding 2: Spike gate output has no documented artifact location

R1 response: Decision 9 introduces a _SDK_API_NOTES module-level string constant in mcp_server.py. Step 1 has an explicit checklist item to update this constant with observed imports/signatures. The spike result now lives in source code.

Assessment: Resolved. ✓ The artifact location is now concrete and named.

Prior Minor Finding 3: NATS state query deferred

R1 response: Acknowledged, scope call retained. docs/mcp.md Non-goals section will name NATS explicitly. This is defensible per CLAUDE.md (NATS subscriber not yet wired under #92).

Assessment: Appropriately handled. The issue title is [MINOR] and the issue body frames NATS as aspirational ("would be highly useful"). Per CLAUDE.md, there's nothing to expose yet. Scope retention is sound.

---

Fresh Dimension-by-Dimension Review

Dimension 1 — Requirements Alignment

The issue requires: (a) MCP server configuration in .claude/settings.json or equivalent, (b) AI agents able to query Agamemnon or NATS state during development, (c) part of #92.

Plan addresses: .mcp.json at repo root (correct location for project-scoped MCP, per Decision 1 with evidence), two read-only Agamemnon tools, NATS deferred with explicit rationale. All acceptance criteria have concrete plan steps. ✓

Dimension 2 — Plan Completeness

Design (9 documented decisions), implementation (file-by-file with complete code snippets), tests (7 tests in tests/test_mcp_server.py), docs (docs/mcp.md, README, CHANGELOG, license audit), verification (11 ACs with runnable commands). Rollback is trivially git revert for a purely additive change. ✓

Dimension 3 — Concreteness

All files named with paths, all line numbers verified accurate, code snippets are complete. Implementation order is unambiguous (11 steps). ✓

Dimension 4 — Risk Surface

• Read-only invariant: enforced three ways (only GET client methods, source-text test scanning for write symbols, Decision 2 doc)
• pixi.lock: handled via pixi add not hand-edit
• License audit: AC9 + step 1 checklist
• TLS config: inherited from Settings.client_kwargs(), no new env vars
• SDK upper bound: Decision 8 pins mcp >= 1.0,<2.0 with AC11 verification

One residual concern from the plan's own "Learnings captured" section: the stdio_server() async context contract is unverified at plan time (the spike in step 1 confirms decorator API but not stdio_server signature). The plan notes this explicitly as a known risk, and step 1 is designed to catch it before step 2. This is adequately mitigated by the spike gate pattern. ✓

Dimension 5 — Verification Plan

11 ACs with runnable bash commands. AC10 uses an offline fixture (python -m http.server) — no live Agamemnon required. All commands are executable post-implementation on a clean checkout. ✓

Dimension 6 — Stage Handoff

Complete code snippets for all files. 9 documented decisions with evidence. Implementation order is deterministic. The spike gate is intentionally the first step and bounded at ≤30 min. The plan produces concrete artifacts the implementer can act on. ✓

---

Software Engineering Principles Re-check

P1 (KISS): The Dispatcher class addition from R1 is a good design — it's the simplest seam that makes the code testable without SDK internals. Not over-engineered. build_server() is 12 lines of thin delegation. ✓

P2 (YAGNI): All components map to the issue requirement. The Dispatcher class is genuinely needed as the testable seam; it's not speculative architecture. The _SDK_API_NOTES constant is a documented note, not functionality. ✓

P3 (TDD): The R1 plan fixes this dimension. The Dispatcher class has no mcp dependency, so tests/test_mcp_server.py can be written and produces a clean ImportError RED before any implementation exists. Five of seven tests (all except the two source-text inspection tests) are fully runnable without the spike. The two source-text tests are static file readers, not dependent on SDK internals. The TDD cycle is now: write tests → RED (ImportError) → implement Dispatcher → GREEN → implement build_server → full GREEN. ✓

P4 (DRY): AgamemnonClient and Settings.client_kwargs() reused, no duplication. ✓

P5 (SOLID):

• SRP: mcp_server.py has one responsibility. Dispatcher handles routing; build_server() handles SDK wiring. Two classes, each with one reason to change. ✓
• DIP: Dispatcher takes AgamemnonClient by injection, testable with AsyncMock. ✓

P6 (Modularity): Clean module boundary. MCP adapter doesn't bleed into existing modules. ✓

P7 (POLA): The R1 plan eliminates the P7 violation from the prior review. Tests no longer access private SDK attributes. The Dispatcher API is intuitive: list_tools() returns tool descriptors, call_tool(name, arguments) returns JSON string. UnknownToolError is a typed exception, not a silent failure. ✓

---

Residual Observations (Minor, not blocking)

1. The test_build_server_is_thin_adapter_that_delegates_to_dispatcher test does a source-text string search ("dispatcher.list_tools()" in src). This is fragile if formatting changes slightly (e.g., dispatcher.list_tools () or a local variable rename), but it's a legitimate belt-and-suspenders check and clearly documented as such. Not a finding.

2. The "Learnings captured" section notes three unverified SDK claims (stdio_server() signature, Tool/TextContent constructor signatures, .mcp.json auto-discovery). These are honest disclosures, mitigated by the spike gate in step 1. The plan doesn't pretend these are verified — it names them as risks the spike will resolve. ✓

3. The plan's AC9 (pixi run license-audit | grep -i '^ mcp') verifies the entry exists but not its correctness. This is a pre-existing limitation of the license audit tooling, not a new gap introduced by this plan.

---

Verdict Derivation

The prior review's major finding (unrunnable test code, P3/TDD + P7/POLA) has been substantively resolved through the Dispatcher seam architecture — not merely acknowledged. The three minor findings are also resolved. No new critical or major findings emerge from the R1 plan. All six stage-specific dimensions are covered. All P1–P7 principles are satisfied.

Grade: B
Verdict: GO

---

Generated by Claude Code Planner (strict review loop)

[mvillmow]

🔍 Plan Review

Good. The codebase claims are accurate (claims 5 and 6 are correctly about files to be created, not pre-existing). Now let me analyze the R1 plan carefully against what the prior review flagged.

R1 Plan Analysis

Prior Review Major Finding: Unrunnable Test Code (P3/TDD + P7/POLA)

The prior review's major finding was:

"Test code as written accesses server._tool_handlers — a private attribute that may not exist. TDD RED may fail with AttributeError from the test harness itself, not from missing implementation."

R1 response: The plan introduced a Dispatcher class as a plan-owned seam. The new tests/test_mcp_server.py imports only Dispatcher, UnknownToolError, and _TOOLS — zero references to mcp.server internals. The seven tests in the snippet never touch private SDK attributes. RED is now a clean ImportError: cannot import name 'Dispatcher' at step 2, which is exactly the right kind of RED.

Assessment: Major finding is genuinely resolved. The architectural change (Dispatcher class as testable seam, build_server() as thin SDK adapter) eliminates the private-attribute access entirely. The test suite is fully runnable from a clean checkout — no spike required before writing tests. This is a substantive fix, not just an acknowledgment.

Prior Minor Finding 1: config.py line number (64 → 55)

R1 response: Decision 6 now explicitly cites src/telemachy/config.py:55. Verified correct by the Explore agent above.

Assessment: Resolved. ✓

Prior Minor Finding 2: Spike gate output has no documented artifact location

R1 response: Decision 9 introduces a _SDK_API_NOTES module-level string constant in mcp_server.py. Step 1 has an explicit checklist item to update this constant with observed imports/signatures. The spike result now lives in source code.

Assessment: Resolved. ✓ The artifact location is now concrete and named.

Prior Minor Finding 3: NATS state query deferred

R1 response: Acknowledged, scope call retained. docs/mcp.md Non-goals section will name NATS explicitly. This is defensible per CLAUDE.md (NATS subscriber not yet wired under #92).

Assessment: Appropriately handled. The issue title is [MINOR] and the issue body frames NATS as aspirational ("would be highly useful"). Per CLAUDE.md, there's nothing to expose yet. Scope retention is sound.

---

Fresh Dimension-by-Dimension Review

Dimension 1 — Requirements Alignment

The issue requires: (a) MCP server configuration in .claude/settings.json or equivalent, (b) AI agents able to query Agamemnon or NATS state during development, (c) part of #92.

Plan addresses: .mcp.json at repo root (correct location for project-scoped MCP, per Decision 1 with evidence), two read-only Agamemnon tools, NATS deferred with explicit rationale. All acceptance criteria have concrete plan steps. ✓

Dimension 2 — Plan Completeness

Design (9 documented decisions), implementation (file-by-file with complete code snippets), tests (7 tests in tests/test_mcp_server.py), docs (docs/mcp.md, README, CHANGELOG, license audit), verification (11 ACs with runnable commands). Rollback is trivially git revert for a purely additive change. ✓

Dimension 3 — Concreteness

All files named with paths, all line numbers verified accurate, code snippets are complete. Implementation order is unambiguous (11 steps). ✓

Dimension 4 — Risk Surface

• Read-only invariant: enforced three ways (only GET client methods, source-text test scanning for write symbols, Decision 2 doc)
• pixi.lock: handled via pixi add not hand-edit
• License audit: AC9 + step 1 checklist
• TLS config: inherited from Settings.client_kwargs(), no new env vars
• SDK upper bound: Decision 8 pins mcp >= 1.0,<2.0 with AC11 verification

One residual concern from the plan's own "Learnings captured" section: the stdio_server() async context contract is unverified at plan time (the spike in step 1 confirms decorator API but not stdio_server signature). The plan notes this explicitly as a known risk, and step 1 is designed to catch it before step 2. This is adequately mitigated by the spike gate pattern. ✓

Dimension 5 — Verification Plan

11 ACs with runnable bash commands. AC10 uses an offline fixture (python -m http.server) — no live Agamemnon required. All commands are executable post-implementation on a clean checkout. ✓

Dimension 6 — Stage Handoff

Complete code snippets for all files. 9 documented decisions with evidence. Implementation order is deterministic. The spike gate is intentionally the first step and bounded at ≤30 min. The plan produces concrete artifacts the implementer can act on. ✓

---

Software Engineering Principles Re-check

P1 (KISS): The Dispatcher class addition from R1 is a good design — it's the simplest seam that makes the code testable without SDK internals. Not over-engineered. build_server() is 12 lines of thin delegation. ✓

P2 (YAGNI): All components map to the issue requirement. The Dispatcher class is genuinely needed as the testable seam; it's not speculative architecture. The _SDK_API_NOTES constant is a documented note, not functionality. ✓

P3 (TDD): The R1 plan fixes this dimension. The Dispatcher class has no mcp dependency, so tests/test_mcp_server.py can be written and produces a clean ImportError RED before any implementation exists. Five of seven tests (all except the two source-text inspection tests) are fully runnable without the spike. The two source-text tests are static file readers, not dependent on SDK internals. The TDD cycle is now: write tests → RED (ImportError) → implement Dispatcher → GREEN → implement build_server → full GREEN. ✓

P4 (DRY): AgamemnonClient and Settings.client_kwargs() reused, no duplication. ✓

P5 (SOLID):

• SRP: mcp_server.py has one responsibility. Dispatcher handles routing; build_server() handles SDK wiring. Two classes, each with one reason to change. ✓
• DIP: Dispatcher takes AgamemnonClient by injection, testable with AsyncMock. ✓

P6 (Modularity): Clean module boundary. MCP adapter doesn't bleed into existing modules. ✓

P7 (POLA): The R1 plan eliminates the P7 violation from the prior review. Tests no longer access private SDK attributes. The Dispatcher API is intuitive: list_tools() returns tool descriptors, call_tool(name, arguments) returns JSON string. UnknownToolError is a typed exception, not a silent failure. ✓

---

Residual Observations (Minor, not blocking)

1. The test_build_server_is_thin_adapter_that_delegates_to_dispatcher test does a source-text string search ("dispatcher.list_tools()" in src). This is fragile if formatting changes slightly (e.g., dispatcher.list_tools () or a local variable rename), but it's a legitimate belt-and-suspenders check and clearly documented as such. Not a finding.

2. The "Learnings captured" section notes three unverified SDK claims (stdio_server() signature, Tool/TextContent constructor signatures, .mcp.json auto-discovery). These are honest disclosures, mitigated by the spike gate in step 1. The plan doesn't pretend these are verified — it names them as risks the spike will resolve. ✓

3. The plan's AC9 (pixi run license-audit | grep -i '^ mcp') verifies the entry exists but not its correctness. This is a pre-existing limitation of the license audit tooling, not a new gap introduced by this plan.

---

Verdict Derivation

The prior review's major finding (unrunnable test code, P3/TDD + P7/POLA) has been substantively resolved through the Dispatcher seam architecture — not merely acknowledged. The three minor findings are also resolved. No new critical or major findings emerge from the R1 plan. All six stage-specific dimensions are covered. All P1–P7 principles are satisfied.

Grade: B
Verdict: GO

[mvillmow]

Filed consolidated follow-up issue #283 covering 1 item(s); 3 additional item(s) were considered and rejected as out of scope (see PR body).