Skip to content

Commit 461be44

Browse files
jfallowsclaude
jfallows
and
claude
authored
fix(binding-mcp): flush hydration registrants queued before a real preauthorize challenge (#2447)
McpLifecycleClient.onClientChallenge called settleLifecycle(traceId) but never settleRequests(traceId), unlike onClientBegin/onClientEnd/onClientAbort/ onClientReset, which all flush it. A hydration-mode McpListClient registered via register() while this connect is still establishing sits in requests until flushed -- and if the connect's very first event is a real preauthorize elicitation challenge (e.g. a toolkit whose south server requires interactive browser-driven OAuth, which hydration can never complete), nothing ever flushes it. That kind's pending count never reaches zero, cache.populated never becomes true, and every real caller's own initialize blocks on it indefinitely, project-wide -- not just for the affected toolkit. Also narrow `challenged` to only latch on a real preauthorize elicitation (McpChallengeExFW.KIND_ELICIT_CREATE), not any challenge kind. KIND_RESUME and KIND_SUSPENDED are the south client's own SSE transport resuming or suspending its long-poll stream, an ordinary occurrence on any healthy connection unrelated to authorization -- treating them as an elicitation would latch a healthy route into interactiveAuthRoutes the moment its SSE stream first happens to suspend, causing hydration to give up retrying a route that was never actually stuck on a human. Adds McpProxyCacheIT#shouldHydrateToolkitMultiSettlingChallengedRoute, mirroring shouldHydrateToolkitMultiSettlingResetRoute's two-toolkit shape: one route's south connect issues a real elicitCreate challenge and never resolves, the other resolves normally, and the client still receives the second route's tool instead of the whole request hanging. Verified the new test fails with TestTimedOut when settleRequests(traceId) is reverted out of onClientChallenge, confirming it actually exercises the fix. Full binding-mcp module suite (178 unit tests, 306 K3PO ITs including the new scenario) passes; license and checkstyle checks are clean. Co-authored-by: Claude <noreply@anthropic.com>
1 parent d3bfe5a commit 461be44

4 files changed

Lines changed: 181 additions & 1 deletion

File tree

  • runtime/binding-mcp/src
  • specs/binding-mcp.spec/src/main/scripts/io/aklivity/zilla/specs/binding/mcp/streams/application/cache.hydrate.toolkit.multi.challenge.route

‎runtime/binding-mcp/src/main/java/io/aklivity/zilla/runtime/binding/mcp/internal/stream/McpProxyLifecycleFactory.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1205,8 +1205,31 @@ private void onClientChallenge(
12051205
final long authorization = challenge.authorization();
12061206
final OctetsFW extension = challenge.extension();
12071207

1208-
challenged = true;
1208+
// only a real preauthorize elicitation means this route needs a human to answer --
1209+
// KIND_RESUME and KIND_SUSPENDED are the south client's own SSE transport resuming
1210+
// or suspending its long-poll stream, an ordinary occurrence on any healthy
1211+
// connection and unrelated to authorization, so must not be mistaken for one:
1212+
// doing so latches this route into interactiveAuthRoutes (via settleLifecycle
1213+
// below) the moment its stream first happens to suspend, well before any real
1214+
// elicitation, and hydration then gives up retrying a route that was never
1215+
// actually stuck on a human
1216+
final McpChallengeExFW challengeEx = extension.sizeof() > 0
1217+
? mcpChallengeExRO.tryWrap(extension.buffer(), extension.offset(), extension.limit())
1218+
: null;
1219+
if (challengeEx != null && challengeEx.kind() == McpChallengeExFW.KIND_ELICIT_CREATE)
1220+
{
1221+
challenged = true;
1222+
}
12091223
settleLifecycle(traceId);
1224+
// a registrant added via register() before this challenge arrived is still
1225+
// waiting in requests -- unlike onClientBegin/onClientEnd/onClientAbort/onClientReset,
1226+
// nothing else flushes it. Without this, a list-style registrant (hydration) on a
1227+
// route whose very first challenge is a real preauthorize elicitation never learns
1228+
// "no session yet, try again later": it simply waits forever, since no human will
1229+
// ever answer that elicitation on hydration's behalf. Safe to call unconditionally --
1230+
// by the time any OTHER challenge kind (RESUME/SUSPENDED) can occur, onClientBegin has
1231+
// already settled this connect and flushed requests, so this is a no-op then.
1232+
settleRequests(traceId);
12101233

12111234
server.doServerChallenge(traceId, authorization, prefixChallengeCorrelationId(extension));
12121235
}

‎runtime/binding-mcp/src/test/java/io/aklivity/zilla/runtime/binding/mcp/internal/stream/McpProxyCacheIT.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,25 @@ public void shouldHydrateToolkitMultiSettlingResetRoute() throws Exception
147147
k3po.finish();
148148
}
149149

