open/portfolio-optimiser
1
0
Fork 0
Code Issues Releases 1 Activity

feat(ingest): add the MCP connector as a transport inside the http family (S2.2)

Browse source 
Commons settled on 2026-08-01 that MCP is an extension of the `http` source
family, not a fourth family (`shared/ingest-spec.md` §4). This implements it
with ZERO schema change and zero spec amendment.

The transport discriminator lives in `base_url`, not in new manifest fields:
the shared library rejects unknown manifest keys fail-fast, and we consume it
pull-only at a pinned v0.3.1, so `server_ref`/`tool` as fields would have meant
a spec amendment plus a library release. It buys nothing — the library already
joins `base_url` + `/` + `query`, so `mcp+stdio://<server_ref>` + `<tool>`
reproduces exactly the two-part structure the (now stale) reference plan wanted.

Staying inside the family INHERITS what a fourth family would have had to write
and could have forgotten: the §8 network grant (measured to fire before any tool
call), the `max_rows` cap, §5 verbatim fenced rendering, and the §7 provenance
stamp. The discriminator gates rather than labels — `mcp_get` refuses a URL it
does not own, so an MCP transport can never quietly serve an `https://` manifest
and leave the bundle's provenance claiming a transport that was never used.

Parsing is string-based, not `urlsplit`-based: `urlsplit().hostname` lowercases
the host, which would silently break the case-sensitive env lookup `server_ref`
depends on.

`ingest.py` is untouched — it is AST-guarded mcp-free, so the transport lives in
its own module and is opt-in at the call site. `ingest_mcp.py` imports the open
`mcp` protocol client but never `agent_framework`, keeping the seam D7-portable.

Load-bearing, six mutations all measured RED: detach the scheme guard · make the
refusal unconditional · swap parsing to `urlsplit().hostname` · skip non-text
content instead of raising · force `allow_network=True` · smuggle in a MAF
import. Both source files restored byte-identical (`shasum -c`) after each.

Honesty: `stdio_call_tool` (the real stdio path) is written but never executed
end to end — every test injects a canned tool call, so the suite spawns no
subprocess and opens no socket. No golden fixture, and MCP stays unwired in the
optimiser run path. Stated in docs/extending.md rather than implied away.

555 -> 578 tests; ruff + mypy green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112FPR5TX6pDLiNicBPzE8i
This commit is contained in:
Kjell Tore Guttormsen 2026-08-02 21:14:57 +02:00
commit ddd6338f02
4 changed files with 683 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

55
docs/extending.md
View file

