Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 89 additions & 0 deletions docs/issue24-wire-alignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Issue #24: modern MCP wire alignment

PR #37 extends the official-transport foundation from PRs #34 and #36. The
official MCP implementation owns JSON-RPC parsing, dispatch, negotiation, SSE and
the modern method/name header checks. NitroStack supplies its handlers and era
policy; it does not recreate the deleted JSON-RPC sidecar.

## Behavior

| Area | Result |
|---|---|
| Ping | Available only in the legacy era through MCP's public `on_ping` hook. Auto and modern reject it. |
| Retired methods | Modern requests reject retired methods even when the application is configured as auto. Legacy-compatible auto requests retain their existing supported behavior. |
| Discovery | The shared builder supplies `_meta.serverInfo`, preserving cache hints and its top-level compatibility copy. MCP may omit legacy-only top-level fields when serializing the modern wire. |
| Ordinary results | Tool, resource and prompt handlers use native top-level `resultType`; existing metadata and specialized result types are preserved. |
| Capability error | `JsonRpcErrorCode.MISSING_REQUIRED_CLIENT_CAPABILITY` defines `-32021`, with its default message and public protocol export. NitroStack-specific live task-capability enforcement is deferred to the tasks issue. |
| Tasks extension | The application does not advertise the unimplemented 2026 tasks extension, including through custom discovery extensions. Existing legacy task functionality remains available. |

## Why a small HTTP header check is still necessary

MCP 2.2.0 selects its strict modern HTTP entry using `MCP-Protocol-Version`. On
that entry, MCP validates `Mcp-Method`, `Mcp-Name`, protocol metadata, duplicates
and unsupported versions. A missing version header instead selects the SDK's
legacy-compatible HTTP entry, so the strict path cannot enforce its own entry
condition.

The existing mount's `HeaderCompatMiddleware` therefore rejects a modern-era
POST with a missing or blank version header using HTTP 400 / error `-32020`.
It does not read the body, infer a method, replay input or dispatch a request.
The response uses a null ID because the JSON-RPC body was not inspected. Health
and other routes remain outside this mount-specific check.

Auto preserves genuine 2025 requests. If a request's body explicitly claims the
modern protocol but lacks the HTTP version header, the existing server middleware
rejects the inconsistency. Stdio has no HTTP headers and is exempt from HTTP-only
requirements. Version-header normalization retains duplicate entries so the
official validator can reject them.

The retirement middleware also evaluates the request's negotiated protocol, not
only the application's era. This prevents auto's legacy task handlers from
answering methods retired on a modern request. Handler registration still uses
MCP's public APIs.

## Result fields and compatibility

The SDK's models provide a native `result_type` field serialized as `resultType`.
The handlers use that field rather than inventing `_meta.resultType`. Model
updates retain application metadata and any already-selected result type.

Discovery's requested `_meta.serverInfo` is distinct from MCP 2.2.0's reserved
`io.modelcontextprotocol/serverInfo` metadata stamp on ordinary modern results.
Tests check the correct location for each. Existing MRTR and legacy task handling
is retained; this PR does not implement a new tasks or MRTR protocol.

## Removed machinery and replacement coverage

Removed code includes the unused sessionless initialize payload builder and
application/server aliases, the test-only modern method-contract table, and the
duplicate optional/required method/name validators. Initialization remains owned
by MCP.

| Previous coverage | Replacement |
|---|---|
| Direct checks of deleted dispatch/pipeline objects | PR #36's migrated HTTP probes retained through the dependency merge. |
| Required/optional method and name helper tests | HTTP requests with missing and mismatched headers for tools, resources and prompts in auto/modern. |
| Static method-contract membership assertion | Successful real HTTP operations covering the requested tool/resource/prompt surface. |
| Sessionless initialize helper inspection | Actual auto initialization response through the official transport. |
| Ping as an HTTP success probe | Tool listing as the success probe, plus explicit legacy success and modern/auto retirement tests for ping. |

Pure JSON-RPC parsing/error mapping, protocol-version consistency, CORS utilities,
SSE formatting and response-header utility tests remain. No new skips, xfail
markers or exclusions are used.

## Dependency merge details

The updated PR #34 contained the new custom CORS middleware and older versions of
the four migrated test modules. The conflict resolution retains the corrected
PR #36 tests and the single Starlette `CORSMiddleware`. Browser-defined parameter
headers are supported through Starlette's request-header mirroring, while allowed
origins remain governed by `MCP_CORS_ALLOWED_ORIGINS`.

CORS tests assert Starlette's HTTP 200 preflight response and inspect exposed
headers on actual responses. The dual HTTP/stdio test now includes the required
HTTP version header with its modern body metadata.

