fix(ingest): run the MCP stdio transport against a real server, and repair its error contract (kø-x)
`stdio_call_tool` shipped never having been executed end to end — docs said so explicitly. Running it found a real defect: `stdio_client` and `ClientSession` are each an anyio task group, and anyio re-packages anything leaving one in a `BaseExceptionGroup`. Both errors the transport raises from inside the session (`mcp_tool_error`, `mcp_non_text_content`) therefore reached callers as exception groups, never as the `IngestError` the whole Door A path catches and switches on by `code`. No canned-tool test could see this: they never enter a task group. `_unwrap_ingest_error` recovers the owned error and re-raises it; anything unowned is re-raised untouched, so this narrows an exception group rather than blanket-catching. Duck-typed on `.exceptions` because `except*`/`ExceptionGroup` are 3.11+ and this project supports >=3.10. Verified against a REAL server subprocess (a local process costs no model tokens, so the repo's cost discipline is untouched; the contract tests still spawn nothing): `examples/ingest-golden-mcp/` + `tests/test_ingest_golden_mcp.py` — byte-identical golden extraction mirroring the http/sql goldens, plus the tool-error and missing-`server_ref` branches. Also recorded: a server on the ingest path must expose a NULL-ARGUMENT tool, so `datasource.build_mcp_server` cannot serve it (`retrieve_cost_docs(query)` has a required parameter, verified to return an error result). The two are separate seams by design. Load-bearing MEASURED, five mutations all RED: detach the unwrap · detach `initialize()` · make the error code generic · detach the `isError` branch · change one byte of the served body. 612 -> 615 tests. ruff + format + mypy clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WiY53sm8JFqk7NN75g5wRS
This commit is contained in:
parent
126807aee7
commit
9dc3722161
9 changed files with 272 additions and 9 deletions
14
CLAUDE.md
14
CLAUDE.md
|
|
@ -53,6 +53,20 @@ Python ≥3.10. MAF (`agent-framework-core` 1.9.0). Pakkehåndtering: `uv`. To b
|
|||
`IngestStampError` — mens hver halvdel alene er lovlig (kuratert innhold kan bære ett
|
||||
provenance-felt). Validering, ALDRI reparasjon: ingenting skrives. Uten dette kunne en kuratert fil
|
||||
bli stille slettet av en senere re-materialisering, som fjerner nøyaktig det som bærer stempelet.
|
||||
- **`IngestError` må overleve anyio-task-gruppene (kø-(x), 2026-08-03):** `stdio_client` og
|
||||
`ClientSession` er hver sin task group, og anyio pakker ALT som forlater en av dem i en
|
||||
`BaseExceptionGroup`. Derfor nådde `stdio_call_tool`s egne feil (`mcp_tool_error`,
|
||||
`mcp_non_text_content`) kalleren som en exception group — aldri som `IngestError`, som er typen
|
||||
hele Door A fanger og switcher på via `code`. `_unwrap_ingest_error` pakker ut og re-raiser den
|
||||
eide feilen; alt annet re-raises URØRT (innsnevring, aldri blanket re-raise). Duck-typet på
|
||||
`.exceptions`, fordi `except*`/`ExceptionGroup` er 3.11+ og repoet er `>=3.10`. **Ingen
|
||||
canned-tool-test kunne fanget dette** — de går aldri inn i en task group; defekten dukket opp
|
||||
første gang koden faktisk ble kjørt. En MCP-server på ingest-stien må eksponere en
|
||||
**null-argument-tool** (URL-en bærer begge koordinatene, tool kalles med `{}`), så
|
||||
`datasource.build_mcp_server` kan IKKE serve den — `retrieve_cost_docs(query)` har et påkrevd
|
||||
argument og returnerer et error-result. De to er separate sømmer med vilje. Load-bearing MÅLT
|
||||
(`tests/test_ingest_golden_mcp.py`), fem mutasjoner alle røde: detach unwrappingen · detach
|
||||
`initialize()` · gjør feilkoden generisk · detach `isError`-grenen · endre ett byte av bodyen.
|
||||
- **Stoppkriterier + budsjett-tak påkrevd ved oppstart** (fail-fast, aldri ubegrenset loop).
|
||||
- **Group Chat maker-checker** som debatt-default (IKKE Magentic, som er eksperimentell).
|
||||
- **To falsifiserere, samme kandidat (Steg 3/4, målbilde §2/§6):** den deterministiske validatoren
|
||||
|
|
|
|||
|
|
@ -150,12 +150,28 @@ forget. The transport discriminator **gates rather than labels**: `mcp_get` refu
|
|||
not own, so an MCP transport can never quietly serve a plain `https://` manifest and leave the
|
||||
bundle's provenance claiming a transport that was never used.
|
||||
|
||||
**Not verified against a live MCP server.** Every test injects a canned tool call, so the suite
|
||||
spawns no subprocess and opens no socket (cost discipline). `stdio_call_tool` — the real stdio path
|
||||
— is therefore **written but never executed end to end**; a deployer using it should expect to shake
|
||||
it out. The seam it plugs into (`mcp_get`, the discriminator, the gate) *is* measured, including six
|
||||
detach mutations. There is no `examples/ingest-golden-mcp/` fixture, and MCP remains **unwired in
|
||||
the optimiser run path** — the in-process `FunctionTool` seam stays the default there.
|
||||
**Verified against a real MCP server subprocess (2026-08-03).** `stdio_call_tool` was previously
|
||||
written but never executed end to end; `examples/ingest-golden-mcp/` + `tests/test_ingest_golden_mcp.py`
|
||||
now run it against a live server process — byte-identical golden extraction, plus the tool-error and
|
||||
missing-`server_ref` branches. Five detach mutations measured RED (unwrap, `initialize()`, error-code
|
||||
identity, the `isError` branch, one body byte). A local subprocess costs no model tokens, so the cost
|
||||
discipline is untouched; the contract tests still inject a canned tool and spawn nothing.
|
||||
|
||||
**What running it actually found — the error contract was broken.** `stdio_client` and
|
||||
`ClientSession` are each an anyio task group, and anyio re-packages anything leaving one in a
|
||||
`BaseExceptionGroup`. Every error raised inside the session (`mcp_tool_error`,
|
||||
`mcp_non_text_content`) therefore reached callers as an exception group, never as the `IngestError`
|
||||
the whole Door A path catches and switches on by `code`. Fixed by unwrapping the group and
|
||||
re-raising the owned error; anything unowned is re-raised untouched. **No canned-tool test could
|
||||
have caught this** — they never enter a task group. This is the case for running what you ship.
|
||||
|
||||
**A server on this path must expose a null-argument tool.** The URL carries both coordinates and the
|
||||
tool is called with an empty argument dict, so `datasource.build_mcp_server` **cannot** serve ingest:
|
||||
its `retrieve_cost_docs(query)` has a required parameter (verified — it returns an error result).
|
||||
The two are separate seams by design: `build_mcp_server` serves the agents' retrieval path.
|
||||
|
||||
**Still true:** MCP remains **unwired in the optimiser run path** — the in-process `FunctionTool`
|
||||
seam stays the default there. The timeout path (`asyncio.wait_for`) is not covered by a test.
|
||||
|
||||
**Where the D7 sibling stands (målbilde §11 boundary).** The Claude Agent SDK sibling built the
|
||||
**file/CSV and SQL** connectors — mirroring I3/I5 — with bit-identical golden extractions. **HTTP
|
||||
|
|
|
|||
2
examples/ingest-golden-mcp/expected-bundle/index.md
Normal file
2
examples/ingest-golden-mcp/expected-bundle/index.md
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
Golden extraction case: one MCP stdio extract (http source family, mcp+stdio transport).
|
||||
- [Cost documentation](ingest-cost-docs.md)
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
---
|
||||
type: concept
|
||||
title: Cost documentation
|
||||
source_system: docs-mcp
|
||||
source_query: cost_docs
|
||||
ingested_at: 2026-08-03T12:00:00Z
|
||||
ingest_manifest: manifest@18577a264477cd11
|
||||
generated: true
|
||||
---
|
||||
|
||||
```
|
||||
{"service": "cost-docs", "state": "ready | partial", "path": "c:\temp\cache"}
|
||||
```
|
||||
1
examples/ingest-golden-mcp/ingested-at.txt
Normal file
1
examples/ingest-golden-mcp/ingested-at.txt
Normal file
|
|
@ -0,0 +1 @@
|
|||
2026-08-03T12:00:00Z
|
||||
14
examples/ingest-golden-mcp/manifest.json
Normal file
14
examples/ingest-golden-mcp/manifest.json
Normal file
|
|
@ -0,0 +1,14 @@
|
|||
{
|
||||
"manifest_version": 1,
|
||||
"source": {"type": "http", "id": "docs-mcp", "base_url": "mcp+stdio://PORTFOLIO_GOLDEN_MCP", "credential_ref": null},
|
||||
"bundle_summary": "Golden extraction case: one MCP stdio extract (http source family, mcp+stdio transport).",
|
||||
"extractions": [
|
||||
{
|
||||
"id": "cost-docs",
|
||||
"title": "Cost documentation",
|
||||
"query": "cost_docs",
|
||||
"okf_type": "concept",
|
||||
"max_rows": 10
|
||||
}
|
||||
]
|
||||
}
|
||||
27
examples/ingest-golden-mcp/server.py
Normal file
27
examples/ingest-golden-mcp/server.py
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
r"""Golden-case MCP stdio server — the counterpart `stdio_call_tool` is measured against.
|
||||
|
||||
Deliberately a NULL-ARGUMENT tool. The ingest transport carries its two coordinates in the URL
|
||||
(`mcp+stdio://<server_ref>/<tool>`) and calls the tool with an empty argument dict, so a server
|
||||
consumed by Door A must expose a tool that needs none. `datasource.build_mcp_server` does NOT
|
||||
qualify — its `retrieve_cost_docs(query)` takes a required argument; it serves the agents'
|
||||
retrieval path, not the ingest path.
|
||||
|
||||
The body is a fixed literal so the golden is byte-deterministic, and it carries a `|` and a `\`
|
||||
to pin the §5 rule that an http-family body is rendered VERBATIM inside a fence, never escaped
|
||||
into a markdown table.
|
||||
"""
|
||||
|
||||
from mcp.server.fastmcp import FastMCP
|
||||
|
||||
server = FastMCP("portfolio-optimiser-golden")
|
||||
|
||||
_BODY = '{"service": "cost-docs", "state": "ready | partial", "path": "c:\\temp\\cache"}\n'
|
||||
|
||||
|
||||
@server.tool()
|
||||
def cost_docs() -> str:
|
||||
return _BODY
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
server.run()
|
||||
|
|
@ -127,8 +127,16 @@ def stdio_call_tool(
|
|||
variable rather than as a tool argument: tool arguments are echoed in MCP progress and result
|
||||
payloads and would land in logs.
|
||||
|
||||
Kept thin and separate from :func:`mcp_get` on purpose — every contract test injects a canned
|
||||
``McpCallTool`` instead, so the suite spawns no subprocess (cost discipline: no heavy runs).
|
||||
Kept thin and separate from :func:`mcp_get` on purpose — the CONTRACT tests
|
||||
(``tests/test_ingest_mcp_loadbearing.py``) inject a canned ``McpCallTool``, so they spawn no
|
||||
subprocess. The stdio path itself is covered separately by ``tests/test_ingest_golden_mcp.py``,
|
||||
which runs it against a REAL server subprocess (a local process costs no model tokens, so the
|
||||
cost discipline that governs this repo is untouched).
|
||||
|
||||
A server consumed here must expose a NULL-ARGUMENT tool: the URL carries both coordinates and
|
||||
the tool is invoked with an empty argument dict. ``datasource.build_mcp_server`` does not
|
||||
qualify — its ``retrieve_cost_docs(query)`` has a required parameter; it serves the agents'
|
||||
retrieval path, not ingest.
|
||||
"""
|
||||
import asyncio
|
||||
|
||||
|
|
@ -161,11 +169,42 @@ def stdio_call_tool(
|
|||
)
|
||||
return _body_from_content(result.content, tool=tool, server_ref=server_ref)
|
||||
|
||||
return asyncio.run(asyncio.wait_for(run(), timeout=timeout_seconds))
|
||||
try:
|
||||
return asyncio.run(asyncio.wait_for(run(), timeout=timeout_seconds))
|
||||
except BaseException as exc: # noqa: BLE001 — re-raised unchanged unless we own it
|
||||
owned = _unwrap_ingest_error(exc)
|
||||
if owned is None:
|
||||
raise
|
||||
raise owned from exc
|
||||
|
||||
return call_tool
|
||||
|
||||
|
||||
def _unwrap_ingest_error(exc: BaseException) -> IngestError | None:
|
||||
"""Recover an :class:`IngestError` that anyio re-packaged into a task-group exception group.
|
||||
|
||||
``stdio_client`` and ``ClientSession`` are each an anyio task group, and anyio wraps ANYTHING
|
||||
leaving one in a ``BaseExceptionGroup`` — nested once per group. So every error this transport
|
||||
raises from inside the session (``mcp_tool_error``, ``mcp_non_text_content``) reached callers as
|
||||
an exception group, never as the ``IngestError`` the whole Door A path catches and switches on by
|
||||
``code``. Found only by running against a real server; the canned-tool tests never enter a task
|
||||
group.
|
||||
|
||||
Duck-typed on ``.exceptions`` rather than ``except*`` / ``ExceptionGroup``: both are 3.11+, and
|
||||
this project supports ``>=3.10``, where anyio raises the ``exceptiongroup`` backport instead.
|
||||
|
||||
Returns ``None`` for anything we do not own, so the caller re-raises it untouched — this
|
||||
narrows an exception group, it is never a blanket re-raise.
|
||||
"""
|
||||
if isinstance(exc, IngestError):
|
||||
return exc
|
||||
for sub in getattr(exc, "exceptions", ()):
|
||||
owned = _unwrap_ingest_error(sub)
|
||||
if owned is not None:
|
||||
return owned
|
||||
return None
|
||||
|
||||
|
||||
def _body_from_content(content: list[Any], *, tool: str, server_ref: str) -> str:
|
||||
"""Concatenate an MCP tool result's TEXT blocks into the verbatim body (§5).
|
||||
|
||||
|
|
|
|||
137
tests/test_ingest_golden_mcp.py
Normal file
137
tests/test_ingest_golden_mcp.py
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
"""Golden regression for the MCP stdio transport — run against a REAL MCP server subprocess.
|
||||
|
||||
Mirrors ``tests/test_ingest_golden_http.py`` / ``_sql.py``, and closes the one gap those two could
|
||||
not: every other MCP test injects a canned ``McpCallTool``, so ``stdio_call_tool`` — the real stdio
|
||||
path — was written but never executed. Docs said so explicitly. This module executes it.
|
||||
|
||||
**Why a real subprocess and not an in-process server.** Driving ``datasource.build_mcp_server``
|
||||
in-process would exercise ``mcp_get`` (already measured, six detach mutations) and not one line of
|
||||
``stdio_call_tool``: the env-var resolution, ``StdioServerParameters``, the ``stdio_client``
|
||||
transport, session initialisation, the ``isError`` branch and the timeout wrapper all live on the
|
||||
subprocess path. Only spawning a server makes "run against a real MCP server" a true claim. The cost
|
||||
discipline this repo enforces is about MODEL calls and Azure — a local Python subprocess costs
|
||||
neither, and the canned-tool contract tests stay subprocess-free.
|
||||
|
||||
**Why the golden server takes no arguments.** The transport carries its two coordinates in the URL
|
||||
(``mcp+stdio://<server_ref>/<tool>``) and calls the tool with an empty argument dict.
|
||||
``datasource.build_mcp_server`` therefore CANNOT serve this path — its ``retrieve_cost_docs(query)``
|
||||
has a required parameter, so the call fails with an error result. That is a real property of the two
|
||||
seams, not an accident of the fixture: ``build_mcp_server`` serves the agents' retrieval path, this
|
||||
serves ingest.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from portfolio_optimiser.ingest import materialize
|
||||
from portfolio_optimiser.ingest_mcp import IngestError, mcp_get, stdio_call_tool
|
||||
|
||||
GOLDEN = Path(__file__).resolve().parents[1] / "examples" / "ingest-golden-mcp"
|
||||
_SERVER_REF = "PORTFOLIO_GOLDEN_MCP"
|
||||
|
||||
|
||||
def _bundle_bytes(directory: Path) -> dict[str, bytes]:
|
||||
return {p.name: p.read_bytes() for p in sorted(directory.iterdir()) if p.is_file()}
|
||||
|
||||
|
||||
def _transport(server_script: Path, monkeypatch: pytest.MonkeyPatch, **kwargs: object):
|
||||
"""Point ``server_ref`` at this interpreter and pass the server script as its argument.
|
||||
|
||||
The manifest names an ENV VAR, never an executable path (spec §4), which is exactly why the
|
||||
golden stays checkout-location independent: the path travels in ``args``, not in the bundle.
|
||||
"""
|
||||
monkeypatch.setenv(_SERVER_REF, sys.executable)
|
||||
return mcp_get(stdio_call_tool(args=(str(server_script),), **kwargs)) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_golden_mcp_extraction_is_bit_deterministic(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""The §11 golden case for ``mcp+stdio``, byte-for-byte, against a live server subprocess.
|
||||
|
||||
RED when any byte of the expected bundle diverges — and, unlike every other MCP test, RED if the
|
||||
real stdio transport stops working at all.
|
||||
"""
|
||||
ingested_at = (GOLDEN / "ingested-at.txt").read_text(encoding="utf-8").strip()
|
||||
get = _transport(GOLDEN / "server.py", monkeypatch)
|
||||
out = tmp_path / "bundle"
|
||||
|
||||
materialize(
|
||||
GOLDEN / "manifest.json",
|
||||
out,
|
||||
ingested_at=ingested_at,
|
||||
allow_network=True,
|
||||
http_get=get,
|
||||
)
|
||||
|
||||
expected = _bundle_bytes(GOLDEN / "expected-bundle")
|
||||
actual = _bundle_bytes(out)
|
||||
# File-SET equality first — catches extra AND missing files, not just diverging bytes.
|
||||
assert actual.keys() == expected.keys()
|
||||
for name, content in expected.items():
|
||||
assert actual[name] == content, f"golden byte divergence in {name}"
|
||||
|
||||
# §10: a second run over the same output leaves every byte unchanged.
|
||||
materialize(
|
||||
GOLDEN / "manifest.json",
|
||||
out,
|
||||
ingested_at=ingested_at,
|
||||
allow_network=True,
|
||||
http_get=get,
|
||||
)
|
||||
assert _bundle_bytes(out) == expected
|
||||
|
||||
|
||||
def _write_server(tmp_path: Path, body: str) -> Path:
|
||||
"""A throwaway MCP stdio server for the failure paths — never the committed golden fixture."""
|
||||
script = tmp_path / "failing_server.py"
|
||||
script.write_text(
|
||||
"from mcp.server.fastmcp import FastMCP\n"
|
||||
"server = FastMCP('probe')\n" + body + "\nif __name__ == '__main__':\n server.run()\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
return script
|
||||
|
||||
|
||||
def test_tool_error_reaches_the_caller_as_ingest_error(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A failing tool must surface as ``IngestError(code='mcp_tool_error')`` AT THE CALL SITE.
|
||||
|
||||
This is the property only a real server can prove. ``stdio_call_tool`` raises inside two nested
|
||||
anyio task groups (``stdio_client`` and ``ClientSession``), and anyio re-packages anything
|
||||
leaving a task group as a ``BaseExceptionGroup``. So the raise the module carefully wrote never
|
||||
reached callers as an ``IngestError`` — and the entire ingest path is built on catching that
|
||||
type, by ``code``. RED before the unwrap; the canned-tool tests cannot see this at all, because
|
||||
they never enter a task group.
|
||||
"""
|
||||
script = _write_server(
|
||||
tmp_path,
|
||||
"@server.tool()\ndef cost_docs() -> str:\n raise RuntimeError('server-side failure')\n",
|
||||
)
|
||||
get = _transport(script, monkeypatch)
|
||||
|
||||
with pytest.raises(IngestError) as excinfo:
|
||||
get(f"mcp+stdio://{_SERVER_REF}/cost_docs", None)
|
||||
assert excinfo.value.code == "mcp_tool_error"
|
||||
|
||||
|
||||
def test_missing_server_ref_fails_before_any_process_is_spawned(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""CONTROL — the fail-fast branch that runs BEFORE the async machinery.
|
||||
|
||||
It proves the unwrap above is not papering over a blanket re-raise: this error never enters a
|
||||
task group, so it must arrive as a plain ``IngestError`` both before and after the fix. It also
|
||||
pins the §4 promise that a missing env var is refused without spawning anything.
|
||||
"""
|
||||
monkeypatch.delenv(_SERVER_REF, raising=False)
|
||||
get = mcp_get(stdio_call_tool())
|
||||
|
||||
with pytest.raises(IngestError) as excinfo:
|
||||
get(f"mcp+stdio://{_SERVER_REF}/cost_docs", None)
|
||||
assert excinfo.value.code == "mcp_server_ref_unset"
|
||||
Loading…
Add table
Add a link
Reference in a new issue