fix: unify HTTP factory and migrate tests to official MCP transport

docs/pr36-review-verification.md

@@ -0,0 +1,161 @@
+# PR #36: HTTP factory review fixes and verification
+
+## Baseline and dependency
+
+The review was reproduced on PR #36 head `e458282b24ef81f05dd6577f336f0d018541c118`.
+Complete collection failed on the four modules listed by the reviewer. Running
+the importability and supported-version tests directly reproduced both additional
+failures.
+
+Refs recorded before implementation:
+
+| Ref | Commit |
+| --- | --- |
+| PR #36 / `feature/issue-23-http-factory` | `e458282b24ef81f05dd6577f336f0d018541c118` |
+| PR #34 head | `55512e0b8b5231b066b9adb6b2284bdfc0989160` |
+| Upstream `develop` | `b3a2e13ae9f6af024f568b10d7c689927ae8ec2a` |
+
+Upstream develop was integrated with normal merge commit `8df5fc4`. The merge
+kept the sidecar files deleted. The supported-version test fix was already on
+develop and arrived with that merge; no supported protocol date was removed.
+
+PR #36 remains dependent on PR #34 and should land after it. PR #34 was not
+modified, and its newer test exclusions were not imported. The separate
+spillover branch and PR are not part of this change.
+
+## Architecture and behavior
+
+Official MCP 2.x remains the sole owner of `/mcp`: it parses JSON-RPC, dispatches
+requests, and serializes responses. No removed ping helper, dispatch pipeline,
+body-replay middleware, or wrapper was restored.
+
+A small public `Server.middleware` hook applies era policy to the SDK's already
+parsed request context. It rejects initialization on modern-only servers,
+rejects explicit unsupported version claims, and checks that a supplied HTTP
+version agrees with its envelope. It delegates valid calls to the official
+runner. It does not read or buffer the HTTP request body, dispatch a second
+handler, or construct JSON-RPC responses.
+
+The official SDK uses an implicit `2025-03-26` version when compatible HTTP
+requests omit a protocol header. That SDK fallback remains allowed for legacy
+compatibility; it is not added as a newly advertised protocol date. Explicit
+versions still follow the existing era policy.
+
+| Era | Explicit supported dates | Resource subscribe capability |
+| --- | --- | --- |
+| legacy | 2025-06-18, 2025-11-25 | true; existing compatibility behavior |
+| auto | 2025-06-18, 2025-11-25, 2026-07-28 | false; subscription requests rejected |
+| modern | 2026-07-28 | false; subscription requests rejected |
+
+The shared subscription predicate is used by discover, capabilities, and
+subscribe/unsubscribe handling. Server instances receive the resolved era when
+their handlers are configured, so capabilities are correct before HTTP startup,
+for additional server instances, and during stdio initialization.
+
+The existing single CORS layer, era-driven `/json/version`, official session
+manager, health routes, slash compatibility, and public stdio runner remain.
+
+## Test migration map
+
+| Previous failure or test target | Replacement and retained coverage |
+| --- | --- |
+| Blueprint import of deleted dispatch classes | Real `server/discover` and tool calls through the mounted HTTP application; capabilities and no-session assertions retained |
+| Deprecated-method tests using pipeline and replay doubles | Official HTTP dispatch and registered handlers; sessionless subscription rejection and legacy compatibility tested separately |
+| Wire tests using deleted pipeline classes | Actual HTTP method/name/version rejection and successful supported-version requests; parsing, error mapping, identity, and pure validators retained |
+| Stateless HTTP imports of deleted ping/dispatch helpers | Mounted-app tool/resource round trips, JSON and SSE responses, all eras, header-only malformed request rejection, unknown-method errors, CORS, slash/health/version checks, verified identity, parameter mirroring, and sessionless isolation |
+| Test-module importability | Existing registry remains enabled and unchanged; all registered modules import |
+| Supported versions per era | Existing additional legacy date included in test expectations through the develop merge |
+| Private stdio dependency | AST checks of runner imports, real memory-stream exchanges, legacy initialization, modern calls, initialization rejection, notifications, and existing stdout safety tests |
+
+The large test deletion reflects removal of tests of retired implementation
+details and duplicate pipeline/replay entry points. It does not exclude files
+from pytest. Pure header, CORS, parser, SSE, method-contract-table, and response
+echo utility tests remain. Coverage of current behavior is exercised at the
+official transport boundary instead of through a replacement sidecar.
+
+MCP 2.2 maps modern method-not-found errors to HTTP 404; the legacy transport
+maps those errors to HTTP 200 with a JSON-RPC error body. Tests assert those
+official transport mappings rather than preserving a deleted sidecar's status.
+
+## Verification
+
+All tests used MCP **2.2.0**. Full runs used:
+
+```text
+python -m pytest tests --cov=nitrostack --cov-report=term-missing --timeout=60
+```
+
+| Platform | Python | Complete suite | Coverage |
+| --- | --- | --- | --- |
+| Windows | 3.12.14 | 807 passed | 86.02% |
+| Ubuntu / WSL | 3.10.21 | 807 passed | 86.04% |
+| Ubuntu / WSL | 3.11.16 | 807 passed | 86.02% |
+| Ubuntu / WSL | 3.12.14 | 807 passed | 86.02% |
+
+No tests were skipped, marked xfail, ignored, or deselected in these complete
+runs. Coverage's separate "1 empty file skipped" message refers to an empty
+source file in its report, not a skipped pytest test.
+
+The final review restored two additional pre-existing pure utility checks:
+the method-contract surface and supported response-version echo. Both passed
+on all four runtimes after the complete runs above. Final collection contains
+**809 tests**, with no collection errors. These two additions do not change
+production behavior. The hosted workflow should run all 809 tests on the final
+commit; local Linux matrix results above are not a claim about hosted CI status.
+
+Other recorded checks:
+
+- The six reviewer-reported test modules: **225 passed** before the two final
+  utility checks were restored.
+- The original issue verification command: **34 passed**.
+- The issue #22/#23 ownership/factory and related HTTP/stdio/deprecation set:
+  **76 passed**.
+- AST validation rejects private `mcp.server.runner` imports.
+- Production searches find no deleted sidecar wrappers or ping helpers.
+- `git diff --check` passes.
+
+The exact original issue verification command is:
+
+```text
+python -m pytest tests/test_transport_http.py tests/test_lifecycle_http.py tests/test_mcp20_stdio.py tests/test_transports.py -q
+```
+
+For a fresh environment, install the repository's development extra first:
+
+```text
+python -m pip install -e ".[dev]"
+```
+
+The existing GitHub workflow provides the Linux Python 3.10/3.11/3.12 matrix.
+Its status must be checked separately on the pushed commit. No reviewer reply
+was posted as part of the implementation; a response is prepared for user review.
+
+## Rebase after PR #34 merged (2026-10-28)
+
+PR #34 merged normally as `805db05`. PR #36 was rebased onto current develop
+`63ecd9240d98bbe06c879089dd887494c7e99293`, which also includes the upstream
+SecretValue work. The previous head `71e47f1` is preserved locally as
+`backup/pr36-before-develop-rebase-20261028`.
+
+The rebased branch preserves the migrated tests and single Starlette CORS layer.
+Conflicts with the merged foundation's older tests/custom CORS layer were
+resolved while retaining browser-defined Mcp-Param headers and origin checks.
+Comparison against the previously tested merged foundation plus new develop
+found no functional differences beyond these retained corrections.
+
+Complete collection succeeds with **866 tests**. Exact-source local full suites
+at rebased implementation head `682630b`, using MCP **2.2.0**, report:
+
+| Platform | Python | Result | Coverage |
+|---|---|---|---|
+| Ubuntu / WSL | 3.10.21 | 866 passed | 86.11% |
+| Ubuntu / WSL | 3.11.16 | 866 passed | 86.11% |
+| Ubuntu / WSL | 3.12.14 | 866 passed | 86.11% |
+| Windows | 3.12.14 | 866 passed | 86.11% |
+
+No ignored files, deselections, skips or xfails were used. `git diff --check`
+passes. Logs are retained locally in `.pytest_cache/rebase-linux-3.10.log`,
+`rebase-linux-3.11.log`, `rebase-linux-3.12.log` and
+`rebase-windows-full.tmp.log`. The hosted CI results above apply to the old head;
+hosted CI must be checked again on the rebased published head. PR #34 is no
+longer an unmerged dependency. PR #37 remains stacked on this PR until #36 lands.

nitrostack/core/app.py

@@ -66,6 +66,7 @@
     resolve_http_engine,
     resolve_protocol_era_resolution,
     wire_mode_for_era,
+    supports_resource_subscriptions,
 )
 from nitrostack.protocol.mrtr import InputRequiredResult, split_mrtr_from_arguments
 from nitrostack.protocol.cache_hints import (
@@ -869,10 +870,14 @@ def handle_server_discover(self, protocol_version: Optional[str] = None) -> Dict
             getattr(self, "protocol_era", None),
             self.server_config.protocol_version,
         )
+        # Same source of truth as `NitroStackMcpServer.get_capabilities`, so
+        # `server/discover` and real capability negotiation never disagree.
+        resources_subscribe = supports_resource_subscriptions(self.protocol_era)
         return build_discover_result(
             server_name=self.server_config.name,
             server_version=self.server_config.version,
             protocol_version=version,
+            resources_subscribe=resources_subscribe,
             advertise_tasks=self._advertise_tasks_extension(),
             advertise_app=has_widgets,
             custom_extensions=self._custom_extensions(),
@@ -904,6 +909,7 @@ def _list_endpoint_cache_meta(self) -> Dict[str, Any]:
         return build_list_endpoint_cache_hint_meta()
 
     def _setup_handlers(self, server: NitroStackMcpServer) -> None:
+        server.protocol_era = self.protocol_era
         async def _list_tools(
             ctx: ServerRequestContext,
             _params: Optional[PaginatedRequestParams],
@@ -967,14 +973,16 @@ async def _read_resource(ctx: ServerRequestContext, params: ReadResourceRequestP
             return _read_contents_to_result(uri, await self._read_resource(uri))
 
         async def _subscribe_resource(_ctx: ServerRequestContext, params: SubscribeRequestParams):
-            if rejects_deprecated_method("resources/subscribe", self.protocol_era):
+            if not supports_resource_subscriptions(self.protocol_era):
                 message = deprecated_method_message("resources/subscribe")
                 raise MCPError(types.METHOD_NOT_FOUND, message or "Not supported")
             if str(params.uri) not in self._resources:
                 raise ResourceNotFoundError(str(params.uri))
             return types.EmptyResult()
 
         async def _unsubscribe_resource(_ctx: ServerRequestContext, _params: UnsubscribeRequestParams):
+            if not supports_resource_subscriptions(self.protocol_era):
+                raise MCPError(types.METHOD_NOT_FOUND, "Resource subscriptions are not supported in this era")
             return types.EmptyResult()
 
         async def _list_prompts(_ctx: ServerRequestContext, _params: Optional[PaginatedRequestParams]):

nitrostack/core/mcp_server.py

@@ -8,9 +8,19 @@
 from typing import Any, Callable, Dict, Optional
 
 import mcp.types as types
+from mcp import MCPError
 from mcp.server.lowlevel import Server as LowLevelServer
 from mcp.server.lowlevel.server import NotificationOptions
 from mcp.server.subscriptions import InMemorySubscriptionBus, ListenHandler
+from nitrostack.protocol.errors import JsonRpcErrorCode
+from nitrostack.protocol.version import (
+    MODERN_PROTOCOL_VERSION,
+    ProtocolEra,
+    supports_resource_subscriptions,
+    supported_protocol_versions_for_era,
+)
+from nitrostack.protocol.meta import flatten_request_meta_object
+from nitrostack.protocol.jsonrpc import HeaderBodyMismatchError, validate_protocol_version_header_meta
 
 
 class NitroStackMcpServer(LowLevelServer):
@@ -33,12 +43,41 @@ def __init__(self, name: str, version: Optional[str] = None):
         self.has_task_support: bool = False
         self.http_engine: Optional[str] = None
         self.sessionful: bool = False
+        self.protocol_era: Optional[ProtocolEra] = None
+        self.middleware.append(self._enforce_protocol_era)
         self.discover_handler: Optional[Callable[[], dict[str, Any]]] = None
         self.initialize_handler: Optional[Callable[[Optional[str]], dict[str, Any]]] = None
         # In-process tests still look up handlers by request type.
         self.request_handlers: Dict[Any, Callable[..., Any]] = {}
         self.notification_handlers: Dict[Any, Callable[..., Any]] = {}
 
+    async def _enforce_protocol_era(self, context, call_next):
+        """Apply era policy inside the official runner, after its JSON-RPC parsing."""
+        if self.protocol_era == "modern" and context.method in (
+            "initialize", "notifications/initialized",
+        ):
+            raise MCPError(types.METHOD_NOT_FOUND, f"Method not found: {context.method}")
+        request_headers = getattr(getattr(context, "request", None), "headers", {})
+        meta = flatten_request_meta_object(context.meta)
+        header_version = request_headers.get("mcp-protocol-version")
+        meta_version = meta.get("io.modelcontextprotocol/protocolVersion")
+        try:
+            validate_protocol_version_header_meta(header_version, meta_version)
+        except HeaderBodyMismatchError as exc:
+            raise MCPError(int(exc.code), exc.message) from None
+        claimed_version = header_version or meta_version
+        # The official SDK uses a 2025-03-26 default when a legacy-compatible
+        # HTTP request omits the header. Do not reject that implicit fallback.
+        unsupported_claim = self.protocol_era is not None and claimed_version and (
+            claimed_version not in supported_protocol_versions_for_era(self.protocol_era)
+        )
+        modern_legacy_request = (
+            self.protocol_era == "modern" and context.protocol_version != MODERN_PROTOCOL_VERSION
+        )
+        if unsupported_claim or modern_legacy_request:
+            raise MCPError(int(JsonRpcErrorCode.UNSUPPORTED_PROTOCOL_VERSION), "Unsupported protocol version")
+        return await call_next(context)
+
     def handle_server_discover(self) -> dict[str, Any]:
         """Answer ``server/discover`` from this server instance."""
         if self.discover_handler is None:
@@ -67,7 +106,7 @@ def get_capabilities(
         )
 
         if caps.resources is not None:
-            caps.resources.subscribe = True
+            caps.resources.subscribe = supports_resource_subscriptions(self.protocol_era)
 
         if self.has_task_support:
             caps.tasks = types.ServerTasksCapability(

nitrostack/protocol/discovery.py

@@ -36,11 +36,17 @@ def build_discover_result(
     custom_extensions: Optional[dict[str, str]] = None,
     tools_list_changed: bool = True,
     resources_list_changed: bool = True,
+    resources_subscribe: bool = False,
     prompts_list_changed: bool = True,
     ttl_ms: int = DEFAULT_LIST_CACHE_TTL_MS,
     cache_scope: Literal["public", "private"] = "private",
 ) -> dict[str, Any]:
-    """Build the ``server/discover`` result for the mounted HTTP engine."""
+    """Build the ``server/discover`` result for the mounted HTTP engine.
+
+    ``resources_subscribe`` must match ``NitroStackMcpServer.get_capabilities``
+    for the same era, so ``server/discover`` and real capability negotiation
+    never disagree on ``resources.subscribe``.
+    """
     versions = list(supported_versions or SUPPORTED_PROTOCOL_VERSIONS)
     extensions: dict[str, dict[str, str]] = {}
     if advertise_app:
@@ -53,7 +59,7 @@ def build_discover_result(
 
     capabilities: dict[str, Any] = {
         "tools": {"listChanged": tools_list_changed},
-        "resources": {"subscribe": False, "listChanged": resources_list_changed},
+        "resources": {"subscribe": resources_subscribe, "listChanged": resources_list_changed},
         "prompts": {"listChanged": prompts_list_changed},
     }
     if extensions:

nitrostack/protocol/version.py

@@ -207,6 +207,11 @@ def needs_sessionful_engine(era: ProtocolEra) -> bool:
     return era == "legacy"
 
 
+def supports_resource_subscriptions(era: Optional[ProtocolEra]) -> bool:
+    """Only the legacy compatibility era serves resource subscriptions."""
+    return era == "legacy"
+
+
 def http_engine_for_era(era: ProtocolEra) -> HttpEngine:
     """
     Select the /mcp HTTP engine for an era.

nitrostack/transports/http.py

@@ -34,13 +34,15 @@
 
 from starlette.applications import Starlette
 from starlette.middleware import Middleware
+from starlette.middleware.cors import CORSMiddleware
 from starlette.responses import HTMLResponse, JSONResponse, Response
 from starlette.routing import Mount, Route
 from starlette.types import ASGIApp, Receive, Scope, Send
 
 from nitrostack.transports.landing_page import render_landing_page
 from nitrostack.widgets.preview_page import render_preview_page
 from nitrostack.core.di import DIContainer
+from nitrostack.transports.cors import configured_cors_origins
 from pydantic_core import PydanticUndefined
 from mcp.server.sse import SseServerTransport
 from mcp.server.streamable_http_manager import StreamableHTTPSessionManager
@@ -56,10 +58,10 @@
 from nitrostack.protocol.constants import LEGACY_SESSION_HEADER
 from nitrostack.transports.headers import (
     CORS_ALLOW_HEADER_NAMES,
+    CORS_EXPOSE_HEADER_NAMES,
     SSE_SUBSCRIPTIONS_PATH,
     strip_legacy_session_headers_asgi,
 )
-from nitrostack.transports.cors import McpCorsMiddleware
 from nitrostack.transports.subscriptions import subscriptions_listen_endpoint
 from nitrostack.transports.proxy import public_url_for_request
 from nitrostack.core.log_emitter import install_log_handler
@@ -490,6 +492,7 @@ def build_http_app(
     if server is not None:
         server.http_engine = http_engine
         server.sessionful = http_engine == "sessionful"
+        server.protocol_era = protocol_era
 
     security_settings = None
     if not enable_cors:
@@ -662,7 +665,7 @@ async def json_version(request):
         return JSONResponse(
             {
                 "Browser": meta["name"],
-                "Protocol-Version": "2025-06-18",
+                "Protocol-Version": protocol_version_for_era(protocol_era),
                 "User-Agent": f"NitroStack/{meta['version']}",
                 "webSocketDebuggerUrl": "",
                 "transport": "mcp",
@@ -741,14 +744,23 @@ async def lifespan(app):
         Middleware(ExactEndpointSlashMiddleware, endpoint=endpoint),
     ]
     if enable_cors:
-        # CORS policy lives in transports/cors.py (MCP_CORS_ALLOWED_ORIGINS, Mcp-* headers).
-        # Mcp-Session-Id is exposed only on the sessionful (legacy) engine.
+        expose_headers = list(CORS_EXPOSE_HEADER_NAMES)
+        if http_engine == "sessionful":
+            expose_headers.append(LEGACY_SESSION_HEADER)
+        # Single CORS layer for `/mcp`: the sidecar that used to add a second
+        # one has been removed (#22), so `MCP_CORS_ALLOWED_ORIGINS` is honored
+        # here and nowhere else.
+        allow_origins = list(configured_cors_origins()) or ["*"]
         middleware.insert(
             0,
             Middleware(
-                McpCorsMiddleware,
-                expose_session_id=http_engine == "sessionful",
-                fallback_allow_headers=CORS_ALLOW_HEADERS,
+                CORSMiddleware,
+                allow_origins=allow_origins,
+                allow_methods=["GET", "POST", "DELETE", "OPTIONS"],
+                # SEP-2243 permits application-defined Mcp-Param-* names.
+                # Starlette mirrors requested names when '*' is enabled.
+                allow_headers=[*CORS_ALLOW_HEADERS, "*"],
+                expose_headers=expose_headers,
             ),
         )