The stash was inspected selectively and remains intact. Generated bytecode,
scaffolded applications, OAuth client changes and spillover changes were not
ported into this implementation. See `pr37-review-verification.md` for exact
commit IDs, reproduction commands and validation results.
165 changes: 165 additions & 0 deletions docs/pr37-review-verification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# PR #37 / issue #24 verification

Validation date: 2026-10-28. Branch: `feature/issue-24-wire-alignment`.
Implementation commit: `0ec5fe265c46d6cbcf35900eca54dcded8865de0`.

## Recorded inputs and dependency history

| Input | Commit |
|---|---|
| Published PR #37 head | `e458282b24ef81f05dd6577f336f0d018541c118` |
| Current upstream develop | `e9cf1dbd8127aaa0154d9734586a0245f1bd2e84` |
| PR #34 head | `680c722837f2fa86da03a8aca9b1f10e97dc239f` |
| Corrected PR #36 head | `71e47f10577bb255e18ec745f0ece18c669fedd0` |
| Preserved local stash | `4163a6583b2fc2d3b95f471f64e05d046bd7c318` |

PRs #34 and #36 were open and unmerged when fetched. The issue branch received
normal merges of PR #36 (`d0391ef`) and the updated PR #34 (`749d4d6`), followed
by the CORS conflict-resolution correction (`e81d34c`). Develop is already an
ancestor through the updated dependency; a subsequent develop merge reported
already up to date. No history was rewritten or force-pushed.

The issue implementation selectively ports relevant ideas from the stash without
applying or dropping it. Its generated bytecode and scaffolded application were
excluded. OAuth and spillover feature commits are not included.

## Reviewer failure reproduction

Collection against an isolated archive of the published head reproduced all four
reviewer-reported errors: **566 tests collected, four collection errors**.

| Module | Original failure | Correction |
|---|---|---|
| `test_mcp20_blueprint.py` | Deleted dispatch module import | Retain PR #36's actual HTTP discovery/statelessness tests. |
| `test_mcp20_deprecated.py` | Deleted dispatch module import | Retain official HTTP and registered-handler retirement tests. |
| `test_mcp20_jsonrpc_wire.py` | Deleted dispatch module import | Retain pure parsing/error tests and official HTTP validation probes. |
| `test_mcp20_stateless_http.py` | Removed ping helper import | Retain migrated HTTP, malformed-body, CORS and SSE coverage. |

After dependency integration, collection succeeded with **826 tests**. The final
branch collects **914 tests**, including **99 issue #24 wire cases**. Obsolete
method/name-validator and static method-table checks were replaced with real HTTP
header and successful-operation tests. See `issue24-wire-alignment.md` for the
coverage mapping and architecture explanation.

## Final results

All complete runs use **MCP 2.2.0**, the development extra and a 60-second test
timeout. No ignore flags, deselection expressions, new skips or xfail markers
were used. The coverage tool's empty-file message does not indicate a skipped
test.

| Platform | Python | Complete suite | Coverage |
|---|---|---|---|
| Ubuntu / WSL | 3.10.21 | 914 passed | 86.15% |
| Ubuntu / WSL | 3.11.16 | 914 passed | 86.11% |
| Ubuntu / WSL | 3.12.14 | 914 passed | 86.11% |
| Windows | 3.12.14 | 914 passed | 86.13% |

The Linux matrix runs an exact `git archive` of implementation commit `0ec5fe2`
on WSL's native filesystem in isolated environments. This avoids the mounted
Windows filesystem's substantial test/build overhead. Logs are copied to the
worktree's ignored `.pytest_cache/linux-final-3.10.log`,
`linux-final-3.11.log` and `linux-final-3.12.log`. The complete final Windows log
is `windows-final.tmp.log`, also ignored. These are local matrix runs, not hosted
GitHub Actions results.

The combined reviewer/protocol/transport verification set passed **379 tests**:

```powershell
python -m pytest tests/test_mcp24_wire_alignment.py tests/test_mcp20_blueprint.py tests/test_mcp20_deprecated.py tests/test_mcp20_jsonrpc_wire.py tests/test_mcp20_stateless_http.py tests/test_mcp20_acceptance.py tests/test_mcp20_foundation.py tests/test_mcp22_official_http_owner.py tests/test_mcp23_http_factory.py tests/test_transport_http.py tests/test_lifecycle_http.py tests/test_mcp20_stdio.py tests/test_transports.py -o addopts= -q --timeout=60 --tb=short
```

Complete verification commands:

```powershell
python -m pytest tests --collect-only -o addopts= -q
python -m pytest tests --cov=nitrostack --cov-report=term-missing --timeout=60 --tb=short -o addopts= -q
git diff --check
```