@ -105,15 +105,52 @@ client wired into the optimiser run path — that stays a **Non-Goal** here: the
`FunctionTool` seam is the default in the run path, and MCP is demonstrated (via `build_mcp_server`
in `datasource.py`), not wired in.
**Where the D7 sibling stands (målbilde §11 boundary).** The Claude Agent SDK sibling (D7) built
the **file/CSV and SQL** connectors — mirroring I3/I5 — with bit-identical golden extractions.
**HTTP and MCP are demonstrated on the MAF side only** (MAF-only), against a local mock; the
sibling ships no network connector and no live-source integration. On D7 the in-process server
hook is `create_sdk_mcp_server(name, version="1.0.0", tools=...) -> McpSdkServerConfig` (package
`claude-agent-sdk`) — verified 2026-07-04 against the official Claude Agent SDK Python docs — but
that is a **documented hook a deployer would reach for, not a shipped D7 connector**; no D7 HTTP
or MCP session is planned. A deployer who wants a network- or MCP-mediated source extends this
family behind the same explicit, per-run network grant; nothing here contacts a live endpoint.
**The connector (S2.2, 2026-08-02).** `ingest_mcp.py` implements it. There is **no fourth source
family and no schema change**: an MCP source is an ordinary `type: "http"` source whose transport is
declared by the `base_url` scheme, and whose two parts fall straight out of the join the library
already performs (`base_url` + `/` + `query`):
```json
{ "type": "http", "id": "docs", "base_url": "mcp+stdio://PORTFOLIO_DOCS_MCP" }
```
```python
from portfolio_optimiser.ingest import materialize
from portfolio_optimiser.ingest_mcp import mcp_get, stdio_call_tool
materialize(manifest, bundle_dir, ingested_at="2026-08-02T00:00:00Z",
allow_network=True, http_get=mcp_get(stdio_call_tool()))
```
`server_ref` is the **name of an environment variable** holding the server command — mirroring the
`sql` family's `connection_ref`, so the manifest carries a reference and never an executable path —
and the extraction's `query` is the tool name. Parsing is string-based, not `urlsplit`-based:
`urlsplit().hostname` lowercases the host, which would silently break a case-sensitive env lookup.
Two properties are worth stating because they are **inherited, not written**: the §8 network grant
covers MCP for free (an MCP source *is* `http`, so it is refused before any tool call unless
`allow_network=True`), as do the `max_rows` cap, verbatim fenced rendering, and the §7 provenance
stamp. Had MCP become a fourth family, each of those would have been ours to write — and ours to
forget. The transport discriminator **gates rather than labels**: `mcp_get` refuses a URL it does
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.
**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
and MCP are implemented on the MAF side only**; the sibling ships no network connector and no
live-source integration. `ingest_mcp.py` is deliberately MAF-free (it imports the open `mcp`
protocol client, never `agent_framework`), so the seam is portable to D7 unchanged. On D7 the
in-process server hook is `create_sdk_mcp_server(name, version="1.0.0", tools=...) ->
McpSdkServerConfig` (package `claude-agent-sdk`) — verified 2026-07-04 against the official Claude
Agent SDK Python docs — but that is a **documented hook a deployer would reach for, not a shipped D7
connector**; no D7 MCP session is planned. Nothing here contacts a live endpoint.
## Bytt ut henteren (Embedder / Retriever, S3.1)

186
src/portfolio_optimiser/ingest_mcp.py Normal file
View file

