I2.1 — Protocol and Contract Skeleton for v0.4.0 MCP vertical slice

[michaelegner]

Summary

Delivers I2.1 of the v0.4.0 MCP Vertical Slice and Evidence Drill-Down spec
(docs/specifications/0.4.0/i2-mcp-vertical-slice-and-evidence-drill-down.md §19): freeze MCP
2026-07-28, add evidence wire contracts, generalize the frozen ArchitectureAnswer<T> envelope for
a second tool without weakening I1, and expose deterministic two-tool discovery. No tool has a
working body yet — get_service_dependencies's real dispatch is I2.2, get_evidence's service
logic is I2.3.

• Contracts (app/architecture_intelligence/contracts.py, request.py): new EvidenceData /
  EvidenceRecord / SupportedFact / ObservedEvidenceMetadata (reusing the existing
  app.provenance.model SourceType/EvidenceType enums, not redefining them) and EvidenceRequest.
  tool tightened to Literal["get_service_dependencies", "get_evidence"]; the
  observation_context-required invariant is now gated on tool (the only field always present,
  since data can be None for either tool in some outcomes) rather than on data's type. Both
  answer schemas are frozen/committed (schema_export.py extended; new
  schemas/architecture_intelligence/v0.4/evidence-answer.schema.json).
• MCP layer (app/mcp/): built on the official mcp==2.1.1 SDK (pinned), per direction — the
  narrow, stateless 2026-07-28 subset this release needs is real, current SDK behavior, not
  functionality worth hand-rolling. tools.py registers both tools (stub bodies raising ToolError
  — discoverable via tools/list, not callable) directly against the I1 request/answer types, so
  in/out schemas come from the real contracts, not a parallel hand-written layer.
