Switch to upstream rmcp.#10754
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
vorporeal
force-pushed
the
david/switch-to-upstream-rmcp
branch
from
9b73bb4 to
6f53c6d
Compare
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR switches Warp to upstream rmcp and restores legacy SSE support in a new mcp crate.
Concerns
- Clean SSE stream termination currently ends the transport instead of reconnecting, so normal idle timeouts or server restarts can leave MCP connections dead.
- The new stream-error log includes the full SSE URI, which may contain query-string credentials or session tokens.
Security
- Redact or omit the logged SSE URI before writing stream errors to logs.
Verdict
Found: 0 critical, 2 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| retrying, | ||
| } | ||
| } | ||
| None => { |
There was a problem hiding this comment.
| last_event_id: Option<&str>, | ||
| ) { | ||
| tracing::warn!( | ||
| uri = %self.uri, |
There was a problem hiding this comment.
| matches!(sse.event.as_deref(), None | Some("") | Some("message")); | ||
| if !is_message_event { | ||
| match this.connector.handle_control_event(&sse) { | ||
| Ok(()) => return self.poll_next(cx), |
There was a problem hiding this comment.
💡 [SUGGESTION] [SECURITY] A server can keep this poll recursive by sending buffered control or malformed events; replace the self.poll_next(cx) recursion paths with a loop so untrusted SSE input cannot grow the stack.
1 participant
Description
Switches Warp from our fork of
rmcpback to the upstream crates.io release so we no longer have to carry and maintain a fork just for Warp-specific compatibility.Our fork primarily existed because older upstream
rmcpsupported the legacy SSE client transport that some MCP servers still rely on, but current upstream releases no longer include that transport. To preserve compatibility while using upstreamrmcp, this PR adds a small localmcpworkspace crate that contains the old upstream SSE transport implementation (SseClientTransport, reconnect handling, reqwest integration, and auth integration), copied from an earlier version of upstreamrmcpunder its Apache-2.0 license with attribution in the copied files.This also updates Warp's MCP integration for the newer upstream APIs and related dependency changes:
rmcpgit dependency with upstreamrmcp = "1.6".crates/mcpto host the legacy SSE transport shim used by Warp.rmcpAPI changes.StoredCredentialsshape for MCP OAuth so refreshed tokens,token_received_at, and DCR client secrets survive restarts.Testing
cargo check -p warppasses.Added/updated unit coverage for MCP OAuth credential deserialization, token timestamp persistence, skipped empty-token persistence, and refresh-token carry-forward behavior.
Updated MCP capability helper tests for the upstream builder-style APIs.
Manually tested MCP server connections using the legacy SSE transport with a local build.
I have manually tested my changes locally with
./script/runAgent Mode
Co-Authored-By: Oz oz-agent@warp.dev