@ -0,0 +1,186 @@
"""Door A ingest — the MCP connector, as a TRANSPORT inside the ``http`` source family (§4).
``shared/ingest-spec.md`` §4: "An MCP-based connector is an extension of this family and MUST
honour the same extraction, materialization, and gate contracts." Commons settled this on
2026-08-01 — MCP is **not** a fourth source family. This module is that extension, and nothing
more: it adapts an MCP tool call into the shape the shared library's transport seam already
expects, so every contract downstream of the transport is the EXISTING one.
**Where the transport discriminator lives, and why.** The shared library validates manifests
fail-fast and rejects unknown keys (``llm_ingestion_okf.manifest._require_keys``): an ``http``
source admits exactly ``{type, id, base_url}`` plus optional ``credential_ref``. We consume that
library PULL-ONLY at a pinned ``v0.3.1``, so introducing ``server_ref``/``tool`` as manifest fields
would mean a spec amendment plus a library release. It buys nothing: the library's own URL join
composes ``base_url`` + ``/`` + ``query``, so
base_url: "mcp+stdio://<server_ref>" query: "<tool>"
reproduces exactly the two-part structure a dedicated ``mcp`` family would have carried — with zero
schema change. The spec's MUST sits on the CONTRACTS (extraction, materialization, gates), not on
field names; naming was explicitly left to the consumer.
**What we inherit for free by staying inside the family** — and would have had to write, and could
have forgotten, as a fourth family:
- the §8 network gate (an MCP source is a non-local transport; ``allow_network`` is a run argument,
never a manifest field, and it refuses BEFORE any transport call);
- the §8 ``max_rows`` cap, enforced as an ERROR rather than a silent truncation;
- §5 verbatim fenced-body rendering, including the code-fence-marker refusal;
- the §7 ``generated: true`` + ``ingest_manifest`` provenance stamp;
- the §4 rule that credentials resolve at run time from the environment, never from the manifest.
**MAF-free (D7-portable).** MCP is an open protocol; ``agent_framework`` is one framework's runtime.
This module may import the former and never the latter, so the sibling Claude-SDK implementation can
consume the identical seam. Guarded by
``tests/test_ingest_mcp_loadbearing.py::test_ingest_mcp_is_maf_free``.
Note this module is deliberately NOT imported by ``portfolio_optimiser.ingest``: that module is
AST-guarded against importing ``mcp`` at all
(``tests/test_ingest_loadbearing.py::test_ingest_module_is_maf_free_and_context_layer_pure``).
The MCP transport is opt-in at the CALL SITE — ``materialize(..., http_get=mcp_get(...))`` — which
is also why an ``http`` manifest can never quietly acquire an MCP transport.
"""
from __future__ import annotations
import os
from collections.abc import Callable
from typing import Any
from llm_ingestion_okf.connectors import HttpGet
from portfolio_optimiser.ingest import IngestError
#: The transport token in ``base_url``. One source of truth: the parser accepts this and nothing
#: else, so manifest-authoring docs and the parser cannot drift apart.
MCP_SCHEME = "mcp+stdio"
#: An MCP tool invocation reduced to what the transport seam needs: given the server reference,
#: the tool name and an optional run-time-resolved credential, return the response body verbatim.
#: This is the extension point — ``stdio_call_tool`` is the real implementation, and tests inject
#: a canned one so the suite runs with no subprocess and no socket.
McpCallTool = Callable[[str, str, "str | None"], str]
def parse_mcp_url(url: str) -> tuple[str, str]:
"""Split a joined MCP URL back into ``(server_ref, tool)``, fail-fast on anything else.
The library hands the transport ``base_url`` + ``/`` + ``query``; this recovers the two parts.
Parsing is deliberately STRING-based rather than ``urllib.parse.urlsplit``: ``urlsplit``
lowercases the host, and ``server_ref`` names an environment variable, whose name is
case-sensitive. Going through ``urlsplit`` would turn ``PORTFOLIO_DOCS_MCP`` into
``portfolio_docs_mcp`` and fail the lookup for a reason invisible at the call site.
Refusing a URL this transport does not own is a GATE, not a label: it stops an MCP transport
from quietly serving a plain ``https://`` manifest, which would leave the bundle's provenance
claiming a transport that was never used.
"""
prefix = f"{MCP_SCHEME}://"
if not url.startswith(prefix):
raise IngestError(
f"MCP transport received a URL it does not own: {url!r} "
f"(expected a {prefix}… base_url; spec §4 — the manifest declares its transport)",
code="mcp_scheme_mismatch",
)
remainder = url[len(prefix) :]
server_ref, separator, tool = remainder.partition("/")
if not separator or not server_ref or not tool:
raise IngestError(
f"MCP URL must be {prefix}<server_ref>/<tool>, got {url!r} "
"(server_ref names an environment variable holding the server command; "
"tool is the extraction's `query`)",
code="mcp_url_malformed",
)
return server_ref, tool
def mcp_get(call_tool: McpCallTool) -> HttpGet:
"""Adapt an MCP tool call into the library's ``HttpGet`` transport seam.
Returns a ``(url, credential) -> str`` callable — the exact shape the library injects as
``http_get`` — which parses the URL, refuses one it does not own, and delegates the fetch to
``call_tool``. The credential arrives already resolved from the environment by the library and
is passed out-of-band; it is never joined into the URL, never logged, never stamped.
"""
def get(url: str, credential: str | None) -> str:
server_ref, tool = parse_mcp_url(url)
return call_tool(server_ref, tool, credential)
return get
def stdio_call_tool(
*,
args: tuple[str, ...] = (),
timeout_seconds: float = 30.0,
) -> McpCallTool:
"""The real stdio transport: launch the MCP server named by ``server_ref`` and call its tool.
``server_ref`` is the NAME of an environment variable holding the server command — mirroring
the ``sql`` family's ``connection_ref`` — so the manifest carries a reference, never an
executable path, and stays versionable and shareable. A missing or empty variable is fail-fast
BEFORE any process is spawned.
The credential, when present, is passed to the server as the ``MCP_CREDENTIAL`` environment
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).
"""
import asyncio
from mcp import ClientSession, StdioServerParameters
from mcp.client.stdio import stdio_client
def call_tool(server_ref: str, tool: str, credential: str | None) -> str:
command = os.environ.get(server_ref)
if not command:
raise IngestError(
f"MCP server_ref {server_ref!r} is not set in the environment "
"(the manifest names a reference; the command resolves at run time — spec §4)",
code="mcp_server_ref_unset",
)
env = dict(os.environ)
if credential is not None:
env["MCP_CREDENTIAL"] = credential
async def run() -> str:
params = StdioServerParameters(command=command, args=list(args), env=env)
async with stdio_client(params) as (read, write):
async with ClientSession(read, write) as session:
await session.initialize()
result = await session.call_tool(tool, {})
if result.isError:
raise IngestError(
f"MCP tool {tool!r} on {server_ref!r} returned an error result",
code="mcp_tool_error",
)
return _body_from_content(result.content, tool=tool, server_ref=server_ref)
return asyncio.run(asyncio.wait_for(run(), timeout=timeout_seconds))
return call_tool
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).
Non-text content (images, embedded resources) is an ERROR rather than a silent drop: the body
is rendered verbatim into the concept file, and quietly discarding part of an extraction would
produce a bundle that looks complete and is not.
"""
parts: list[str] = []
for block in content:
text = getattr(block, "text", None)
if text is None:
raise IngestError(
f"MCP tool {tool!r} on {server_ref!r} returned non-text content "
f"({type(block).__name__}); ingest renders bodies verbatim and never drops content",
code="mcp_non_text_content",
)
parts.append(text)
return "".join(parts)

232
tests/test_ingest_mcp.py Normal file
View file

@ -0,0 +1,232 @@
"""S2.2 — the MCP connector as a TRANSPORT inside the ``http`` source family (§4).
Commons settled this on 2026-08-01: MCP is **not** a fourth source family. ``shared/ingest-spec.md``
§4 says an MCP-based connector "is an extension of this family and MUST honour the same extraction,
materialization, and gate contracts". The reference plan
(``docs/plan/2026-07-10-sesjonsplan-fase2-6.md`` lines 193-208) predates that ruling and still
describes a fourth family ``type: "mcp"`` with ``server_ref``/``tool`` manifest fields — that part
is STALE and deliberately not followed here.
**Why the discriminator lives in ``base_url``, not in new manifest fields.** The shared library
rejects unknown manifest keys fail-fast (``llm_ingestion_okf.manifest._require_keys``): an ``http``
source admits exactly ``{type, id, base_url}`` plus optional ``credential_ref``. We are a PULL-ONLY
consumer pinned at ``v0.3.1``, so adding ``server_ref``/``tool`` as fields would require a spec
amendment plus a library release. It is not needed: the library's own URL join already composes
``base_url`` + ``/`` + ``query``, so ``mcp+stdio://<server_ref>`` + ``<tool>`` reproduces exactly
the two-part structure the stale plan wanted — with zero schema change. Field names were explicitly
left to us ("en implementasjonsbeslutning HOS OSS"); the MUST sits on the CONTRACTS, not the names.
**The §8 network gate is inherited, not re-implemented.** Because an MCP source IS ``type: "http"``,
the library refuses it fail-fast unless a run passes ``allow_network=True`` — measured below to fire
BEFORE any transport call. That is the whole point of staying inside the family: the gate cannot be
forgotten, because we never got the chance to write it.
Every test here injects a canned ``call_tool`` — NO subprocess, NO socket, NO server. Mirrors the
transport-seam discipline of ``tests/test_ingest_http.py``.
"""
from __future__ import annotations
import inspect
import json
from pathlib import Path
import pytest
from portfolio_optimiser.ingest import (
IngestError,
NetworkGateError,
materialize,
)
from portfolio_optimiser.ingest_mcp import (
MCP_SCHEME,
mcp_get,
parse_mcp_url,
)
def _make_call_tool(body: str, *, recorder: list[tuple[str, str, str | None]] | None = None):
"""A canned MCP tool call: records ``(server_ref, tool, credential)``, returns a fixed body.
Local to this file (mirrors ``_make_get``/``_make_db`` locality; never conftest, which is
MAF/LLM-only)."""
def call_tool(server_ref: str, tool: str, credential: str | None) -> str:
if recorder is not None:
recorder.append((server_ref, tool, credential))
return body
return call_tool
def _write_manifest(tmp_path: Path, base_url: str, query: str = "retrieve_cost_docs") -> Path:
manifest = {
"manifest_version": 1,
"source": {"type": "http", "id": "docs", "base_url": base_url},
"bundle_summary": "MCP-sourced cost documentation.",
"extractions": [
{
"id": "cost-docs",
"title": "Cost documentation",
"query": query,
"okf_type": "concept",
"max_rows": 50,
}
],
}
path = tmp_path / "manifest.json"
path.write_text(json.dumps(manifest), encoding="utf-8")
return path
# --- the transport discriminator (§4) ------------------------------------------------------------
def test_mcp_url_splits_into_server_ref_and_tool() -> None:
"""The library joins ``base_url`` + ``/`` + ``query``; we split that back into the two parts
the stale plan wanted as separate manifest fields."""
assert parse_mcp_url("mcp+stdio://docs-server/retrieve_cost_docs") == (
"docs-server",
"retrieve_cost_docs",
)
def test_mcp_url_preserves_case_of_server_ref() -> None:
"""``server_ref`` names an environment variable (mirroring ``connection_ref``), and env-var
names are CASE-SENSITIVE. ``urlsplit().hostname`` lowercases the host — so parsing MUST be
string-based, not urlsplit-based. Without this, ``PORTFOLIO_DOCS_MCP`` silently becomes
``portfolio_docs_mcp`` and the lookup fails for a reason nobody can see."""
server_ref, tool = parse_mcp_url("mcp+stdio://PORTFOLIO_DOCS_MCP/retrieve_cost_docs")
assert server_ref == "PORTFOLIO_DOCS_MCP"
assert tool == "retrieve_cost_docs"
@pytest.mark.parametrize(
"url",
[
"https://example.test/api/retrieve",
"http://example.test/retrieve",
"mcp://docs-server/tool", # near-miss: right family, wrong transport token
"mcp+stdio://docs-server", # server but no tool
"mcp+stdio:///retrieve", # tool but no server
],
)
def test_mcp_transport_refuses_urls_it_does_not_own(url: str) -> None:
"""The discriminator GATES, it does not merely label. An MCP transport pointed at a plain
``https://`` manifest must refuse fail-fast rather than quietly serve it — otherwise the
manifest's declared transport and the transport actually used can silently disagree, and the
bundle's provenance would claim something untrue."""
with pytest.raises(IngestError):
parse_mcp_url(url)
def test_mcp_get_refuses_non_mcp_url_through_the_transport_seam() -> None:
"""Same gate, reached the way the library reaches it — and the tool is never called."""
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_make_call_tool("a,b\n1,2", recorder=calls))
with pytest.raises(IngestError):
get("https://example.test/retrieve", None)
assert calls == [], "transport must refuse BEFORE invoking the MCP tool"
# --- HttpGet compatibility (the seam contract) ---------------------------------------------------
def test_mcp_get_is_shape_compatible_with_the_library_transport_seam() -> None:
"""``mcp_get`` must return something the library can use as its ``http_get``: a callable of
``(url, credential) -> str``. If this drifts, the library binds it and fails at call time
instead of import time."""
get = mcp_get(_make_call_tool("x"))
params = list(inspect.signature(get).parameters)
assert len(params) == 2, f"HttpGet takes (url, credential), got {params}"
assert isinstance(get("mcp+stdio://s/t", None), str)
def test_mcp_get_returns_the_tool_body_verbatim() -> None:
"""§5 verbatim-body rule: the transport transports, it does not reformat."""
body = "region,saving\nnorth,1200\nsouth,900"
get = mcp_get(_make_call_tool(body))
assert get("mcp+stdio://docs-server/retrieve_cost_docs", None) == body
def test_mcp_get_forwards_credential_without_placing_it_in_the_url() -> None:
"""Credentials resolve at run time and never live in the manifest (§4). The library resolves
``credential_ref`` from the environment and hands the VALUE to the transport; the MCP tool call
receives it out-of-band, never joined into the URL."""
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_make_call_tool("a,b", recorder=calls))
get("mcp+stdio://docs-server/retrieve_cost_docs", "secret-token")
assert calls == [("docs-server", "retrieve_cost_docs", "secret-token")]
def test_mcp_scheme_constant_matches_what_the_parser_accepts() -> None:
"""One source of truth for the transport token — a drifting constant would make the
manifest-authoring docs and the parser disagree."""
server_ref, tool = parse_mcp_url(f"{MCP_SCHEME}://docs-server/retrieve_cost_docs")
assert (server_ref, tool) == ("docs-server", "retrieve_cost_docs")
# --- end-to-end through the real materialization path (§5) ---------------------------------------
def test_mcp_source_materializes_a_bundle_through_the_http_family(tmp_path: Path) -> None:
"""The whole point of staying inside ``type: "http"``: materialization, rendering and stamping
are the EXISTING code paths, reached with an MCP transport injected. Nothing bespoke."""
manifest_path = _write_manifest(tmp_path, "mcp+stdio://docs-server")
bundle_dir = tmp_path / "bundle"
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_make_call_tool("region,saving\nnorth,1200", recorder=calls))
written = materialize(
manifest_path,
bundle_dir,
ingested_at="2026-08-02T00:00:00Z",
allow_network=True,
http_get=get,
)
assert calls == [("docs-server", "retrieve_cost_docs", None)]
assert [p.name for p in written] == ["ingest-cost-docs.md"]
text = written[0].read_text(encoding="utf-8")
assert "north,1200" in text, "extracted body must reach the concept file"
assert "generated: true" in text, "§7 honesty stamp must survive the MCP path"
def test_mcp_source_is_refused_without_the_per_run_network_optin(tmp_path: Path) -> None:
"""§8, INHERITED for free: an MCP source is a non-local transport, so the network gate applies
and fires BEFORE the transport is touched. The manifest cannot grant itself network access.
This is the strongest argument for the http-family decision: had MCP become a fourth family,
this gate would have been ours to write, and ours to forget."""
manifest_path = _write_manifest(tmp_path, "mcp+stdio://docs-server")
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_make_call_tool("a,b", recorder=calls))
with pytest.raises(NetworkGateError):
materialize(
manifest_path,
tmp_path / "bundle",
ingested_at="2026-08-02T00:00:00Z",
http_get=get, # allow_network defaults to False
)
assert calls == [], "the gate must fire BEFORE any MCP tool call"
def test_mcp_materialization_is_byte_deterministic(tmp_path: Path) -> None:
"""Ingest is deterministic end to end (§11): same manifest + same ``ingested_at`` + same body
⇒ byte-identical output. ``ingested_at`` is stamped verbatim, never wall-clock."""
outputs = []
for run in ("a", "b"):
root = tmp_path / run
root.mkdir()
manifest_path = _write_manifest(root, "mcp+stdio://docs-server")
written = materialize(
manifest_path,
root / "bundle",
ingested_at="2026-08-02T00:00:00Z",
allow_network=True,
http_get=mcp_get(_make_call_tool("region,saving\nnorth,1200")),
)
outputs.append(written[0].read_bytes())
assert outputs[0] == outputs[1]

