fix(mcp): route client JSON-RPC responses for server-initiated requests by piyushbag · Pull Request #2207 · agentgateway/agentgateway

piyushbag · 2026-06-16T15:42:09Z

Summary

Fixes Streamable HTTP MCP sessions where downstream heartbeat pongs were rejected with HTTP 500 (unsupported message type).

Track server-initiated JSON-RPC request ids from the GET event stream and map them to the issuing upstream
Forward ClientJsonRpcMessage::Response / Error posts from the downstream client back to the correct upstream (streamable HTTP, SSE, and stdio transports)
For a single-upstream session, fall back to that upstream when no explicit mapping exists

Fixes #2187

Problem

In stateful Streamable HTTP mode, upstream servers can send JSON-RPC requests (for example ping) on the GET SSE stream. The downstream MCP client auto-replies with a JSON-RPC response on POST. Session::send_internal only handled client requests and notifications, so pongs hit the catch-all arm and failed.

Approach

Add per-session pending_server_requests (RequestId → upstream name), populated when forwarding ServerJsonRpcMessage::Request on the GET stream.
Handle ClientJsonRpcMessage::Response and Error in send_internal, routing via Relay::send_client_response.
Add Upstream::generic_client_message so all MCP transports can accept arbitrary downstream replies (streamable HTTP expects 202 Accepted).

Test plan

cargo test -p agentgateway track_outbound_server_requests_registers_server_initiated_request_id
cargo test -p agentgateway mcp::handler::tests
cargo build -p agentgateway

Local CI verification (macOS)

First contribution to this repo. Ran the Branch workflow checks locally while CI is waiting on maintainer workflow approval for fork PRs.

Host: macOS arm64 (darwin 25.4.0)
Toolchain: Rust 1.96.0, Go 1.26.4, Node v26.3.0, Docker 29.5.2, kind 0.32.0, Helm 3.21.2, bash 5.3.15
Branch checked: fix/mcp-streamable-http-client-response-routing @ de043d9
Worktree: /Users/piyush/ag-pr2207-local-1782641235 (no spaces in path; required for controller Makefile/scripts)

Workflow job	Local command	Result
DCO	signed-off commits	green on GitHub
Proxy Test (Mac)	`env -u DYNAMIC_CA_CERT_CACHE_CAPACITY RUST_TEST_THREADS=1 make test`	pass
proxy-lint	`make generate-schema check-clean-repo && make lint`	pass
ui-lint	`(cd ui && npm ci && npm run lint)`	pass
controller-test	`PATH=/opt/homebrew/opt/helm@3/bin:$PATH go test -race ./...`	pass
controller-lint	`make -C controller analyze`; `go mod tidy && git diff --exit-code go.mod go.sum`; `make generate-apis check-clean-repo && make -C controller verify`	pass
Proxy Test (Linux)	not run locally	deferred to CI (Linux runner + validation deps)
Proxy Test (Windows)	not run locally	deferred to CI (Windows runner)
controller-e2e	`TEST_MODE=e2e /opt/homebrew/bin/bash ./controller/test/setup/setup-kind-ci.sh` then `CGO_ENABLED=0 PERSIST_INSTALL=true go test -tags=e2e -v ./controller/test/e2e`	blocked locally: push to kind local registry `localhost:5000` timed out on macOS Docker
controller-conformance	`TEST_MODE=conformance /opt/homebrew/bin/bash ./controller/test/setup/setup-kind-ci.sh` then `make -C controller all-conformance`	blocked locally: incomplete kind install (`GatewayClass agentgateway` not found)

Notes

make test failed once with a pre-existing local env var DYNAMIC_CA_CERT_CACHE_CAPACITY=0; reran with it unset and RUST_TEST_THREADS=1.
Controller Makefile/scripts fail from the iCloud checkout path because of spaces in Mobile Documents; reran from a no-space worktree.
Helm chart golden tests require Helm 3 (helm@3); Helm 4 produced template whitespace diffs only.

piyushbag · 2026-07-11T11:48:19Z

Rebased onto current main. Resolved import conflict in handler.rs and updated JSON-RPC error id handling for the current Option type.

Local: cargo check -p agentgateway passes.

piyushbag · 2026-07-17T13:57:55Z

Rebased onto current main and fixed post-rebase compile/size issues:

Restored use crate::mcp so mcp::Error / mcp::session resolve again
Box::pin client response routing so send_internal stays under the assert_size limit

Local: cargo clippy -p agentgateway --lib and cargo test -p agentgateway --lib mcp:: (227 passed).

realmehmetali

I reviewed the current head (8a2f2bfd) and found two multiplexing cases that appear to need coverage before this routing is complete:

pending_server_requests is keyed only by the upstream-provided RequestId. JSON-RPC ids are scoped to a connection, so two multiplexed upstreams can both issue (for example) id 0. The later insert overwrites the first upstream; the first client reply is then routed to the wrong backend, and the second has no mapping. I think the downstream-facing request id needs to be namespaced/remapped, with both the upstream name and original id retained for the reverse path. A regression test with two upstreams issuing the same id would capture this.
Tracking is currently installed only in send_fanout_get. A Streamable HTTP server can also send a server-initiated request on an SSE response to a client POST. Those messages flow through send_single / send_fanout_to without track_outbound_server_requests. The single-upstream fallback masks that case, but a multiplexed POST-originated stream will fail with “no upstream registered”. Could the same tracking/remapping wrapper be applied to every outbound server-message stream, with a test for a POST-originated server request?

The existing GET-heartbeat path and focused unit test look directionally good; these cases seem important to prevent the fix from becoming connection-order-dependent under multiplexing.

piyushbag · 2026-07-18T15:09:21Z

Rebased onto current main and addressed both multiplexing cases from the review:

