mcp: copy all extensions over#1518
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates MCP upstream request handling to propagate all http::Extensions from the incoming HTTP request into downstream MCP upstream requests, enabling access to additional request-scoped metadata (e.g., authz metadata) in MCP context.
Changes:
- Replace per-extension propagation (
Claims,BufferLimit) with cloning and forwarding the fullhttp::Extensionsmap. - Extend outgoing MCP upstream requests with the captured extensions in
IncomingRequestContext::apply. - Add
TransformationMetadatatoDebugExtensionsoutput for improved observability.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/agentgateway/src/mcp/upstream/mod.rs | Capture and forward all request extensions via IncomingRequestContext rather than copying a small subset. |
| crates/agentgateway/src/http/mod.rs | Include TransformationMetadata in extension debug output for logging/troubleshooting. |
Comments suppressed due to low confidence (1)
crates/agentgateway/src/mcp/upstream/mod.rs:26
- The imports
crate::http::jwt::Claims,crate::http::transformation_cel::TransformationMetadata, andcrate::transport::BufferLimitappear unused in this module (they’re not referenced after switching to cloningparts.extensions). Please remove them to avoid unused-import warnings (which can fail CI if warnings are denied).
use crate::http::jwt::Claims;
use crate::http::transformation_cel::TransformationMetadata;
use crate::mcp::FailureMode;
use crate::mcp::mergestream::Messages;
use crate::mcp::router::{McpBackendGroup, McpTarget};
use crate::mcp::streamablehttp::StreamableHttpPostResponse;
use crate::mcp::{mergestream, upstream};
use crate::proxy::ProxyError;
use crate::proxy::httpproxy::PolicyClient;
use crate::transport::BufferLimit;
use crate::types::agent::McpTargetSpec;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn cpy_ext<T: Clone + Send + Sync + 'static>( | ||
| inp: &::http::request::Parts, | ||
| ext: &mut ::http::Extensions, | ||
| ) { | ||
| if let Some(v) = inp.extensions.get::<T>() { | ||
| ext.insert(v.clone()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
cpy_ext is currently unused. Please remove it, or wire it up if the intent is to selectively propagate only specific extension types (especially if you end up not cloning the entire Extensions map).
| fn cpy_ext<T: Clone + Send + Sync + 'static>( | |
| inp: &::http::request::Parts, | |
| ext: &mut ::http::Extensions, | |
| ) { | |
| if let Some(v) = inp.extensions.get::<T>() { | |
| ext.insert(v.clone()); | |
| } | |
| } |
| #[derive(Debug, Clone)] | ||
| pub struct IncomingRequestContext { | ||
| headers: http::HeaderMap, | ||
| claims: Option<Claims>, | ||
| buffer_limit: Option<BufferLimit>, | ||
| ext: ::http::Extensions, | ||
| } |
There was a problem hiding this comment.
IncomingRequestContext is cloned in a few hot paths (e.g., stdio and SSE transports clone ctx per message). With ext: http::Extensions added, each ctx.clone() now clones the full extensions map, which may include relatively large metadata maps. Consider mitigating the cost (e.g., storing only the specific extensions MCP needs, or storing extensions behind an Arc and only cloning selected heavy values) if this becomes a throughput/latency issue.
This lets us access any fields from the initial request in MCP context, such as extauthz metadata, etc Signed-off-by: John Howard <john.howard@solo.io>
howardjohn
force-pushed
the
mcp/cpy-all-ext
branch
from
42db3b8 to
34a3c8d
Compare
This lets us access any fields from the initial request in MCP context,
such as extauthz metadata, etc