219
tests/test_ingest_mcp_loadbearing.py Normal file
View file

@ -0,0 +1,219 @@
"""S2.2 load-bearing seams — MCP as an ``http``-family transport (ingest spec §11's detach-RED regime).
Each test is built so the seam under test is the ONLY thing preventing the observable artifact (the
Fase-2 green-but-dead trap). Every test injects a canned ``McpCallTool`` — no subprocess, no socket.
- TRANSPORT DISCRIMINATOR (§4): ``parse_mcp_url`` refuses a URL the MCP transport does not own, and
accepts one it does. Asserted on BOTH branches with the SAME parser: dropping the prefix check →
the refuse assertion goes RED (an ``https://`` manifest is silently served over MCP); making the
refusal unconditional → the accept assertion goes RED. A refuse-only test would false-green on the
second detach — the trap called out in ``test_ingest_http_loadbearing``.
- CASE PRESERVATION (§4, ``server_ref`` = env-var name): parsing is string-based, not
``urlsplit``-based. RED the moment someone "cleans this up" to ``urlsplit().hostname``, which
lowercases the host and breaks a case-sensitive environment lookup. Both branches again: an
uppercase ref survives AND a lowercase ref still works, so an ``upper()``-everywhere mutation
cannot false-green it.
- NETWORK GATE IS INHERITED, NOT RE-IMPLEMENTED (§8): the payoff of the http-family decision. The
gate fires before any MCP tool call, on BOTH branches (flag off → refuse + tool never called;
flag on → succeeds + tool called). The CONTROL that gives this test its meaning: the same
extraction driven around ``materialize`` reaches the tool with no gate at all — proving the
refusal comes from staying inside the family, not from anything this module wrote.
- NON-TEXT CONTENT IS AN ERROR (§5 verbatim body): a tool returning an image/resource block raises
rather than silently contributing nothing. RED if the refusal degrades to skipping, which would
produce a bundle that looks complete and is not. Control: an all-text result still concatenates.
- MAF-FREE (D7): this module may import ``mcp`` (open protocol) but never ``agent_framework`` (one
framework's runtime), so the sibling Claude-SDK implementation consumes the identical seam. RED on
the first MAF import.
"""
from __future__ import annotations
import ast
import json
from pathlib import Path
import pytest
from portfolio_optimiser.ingest import NetworkGateError, materialize
from portfolio_optimiser.ingest_mcp import (
IngestError,
_body_from_content,
mcp_get,
parse_mcp_url,
)
_INGESTED_AT = "2026-08-02T00:00:00Z"
def _manifest(tmp_path: Path, base_url: str) -> Path:
payload = {
"manifest_version": 1,
"source": {"type": "http", "id": "docs", "base_url": base_url},
"bundle_summary": "MCP-sourced cost documentation.",
"extractions": [
{
"id": "cost-docs",
"title": "Cost documentation",
"query": "retrieve_cost_docs",
"okf_type": "concept",
"max_rows": 50,
}
],
}
path = tmp_path / "manifest.json"
path.write_text(json.dumps(payload), encoding="utf-8")
return path
def _recording_call_tool(body: str, calls: list[tuple[str, str, str | None]]):
def call_tool(server_ref: str, tool: str, credential: str | None) -> str:
calls.append((server_ref, tool, credential))
return body
return call_tool
# --- the transport discriminator gates BOTH ways -------------------------------------------------
def test_transport_discriminator_refuses_foreign_and_accepts_own() -> None:
"""RED if the prefix check is dropped (foreign URL served) OR made unconditional (own URL
refused). One parser, both branches, so neither mutation survives."""
with pytest.raises(IngestError):
parse_mcp_url("https://example.test/api/retrieve")
assert parse_mcp_url("mcp+stdio://docs-server/retrieve_cost_docs") == (
"docs-server",
"retrieve_cost_docs",
)
def test_transport_discriminator_gates_through_the_seam_before_calling_the_tool() -> None:
"""The gate must sit in FRONT of the tool call, not after it: a refusal that still invoked the
MCP server would already have paid the call it was meant to prevent."""
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_recording_call_tool("a,b\n1,2", calls))
with pytest.raises(IngestError):
get("https://example.test/api/retrieve", None)
assert calls == [], "refusal must precede the MCP tool call"
assert get("mcp+stdio://docs-server/retrieve_cost_docs", None) == "a,b\n1,2"
assert calls == [("docs-server", "retrieve_cost_docs", None)]
def test_server_ref_case_survives_parsing_both_ways() -> None:
"""RED if parsing moves to ``urlsplit().hostname``, which lowercases the host and silently
breaks a case-sensitive env lookup. The lowercase branch stops an ``upper()`` mutation from
false-greening the uppercase one."""
assert parse_mcp_url("mcp+stdio://PORTFOLIO_DOCS_MCP/t") == ("PORTFOLIO_DOCS_MCP", "t")
assert parse_mcp_url("mcp+stdio://docs-server/t") == ("docs-server", "t")
# --- the §8 gate is inherited from the http family, not written here -----------------------------
def test_network_gate_covers_mcp_on_both_branches(tmp_path: Path) -> None:
"""Same manifest, same recording tool, both flag states. Removing the gate → the refuse
assertion goes RED (tool called, no error); making the refusal unconditional → the allow
assertion goes RED."""
manifest_path = _manifest(tmp_path, "mcp+stdio://docs-server")
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_recording_call_tool("region,saving\nnorth,1200", calls))
with pytest.raises(NetworkGateError):
materialize(manifest_path, tmp_path / "off", ingested_at=_INGESTED_AT, http_get=get)
assert calls == [], "gate must fire BEFORE any MCP tool call"
written = materialize(
manifest_path,
tmp_path / "on",
ingested_at=_INGESTED_AT,
allow_network=True,
http_get=get,
)
assert len(calls) == 1, "with the opt-in set, the tool must actually be reached"
assert written and written[0].exists()
def test_control_the_gate_comes_from_the_family_not_from_this_module(tmp_path: Path) -> None:
"""CONTROL — this is what makes the test above mean something.
Driving the SAME extraction around ``materialize`` (i.e. what a bespoke fourth-family connector
would have done) reaches the MCP tool with no gate whatsoever. So the refusal proven above is
inherited by staying inside ``type: "http"`` — it is not something this module implements, and a
fourth family would have had to write it, and could have forgotten it."""
calls: list[tuple[str, str, str | None]] = []
get = mcp_get(_recording_call_tool("region,saving\nnorth,1200", calls))
get("mcp+stdio://docs-server/retrieve_cost_docs", None)
assert calls == [("docs-server", "retrieve_cost_docs", None)], (
"bypassing materialize reaches the tool ungated — the gate lives in the family path"
)
# --- verbatim body: non-text content is an error, never a silent drop ----------------------------
class _ImageBlock:
"""An MCP content block with no ``text`` attribute (image / embedded resource)."""
mime_type = "image/png"
class _TextBlock:
def __init__(self, text: str) -> None:
self.text = text
def test_non_text_content_raises_rather_than_dropping_silently() -> None:
"""RED if the refusal degrades to skipping non-text blocks: the body is rendered verbatim into
the concept file, so dropping part of an extraction yields a bundle that looks complete and is
not. Control: an all-text result still concatenates in order."""
with pytest.raises(IngestError):
_body_from_content([_TextBlock("region,saving\n"), _ImageBlock()], tool="t", server_ref="s")
assert (
_body_from_content([_TextBlock("a,b\n"), _TextBlock("1,2")], tool="t", server_ref="s")
== "a,b\n1,2"
)
# --- D7 portability ------------------------------------------------------------------------------
def test_ingest_mcp_is_maf_free() -> None:
"""MCP is an open protocol and may be imported; ``agent_framework`` is one framework's runtime
and may not. Keeps this transport consumable unchanged by the sibling Claude-SDK stack. RED on
the first MAF import."""
module_path = (
Path(__file__).resolve().parents[1] / "src" / "portfolio_optimiser" / "ingest_mcp.py"
)
tree = ast.parse(module_path.read_text(encoding="utf-8"))
roots: list[str] = []
for node in ast.walk(tree):
if isinstance(node, ast.Import):
roots.extend(alias.name.split(".")[0] for alias in node.names)
elif isinstance(node, ast.ImportFrom) and node.module and node.level == 0:
roots.append(node.module.split(".")[0])
assert "agent_framework" not in roots, f"MAF import in ingest_mcp.py: {roots}"
assert "mcp" in roots, (
"the MCP transport is expected to import the open-protocol client; if this moved, "
"re-point the guard rather than deleting it"
)
def test_the_seam_is_opt_in_at_the_call_site() -> None:
"""``portfolio_optimiser.ingest`` must NOT import this module: it is AST-guarded mcp-free, and
the MCP transport is opt-in via ``materialize(..., http_get=mcp_get(...))``. That is also what
stops a plain ``http`` manifest from quietly acquiring an MCP transport."""
ingest_path = Path(__file__).resolve().parents[1] / "src" / "portfolio_optimiser" / "ingest.py"
source = ingest_path.read_text(encoding="utf-8")
assert "ingest_mcp" not in source, "ingest.py must stay independent of the MCP transport"