RequestId namespacing. Outbound server-initiated requests are remapped to agw:{upstream}:n:{id} / agw:{upstream}:s:{id} before the downstream client sees them. pending_server_requests now stores (upstream, original_id) under the remapped key, and send_client_response restores the original id before forwarding upstream. Colliding ids from two upstreams no longer overwrite each other.
POST-originated SSE. The same track_outbound_server_requests wrapper now runs on send_single and send_fanout_to, not only send_fanout_get. Pending state lives on Relay so every outbound server-message stream shares one session map.
Regression tests.
- track_outbound_server_requests_namespaces_colliding_ids_across_upstreams
- track_outbound_server_requests_covers_post_originated_sse_streams
- updated remapping registration test

Local verification: focused unit tests above plus the full mcp:: lib suite (237 passed); clippy on the lib crate with warnings denied.

stevenctl · 2026-07-19T18:03:51Z

 			Box::pin(us.generic_stream(r, &ctx).assert_size::<{ 3 * 1024 }>()).await?,
 			cel,
 		);
+		let stream = track_outbound_server_requests(


the calls to track_outbound_server_requests are before into_sse_stream, which for 2026-07-28+ downstreams reject server->client for multiplexed stuff because we can't route them; these entries are accumulated but the client will never answer them

we should probably skip tracking when ctx_downstream_modern -> true

Wrapped tracking in track_outbound_server_requests_for_downstream: when ctx_downstream_modern is true we pass the stream through without remapping ids or touching the pending map (send_single, send_fanout_get, send_fanout_to). Modern SSE already rejects direct server→client requests, so those entries were unreachable anyway.

Follow-up on head 879c6fe5: 1a0f0cf4 dropped the session pending_server_requests map entirely. Remapped ids are now stateless (agw:{upstream}:{n|s}:{original_id} parsed in parse_downstream_server_request_id), so modern downstreams still skip tracking via track_outbound_server_requests_for_downstream and there is no map growth path for unreachable entries.

stevenctl · 2026-07-19T18:06:35Z

+			pending_server_requests
+				.lock()
+				.expect("pending_server_requests lock poisoned")
+				.insert(


as pings occur, if they're unanswered this will grow indefinitely and live until the session ends

Added insert_pending_server_request with MAX_PENDING_SERVER_REQUESTS (256): before insert at capacity we remove one existing entry so unanswered pings cannot grow the map for the whole session. Test: insert_pending_server_request_evicts_when_at_capacity.

Superseded on 1a0f0cf4: the pending map and FIFO eviction are gone. Server-initiated ids are remapped on the outbound stream only; client replies recover upstream + original id from the encoded id string (or single-upstream passthrough in resolve_pending_server_request).

stevenctl · 2026-07-19T18:08:31Z

-			_ => Err(UpstreamError::InvalidRequest(
-				"unsupported message type".to_string(),
-			)),
+			ClientJsonRpcMessage::Response(r) => {


these branches could be deduped right?

Merged the Response and Error arms into a single match that calls send_client_response(message, ctx) with shared logging setup.

Still in place on 879c6fe5: session.rs uses one arm for ClientJsonRpcMessage::Response(_) | ClientJsonRpcMessage::Error(_) calling send_client_response.

stevenctl

The ID rewriting carries all the routing state, do we need the pending request map?

piyushbag · 2026-07-20T16:02:40Z

Addressed in 4556659:

Modern downstream skip: track_outbound_server_requests_for_downstream returns the stream unchanged when ctx_downstream_modern is true (send_single, send_fanout_get, send_fanout_to), so we do not remap ids or populate the pending map for clients that cannot reply on SSE anyway.
Pending map bound: insert_pending_server_request enforces MAX_PENDING_SERVER_REQUESTS (256) and evicts an existing entry before insert when at capacity. Unit tests cover modern skip and eviction.
session.rs dedup: Combined the Response and Error arms into one match that forwards via send_client_response(message, ctx).

stevenctl · 2026-07-21T15:10:12Z

+		.lock()
+		.expect("pending_server_requests lock poisoned");
+	while map.len() >= MAX_PENDING_SERVER_REQUESTS {
+		let Some(evict) = map.keys().next().cloned() else {


Is this FIFO? Seems a bit arbitrary.

Also, I really think we should drop the pending_server_requests map

agw:{upstream}:{n|s}:{original_id} contians everything we need to call resolve_pending_server_request; this would simplify things quite a bit.

Done on 1a0f0cf4 (879c6fe5 head). Dropped pending_server_requests; routing uses encoded ids (agw:{upstream}:{n|s}:{original_id}) parsed in parse_downstream_server_request_id + resolve_pending_server_request. Single-upstream sessions still passthrough raw ids when the encoded prefix is absent.

stevenctl · 2026-07-21T15:17:44Z

+	}
+
+	#[tokio::test]
+	async fn track_outbound_server_requests_covers_post_originated_sse_streams() {


this tests the tracking mechanism rather than the behavior we're trying to fix

something in mcp_tests.rs that looks more e2e that emits a ping and has the client POST {"jsonrpc":"2.0","id":"<remapped>","result":{}} and assert that the upstream saw the pong with the original ID would be the best proof of the fix

Acknowledged. I kept handler-level unit coverage (resolve_pending_server_request / id restore) rather than adding a new mcp_tests.rs e2e in this pass. Happy to add an integration ping/pong test in a follow-up if you want that as merge gate.

... I don't think this test is needed. It doesn't actually cover send_single and just runs the same wrapper that other tests already have coverage for.

Removed track_outbound_server_requests_covers_post_originated_sse_streams in 22a4238.

Added e2e coverage in mcp_tests.rs based on your tip branch (stevenctl/mcp-server-requests-tests):

multiplexed_pong_routes_to_pinging_upstream

elicitation_roundtrip_completes_tool_call

Both pass locally; see the top-level comment for the command output.

stevenctl · 2026-07-21T15:19:41Z

+			Upstream::McpStreamable(c) => {
+				let res = c.send_client_message(message, ctx).await?;
+				match res {
+					StreamableHttpPostResponse::Accepted => Ok(()),


should we be more forgiving here (allow 200, not just 202) rather than turn it into a hard error?

Addressed in 3cf6d9a9: streamable upstream client replies now accept StreamableHttpPostResponse::Json / Sse (200 + body) as well as 202 Accepted, so we no longer hard-error on spec-adjacent 200 responses.

stevenctl · 2026-07-21T15:22:46Z

+	) -> Result<Response, UpstreamError> {
+		let request_id = match &message {
+			ClientJsonRpcMessage::Response(r) => r.id.clone(),
+			ClientJsonRpcMessage::Error(e) => e.id.clone().ok_or_else(|| {


https://www.jsonrpc.org/specification#error_object

errors don't have to have IDs

but we can't route with the ID.. should we just warn and return 202 here? for single upstream we can easily forward as-is with out an ID

Addressed in 3cf6d9a9 on 879c6fe5:

Single upstream: id-less client JSON-RPC errors are forwarded as-is via generic_client_message, then we return 202 (accepted_response()).

Multiplexed: we warn and still return 202 (cannot route without id); no hard error.

Id-present responses/errors still go through resolve_pending_server_request + id restore.

piyushbag · 2026-07-21T17:36:33Z

Addressed in 2441a3c4:

200 vs 202 on client replies: generic_client_message for streamable upstream now treats Accepted, Json, and Sse post responses as success (upstream 200 with JSON no longer hard-errors).
Errors without id: per JSON-RPC, notification-style errors are allowed without an id. Single-upstream sessions forward the message as-is; multiplexed sessions log a warning and return 202 because we cannot route without an id.

Local: handler unit tests green on this head.

Still open from earlier review: stateless pending map vs encoded id (happy to refactor in a follow-up if you prefer that direction) and an e2e ping/pong test in mcp_tests.rs.

piyushbag · 2026-07-23T19:00:28Z

Rebased onto latest main (95318c97). Local cargo test -p agentgateway pending_server passes.

Review follow-ups already on head:

HTTP 200 client replies (upstream/mod.rs): StreamableHttpPostResponse::Json and Sse are accepted alongside Accepted when forwarding client JSON-RPC responses (263c205).

Id-less JSON-RPC errors (handler.rs): single-upstream sessions forward as-is and return 202; multiplexed sessions log a warning and return 202 because there is no id to route (263c205).

pending_server_requests map: kept because the downstream-facing id is the remapped agw:{upstream}:{n|s}:{original_id} string, but resolve_pending_server_request must recover (upstream, original_id) on the return path. The map is that reverse index. Tracking is skipped entirely for modern (2026-07-28+) downstreams via track_outbound_server_requests_for_downstream. Unanswered legacy pings are bounded by FIFO eviction at MAX_PENDING_SERVER_REQUESTS.

E2E ping→client POST→upstream pong proof in mcp_tests.rs is still on my list; the handler unit test covers tracking/eviction today.

Signed-off-by: Piyush Jagadish Bag [email protected]

stevenctl · 2026-07-23T19:48:40Z

map: kept because the downstream-facing id is the remapped agw:{upstream}:{n|s}:{original_id} string, but resolve_pending_server_request must recover (upstream, original_id)

we don't need a stateful map to extract info that we could parse out of that remapped id

piyushbag · 2026-07-24T17:58:24Z

Addressed the stateful-map feedback in 1a0f0cf4:

Stateless reverse routing: dropped pending_server_requests. Outbound ids are still remapped to agw:{upstream}:{n|s}:{original_id}; resolve_pending_server_request now parses upstream + original id from that string on the client POST reply path. Single-upstream sessions still accept non-remapped ids as before.

Already on head from prior commits:

HTTP 200 client replies: StreamableHttpPostResponse::Json / Sse accepted alongside Accepted in generic_client_message.
Id-less JSON-RPC errors: single-upstream forwards as-is (202); multiplexed logs a warning and returns 202.

Tests: updated unit coverage for remapping + parse roundtrip (including string ids with colons). Full mcp_tests.rs ping→POST→upstream pong e2e is still on my list; the mock streamable servers in that file do not emit server-initiated pings on GET today, so I kept the proof at handler unit + integration level for this push.

Signed-off-by: Piyush Jagadish Bag [email protected]

realmehmetali

I rechecked the current head after the multiplexing follow-ups. The original collision and POST-stream coverage concerns are addressed; I found one remaining ambiguity in the new stateless encoding.

realmehmetali · 2026-07-28T00:59:48Z

+		return None;
+	};
+	let rest = encoded.as_ref().strip_prefix("agw:")?;
+	if let Some(idx) = rest.rfind(":n:") {


The delimiter encoding is not round-trippable for arbitrary JSON-RPC string IDs. For upstream backend and original string ID part:n:7, the generated ID is agw:backend:s:part:n:7; because this parser checks the last :n: first, it decodes that as upstream backend:s:part and numeric ID 7. An ID containing :s: similarly shifts part of the original ID into the upstream name, so the reply can be routed to a nonexistent or wrong backend. Could this use an unambiguous representation (for example, length-prefixed or encoded components) and add round-trip cases containing both marker strings?

Fixed on c647e33: string original ids are URL-safe base64-encoded in the remapped id (agw:{upstream}:s:{b64}), parsed with split_once(":s:") + decode. Added unit test for part:n:7. Legacy plain-string payloads still decode for in-flight ids from earlier heads.

Signed-off-by: Piyush Jagadish Bag [email protected]

piyushbag · 2026-07-30T16:47:00Z

Addressed the round-trip encoding concern on c647e33:

String ids: remapped ids now URL-safe base64-encode the original string payload after the first :s: delimiter (agw:{upstream}:s:{b64}). Parsing uses split_once(":s:") plus base64 decode, so ids like part:n:7 no longer decode as numeric 7.

Numeric ids: unchanged (agw:{upstream}:n:{n}), still parsed via rsplit_once(":n:").

Legacy: plain-string payloads after :s: still decode for any in-flight remapped ids from earlier heads.

Unit tests cover both part:a:part and part:n:7. Signed-off-by on commit.

Prior stevenctl threads (stateless map, HTTP 200 replies, id-less errors, modern downstream skip) remain on head; e2e ping/pong in mcp_tests.rs still deferred.

stevenctl · 2026-08-11T18:47:50Z

+/// Namespace a server-initiated request id for the downstream connection.
+///
+/// Upstream JSON-RPC ids are scoped per connection. Multiplexed backends can
+/// both emit id `0`; remapping keeps downstream ids collision-free and encodes
+/// the upstream name plus original id for the reverse path.
+fn downstream_server_request_id(upstream: &str, original: &RequestId) -> RequestId {
+	match original {
+		RequestId::Number(n) => RequestId::String(format!("agw:{upstream}:n:{n}").into()),
+		RequestId::String(s) => {
+			let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(s.as_bytes());
+			RequestId::String(format!("agw:{upstream}:s:{encoded}").into())
+		},
+	}
+}
+
+fn parse_downstream_server_request_id(id: &RequestId) -> Option<PendingServerRequest> {
+	let RequestId::String(encoded) = id else {
+		return None;
+	};
+	let rest = encoded.as_ref().strip_prefix("agw:")?;
+	if let Some((upstream, n_str)) = rest.rsplit_once(":n:") {
+		let n: i64 = n_str.parse().ok()?;
+		return Some(PendingServerRequest {
+			upstream: upstream.into(),
+			original_id: RequestId::Number(n),
+		});
+	}
+	if let Some((upstream, payload)) = rest.split_once(":s:") {
+		let original = decode_downstream_string_request_id(payload)?;
+		return Some(PendingServerRequest {
+			upstream: upstream.into(),
+			original_id: RequestId::String(original.into()),
+		});
+	}
+	None
+}
+
+fn decode_downstream_string_request_id(payload: &str) -> Option<String> {
+	if let Ok(decoded) = base64::engine::general_purpose::URL_SAFE_NO_PAD.decode(payload) {
+		return String::from_utf8(decoded).ok();
+	}
+	// Legacy remapped ids stored the raw string after the first `:s:` delimiter.
+	Some(payload.to_string())
+}


I don't think we need base64 in the encoding.. wouldn't the following be unambiguous, left to right?

agw_{upstream}_{n|s}_{original}

agw_ ensures its our ID to rewrite

we already use _ for tool name multiplexing, upstream can't contain it

cut off n/s for type ambiguity

the tail at this point is the original ID, we don't care if it contains our delimiter

Suggested change

/// Namespace a server-initiated request id for the downstream connection.

///

/// Upstream JSON-RPC ids are scoped per connection. Multiplexed backends can

/// both emit id `0`; remapping keeps downstream ids collision-free and encodes

/// the upstream name plus original id for the reverse path.

fn downstream_server_request_id(upstream: &str, original: &RequestId) -> RequestId {

match original {

RequestId::Number(n) => RequestId::String(format!("agw:{upstream}:n:{n}").into()),

RequestId::String(s) => {

let encoded = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(s.as_bytes());

RequestId::String(format!("agw:{upstream}:s:{encoded}").into())

},

}

}

fn parse_downstream_server_request_id(id: &RequestId) -> Option<PendingServerRequest> {

let RequestId::String(encoded) = id else {

return None;

};

let rest = encoded.as_ref().strip_prefix("agw:")?;

if let Some((upstream, n_str)) = rest.rsplit_once(":n:") {

let n: i64 = n_str.parse().ok()?;

return Some(PendingServerRequest {

upstream: upstream.into(),

original_id: RequestId::Number(n),

});

}

if let Some((upstream, payload)) = rest.split_once(":s:") {

let original = decode_downstream_string_request_id(payload)?;

return Some(PendingServerRequest {

upstream: upstream.into(),

original_id: RequestId::String(original.into()),

});

}

None

}

fn decode_downstream_string_request_id(payload: &str) -> Option<String> {

if let Ok(decoded) = base64::engine::general_purpose::URL_SAFE_NO_PAD.decode(payload) {

return String::from_utf8(decoded).ok();

}

// Legacy remapped ids stored the raw string after the first `:s:` delimiter.

Some(payload.to_string())

}

/// Namespace a server-initiated request id for the downstream connection.

///

/// Upstream JSON-RPC ids are scoped per connection. Multiplexed backends can

/// both emit id `0`; remapping keeps downstream ids collision-free and encodes

/// the upstream name plus original id for the reverse path.

///

/// Layout is `agw_{upstream}_{n|s}_{original}`:

/// * `agw_` helps us ensure we don't rewrite IDs that we didn't generate.

/// * `{upstream}` is the name of the upstream connection that generated the request.

/// * `{n|s}` is a tag for the original id type: `n`

/// * `{original}` is the original id, either a number or string.

fn downstream_server_request_id(upstream: &str, original: &RequestId) -> RequestId {

match original {

RequestId::Number(n) => {

RequestId::String(format!("agw{DELIMITER}{upstream}{DELIMITER}n{DELIMITER}{n}").into())

},

RequestId::String(s) => {

RequestId::String(format!("agw{DELIMITER}{upstream}{DELIMITER}s{DELIMITER}{s}").into())

},

}

}

fn parse_downstream_server_request_id(id: &RequestId) -> Option<ServerRequestRoute> {

let RequestId::String(encoded) = id else {

return None;

};

let rest = encoded

.as_ref()

.strip_prefix("agw")?

.strip_prefix(DELIMITER)?;

let (upstream, rest) = rest.split_once(DELIMITER)?;

let (tag, payload) = rest.split_once(DELIMITER)?;

let original_id = match tag {

"n" => RequestId::Number(payload.parse().ok()?),

"s" => RequestId::String(payload.into()),

_ => return None,

};

Some(ServerRequestRoute {

upstream: upstream.into(),

original_id,

})

}

i'm not fully convinced we need the agw_ prefix either.. we only go through this encode/decode when we're multiplexing.. clients wouldn't really send us ClientResponse that have IDs we didn't encode first.. even if they did they'd have to craft them in a specific way that matches one of the targets being mmultiplexed and have that _n_/_s_ in the middle..

Done in 22a4238. Switched remapped ids to agw_{upstream}_{n|s}_{original} with _ delimiters, removed base64 encode/decode, and kept round-trip tests for colons, :n:, and underscores in original ids.

stevenctl · 2026-08-11T18:50:57Z

+			ClientJsonRpcMessage::Response(_) | ClientJsonRpcMessage::Error(_) => {
+				let ctx = IncomingRequestContext::new(&parts);
+				let (_span, log, _cel) = mcp::handler::setup_request_log(parts, "response");
+				let session_id = self.id.to_string();


Consistent with other sites that do this

Suggested change

let session_id = self.id.to_string();

let session_id = (!self.synthetic).then(|| self.id.to_string());

Done in 22a4238. Response handler now uses (!self.synthetic).then(|| self.id.to_string()) for session_id logging.

stevenctl · 2026-08-11T18:58:43Z

+	pub async fn send_client_response(
+		&self,
+		message: ClientJsonRpcMessage,
+		ctx: IncomingRequestContext,
+	) -> Result<Response, UpstreamError> {
+		match &message {
+			ClientJsonRpcMessage::Error(e) if e.id.is_none() => {
+				if self.upstreams.size() == 1 {
+					let name = self
+						.upstreams
+						.iter_named()
+						.next()
+						.map(|(name, _)| name)
+						.ok_or_else(|| UpstreamError::InvalidRequest("no upstreams available".into()))?;
+					let us = self.upstreams.get(name.as_str())?;
+					us.generic_client_message(message, &ctx).await?;
+					return Ok(accepted_response());
+				}
+				warn!(
+					"dropping client JSON-RPC error without id in multiplexed session; cannot route upstream"
+				);
+				return Ok(accepted_response());
+			},
+			ClientJsonRpcMessage::Response(_) | ClientJsonRpcMessage::Error(_) => {},
+			_ => {
+				return Err(UpstreamError::InvalidRequest(
+					"expected client JSON-RPC response".into(),
+				));
+			},
+		}
+
+		let request_id = match &message {
+			ClientJsonRpcMessage::Response(r) => r.id.clone(),
+			ClientJsonRpcMessage::Error(e) => e.id.clone().expect("id checked above"),
+			_ => unreachable!(),
+		};
+
+		let pending = resolve_pending_server_request(self, &request_id)?;
+		let message = restore_client_response_id(message, pending.original_id)?;
+		let us = self.upstreams.get(pending.upstream.as_str())?;
+		us.generic_client_message(message, &ctx).await?;
+		Ok(accepted_response())
+	}


Avoid the repeated match, and the unreachable!() by extracing the request_id once and matching on it

also, you can use upstreams.defualt_target_name rather than re-deriving the sole upstream name here

this also makes sure we work with prefixMode: Always

Suggested change

pub async fn send_client_response(

&self,

message: ClientJsonRpcMessage,

ctx: IncomingRequestContext,

) -> Result<Response, UpstreamError> {

match &message {

ClientJsonRpcMessage::Error(e) if e.id.is_none() => {

if self.upstreams.size() == 1 {

let name = self

.upstreams

.iter_named()

.next()

.map(|(name, _)| name)

.ok_or_else(|| UpstreamError::InvalidRequest("no upstreams available".into()))?;

let us = self.upstreams.get(name.as_str())?;

us.generic_client_message(message, &ctx).await?;

return Ok(accepted_response());

}

warn!(

"dropping client JSON-RPC error without id in multiplexed session; cannot route upstream"

);

return Ok(accepted_response());

},

ClientJsonRpcMessage::Response(_) | ClientJsonRpcMessage::Error(_) => {},

_ => {

return Err(UpstreamError::InvalidRequest(

"expected client JSON-RPC response".into(),

));

},

}

let request_id = match &message {

ClientJsonRpcMessage::Response(r) => r.id.clone(),

ClientJsonRpcMessage::Error(e) => e.id.clone().expect("id checked above"),

_ => unreachable!(),

};

let pending = resolve_pending_server_request(self, &request_id)?;

let message = restore_client_response_id(message, pending.original_id)?;

let us = self.upstreams.get(pending.upstream.as_str())?;

us.generic_client_message(message, &ctx).await?;

Ok(accepted_response())

}

pub async fn send_client_response(

&self,

message: ClientJsonRpcMessage,

ctx: IncomingRequestContext,

) -> Result<Response, UpstreamError> {

let request_id = match &message {

ClientJsonRpcMessage::Response(r) => Some(r.id.clone()),

ClientJsonRpcMessage::Error(e) => e.id.clone(),

_ => {

return Err(UpstreamError::InvalidRequest(

"expected client JSON-RPC response".into(),

));

},

};

let (upstream, message) = match request_id {

Some(id) => {

let route = resolve_server_request_route(self, &id)?;

(

route.upstream,

restore_client_response_id(message, route.original_id),

)

},

// Errors may omit an id; without one we can only route to a default target.

None => match &self.upstreams.default_target_name {

Some(name) => (name.as_str().into(), message),

None => {

warn!("dropping id-less client JSON-RPC error; cannot route in multiplexed session");

return Ok(accepted_response());

},

},

};

let us = self.upstreams.get(upstream.as_str())?;

us.generic_client_message(message, &ctx).await?;

Ok(accepted_response())

}

Done in 22a4238. send_client_response now extracts request_id once, routes id-less errors through default_target_name, and forwards via resolve_server_request_route + restore_client_response_id.

stevenctl · 2026-08-11T19:00:33Z

+			e.id = Some(original_id);
+			Ok(ClientJsonRpcMessage::Error(e))
+		},
+		_ => Err(UpstreamError::InvalidRequest(


this should be unreachable; should this just not return Result; avoids an expect on the caller side..

Done in 22a4238. restore_client_response_id now returns ClientJsonRpcMessage directly; the invalid-message arm is unreachable!() after validation in the caller.

stevenctl · 2026-08-11T19:02:18Z

+	if relay.upstreams.size() == 1 {
+		let name = relay
+			.upstreams
+			.iter_named()
+			.next()
+			.map(|(name, _)| name)
+			.ok_or_else(|| UpstreamError::InvalidRequest("no upstreams available".into()))?;
+		return Ok(PendingServerRequest {
+			upstream: name,
+			original_id: request_id.clone(),
+		});
+	}


Suggested change

if relay.upstreams.size() == 1 {

let name = relay

.upstreams

.iter_named()

.next()

.map(|(name, _)| name)

.ok_or_else(|| UpstreamError::InvalidRequest("no upstreams available".into()))?;

return Ok(PendingServerRequest {

upstream: name,

original_id: request_id.clone(),

});

}

if let Some(name) = &relay.upstreams.default_target_name {

return Ok(ServerRequestRoute {

upstream: name.as_str().into(),

original_id: request_id.clone(),

});

}

Done in 22a4238. Unmapped ids now fall back to relay.upstreams.default_target_name instead of re-deriving the sole upstream from size() == 1.

stevenctl · 2026-08-11T19:03:57Z

+	})
+}
+
+fn resolve_pending_server_request(


Is "pending" the right term here? Should it just be resolve_server_request_route instead..

also, do we need the PendingServerRequest struct? or should we just make it an inline tuple

Done in 22a4238. Renamed to resolve_server_request_route and replaced PendingServerRequest with ServerRequestRoute.

stevenctl · 2026-08-11T19:06:28Z

+			Upstream::McpStreamable(c) => {
+				let res = c.send_client_message(message, ctx).await?;
+				match res {
+					StreamableHttpPostResponse::Accepted
+					| StreamableHttpPostResponse::Json(_, _)
+					| StreamableHttpPostResponse::Sse(_, _) => Ok(()),
+				}
+			},


nit: I don't think the match res does much here?

Suggested change

Upstream::McpStreamable(c) => {

let res = c.send_client_message(message, ctx).await?;

match res {

StreamableHttpPostResponse::Accepted

| StreamableHttpPostResponse::Json(_, _)

| StreamableHttpPostResponse::Sse(_, _) => Ok(()),

}

},

Upstream::McpStreamable(c) => {

c.send_client_message(message, ctx).await?;

Ok(())

},

Done in 22a4238. Streamable arm is now c.send_client_message(message, ctx).await?; Ok(()).

stevenctl · 2026-08-11T19:09:32Z

+	}
+
+	#[tokio::test]
+	async fn track_outbound_server_requests_covers_post_originated_sse_streams() {


... I don't think this test is needed. It doesn't actually cover send_single and just runs the same wrapper that other tests already have coverage for.

stevenctl · 2026-08-11T19:10:12Z

 	}
+
+	#[tokio::test]
+	async fn track_outbound_server_requests_registers_remapped_server_initiated_request_id() {


this tests a pretty narrow part that should already be caught in other higher level tests

Removed track_outbound_server_requests_registers_remapped_server_initiated_request_id in 22a4238.

Streamable HTTP sessions broke when downstream clients replied to upstream heartbeat pings on the GET SSE stream. Track server-initiated request ids, namespace them per upstream so multiplexed backends cannot collide, forward Response/Error messages upstream with the original id restored, and accept 202 on streamable HTTP. Apply the same tracking to POST-originated SSE streams (send_single / send_fanout_to), not only send_fanout_get. Fixes agentgateway#2187 Signed-off-by: Piyush Jagadish Bag <[email protected]>

Signed-off-by: Piyush Jagadish Bag <[email protected]>

Skip pending-map tracking when the downstream client uses a modern protocol version, since those streams cannot answer server-initiated requests anyway. Cap the pending map size so unanswered heartbeat pings cannot grow without bound. Deduplicate client JSON-RPC response forwarding in session handling. Signed-off-by: Piyush Jagadish Bag <[email protected]>

Allow streamable upstream 200/JSON replies when forwarding client responses. For JSON-RPC errors without an id, forward on single-upstream sessions and warn+accept on multiplexed sessions per review. Signed-off-by: Piyush Jagadish Bag <[email protected]>

Proxy CI treats warnings as errors; the catch-all match arm is unreachable once Accepted, Json, and Sse are handled. Signed-off-by: Piyush Jagadish Bag <[email protected]>

Drop the pending_server_requests map and recover upstream plus original JSON-RPC id from agw:{upstream}:{n|s}:{id} on the client reply path. Signed-off-by: Piyush Jagadish Bag <[email protected]>

Signed-off-by: Piyush Jagadish Bag <[email protected]>

String JSON-RPC ids can contain `:n:` or `:s:` substrings (e.g. `part:n:7`). Remapped ids now URL-safe base64-encode the original string payload after the first `:s:` delimiter so parse is round-trippable. Numeric ids unchanged. Legacy plain-string payloads still decode for in-flight remapped ids. Signed-off-by: Piyush Jagadish Bag <[email protected]>

piyushbag · 2026-08-11T19:27:24Z

Review feedback (Aug 11)

Rebased onto upstream/main and pushed review fixes in 22a4238e.

Changes

Remapped server-initiated request ids use agw_{upstream}_{n|s}_{original} (_ delimiters, no base64).
send_client_response extracts request_id once; id-less errors route via default_target_name.
Renamed resolve_pending_server_request → resolve_server_request_route; PendingServerRequest → ServerRequestRoute.
restore_client_response_id returns ClientJsonRpcMessage (no Result).
Session response logging uses (!self.synthetic).then(|| self.id.to_string()).
Simplified streamable generic_client_message arm.
Removed two redundant unit tests flagged in review; kept collision, modern-downstream skip, and round-trip parse tests.

Verification

cargo check -p agentgateway   # PASS
cargo test -p agentgateway --lib mcp::   # blocked locally by disk space during test harness link; please re-run in CI

Replace base64 remapped id encoding with agw_{upstream}_{n|s}_{original} layout. Refactor send_client_response to extract request_id once, route id-less errors via default_target_name, and rename resolve helper to resolve_server_request_route. Drop redundant unit tests flagged in review. Signed-off-by: Piyush Jagadish Bag <[email protected]>

piyushbag · 2026-08-11T19:30:29Z

Re-pushed tip commit with Signed-off-by so DCO is satisfied (2e0358df). Same tree as the review-feedback patch; only the trailer was missing.

stevenctl · 2026-08-11T20:21:26Z

This still needs some more e2e-ish tests, this branch has a rough example https://github.com/stevenctl/agentgateway/tree/stevenctl/mcp-server-requests-tests

Cover server-initiated request routing under multiplexed streamable HTTP: ping/pong must reach the pinging upstream with the original id, and a mid-tools/call elicitation roundtrip must complete. Signed-off-by: Piyush Jagadish Bag <[email protected]>

piyushbag · 2026-08-11T21:30:07Z

Thanks @stevenctl — added the e2e-style coverage from your tip branch (stevenctl/mcp-server-requests-tests):

multiplexed_pong_routes_to_pinging_upstream — multiplexed streamable HTTP; pong routes to the pinging upstream with original id 7
elicitation_roundtrip_completes_tool_call — mid-tools/call elicitation completes via client reply routing (new elicit mock tool)

Local evidence:

$ cargo test -p agentgateway --lib multiplexed_pong_routes_to_pinging_upstream -- --nocapture
...
test mcp::tests::multiplexed_pong_routes_to_pinging_upstream ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 1707 filtered out; finished in 0.06s

$ cargo test -p agentgateway --lib elicitation_roundtrip_completes_tool_call -- --nocapture
...
test mcp::tests::elicitation_roundtrip_completes_tool_call ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 1707 filtered out; finished in 0.03s

piyushbag · 2026-08-11T21:37:05Z

Follow-up: these are covered by the project's CI gate (make test in Proxy Test), not only a laptop run. Docker in this repo builds the release image; the MCP routing suite runs in CI Proxy Test (real listener + upstream mocks).

CI run (all green): https://github.com/agentgateway/agentgateway/actions/runs/31538166273

From Proxy Test (Linux) logs on cdeaf3db:

test mcp::tests::elicitation_roundtrip_completes_tool_call ... ok
test mcp::tests::multiplexed_pong_routes_to_pinging_upstream ... ok
...
test result: ok. 1707 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 6.82s

Also green on Proxy Test (Mac) and Proxy Test (Windows) for the same commit.

stevenctl · 2026-08-11T22:15:24Z

+			e.id = Some(original_id);
+			ClientJsonRpcMessage::Error(e)
+		},
+		_ => unreachable!("send_client_response validates message type before calling"),


this should passthrough instead of panicking

Changed to passthrough (other => other) in 89446d69 so unexpected variants are left unchanged instead of panicking. Handler unit tests and the two e2e cases still pass.

Avoid panicking on unexpected ClientJsonRpcMessage variants; leave the message unchanged so callers stay defensive. Signed-off-by: Piyush Jagadish Bag <[email protected]>

…ts (agentgateway#2207) ## Summary Fixes Streamable HTTP MCP sessions where downstream heartbeat pongs were rejected with HTTP 500 (`unsupported message type`). - Track server-initiated JSON-RPC request ids from the GET event stream and map them to the issuing upstream - Forward `ClientJsonRpcMessage::Response` / `Error` posts from the downstream client back to the correct upstream (streamable HTTP, SSE, and stdio transports) - For a single-upstream session, fall back to that upstream when no explicit mapping exists Fixes agentgateway#2187 ## Problem In stateful Streamable HTTP mode, upstream servers can send JSON-RPC requests (for example `ping`) on the GET SSE stream. The downstream MCP client auto-replies with a JSON-RPC response on POST. `Session::send_internal` only handled client requests and notifications, so pongs hit the catch-all arm and failed. ## Approach 1. Add per-session `pending_server_requests` (`RequestId` → upstream name), populated when forwarding `ServerJsonRpcMessage::Request` on the GET stream. 2. Handle `ClientJsonRpcMessage::Response` and `Error` in `send_internal`, routing via `Relay::send_client_response`. 3. Add `Upstream::generic_client_message` so all MCP transports can accept arbitrary downstream replies (streamable HTTP expects `202 Accepted`). ## Test plan - [x] `cargo test -p agentgateway track_outbound_server_requests_registers_server_initiated_request_id` - [x] `cargo test -p agentgateway mcp::handler::tests` - [x] `cargo build -p agentgateway` ## Local CI verification (macOS) First contribution to this repo. Ran the Branch workflow checks locally while CI is waiting on maintainer workflow approval for fork PRs. **Host:** macOS arm64 (darwin 25.4.0) **Toolchain:** Rust 1.96.0, Go 1.26.4, Node v26.3.0, Docker 29.5.2, kind 0.32.0, Helm 3.21.2, bash 5.3.15 **Branch checked:** `fix/mcp-streamable-http-client-response-routing` @ `de043d9` **Worktree:** `/Users/piyush/ag-pr2207-local-1782641235` (no spaces in path; required for controller Makefile/scripts) | Workflow job | Local command | Result | |---|---|---| | DCO | signed-off commits | green on GitHub | | Proxy Test (Mac) | `env -u DYNAMIC_CA_CERT_CACHE_CAPACITY RUST_TEST_THREADS=1 make test` | pass | | proxy-lint | `make generate-schema check-clean-repo && make lint` | pass | | ui-lint | `(cd ui && npm ci && npm run lint)` | pass | | controller-test | `PATH=/opt/homebrew/opt/helm@3/bin:$PATH go test -race ./...` | pass | | controller-lint | `make -C controller analyze`; `go mod tidy && git diff --exit-code go.mod go.sum`; `make generate-apis check-clean-repo && make -C controller verify` | pass | | Proxy Test (Linux) | not run locally | deferred to CI (Linux runner + validation deps) | | Proxy Test (Windows) | not run locally | deferred to CI (Windows runner) | | controller-e2e | `TEST_MODE=e2e /opt/homebrew/bin/bash ./controller/test/setup/setup-kind-ci.sh` then `CGO_ENABLED=0 PERSIST_INSTALL=true go test -tags=e2e -v ./controller/test/e2e` | blocked locally: push to kind local registry `localhost:5000` timed out on macOS Docker | | controller-conformance | `TEST_MODE=conformance /opt/homebrew/bin/bash ./controller/test/setup/setup-kind-ci.sh` then `make -C controller all-conformance` | blocked locally: incomplete kind install (`GatewayClass agentgateway` not found) | **Notes** - `make test` failed once with a pre-existing local env var `DYNAMIC_CA_CERT_CACHE_CAPACITY=0`; reran with it unset and `RUST_TEST_THREADS=1`. - Controller Makefile/scripts fail from the iCloud checkout path because of spaces in `Mobile Documents`; reran from a no-space worktree. - Helm chart golden tests require Helm 3 (`helm@3`); Helm 4 produced template whitespace diffs only. --------- Signed-off-by: Piyush Jagadish Bag <[email protected]> Signed-off-by: rayze <[email protected]>

piyushbag requested a review from a team as a code owner June 16, 2026 15:42

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch from 40dab0b to de043d9 Compare June 16, 2026 15:47

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch from de043d9 to 8db78b8 Compare July 11, 2026 11:48

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch from 8db78b8 to 8a2f2bf Compare July 17, 2026 13:56

howardjohn assigned stevenctl Jul 17, 2026

realmehmetali reviewed Jul 18, 2026

View reviewed changes

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch from 8a2f2bf to 79c3212 Compare July 18, 2026 15:09

stevenctl reviewed Jul 19, 2026

View reviewed changes

stevenctl reviewed Jul 21, 2026

View reviewed changes

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch 2 times, most recently from 5d666ad to 09b7cf9 Compare July 22, 2026 14:37

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch 2 times, most recently from 0b6da7f to e717ce3 Compare July 23, 2026 19:18

realmehmetali reviewed Jul 28, 2026

View reviewed changes

howardjohn mentioned this pull request Aug 4, 2026

Bug: MCP Gateway rejects client responses to server-initiated requests (elicitation/sampling) over Streamable HTTP - 500 unsupported message type #2838

Closed

stevenctl reviewed Aug 11, 2026

View reviewed changes

piyushbag added 8 commits August 11, 2026 12:22

style(mcp): rustfmt handler for proxy-lint

e666f72

Signed-off-by: Piyush Jagadish Bag <[email protected]>

fix(mcp): remove unreachable StreamableHttpPostResponse arm

9f160d6

Proxy CI treats warnings as errors; the catch-all match arm is unreachable once Accepted, Json, and Sse are handled. Signed-off-by: Piyush Jagadish Bag <[email protected]>

fix(mcp): parse remapped server-request ids statelessly

a896550

Drop the pending_server_requests map and recover upstream plus original JSON-RPC id from agw:{upstream}:{n|s}:{id} on the client reply path. Signed-off-by: Piyush Jagadish Bag <[email protected]>

style(mcp): apply cargo fmt to handler tests

ceb4525

Signed-off-by: Piyush Jagadish Bag <[email protected]>

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch from c647e33 to 22a4238 Compare August 11, 2026 19:27

piyushbag force-pushed the fix/mcp-streamable-http-client-response-routing branch from 22a4238 to 2e0358d Compare August 11, 2026 19:30

stevenctl reviewed Aug 11, 2026

View reviewed changes

fix(mcp): passthrough unknown messages in restore_client_response_id

89446d6

Avoid panicking on unexpected ClientJsonRpcMessage variants; leave the message unchanged so callers stay defensive. Signed-off-by: Piyush Jagadish Bag <[email protected]>

stevenctl approved these changes Aug 12, 2026

View reviewed changes

stevenctl merged commit acbd7f5 into agentgateway:main Aug 12, 2026
11 checks passed

	let session_id = self.id.to_string();
	let session_id = (!self.synthetic).then(\|\| self.id.to_string());

Conversation

piyushbag commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Approach

Test plan

Local CI verification (macOS)

Uh oh!

piyushbag commented Jul 11, 2026

Uh oh!

piyushbag commented Jul 17, 2026

Uh oh!

realmehmetali left a comment

Choose a reason for hiding this comment

Uh oh!

piyushbag commented Jul 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenctl left a comment

Choose a reason for hiding this comment

Uh oh!

piyushbag commented Jul 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyushbag commented Jul 21, 2026

Uh oh!

piyushbag commented Jul 23, 2026

Uh oh!

stevenctl commented Jul 23, 2026

Uh oh!

piyushbag commented Jul 24, 2026

Uh oh!

realmehmetali left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piyushbag commented Jun 16, 2026 •

edited

Loading