## Earlier failures and Windows comparison

An initial full Windows run reported 912 passed and two failures. One was the
dual HTTP/stdio test sending modern envelope metadata without the required HTTP
version header. Its request now supplies that header and still verifies that
both transports expose the same tools. An earlier Linux 3.10 run also executed
that pre-correction request; the exact-commit final matrix above supersedes it.

The other Windows failure was the unchanged live OAuth localhost socket test,
`test_discovery_dcr_404_when_disabled_or_missing_client_id`, with WinError 10053.
An archive of the unchanged merged foundation (`e81d34c`), using the same Python
3.12.14 environment and MCP version, reproduced WinError 10053 in the companion
`test_discovery_server_idempotent_start_and_dcr_http` test: **34 passed, 1 failed**.

Baseline comparison command:

```powershell
python -m pytest tests/test_oauth.py tests/test_mcp20_stdio.py::test_dual_http_and_stdio_share_the_same_tools -o addopts= -q --timeout=60 --tb=short
```

The final complete Windows run passed all tests. The earlier baseline-reproducible
socket failure remains documented as intermittent; its OS-level cause is not
claimed resolved. No OAuth server or socket test was changed or excluded.

## Static and publication checks

- `git diff --check` passes and no merge-conflict markers remain.
- Production does not reference the deleted sidecar wrappers or ping helpers.
- Duplicate optional/required method/name validators and the obsolete static
method-contract table are gone; their coverage moved to the official transport.
- The existing stdio AST/public-runner and behavioral regressions pass.
- Dependency files and release version are unchanged by the issue implementation.
- Develop and both dependency heads are ancestors of this branch.
- The saved stash remains intact, and other feature worktrees are unchanged.

The branch remains stacked on #34 and #36 until they land. PR #37 must target
develop when its updates are published. No push, PR-base edit, reviewer reply or
merge is included in this implementation step. Hosted CI must be checked on the
eventual pushed head.

## Rebase after PR #34 merged (2026-10-28)

PR #34 merged normally as `805db05`. Both dependent branches now contain
develop `63ecd9240d98bbe06c879089dd887494c7e99293`, including upstream SecretValue
changes. PR #36 was rebased first; PR #37's issue-specific commits were replayed
onto that rebased foundation. The original local PR #37 head is preserved as
`backup/pr37-before-develop-rebase-20261028`. The original published head was
still `e458282`; publishing uses an explicit force-with-lease against that head.

PR #37 now targets develop. It still includes PR #36 until that PR merges, but
the already-merged PR #34 commits are no longer outstanding dependency changes.
Tree comparison against the previously validated implementation plus new develop
shows no functional changes lost during the rebase.

Complete collection succeeds with **954 tests**, including the 99 wire cases.
Exact-source runs at rebased head `a577fe7` use MCP **2.2.0**:

| Platform | Python | Complete suite | Coverage |
|---|---|---|---|
| Ubuntu / WSL | 3.10.21 | 954 passed | 86.32% |
| Ubuntu / WSL | 3.11.16 | 954 passed | 86.30% |
| Ubuntu / WSL | 3.12.14 | 954 passed | 86.30% |
| Windows | 3.12.14 | 953 passed, 1 socket failure on first run | 86.30% |

The Windows failure is `test_discovery_dcr_404_when_disabled_or_missing_client_id`
with WinError 10053, matching the intermittent baseline-reproduced failure
documented above. A complete repeat is recorded separately below. No exclusions,
skips or xfails were added. Logs are retained in
`.pytest_cache/rebase-linux-3.10.log`, `rebase-linux-3.11.log`,
`rebase-linux-3.12.log`, `rebase-windows-full.tmp.log` and
`rebase-windows-repeat.tmp.log`. Hosted CI must be checked on the new published
head; the old 914-test results above are historical.

The complete Windows repeat reported **952 passed, 2 failed** (86.30% coverage),
both unchanged live OAuth discovery socket tests:
`test_discovery_server_idempotent_start_and_dcr_http` and
`test_discovery_dcr_404_when_disabled_or_missing_client_id`, with WinError 10053.
These Windows full suites are not claimed green. No OAuth source or test changes
were introduced by this rebase, and none of these tests were excluded.

A fresh archive of current develop `63ecd92`, tested with the same Windows
Python/MCP environment, reproduced **both** failures: **72 passed, 2 failed**.
The command was `python -m pytest tests/test_oauth.py tests/test_secret_value.py
-o addopts= -q --timeout=60 --tb=short`. The log is
`.pytest_cache/rebase-develop-windows.log`. Import-path inspection confirmed the
archive's package was loaded rather than the editable feature checkout.
44 changes: 12 additions & 32 deletions nitrostack/core/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ def _read_contents_to_result(uri: str, items: List[ReadResourceContents]) -> typ
uri=uri, mimeType=mime, text=str(item.content), **extra
)
)
return types.ReadResourceResult(contents=contents)
return types.ReadResourceResult(contents=contents, resultType="complete")