150+
// a hydration list registrant whose route's south connect never reaches Begin/End/Abort/Reset --
151+
// only a real preauthorize elicitation CHALLENGE, which no human answers on hydration's behalf --
152+
// must still settle (with no session) instead of waiting on it forever: nothing but
153+
// onClientChallenge's own settleRequests() call can flush that registrant, since this route's
154+
// connect never produces any of the other terminal events that flush it elsewhere. Before that
155+
// fix, this route's own kind never reached pending == 0, so cache.populated never became true
156+
// and every caller's own initialize blocked on it project-wide, not just this one route
157+
@Test
158+
@Configuration("proxy.cache.toolkit.multi.yaml")
159+
@Specification({
160+
"${app}/cache.hydrate.toolkit.multi.challenge.route/server",
161+
"${app}/cache.hydrate.toolkit.multi.challenge.route/client" })
162+
@ScriptProperty("affinity \"0000003f\"")
163+
@Configure(name = MCP_HYDRATE_FILTER_NAME, value = "tools")
164+
public void shouldHydrateToolkitMultiSettlingChallengedRoute() throws Exception
165+
{
166+
k3po.finish();
167+
}
168+
150169
@Test
151170
@Configuration("proxy.cache.toolkit.multi.refresh.yaml")
152171
@Specification({

‎specs/binding-mcp.spec/src/main/scripts/io/aklivity/zilla/specs/binding/mcp/streams/application/cache.hydrate.toolkit.multi.challenge.route/client.rpt‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
#
2+
# Copyright 2021-2026 Aklivity Inc
3+
#
4+
# Licensed under the Aklivity Community License (the "License"); you may not use
5+
# this file except in compliance with the License. You may obtain a copy of the
6+
# License at
7+
#
8+
# https://www.aklivity.io/aklivity-community-license/
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
# WARRANTIES OF ANY KIND, either express or implied. See the License for the
13+
# specific language governing permissions and limitations under the License.
14+
#
15+
16+
connect "zilla://streams/app0"
17+
option zilla:window 8192
18+
option zilla:transmission "half-duplex"
19+
20+
write zilla:begin.ext ${mcp:beginEx()
21+
.typeId(zilla:id("mcp"))
22+
.lifecycle()
23+
.build()
24+
.build()}
25+
26+
connected
27+
28+
read zilla:begin.ext ${mcp:matchBeginEx()
29+
.typeId(zilla:id("mcp"))
30+
.lifecycle()
31+
.sessionId("5ca1ab1e-c0de-4a11-b007-000100000000")
32+
.build()
33+
.build()}
34+
35+
read notify LIFECYCLE_INITIALIZED
36+
37+
connect await LIFECYCLE_INITIALIZED
38+
"zilla://streams/app0"
39+
option zilla:window 8192
40+
option zilla:transmission "half-duplex"
41+
42+
write zilla:begin.ext ${mcp:beginEx()
43+
.typeId(zilla:id("mcp"))
44+
.toolsList()
45+
.sessionId("5ca1ab1e-c0de-4a11-b007-000100000000")
46+
.build()
47+
.build()}
48+
49+
connected
50+
51+
read '{"tools":'
52+
'['
53+
'{"name":"quartz__get_current_time"}'
54+
']'
55+
'}'
56+
read closed
57+
58+
write close

‎specs/binding-mcp.spec/src/main/scripts/io/aklivity/zilla/specs/binding/mcp/streams/application/cache.hydrate.toolkit.multi.challenge.route/server.rpt‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
#
2+
# Copyright 2021-2026 Aklivity Inc
3+
#
4+
# Licensed under the Aklivity Community License (the "License"); you may not use
5+
# this file except in compliance with the License. You may obtain a copy of the
6+
# License at
7+
#
8+
# https://www.aklivity.io/aklivity-community-license/
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
# WARRANTIES OF ANY KIND, either express or implied. See the License for the
13+
# specific language governing permissions and limitations under the License.
14+
#
15+
16+
property affinity "00000000"
17+
18+
accept "zilla://streams/app1"
19+
option zilla:window 8192
20+
option zilla:transmission "half-duplex"
21+
22+
accepted
23+
24+
read zilla:begin.ext ${mcp:matchBeginEx()
25+
.typeId(zilla:id("mcp"))
26+
.lifecycle()
27+
.build()
28+
.build()}
29+
30+
connected
31+
32+
read advise zilla:challenge ${mcp:challengeEx()
33+
.typeId(zilla:id("mcp"))
34+
.elicitCreate()
35+
.id("1")
36+
.url("https://server.example.com/authorize?state=7f3a9b1c&redirect_uri=%s".formatted(http:encodeQuery("https://replace.me/callback")))
37+
.build()
38+
.build()}
39+
40+
accept "zilla://streams/app2"
41+
option zilla:window 8192
42+
option zilla:transmission "half-duplex"
43+
44+
accepted
45+
46+
read zilla:begin.ext ${mcp:matchBeginEx()
47+
.typeId(zilla:id("mcp"))
48+
.lifecycle()
49+
.build()
50+
.build()}
51+
52+
connected
53+
54+
write zilla:begin.ext ${mcp:beginEx()
55+
.typeId(zilla:id("mcp"))
56+
.lifecycle()
57+
.sessionId("5ca1ab1e-c0de-4a11-5e55-000a".concat(affinity))
58+
.build()
59+
.build()}
60+
write flush
61+
62+
accepted
63+
64+
read zilla:begin.ext ${mcp:matchBeginEx()
65+
.typeId(zilla:id("mcp"))
66+
.toolsList()
67+
.sessionId("5ca1ab1e-c0de-4a11-5e55-000a".concat(affinity))
68+
.build()
69+
.build()}
70+
71+
connected
72+
73+
write flush
74+
75+
write '{"tools":[{"name":"get_current_time"}]}'
76+
write flush
77+
78+
write close
79+
80+
read closed

0 commit comments

Comments
 (0)