Share MCP subscription sessions with daemon-managed transports

[jolestar]

Summary

• route MCP stdio/http resource subscriptions through the daemon-managed session reuse pool
• fan out session notifications so tool calls and subscriptions can observe the same session state
• fix the session-scoped MCP HTTP test server deadlock and cover shared-session stdio/http e2e cases

Testing

• cargo test --test local_e2e_test test_mcp_http_subscribe_shares_daemon_session_with_tool_calls -- --nocapture
• cargo test --test local_e2e_test test_mcp_http_subscribe_with_read_resource_writes_snapshots -- --nocapture
• cargo test --test local_e2e_test test_mcp_stdio_subscribe_shares_daemon_session_with_tool_calls -- --nocapture
• cargo check --tests

[Copilot]

Pull request overview

This PR routes MCP stdio/http resource subscriptions through the daemon’s MCP session reuse pool so that subscriptions and tool calls can share a single underlying MCP session (and observe a unified notification stream), and adds new E2E coverage around those shared-session semantics.

Changes:

• Adds daemon-side MCP session notification fanout + resource subscription refcounting for both stdio and HTTP transports.
• Updates MCP HTTP transport to use a dedicated reqwest client for the long-lived SSE event stream to avoid request/stream contention.
• Extends the test server scenarios and adds E2E tests validating that MCP subscriptions share session state with tool calls.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/local_e2e_test.rs	Adds shared-session E2E tests for MCP stdio/http subscribe + tool call interactions.
src/test_server/common.rs	Introduces SessionScopedResource scenario + CLI parsing/error message updates.
src/test_server/mcp_stdio.rs	Implements session-scoped resource behavior for stdio test server (tools + notifications).
src/test_server/mcp_http.rs	Implements session-scoped resource behavior for HTTP test server, including session-id headers and SSE updates.
src/test_server/openapi.rs	Allows OpenAPI test server endpoints to respond under the new scenario.
src/test_server/jsonrpc.rs	Extends JSON-RPC test server scenario matching to include the new scenario.
src/test_server/grpc.rs	Extends gRPC test server scenario matching to include the new scenario.
src/test_server/graphql.rs	Extends GraphQL test server scenario matching to include the new scenario.
src/daemon.rs	Adds session reuse lookup for MCP HTTP, notification fanout, and routes MCP subscriptions through daemon-managed sessions.
src/adapters/mcp/http_transport.rs	Adds a dedicated SSE client and exposes ensure_notification_stream on the remote transport wrapper.

Comments suppressed due to low confidence (1)

src/adapters/mcp/http_transport.rs:1029