• app/mcp/guard.py — a thin ingress guard correcting two gaps confirmed live against the
  installed SDK (not assumed from docs): it silently serves pre-2026-07-28/legacy requests instead of
  rejecting them (spec §4/§20's "implementation requires initialize while claiming MCP 2026-07-28" is
  a named release blocker), and it turns an unknown tool name into a normal isError: true result
  instead of a JSON-RPC protocol error (spec §16's determinism/failure-mapping table). Everything else
  — header/_meta validation, unsupported-version rejection, Origin/Host allow-listing, per-tool
  argument-schema validation — is the SDK's own verified, spec-conformant behavior and isn't
  duplicated.
• Also caught and fixed while verifying end-to-end: the SDK reports tools/list in registration
  order, not sorted — get_evidence is now registered first to match the spec's required
  lexicographic order — and a routing/mount-order bug where mounting naively would have
  307-redirected a bare POST /mcp or silently shadowed /health.
• MCPConfig (app/settings.py) adds Origin/Host allow-listing, defaulting to loopback-only (spec
  §15: local/trusted-network evaluation only, never a production-safe public exposure) —
  docs/security-model.md documents this trust boundary alongside the existing ones.

Test plan

• uv run ruff check . / uv run ruff format --check .
• uv run pytest tests/unit — 690 passed (no integration tests needed: nothing in I2.1 opens a
  Neo4j session)
• tests/unit/test_mcp_discovery.py — spec §17 Protocol-and-Discovery scenarios 1-4, plus the
  two guard corrections and the argument-schema-validation passthrough, via a real
  httpx.AsyncClient/ASGI round trip (not the SDK's own client)
• tests/unit/test_architecture_intelligence_contracts.py / test_architecture_intelligence_schema_frozen.py
  extended for EvidenceData/EvidenceRecord/SupportedFact and the second frozen schema
• Manual smoke test of the full mounted app (create_app() + TestClient): POST /mcp
tools/list returns the two tools in order, GET /health still works (regression-tested the
  mount-order bug found above)

🤖 Generated with Claude Code

https://claude.ai/code/session_015VLBD6Vm8GrSDZf2Np6SDb

[Copilot]

🟡 Changes recommended

The /mcp mount currently uses MCPConfig() defaults in create_app(), making config.yaml overrides for the Origin/Host allow-list ineffective despite the new MCPConfig wiring and documentation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Implements the v0.4.0 I2.1 MCP “protocol + contract skeleton” vertical slice by adding the MCP server surface (discovery-only), a thin ingress guard to enforce the modern 2026-07-28 protocol behavior, and freezing a second JSON Schema for the new evidence drill-down wire contracts.

Changes:

• Add MCP SDK integration (mcp==2.1.1) and a minimal MCP server/app layer with deterministic two-tool discovery (tools/list) and stub tool bodies.
• Introduce evidence request/response contract types (EvidenceRequest, EvidenceData/EvidenceRecord/SupportedFact) and freeze the corresponding JSON schema artifacts.
• Add guard + unit tests validating protocol/version handling, discovery determinism, and expected error mapping.

File summaries

File	Description
uv.lock	Lockfile update to include mcp and its transitive dependencies.
pyproject.toml	Adds pinned mcp==2.1.1 dependency.
app/settings.py	Introduces MCPConfig and wires it into AppConfig.
app/main.py	Runs MCP session manager in lifespan and mounts the MCP ASGI app.
app/mcp/app.py	Builds the MCP ASGI app using SDK streamable HTTP + transport security, wrapped by the guard.
app/mcp/guard.py	Middleware enforcing modern protocol header requirements and deterministic protocol errors for unknown tools/unexpected keys.
app/mcp/server.py	Defines the singleton MCPServer and cache hints; registers tools.
app/mcp/tools.py	Registers get_evidence and get_service_dependencies tools (stubbed).
app/mcp/init.py	Package marker for MCP module.
app/architecture_intelligence/contracts.py	Adds evidence contracts and tightens the ArchitectureAnswer.tool typing/invariants.
app/architecture_intelligence/request.py	Adds EvidenceRequest with closed schema + dedup validation.
app/architecture_intelligence/schema_export.py	Exports and writes both dependencies and evidence frozen schemas.
schemas/architecture_intelligence/v0.4/architecture-answer.schema.json	Updates tool enum + observation_context invariant keyed on tool.
schemas/architecture_intelligence/v0.4/evidence-answer.schema.json	Adds frozen schema for ArchitectureAnswer[EvidenceData].
tests/unit/test_mcp_discovery.py	New protocol/discovery/guard behavior tests using ASGI round-trips.
tests/unit/test_architecture_intelligence_schema_frozen.py	Splits frozen-schema assertions into dependencies vs evidence schema.
docs/security-model.md	Documents MCP trust boundary and allow-listing behavior.

Review details

Suppressed comments (1)

app/main.py:116

• create_app() mounts the MCP app using MCPConfig() defaults, so config.yaml overrides for mcp.allowed-origins / mcp.allowed-hosts are ignored (even though MCPConfig was added to Settings/AppConfig and docs say deployments must override defaults). This can lead to unexpected 403s and makes the documented configuration ineffective.

    # ahead of settings/driver being available (create_app() itself must stay env/Neo4j-free), so
    # origin/host allow-listing uses MCPConfig's own defaults rather than the loaded config.yaml -
    # revisit if a deployment needs non-default origins (spec §15: local/trusted-network only).
    mcp_config = MCPConfig()

• Files reviewed: 15/17 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[michaelegner]

Reviewed current head 8c10acc.

Verdict: REQUEST CHANGES. The official SDK choice, two-tool scope, JSON-only transport, deterministic registration order, and raw-httpx protocol tests are good. The following blockers remain:

1. The guard returns the wrong error for a missing protocol header and can pre-empt header/body validation.
   In app/mcp/guard.py:87, a missing MCP-Protocol-Version returns -32022 UnsupportedProtocolVersion. MCP 2026-07-28 requires a missing required standard header to return HTTP 400 with -32020 HeaderMismatch; -32022 is for a present but unsupported version. In addition, the checks at lines 104–116 can return -32602 for an unknown tool or extra argument before the SDK validates mismatched/missing Mcp-Method and Mcp-Name. Validate all required headers and header/body agreement first, then apply tool-specific checks. Add tests for each missing header, each mismatch, and a mismatch combined with an unknown tool.
   Reference: https://modelcontextprotocol.io/specification/2026-07-28/basic/transports/streamable-http#server-validation

2. The advertised tool input schemas are not closed.
   The SDK-generated outer schema around the single request parameter permits extra properties; the test only verifies additionalProperties: false inside the nested request definition. The guard rejects outer extras at runtime, but tools/list still advertises them as valid, violating I2 §9’s closed-inputSchema requirement. Publish an actually closed root schema (or flatten/register the request explicitly) and assert inputSchema.additionalProperties is false for both tools.

3. The new answer contracts are structurally closed but not semantically closed.
   Both frozen schemas allow both tool names, so a dependency answer may identify itself as get_evidence and vice versa. For get_evidence, the model also does not enforce the §12/§14 requirements that observation_context is null and claims plus top-level evidence_refs are empty. EvidenceData does not ensure that returned record IDs and missing IDs form a disjoint, complete partition of the requested IDs, and EvidenceRecord does not couple OBSERVED/non-OBSERVED evidence to non-null/null observation metadata. Freeze these invariants in Pydantic and the exported JSON Schemas, with negative tests.

4. Configured MCP allow-lists are ignored.
   app/settings.py adds Settings.config.mcp, and the security documentation says deployments can override it, but app/main.py:116 constructs a fresh MCPConfig() and therefore always mounts with defaults. Wire the mounted MCP app to the loaded configuration and add a full-app test proving a configured non-default host/origin is honored.

5. Required CI is failing.
   lint + test fails in test_a_wrong_expected_tool_is_caught_as_a_field_mismatch: the new Literal rejects get_something_else in the loader before the comparator test reaches its intended assertion. Update that regression test consistently with the newly frozen contract and rerun the complete suite.

One smaller routing test would also be valuable: because the MCP sub-app is mounted at /, assert that an unrelated POST /not-mcp remains a normal 404 and is not converted into an MCP protocol error.

Nothing here requires expanding I2.1: these are corrections to the protocol and contracts this increment already claims to freeze.

[michaelegner]

CI status on 8a47cc7 (verified via the check-runs API against this exact SHA): all 7 checks green.

Check	Conclusion
lint + test	✅ success
CodeQL	✅ success
analyze (python)	✅ success
analyze (actions)	✅ success
dependency security scan (pip-audit, spec §29)	✅ success

The first push (8c10acc) had one real integration failure: test_a_wrong_expected_tool_is_caught_as_a_field_mismatch (an I1.4 test) relied on an invalid tool value surviving Pydantic load to be caught downstream by the comparator. Tightening tool to a closed Literal[...] in this PR is a deliberate I2.1 contract change, so that value now fails at load time instead — a stronger guarantee. Fixed in 8a47cc7 by asserting the load-time ScenarioValidationError directly; verified locally (tests/unit + tests/integration, both full suites) before pushing.

[michaelegner]

Addressed all 5 blockers in 61373c9, plus the smaller routing suggestion. Detail per point:

1. Wrong error code / guard pre-empting header validation. Rewrote app/mcp/guard.py to delegate directly to mcp.shared.inbound.classify_inbound_request — the SDK's own pure validation-ladder function — instead of hand-rolling it. A missing header now correctly returns -32020 HeaderMismatch (was -32022), and the guard's tool-name/argument checks only run after that classifier has accepted the request, so header/body mismatches always take priority. Added tests for each mismatch (Mcp-Method, Mcp-Name) and a combined mismatch+unknown-tool case proving the priority ordering.

2. Input schemas not closed. app/mcp/tools.py now mutates each registered Tool.parameters after registration (_close_input_schema) to add additionalProperties: false to the SDK's synthesized outer wrapper — confirmed live that the SDK doesn't do this itself. Test now asserts inputSchema.additionalProperties is False for both tools.

3. Contracts not semantically closed. Added a runtime check coupling tool to which generic specialization (T) is actually in use (via __pydantic_generic_metadata__), mirrored as a tool: {"const": ...} in each frozen schema file — the json_schema_extra hook now inspects which T generated the schema. Also added: get_evidence answers must have empty claims/top-level evidence_refs (runtime + schema), EvidenceData.records/missing_evidence_refs must exactly partition requested_evidence_refs (runtime only — no standard JSON Schema expresses a 3-array set partition, documented inline), and EvidenceRecord.observation must be non-null iff evidence_type == OBSERVED (runtime + schema). All with negative tests.

4. Configured allow-lists ignored. create_app() now reads load_config(CONFIG_PATH).mcp instead of a bare MCPConfig(). load_config never touches secrets/Neo4j, so this doesn't reintroduce a hard env-var dependency. New tests/unit/test_mcp_config_wiring.py proves a configured non-default origin/host is honored end-to-end through create_app(), and that the previous default is now correctly rejected.

5. CI failure. Fixed in the prior commit (8a47cc7) — confirmed green on that exact SHA before this round of fixes started.

Routing suggestion: the guard now only inspects MCP_PATH; added a test that POST /not-mcp returns a normal 404, not a synthesized MCP error.

Verified locally before pushing: tests/unit (700 passed) and tests/integration (191 passed), ruff check/ruff format --check.

[michaelegner]

Re-reviewed current head 61373c9.

Verdict: REQUEST CHANGES — one contract blocker remains.

The protocol-validation fix, closed advertised input schemas, tool/specialization coupling, evidence partition and observation-metadata invariants, configurable MCP allow-lists, CI regression fix, and unrelated-path routing test are all implemented correctly. Current-head CI, dependency audit, and CodeQL are green.

Remaining blocker — get_evidence.observation_context is still not constrained to null

I2 §12 and §14 require observation_context: null for every get_evidence answer because evidence lookup is not runtime-context-sensitive. The comments above ArchitectureAnswer state this invariant, but _check_envelope_invariants() currently enforces only empty claims and top-level evidence_refs for get_evidence. The schema conditional for that tool likewise constrains only those two arrays.

As a result, ArchitectureAnswer[EvidenceData] with a non-null, otherwise valid observation context is accepted by both Pydantic and the advertised frozen evidence schema.

Required fix:

• reject non-null observation_context when tool == "get_evidence";
• mirror the rule in the evidence schema conditional;
• add a negative test proving the same payload is rejected by both Pydantic and evidence-answer.schema.json.

Once that invariant is executable, the previous review findings are fully resolved and this I2.1 contract skeleton should be ready to approve.

[michaelegner]

Fixed in 4f43e9c: _check_envelope_invariants now rejects a non-null observation_context when tool == "get_evidence", mirrored in the schema's get_evidence conditional (observation_context: {"type": "null"}). Added the requested negative test proving the same payload is rejected by both Pydantic and evidence-answer.schema.json.

Verified locally before pushing: tests/unit (701 passed) and tests/integration (191 passed), ruff check/ruff format --check.

[michaelegner]

Re-reviewed current head 4f43e9c.

Verdict: APPROVE — no remaining findings.

The final blocker is resolved correctly:

• ArchitectureAnswer[EvidenceData] now rejects any non-null observation_context;
• the same invariant is encoded in the frozen evidence-answer.schema.json;
• the new negative test proves rejection through both Pydantic and JSON Schema validation.

All earlier protocol, schema-closure, semantic-contract, configuration, CI, and routing findings remain resolved. The PR is mergeable, the review thread is closed, and all current-head checks pass: lint/tests, dependency audit, and both CodeQL analyses.

PR #77 is ready to merge.