Skip to content

Commit c9ab0ae

Browse files
committed
docs: park MCP 2026-07-28 compliance review as OUTSTANDING.md
Records the 2026-07-02 re-pin/fix slice's findings, disclosed gaps, and remaining low-severity items as a temporary punch list. Status: PARKED pending upstream draft stabilization; the doc instructs whoever resumes to re-fetch and diff the live schema before trusting any test result or finding recorded here, since the draft has drifted underneath this review twice in one week. Intended to be folded into docs/plans/2026-07-28-spec-compliance.md and deleted at final release preparation, not to become a second status authority.
1 parent d683488 commit c9ab0ae

1 file changed

Lines changed: 72 additions & 0 deletions

File tree

OUTSTANDING.md

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# OUTSTANDING — MCP 2026-07-28 draft-spec compliance & release-docs review
2+
3+
**Status: PARKED.** Snapshot date: 2026-07-02. Schema content sha256: `6e4cba2d…`. Fixture pin: `60dc69e9…`. Reason: awaiting upstream 2026-07-28 draft stabilization / release proximity before investing further compliance-review effort. **On resume, the first action must be re-fetching and diffing the live draft schema against `6e4cba2d…`** — do not assume the findings or test results below still hold; the draft has drifted underneath this document twice already this week. This file is a temporary 0.4 punch list, not a permanent record (see §6) — fold the surviving open items into `docs/plans/2026-07-28-spec-compliance.md` and delete this file during final release preparation.
4+
5+
Date: 2026-07-02 · Branch: `feat/turul-mcp-protocol-2026-07-28` · Current pin: vendored DRAFT-2026-v1 `schema.ts` @ upstream commit with content sha256 `6e4cba2d…` (wire string `"2026-07-28"`), governing spec: <https://modelcontextprotocol.io/specification/draft> (base protocol: <https://modelcontextprotocol.io/specification/draft/basic>).
6+
7+
Sources: multi-agent compliance audit against the (then-current) `1bf94a60…` pin (12 protocol domains, 46 findings upheld / 8 refuted by adversarial verification), a manual release-docs pass, an external review that caught the pin was stale against the live draft, a re-pin + fix slice, and a second external review of that fix slice that caught two test-quality bugs and several documentation errors (§1 "Correction pass" below). See §5 for what the original audit did not cover.
8+
9+
**Verdict: re-pin complete, all identified P0/P1 wire defects fixed, full gate green.** `cargo test` (default-members workspace): all green. `cargo test -p turul-mcp-protocol-2026-07-28 --features compliance`: **376** passed, 0 failed (185 lib + 187 integration + 3 upstream-fixture + 1 doctest; **366** on default features). `cargo clippy` (default-members, all targets): 0 warnings. Upstream fixture pin advanced to `60dc69e9…` (20/20 fixtures pass, 8/87 modeled). Two items remain explicitly disclosed as deferred (§2) rather than silently marked done; §3 carries lower-severity items unrelated to the schema diff that this slice did not address.
10+
11+
---
12+
13+
## 0. Re-pin — closed
14+
15+
The 2026-06-10 pin (`1bf94a60…`) had drifted from the live draft (145 additions / 82 deletions). Re-vendored to `6e4cba2d…` and fixed every wire-breaking and binding change in the same slice:
16+
17+
| # | Change | Status |
18+
|---|--------|--------|
19+
| 0.1 | MCP error codes renumbered (`-32001→-32020` HeaderMismatch, `-32003→-32021` MissingRequiredClientCapability, `-32004→-32022` UnsupportedProtocolVersion; spec now reserves `-32020..-32099`, `-32000..-32019` is implementation-defined) | **Fixed.** Spec-coded `McpError` variants + `headers::ERROR_CODE_HEADER_MISMATCH` updated. Framework-internal codes that squatted on the newly spec-reserved range (`ValidationError`/`InvalidCapability`/`VersionMismatch`/`ConfigurationError`/`SessionError`/`TransportError`/`JsonRpcProtocolError`) renumbered into `-32000..-32019`. New drift-detector test `mcp_error_code_partition` pins both invariants. All downstream hardcoded-code test assertions (6 test files in `turul-mcp-server`) migrated to the new numbers. |
20+
| 0.2 | `CancelledNotificationParams.requestId` optional → required; client→server only (stdio exception for closing `subscriptions/listen`); `ProgressNotification` dropped from `ClientNotification` | **Mostly fixed.** `requestId` is now required `RequestId` (was `Option`); `without_id()` removed. The dispatch-side narrowing (server should stop accepting inbound client `notifications/progress`) is deferred — see §2. |
21+
| 0.3 | `ElicitationCompleteNotification`/`...Params` removed; `ElicitRequestURLParams.elicitationId` removed | **Fixed.** Types deleted from `notifications.rs`/`lib.rs`; `ElicitRequest::new_url`/`ElicitRequestURLParams::new` dropped the parameter. All call sites (protocol crate tests, `turul-mcp-server` session.rs helper, `mrtr_2026.rs`) updated. |
22+
| 0.4 | Subscription-stream metadata: `NotificationMetaObject` (optional `subscriptionId`), new required-`SubscriptionsListenResult` for graceful close | **Fixed (binding) / disclosed gap (emission)**. `subscriptionId` stamping was *already implemented* server-side ahead of the spec formalizing it (`meta::META_KEY_SUBSCRIPTION_ID`, wire-tested in `subscriptions_listen_2026.rs`) — doc comments updated to reflect it's now schema-declared. New `SubscriptionsListenResult`/`SubscriptionsListenResultMeta` bound and wire-tested; server has no shutdown-signal hook to emit it from yet — see §2. |
23+
| 0.5 | `ListRootsRequest.params` retyped from `RequestParams` to bespoke `{ _meta?: MetaObject }` | **Fixed.** Reverted to bespoke `ListRootsRequestParams { meta: Option<HashMap<String, Value>> }`; `with_meta` takes a loose map again. |
24+
25+
Also re-pinned in the same slice: upstream examples fixture commit (`c3e3f09e…`/`1304c8fe…``60dc69e9…`, via `mcp-compliance-2026-07-28 -- refresh --write`), `EXAMPLES_PIN.md`, `COMPLIANCE.md`'s sha/date/declaration-count header and test-gate table, and `docs/adr/027-targeting-mcp-draft-2026-v1.md`'s revision log.
26+
27+
## 1. Defects found during re-audit and fixed in the same slice
28+
29+
| # | Finding | Severity | Fix |
30+
|---|---------|----------|-----|
31+
| 1.1 | `ContentBlock::ResourceLink` duplicated `annotations`/`_meta` — variant-level fields alongside the flattened `ResourceReference`, which also carries them. Schema: `ResourceLink extends Resource` — exactly one of each. Produced duplicate JSON keys on serialize; `resource.annotations`/`resource.meta` always `None` on deserialize. | **High** (outbound wire defect, not accept-side leniency) | Removed the variant-level `annotations`/`meta` fields; `ResourceReference` (flattened) is the sole carrier. `resource_link()`, `with_annotations()`, `with_meta()` updated. |
32+
| 1.2 | `PrimitiveSchemaDefinition` (`StringSchema \| NumberSchema \| BooleanSchema \| EnumSchema`) was `#[serde(untagged)]` with no `type`-literal enforcement. A bare `{"type":"integer"}` or `{"type":"boolean","title":"x"}` silently parsed as `StringSchema` (untagged dispatch tries String first; `schema_type: String` accepts any value). | Medium | Hand-written `Deserialize` dispatches on `type` (plus `enum`/`oneOf` presence, routing enum-shaped payloads to the `EnumSchema` sub-union) before trying variants; unrecognized/missing `type` is rejected. Three regression tests (bare integer, bare boolean, unknown-type-rejected), each confirmed red before the fix. |
33+
34+
### Correction pass (same day — a second external review of the fix slice above)
35+
36+
Caught two test-quality bugs and disclosure/wording errors in the initial fix pass:
37+
38+
- **1.1's regression test didn't prove what it claimed.** It asserted duplicate-key absence via `serde_json::to_value()`, which builds a `Map` — a `Map` cannot represent duplicate keys, so a second `serialize_entry` for the same key silently overwrites the first *before the test ever inspects it*. The test would pass identically whether the underlying `Serialize` impl emitted the key once or twice. Confirmed empirically by isolating the exact pre-fix struct shape in a scratch crate: `to_value()` → 1 key, `to_string()``{"annotations":"from_resource",...,"annotations":"from_variant",...}` (the literal duplicate). **Fixed**: test now asserts against the raw serialized text (`json_str.matches("\"annotations\":").count() == 1`), verified to fail against the actual pre-fix shape and pass against the fix.
39+
- **The same fix pass introduced an identical footgun in new code.** `SubscriptionsListenResultMeta` (added for item 0.4) has a public `extra: HashMap` (`#[serde(flatten)]`) alongside the typed `subscription_id` field renamed to the same reserved key — a caller populating `extra["io.modelcontextprotocol/subscriptionId"]` would collide with the typed field and produce the exact duplicate-key defect just fixed in 1.1. **Fixed**: hand-written `Serialize` for `SubscriptionsListenResultMeta` always emits the typed field and silently drops any colliding `extra` entry. New test `subscriptions::tests::listen_result_meta_extra_cannot_shadow_subscription_id` populates `extra` with the reserved key and asserts exactly one occurrence on the raw wire, confirmed red before the fix. **Disclosed, not fixed**: `RequestMetaObject.extra` has the identical structural risk and predates this slice — out of scope here, flagged in COMPLIANCE.md for a dedicated pass.
40+
- **`handle_subscriptions_listen`'s doc comment overclaimed.** It stated unconditionally "there is no JSON-RPC result" for `subscriptions/listen` — no longer accurate now that `SubscriptionsListenResult` exists for the graceful-teardown case (item 0.4). Fixed to state the split precisely: client-initiated cancellation has no result; the schema's graceful-teardown result exists but has no emission path yet (confirmed no shutdown-signal infrastructure — no `CancellationToken`, no `tokio::signal` wiring — exists anywhere in `turul-http-mcp-server` or `turul-mcp-server` to hook into).
41+
- **ADR-027's test-count arithmetic was wrong** (`177 + 187 + 3 + 1 = 368`, not the claimed 375 — used the default-feature lib count instead of the compliance-feature one) and **COMPLIANCE.md's "Test gate" table was never updated** in the original fix pass (still showed pre-slice-series counts of 160/179/343/333). Both corrected to the verified counts above (185/187/3/1 = 376 compliance, 178/187/1 = 366 default) after this correction round added two more tests.
42+
- **This file's own wording had drifted**: it recommended adding a superseded banner to `docs/plans/2026-07-28-final-readiness-audit.md` that had, in fact, already been added in the same slice; and its verdict line claimed "all identified defects fixed" while §3 (below) lists unresolved low-severity items — narrowed to "all identified P0/P1 wire defects fixed."
43+
- Six `.rs` source comments introduced by the fix slice violated CLAUDE.md §Comments (dev-log/history narrative: "2026-07-02 re-pin", "previously produced duplicate wire keys" in `roots.rs`, `content.rs`, `meta.rs`, `notifications.rs`). Rewritten to state the current contract only; history moved to ADR-027/COMPLIANCE.md where it belongs.
44+
45+
## 2. Disclosed gaps (spec-legal or scoped-out — not silently marked done)
46+
47+
- **`SubscriptionsListenResult` server-side emission is unwired.** The type is bound and wire-tested, but `turul-http-mcp-server`'s `handle_subscriptions_listen` has no graceful server-shutdown hook to emit it from. Schema explicitly permits this ("sent only when the server tears the subscription down… an abrupt transport close carries no response"). No shutdown-signal infrastructure exists anywhere in the transport crates to build this on — it's unbuilt infrastructure, not a missed call site. Close when a shutdown-signal path is added to the SSE stream handler.
48+
- **Server still dispatches inbound `notifications/progress` from the client** (now removed from `ClientNotification`) to a no-op logger, unconditionally on both spec lanes. The same handler also accepts `notifications/message`, which was never a `ClientNotification` member on any pin — this reads as deliberate lenient any-notification handling, not a strict binding to the schema union, so a partial fix (gating only `notifications/progress`) would be inconsistent with the handler's evident design intent. No wire-emission impact (accept-side leniency only). Needs a dedicated slice to decide whether the dispatch table should enforce `ClientNotification` at all.
49+
- **`RequestMetaObject.extra` has the same reserved-key-collision risk fixed on `SubscriptionsListenResultMeta` in §1.** Pre-existing, not introduced by this slice; out of scope here.
50+
51+
## 3. Low-severity items — carried over, not addressed in the re-pin slice
52+
53+
These predate the re-pin and are unrelated to the schema diff; still open:
54+
55+
- **Accept-side loosenings** (each wire-compliant on emit, looser than schema on accept, none recorded in COMPLIANCE.md §Intentional deviations): `ElicitResult.content` values typed `Value` vs `string|number|boolean|string[]` (`elicitation.rs:530`); `CompletionReference` untagged with free-form `ref_type: String`, inconsistent with the crate's own `#[serde(tag="type")]` convention (`completion.rs:34`); `CompleteResult.completion.total` as `Option<u32>` vs the crate's own `number→f64` convention (`completion.rs:153`); `CreateMessageRequestParams.metadata` as `Option<Value>` vs the crate's own `JSONObject→HashMap` convention (`sampling.rs:333`); non-spec `SamplingRequest`/`SamplingResult` carryover types, unused workspace-wide (`sampling.rs:32,43`); `RootsCapabilities.list_changed` extra field vs schema's empty `{}` (`initialize.rs:85` — confirmed still absent from the live schema on 2026-07-02).
56+
- **Comment-hygiene** (CLAUDE.md §Comments): ADR citation in source at `tools.rs:705`; phase-tag "in this slice" at `content.rs:62`; tombstone narrative at `meta.rs:494`; orphaned doc comments describing nonexistent traits at `notifications.rs:659` and `completion.rs:324`.
57+
- **Doc drift**: `caching.rs` module doc describes a `#[serde(flatten)]` embedding pattern no result struct actually uses; `tools.rs:439` rustdoc overstates `CallToolRequest` as "matches the TypeScript interface" (models only the inner `{method, params}`, envelope lives in turul-rpc); `content.rs:21,37` falsely claims `_meta` is "REQUIRED by MCP spec" (it's optional).
58+
- **`docs/plans/2026-07-28-architecture-review.md`** still states stale test counts ("342 tests") predating this and prior slices — a point-in-time snapshot, not re-audited here.
59+
- Cosmetic: `discover.rs` test fixtures advertise `supportedVersions: ["DRAFT-2026-v1"]` (legal open string[]; modernize to `"2026-07-28"` when next touched).
60+
- Coverage notes: `prompt_shape_matches_schema`/`prompt_argument_shape_matches_schema` assert only the `name` key (`tests/compliance.rs:480`); `completion.values` `@maxItems 100` has no producer-side guard; upstream fixture modeling stands at 8/87 (honestly tallied, floor-tested, not a defect); `minItems`/`maxItems` as `u32` vs TS `number` is defensible but undocumented (`elicitation.rs:195`).
61+
62+
## 4. README.md — verified accurate, no changes required
63+
64+
17 framework crates, 54 active examples (+archived), referenced scripts, `server/discover` curl shape, removed-methods table, WIP banner, and the tasks-extension story all match code and disk. Unaffected by the re-pin (none of the changed types are README-documented wire shapes).
65+
66+
## 5. Coverage limits of the original (pre-re-pin) audit
67+
68+
The original 12-domain audit's verify/synthesize phases were cut short by an API spend limit before the re-pin was discovered. Domains not systematically audited: JSON Schema 2020-12 deep-dive, compliance-harness internals, ADR-conformance sweep, envelope/misc/deprecated types, SEP-2577 deprecations sweep, rollout-gap (server/transport enforcement vs protocol crate), comments/purity sweep. Re-run against the current `6e4cba2d…` pin if a comprehensive re-audit is wanted — this document only tracks what was found and fixed, not an exhaustive re-sweep.
69+
70+
## 6. Disposition of this file
71+
72+
This is a temporary 0.4 punch list, not a permanent record. Once the items above are triaged, fold the surviving open items (§2, §3) into `docs/plans/2026-07-28-spec-compliance.md` and delete this file — do not let it become a second, competing status authority alongside the driver doc.

0 commit comments

Comments
 (0)