• McpHttpTransport::subscribe_resource no longer calls ensure_event_stream() before subscribing. For streamable MCP servers that only deliver notifications over the SSE GET stream, subscribing before the reader is running can lose notifications that happen between subscribe and the first later call that opens the stream. Now that the event stream uses a dedicated event_stream_client, consider restoring ensure_event_stream().await? here (or ensure it in the daemon session before subscribing) so notification delivery is reliable.

    pub async fn subscribe_resource(&self, uri: &str) -> Result<()> {
        let params = serde_json::json!({
            "uri": uri
        });
        let _ = self
            .send_request("resources/subscribe", Some(params))
            .await?;
        Ok(())

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[holonbot]

Review for PR #319: Share MCP subscription sessions with daemon-managed transports

Summary

This PR introduces session sharing for MCP resource subscriptions, routing both tool calls and subscriptions through daemon-managed session pools. The implementation adds a notification fanout mechanism that allows multiple consumers (tool calls and subscriptions) to observe the same session state. This is a significant architectural improvement that reduces resource usage and enables better coordination between different MCP operations.

Overall Assessment: ✅ Approve with recommendations

The changes are well-structured, thoroughly tested, and address a real coordination problem in the MCP transport layer. The implementation demonstrates good understanding of async Rust patterns and session lifecycle management.

Key Findings

🔶 Should Fix (2 items)

1. CLI args forwarding without re-validation (src/daemon.rs:573)

   • The try_invoke_existing_mcp_execute_without_detection function forwards already-parsed CLI args without re-validation when reusing sessions. This assumes the args format remains stable between the original call and the reused session.
   • Risk: If the endpoint schema changes between session creation and reuse, the forwarded args may not match the server's expectations.
   • Recommendation: Either re-parse CLI args from the original endpoint string when reusing sessions, or document this as an intentional optimization with the assumption that session reuse implies schema stability.

2. Subscription cleanup error handling (src/daemon.rs:817)

   • In the stdio subscription cleanup, release_resource_subscription errors are logged but don't prevent the function from continuing. This can lead to state divergence where the server believes a subscription is still active while the client has moved on.
   • Risk: Accumulated leaked subscriptions on the server side over time.
   • Recommendation: Consider adding a retry mechanism for failed unsubscriptions, or propagate the error more aggressively to prevent state divergence.

💡 Optional Improvements (2 items)

1. Document event stream timeout rationale (src/adapters/mcp/http_transport.rs:19-22)

   • The event_stream_client uses LEGACY_SSE_STREAM_TIMEOUT_SECS while the regular client uses 30 seconds. The difference in timeout values should be documented to explain the design rationale.

2. Document sequence wraparound behavior (src/daemon.rs:226)

   • The SessionNotificationFanout::extend uses saturating_add for sequence numbering, which wraps at u64::MAX. While practically unlikely, documenting the wraparound behavior would clarify the design intent.

3. Extract keepalive interval to named constant (src/test_server/mcp_http.rs:321)

   • The KeepAlive interval is hardcoded to 250ms. Extracting this to a named constant would improve maintainability and document the policy decision.

Positive Aspects

• Comprehensive test coverage: The PR adds two new e2e tests that specifically validate session sharing behavior for both stdio and HTTP transports.
• Session-scoped resource test scenario: The new SessionScopedResource test scenario is well-designed and effectively validates the core functionality.
• Notification fanout architecture: The SessionNotificationFanout implementation with cursor-based tracking is elegant and handles the multi-consumer case cleanly.
• Dedicated event stream client: Separating the SSE event stream client from the regular JSON-RPC client is a good isolation strategy.
• Subscription reference counting: The ensure_resource_subscription and release_resource_subscription methods properly handle reference counting for shared subscriptions.
• HTTP session lookup optimization: The http_lookup index for session reuse by raw endpoint/auth key is a performance-conscious addition.

Architecture Notes

The PR introduces several important architectural concepts:

1. Notification fanout with cursors: The SessionNotificationFanout maintains a bounded history (256 entries) with sequence numbers, allowing consumers to request updates "since" a cursor. This is a well-established pattern for state synchronization.

2. Session lifecycle management: The daemon now tracks both tool calls and subscriptions through the same session objects, with proper reference counting for subscriptions.

3. Dual-client strategy for HTTP: Using separate clients for regular requests and SSE event streams prevents contention and allows different timeout configurations.

Testing Coverage

The PR includes good e2e test coverage:

• test_mcp_stdio_subscribe_shares_daemon_session_with_tool_calls: Validates stdio session sharing
• test_mcp_http_subscribe_shares_daemon_session_with_tool_calls: Validates HTTP session sharing

Both tests verify that tool calls can modify session-scoped resources that subscriptions observe, which is the core value proposition of this PR.

Recommendations

1. Address the two "should fix" items before merge, particularly the CLI args forwarding validation concern.
2. Consider adding metrics or logging for session reuse hits/misses to help operators understand the effectiveness of the session pooling in production.
3. The MCP_NOTIFICATION_HISTORY_LIMIT constant (256) could be made configurable if different workloads benefit from different history window sizes.

Conclusion

This is a well-executed PR that solves a real problem in the MCP transport layer. The code is clean, well-tested, and follows Rust best practices. The identified issues are minor and don't block merge, but addressing them would improve robustness and maintainability.

Recommendation: Approve after addressing the subscription cleanup error handling concern. The CLI args forwarding issue can be documented as a known trade-off if re-parsing is deemed too expensive.