P13 measured this lift and REFUSED it, because okf >=0.8.5 emits the ownership
stamp as the V1 flow mapping `generated: { by: process:okf-ingest, at: ... }`
where 0.3.2 emitted `true`, and `_carries_complete_ingest_stamp` read the new
form as NOT a stamp -- write_concept_file's forgery refusal would have shipped
DISARMED with the whole fail-closed suite green. That blocker is closed first,
red-first, and then the pin moves.
ROW 1, THE SECURITY HALF. `_claims_ingest_ownership` widens the predicate from
"reads as boolean True" to "claims ingest ownership", of which the boolean is
the pre-V1 spelling. The recogniser for the new half is `decode_flow_value` --
the module's ONE flow decoder, the same argument write_concept_file already
makes for `verified`: the writer refuses exactly what the reader can read. A
value the decoder REFUSES is therefore not an ownership claim and writes
through, which is what keeps this from collapsing into "any non-empty
generated". Two arms red before the fix; no YAML library introduced.
THE PIN. okf v0.3.2 -> v0.8.5, guard v0.3.4 -> v1.4.0 spelled `tag =`, not
`rev =`, and not the declared floor 1.2.0 -- both P13 premises hold and the
reason now lives next to the pin in pyproject.toml. The ":40" comment is
corrected: okf has ONE runtime dependency, the guard, and that is what binds
the two lines together. 27/27 imported names resolve across five modules.
THE GOLDENS, REGENERATED AS A DECISION. Seven concept files across four
examples/ingest-golden-* bundles, one line each. Two were regenerated by the
REAL materializer; the other five are derived (http/sql/mcp cannot materialize
outside the tests' stubs) and then MEASURED -- all four golden suites compare
byte for byte against what the stubs produce, and all four are green. The four
`generated == "true"` asserts now read ONE source, conftest.
expected_generated_stamp: four literals for one emitter fact are four places a
later release can leave half-corrected, which is exactly how the pre-V1 form
survived until P13 measured it. tests/test_okf.py keeps its literal on purpose
-- that one round-trips a CURATED half-stamp through our own writer.
THE BLOCK READER. Measured with the full denominator: all four delivered
knowledge bases write `sources` as a BLOCK sequence and none in flow form
(n100 446/446, n200 1133/1133, n500 270/270, r761 2756/2756 = 4605/4605), and
`evidence_for` reported `unreadable` on 4605 of 4605 -- the falsification layer
had no address for any document in any base. `okf.decode_block_mappings` is the
second CARRIER of one grammar, never a second grammar: colon-SPACE separator,
unquote_scalar, duplicate keys refused, SPEC 5.2's actor rule applied. okf's
consume.read_sources was READ for the form and not called; po calls no okf
reader, which is measured and deliberate. After: 4605 present / 4605 entries.
Reading is not a licence to WRITE -- the emitter is untouched and both writers
still refuse what decode_flow_value refuses.
THREE FINDINGS. (1) The first block reader INVENTED data on `- { k: v }` items
-- SPEC-canonical, and the shape tests/golden/block-form-provenance writes for
`verified` -- decoding it as `{'{ id': '...'}`. No arm caught it: the 5.2 actor
rule shielded the fixture by accident. Closed with a flow-decoder branch and
four new arms. (2) One of my own arms was VACUOUS, found by my own mutation M5:
it claimed to prove the colon-SPACE rule and stayed green under first-colon,
because the two rules agree on every delivered value. Renamed, labelled, and
the claim moved to the arm that actually witnesses it. (3) OPEN, and it needs
the operator: the commons-owned worked example declares its second concept
`unreadable`/`block-sequence`, which is now false for po. `shared/` is
pull-only, so closing it needs a commons amendment; the test asserts the
divergence instead of skipping it, keeping the discriminating half (the example
says two entries were seen and the reader returns exactly two).
NINE EXISTING ARMS REWRITTEN, NONE WEAKENED. All nine pinned "the block form is
unreadable" -- the behaviour this order changes. Each keeps its claim on a
specimen that is still unreadable for a reason of its own (5.2: an entry naming
no actor), or pins the REVERSED direction where the old arm stood so the change
cannot be silent. Two got STRONGER: multi-verified.md was authored for "a reader
keeping the last entry reports machine-confirmed for a concept a human signed",
and that could not be tested while the form was unreadable. Three node ids were
renamed; nothing was removed in substance.
Suite 1582 -> 1606 passed / 5 skipped. Both demo goldens byte-unchanged
(ea8c534... / ede3e2f..., shasum -a 1 of the CONTENT, never the git blob id).
ruff check / ruff format / mypy green. shared/ untouched.
Six mutations, all red against the WHOLE suite, each with its own signature:
row 1 detached (2) / block reader detached (17) / flow-item branch detached (7)
/ a stray indented line folds into an INVENTED entry (4) / separator becomes the
first colon (1 -- and that is finding 2) / the stamp expectation reverts to
"true" (4).
Order: 20260912T195112Z-995611104-from-.claude
Record: docs/2026-09-12-p13b-okf-bump.md
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
235 lines
10 KiB
Python
235 lines
10 KiB
Python
"""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.2``, 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 conftest import expected_generated_stamp
|
|
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 f"generated: {expected_generated_stamp('2026-08-02T00:00:00Z')}" 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]
|