def _http_headers_from_request_ctx(rc: Any) -> Dict[str, str]:
Expand Down Expand Up @@ -960,12 +960,9 @@ def _tasks_visible_on_wire(self) -> bool:
return self.protocol_era not in ("auto", "modern")

def _advertise_tasks_extension(self) -> bool:
if not self._tasks_visible_on_wire():
return False
return any(
entry.config.task_support in ("optional", "required")
for entry in self._tools.values()
)
# Legacy task handlers do not implement the complete 2026 extension.
# Advertising it is deferred to the dedicated tasks issue.
return False

def _custom_extensions(self) -> Optional[Dict[str, str]]:
extensions = getattr(self.server_config, "extensions", None)
Expand Down Expand Up @@ -1018,28 +1015,6 @@ def _stamp_discover_meta(self, result: Dict[str, Any]) -> Dict[str, Any]:
out.setdefault("resultType", "complete")
return out

def handle_sessionless_initialize(self, requested_version: Optional[str] = None) -> Dict[str, Any]:
"""Answer 2025 ``initialize`` on the sessionless ``auto`` path. No session."""
from nitrostack.protocol.discovery import build_sessionless_initialize_result

has_widgets = any(
getattr(entry, "component", None) is not None
for entry in getattr(self, "_tools", {}).values()
)
version = protocol_version_for_era(
getattr(self, "protocol_era", None),
self.server_config.protocol_version,
)
return build_sessionless_initialize_result(
server_name=self.server_config.name,
server_version=self.server_config.version,
requested_version=requested_version,
protocol_version=version,
advertise_tasks=self._advertise_tasks_extension(),
advertise_app=has_widgets,
custom_extensions=self._custom_extensions(),
)

def _list_endpoint_cache_meta(self) -> Dict[str, Any]:
return build_list_endpoint_cache_hint_meta()

Expand Down Expand Up @@ -1074,6 +1049,7 @@ async def _list_tools(
return types.ListToolsResult(
tools=tools,
_meta=self._list_endpoint_cache_meta(),
resultType="complete",
)

async def _ping(_ctx: ServerRequestContext, _params: Optional[RequestParams]):
Expand All @@ -1099,21 +1075,23 @@ async def _call_tool(ctx: ServerRequestContext, params: CallToolRequestParams):
},
isError=False,
)
return result
return result.model_copy(update={"result_type": result.result_type or "complete"})
finally:
request_ctx.reset(token)

async def _list_resources(_ctx: ServerRequestContext, _params: Optional[PaginatedRequestParams]):
return types.ListResourcesResult(
resources=[self._build_resource_definition(entry) for entry in self._resources.values()],
_meta=self._list_endpoint_cache_meta(),
resultType="complete",
)

async def _list_resource_templates(_ctx: ServerRequestContext, _params: Optional[PaginatedRequestParams]):
return types.ListResourceTemplatesResult(
resource_templates=[
self._build_resource_template_definition(entry) for entry in self._resource_templates
],
resultType="complete",
)

async def _read_resource(ctx: ServerRequestContext, params: ReadResourceRequestParams):
Expand All @@ -1137,14 +1115,17 @@ async def _list_prompts(_ctx: ServerRequestContext, _params: Optional[PaginatedR
return types.ListPromptsResult(
prompts=[self._build_prompt_definition(entry) for entry in self._prompts.values()],
_meta=self._list_endpoint_cache_meta(),
resultType="complete",
)

async def _get_prompt(ctx: ServerRequestContext, params: GetPromptRequestParams):
arguments = params.arguments or {}
return await self._get_prompt(
result = await self._get_prompt(
params.name,
{str(key): str(value) for key, value in arguments.items()},
)
return result.model_copy(update={"result_type": result.result_type or "complete"})

server.add_request_handler("ping", RequestParams, _ping)
async def _discover(ctx: ServerRequestContext, _params: Optional[RequestParams]):
return self.handle_server_discover(getattr(ctx, "protocol_version", None))
Expand Down Expand Up @@ -1214,7 +1195,6 @@ async def handle_get_prompt(req):

self._register_task_handlers(server)
self._register_initialized_handler(server)
server.discover_handler = self.handle_server_discover
server.middleware.insert(1, self._request_scope_middleware)

def _build_transform_context(self, rc: Any, *, invoker: Any) -> TransformContext:
Expand Down
Loading
Loading