Skip to content

feat(mcp): add streamable HTTP serve transport#485

Open
andrew-adamson wants to merge 1 commit intoQ00:mainfrom
andrew-adamson:feat/streamable-http-mcp
Open

feat(mcp): add streamable HTTP serve transport#485
andrew-adamson wants to merge 1 commit intoQ00:mainfrom
andrew-adamson:feat/streamable-http-mcp

Conversation

@andrew-adamson
Copy link
Copy Markdown

Summary

  • add streamable HTTP as a supported ouroboros mcp serve transport
  • update the CLI and server adapter to wire the new transport through cleanly
  • cover the transport behavior with focused docs and tests

Testing

  • uv run pytest tests/unit/cli/test_mcp_validate_transport_stderr.py tests/unit/mcp/server/test_adapter.py

Copy link
Copy Markdown
Contributor

@ouroboros-agent ouroboros-agent Bot left a comment

Choose a reason for hiding this comment

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

Review — ouroboros-agent[bot]

Verdict: APPROVE

Reviewing commit 5b69baa for PR #485

Review record: b993855f-396b-4990-861b-0732027e928c

Blocking Findings

No in-scope blocking findings remained after policy filtering.

Non-blocking Suggestions

| 1 | docs/cli-reference.md:656 | Documentation | The startup-cleanup note says orphan-session cleanup is reported “on the console for network transports”, but _run_startup_cleanup() still writes those messages through _stderr_console unconditionally in src/ouroboros/cli/commands/mcp.py. Either the docs should stay transport-agnostic or the implementation should route network-transport cleanup messages through the normal console path. |
| 2 | tests/unit/mcp/server/test_adapter.py:412 | Nice-to-have tests | The new streamable-http coverage only proves the mocked adapter calls run_streamable_http_async(). A higher-value test would exercise the real mcp package interface in an integration test, because this feature depends on an SDK method name/signature that the current unit test can’t validate. |

Design Notes

The change is small and coherent: transport normalization, CLI surface, adapter dispatch, and unit coverage all move in the same direction. The main residual risk is SDK compatibility, since the new server path is only verified through mocks.

Recovery Notes

First recoverable review artifact generated from codex analysis log.


Reviewed by ouroboros-agent[bot] via Codex deep analysis

@shaun0927
Copy link
Copy Markdown
Collaborator

Thanks for the clean slice here — the direction makes sense, and it lines up with the recent MCP transport expansion work.

Before merge, I want to tighten the serve-side contract a bit more:

  1. Please make the user-facing contract explicit for streamable-http:

    • the exact endpoint path clients should use,
    • why http is still only a client-side alias and not a serve transport,
    • and where startup messages go for network transports.
  2. Please add or strengthen one user-visible contract test around the advertised network address/path, not just transport validation.

  3. Since transport work in this repo tends to become security/ops surface area very quickly, please make sure the docs call out any FastMCP-specific limitations/caveats clearly.

The scope is good as-is; I just want the docs + runtime contract to be a bit sharper before this lands.

@andrew-adamson andrew-adamson force-pushed the feat/streamable-http-mcp branch from 5b69baa to 9380198 Compare April 24, 2026 13:43
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