Skip to content

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

Merged
manish-wekan merged 11 commits into
nitrocloudofficial:developfrom
jpsiddharth2008:feature/issue-23-http-factory
Nov 2, 2026
Merged

manish-wekan merged 11 commits into
nitrocloudofficial:developfrom
jpsiddharth2008:feature/issue-23-http-factory

Conversation

@jpsiddharth2008

@jpsiddharth2008 jpsiddharth2008 commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

Purpose

Closes #23. Complete the HTTP factory migration after removal of the duplicate JSON-RPC sidecar, keeping official MCP 2.x as the sole owner of /mcp.

Includes current develop at b4d229b. PRs #34 and #35 are already merged; PR #37 should still land after this PR.

Changes

  • Migrate deleted dispatch/replay and ping-helper test consumers to the official HTTP application, preserving protocol, identity, discovery, SSE and header coverage.
  • Align resource subscription capabilities, discovery and handler behavior across legacy, auto and modern eras, including additional server instances before HTTP startup.
  • Retain one HTTP factory, one Starlette CORS layer, allowed-origin enforcement, exposed headers, health/slash handling and era-specific /json/version reporting.
  • Preserve [mcp-2026] Hide tasks on 2026 wire until tasks/update exists #35's task visibility policy and verify all five task methods remain hidden on auto/modern HTTP.
  • Use public Server.run() and stdio_server() for stdio. Remove every mcp.server.runner import and private MCP stream-protocol imports. A stream guard rejects requests incompatible with the configured era before negotiation; official MCP owns accepted request dispatch, notifications, lifespan and stream shutdown.
  • Preserve trusted stdio connection identity and upstream spillover isolation.

Latest review corrections

  1. Anchor source-scanning paths on Path(nitrostack.__file__).resolve().parent; assert the production scan is nonempty. All three checks pass from outside the checkout on Windows and Linux.
  2. Remove docs/pr36-review-verification.md. Verification notes are no longer shipped as repository documentation.
  3. Strengthen the AST regression to reject runner implementation imports and private MCP module imports, rather than checking only underscore-prefixed symbol names.
  4. Add seven stdio regression cases covering incompatible-era recovery, invalid/missing modern protocol metadata, and EOF cleanup of lifespans, streams and connection identity across all eras. Existing modern calls, legacy handshakes, notifications and spillover round trips remain covered.

Verification on 12637fd

MCP 2.3.0. Collection: 1,052 tests. Factory/stdio-era regression module: 24 passed. No exclusions, deselections or new skips/xfail markers were added.

Environment Complete suite Coverage
Linux/WSL Python 3.10.21 1,052 passed 86.64%
Linux/WSL Python 3.11.16 1,052 passed 86.62%
Linux/WSL Python 3.12.14 1,052 passed 86.62%
Windows Python 3.12.14 1,034 passed, 1 failed, 17 existing skips 84.40%

Linux runs include QuickJS 1.19.4. Windows skips are the existing optional QuickJS checks (16) and symlink capability check (1).

The Windows failure is the unchanged OAuth localhost test test_discovery_dcr_404_when_disabled_or_missing_client_id, raising WinError 10053. The exact test passed on isolated reruns against both this branch and unchanged current develop. The full Windows run is not reported as green; no OAuth code or test was changed.

git diff --check passes. Hosted CI passed on Python 3.10, 3.11 and 3.12 for the final head: run 41454280044.

@Shreyavemana Shreyavemana left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested PR #36 locally.

  • The issue verification tests pass (46 passed).
  • However, the full test suite fails: four test files fail during collection because of removed modules/functions, and two additional tests fail after excluding them:
    • test_mcp20_blueprint.py — missing nitrostack.transports.dispatch
    • test_mcp20_deprecated.py — missing nitrostack.transports.dispatch
    • test_mcp20_jsonrpc_wire.py — missing nitrostack.transports.dispatch
    • test_mcp20_stateless_http.py — cannot import build_ping_response

(Additional failed test after removing the above ones)

  • test_mcp20_test_modules_importable
  • test_supported_versions_per_era

Please address the remaining test failures and rerun the full test suite.

@manish-wekan
manish-wekan changed the base branch from main to develop October 21, 2026 10:14
@jpsiddharth2008 jpsiddharth2008 changed the title Feature/issue 23 http factory fix: unify HTTP factory and migrate tests to official MCP transport Oct 27, 2026
@manish-wekan

manish-wekan commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Restore Access-Control-Expose-Headers, which is missing compared with develop.

Make the source-scanning tests independent of the directory pytest runs from.

Do the #22 request-context items or link a follow-up issue.

Finally Merge it ahead of #34, #35 and #91 , since it already includes #34's commit.

~sent from claude-code

@manish-wekan

manish-wekan commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Please also rebase onto develop: #35 was merged, and it conflicts in tests/test_mcp20_deprecated.py and tests/test_mcp20_stateless_http.py.

~sent from claude-code

@jpsiddharth2008
jpsiddharth2008 force-pushed the feature/issue-23-http-factory branch from 90ddc61 to c222cc8 Compare November 2, 2026 12:11
@manish-wekan

manish-wekan commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the rebase. Three things are left before merge:

  1. The source-scanning tests still read Path("nitrostack/...") relative to the working directory (test_mcp22_official_http_owner.py:47-57, test_mcp23_http_factory.py:115). Please anchor them on Path(nitrostack.__file__).resolve().parent. Run from another directory, test_stdio_module_has_no_private_mcp_runner_import fails, and the exists()/rglob checks pass without checking anything.
  2. Please remove docs/pr36-review-verification.md.
  3. stdio.py still imports mcp.server.runner internals, but [mcp-2026] Unify HTTP factory: CORS once, subscribe flag, public stdio APIs #23 asks for public stdio APIs only. Please fix it here, or open a follow-up issue and link it.

~sent from claude-code

@manish-wekan
manish-wekan merged commit fb0faf8 into nitrocloudofficial:develop Nov 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants