feat(binding-mcp): MCP elicitation on server kind (#1739)#1752
Open
feat(binding-mcp): MCP elicitation on server kind (#1739)#1752
Conversation
Define typed extension flyweights for the MCP elicitation flow on the
originating tools/call stream.
mcp.idl:
- McpChallengeEx union with elicitCreate case carrying
McpElicitCreateChallengeEx { id, url }. CHALLENGE matches MCP's
"no unsolicited elicitation" requirement structurally -- target ->
initiator on INITIAL stream, only valid on an active request.
- McpFlushEx union with elicitCallback (FLUSH carrying OAuth callback
url) and elicitComplete (FLUSH carrying id + status).
- McpElicitStatus enum (COMPLETED, DECLINED, CANCELLED).
McpFunctions:
- challengeEx() / matchChallengeEx() builder + matcher with nested
elicitCreate sub-builder.
- flushEx() / matchFlushEx() builder + matcher with nested
elicitCallback and elicitComplete sub-builders.
McpFunctionsTest: 35 tests covering each builder/matcher pair plus
empty-matcher, case-mismatch and field-mismatch failure paths.
…1739) Application-level scripts and IT method for the elicitation happy path on a tools/call mcp app stream: - read tools/call BEGIN+DATA on initial - write CHALLENGE elicitCreate on initial reverse (target -> initiator) - read FLUSH elicitCallback on initial forward (initial held open until callback arrives, deferred-END convention) - read closed (initial END after callback) - write reply BEGIN - write FLUSH elicitComplete on reply forward - write tool result DATA, close reply Verified peer-to-peer self-consistent via ApplicationIT#shouldCallToolElicitCompleted.
…1739) Application-level scripts and IT method for the elicitation declined path: - read tools/call BEGIN+DATA on initial - write CHALLENGE elicitCreate - read FLUSH elicitCallback (callback URL carries error=access_denied) - read closed - write reply BEGIN - write FLUSH elicitComplete{DECLINED} - write abort (original tools/call cannot proceed without credentials) Verified peer-to-peer self-consistent via ApplicationIT#shouldCallToolElicitDeclined.
) Application-level scripts and IT method for the elicitation timeout path: - read tools/call BEGIN+DATA on initial - write CHALLENGE elicitCreate - (no elicitCallback within timeout window — user did not complete OAuth) - write reply BEGIN - write FLUSH elicitComplete{CANCELLED} - write abort Verified peer-to-peer self-consistent via ApplicationIT#shouldCallToolElicitTimeout.
…1739) Application-level scripts and IT method exercising the elicitation happy path on the lifecycle (initialize) stream: - read lifecycle BEGIN - write lifecycle BEGIN reply - write CHALLENGE elicitCreate - read FLUSH elicitCallback - write FLUSH elicitComplete{COMPLETED} - (lifecycle stays open — no END) Verified peer-to-peer self-consistent via ApplicationIT#shouldInitializeElicitCompleted.
…1739) Application-level scripts and IT method for the elicitation declined path on the lifecycle (initialize) stream: - read lifecycle BEGIN, write lifecycle BEGIN reply - write CHALLENGE elicitCreate - read FLUSH elicitCallback (callback URL carries error=access_denied) - write FLUSH elicitComplete{DECLINED} - write abort Verified peer-to-peer self-consistent via ApplicationIT#shouldInitializeElicitDeclined.
) Application-level scripts and IT method for the elicitation timeout path on the lifecycle (initialize) stream: - read lifecycle BEGIN, write lifecycle BEGIN reply - write CHALLENGE elicitCreate - (no elicitCallback within timeout window) - write FLUSH elicitComplete{CANCELLED} - write abort Verified peer-to-peer self-consistent via ApplicationIT#shouldInitializeElicitTimeout.
) Application-level scripts for the elicitation flow through an mcp proxy with a single bluesky-tagged toolkit route. Pattern (mirrors tools.call.toolkit): - client (upstream of proxy) sends tools/call name="bluesky__get_weather" with json data carrying same prefixed name - server (downstream of proxy) accepts tools/call name="get_weather" (toolkit prefix stripped from BEGIN extension) - elicitCreate URL state on client side carries "bluesky__7f3a9b1c" (proxy prepended toolkit prefix going upstream) - server side sees state="7f3a9b1c" (no toolkit prefix yet) - elicitCallback: client sends URL state="bluesky__7f3a9b1c"; server reads state="7f3a9b1c" (proxy stripped toolkit prefix going downstream and routed to bluesky-tagged backend) - elicitComplete and tool result flow back upstream No ApplicationIT method — toolkit scenarios require the proxy in the pipeline, exercised by McpProxyIT in the runtime project once implementation lands. Script syntax validated via k3po:validate.
Network-side scripts and IT method exercising the wire emission of
elicitation/create as an SSE event on a tools/call response.
- standard initialize + notifications/initialized handshake
- POST /mcp tools/call (Accept includes text/event-stream)
- 200 response with content-type text/event-stream
- SSE event id: 2:1 carrying
data: {"jsonrpc":"2.0","method":"elicitation/create",
"params":{"mode":"url","elicitationId":"elicit-1",
"url":"https://provider.example/authorize?
state=session-1.7f3a9b1c&
redirect_uri=https://localhost:8080/mcp/auth/callback"}}
State carries only sessionId prefix (no toolkit__) — server-self-contained
case where the proxy is not in the pipeline. Callback URL substituted by
mcp-server in place of CALLBACK_PLACEHOLDER.
Verified peer-to-peer self-consistent via
NetworkIT#shouldCallToolElicitCreate.
Network-side scripts and IT method exercising the OAuth callback endpoint that the browser is redirected to: - GET /mcp/auth/callback?code=xyz&state=session-1.7f3a9b1c - 200 text/plain "Authorization complete, you may close this tab." The state query parameter carries the sessionId prefix added by the mcp-server binding when emitting elicitation/create upstream. The http-server affinity routing extracts the first [^.]+ segment to hash-route the callback to the correct worker thread. Verified peer-to-peer self-consistent via NetworkIT#shouldReceiveAuthCallback.
Network-side scripts and IT method exercising the full happy-path wire
trace of the elicitation flow on a tools/call response SSE stream:
- standard initialize + notifications/initialized handshake
- POST /mcp tools/call
- 200 text/event-stream response carrying three SSE events:
id: 2:1 data: elicitation/create (mode=url, elicitationId, url)
id: 2:2 data: elicitation/complete (elicitationId, status=completed)
id: 2:3 data: tool result (jsonrpc id=2)
- response stream closes
Verified peer-to-peer self-consistent via
NetworkIT#shouldCallToolElicitComplete.
…cenario (#1739) Network-side scripts and IT method exercising the OAuth callback endpoint when the state's elicitationId portion does not map to any in-flight elicitation (e.g., timed out and evicted, or never existed): - GET /mcp/auth/callback?code=xyz&state=session-1.unknown - 410 Gone "Authorization session expired or unknown." Verified peer-to-peer self-consistent via NetworkIT#shouldRejectAuthCallbackUnknownElicitation.
#1739) Align network scenario name with its application-level counterpart (tools.call.elicit.completed). Both layers now share the consistent ".completed" suffix denoting the COMPLETED elicitation outcome. The focused tools.call.elicit.create network scenario stays distinct because it tests just the elicitCreate emission rather than the full happy path.
Network counterpart of the application-level declined scenario. Wire
trace shows:
- standard initialize + notifications/initialized handshake
- POST /mcp tools/call
- 200 text/event-stream response carrying:
id: 2:1 data: elicitation/create
id: 2:2 data: elicitation/complete (status=declined)
id: 2:3 data: jsonrpc error -32000 "Authorization declined"
- response stream closes
Verified peer-to-peer self-consistent via
NetworkIT#shouldCallToolElicitDeclined.
Network counterpart of the application-level timeout scenario. Wire
trace shows:
- standard initialize + notifications/initialized handshake
- POST /mcp tools/call
- 200 text/event-stream response carrying:
id: 2:1 data: elicitation/create
id: 2:2 data: elicitation/complete (status=cancelled)
id: 2:3 data: jsonrpc error -32000 "Authorization timed out"
- response stream closes
Verified peer-to-peer self-consistent via
NetworkIT#shouldCallToolElicitTimeout.
Network counterpart of the application-level initialize.elicit.completed
scenario. Wire trace shows:
- POST /mcp initialize (Accept: application/json, text/event-stream)
- 200 text/event-stream with mcp-session-id header
(server upgrades response to SSE because elicitation is required mid-flow)
- SSE events:
id: 1:1 data: elicitation/create
id: 1:2 data: elicitation/complete (status=completed)
id: 1:3 data: jsonrpc id=1 result (initialize result)
- response stream closes
Verified peer-to-peer self-consistent via
NetworkIT#shouldInitializeElicitCompleted.
…rios (#1739) Network counterparts of the application-level declined/timeout initialize scenarios. Wire trace shows: - POST /mcp initialize (Accept: application/json, text/event-stream) - 200 text/event-stream with mcp-session-id header - SSE events: id: 1:1 data: elicitation/create id: 1:2 data: elicitation/complete (status=declined or cancelled) id: 1:3 data: jsonrpc id=1 error -32000 ("Authorization declined" or "Authorization timed out") - response stream closes Verified peer-to-peer self-consistent via NetworkIT#shouldInitializeElicitDeclined and NetworkIT#shouldInitializeElicitTimeout.
Network counterpart of the application-level toolkit scenario. Wire
trace shows the toolkit prefix visible end-to-end:
- POST /mcp tools/call with name="bluesky__get_weather"
- 200 text/event-stream response carrying:
id: 2:1 data: elicitation/create
url state="session-1.bluesky__7f3a9b1c"
(server prefixed sessionId; proxy prefixed toolkit)
id: 2:2 data: elicitation/complete (status=completed)
id: 2:3 data: jsonrpc id=2 tool result
No NetworkIT method — toolkit scenarios require the proxy in the
pipeline, exercised by runtime ITs once implementation lands. Script
syntax validated via k3po:validate.
Per the MCP spec, server-initiated requests including elicitation/create are not permitted before notifications/initialized. Sending elicitation/create on the initialize response would be a protocol violation that an MCP-compliant client would reject. The mcp-client binding instead defers backend communication until after init success: the Zilla-facing initialize completes without backend contact; the first post-init request (typically tools/call) is what triggers elicitation, which is exactly what the tools.call.elicit.* scenarios cover. A focused lifecycle.initialize.elicit.deferred scenario will be added separately to illustrate the deferred behavior.
…licit (#1739) Match the existing convention where toolkit-bearing scenarios place ".toolkit" immediately after the operation (e.g., tools.call.toolkit, tools.call.toolkit.with.progress). The elicitation suffix follows last: tools.call.toolkit.elicit at both application and network layers. No content change — directory rename only.
…#1739) The focused tools.call.elicit.create network scenario is fully covered by tools.call.elicit.{completed,declined,timeout} -- they all start with the same handshake, POST /mcp tools/call, 200 text/event-stream, and emit the elicitation/create event before continuing. The standalone .create scenario only stopped early and added no test coverage beyond what the three full-flow scenarios already exercise.
Apply the existing multi-line single-quote concatenation pattern to
the elicitation JSON structures inside SSE data: events for the
tools.call.elicit.{completed,declined,timeout} network scenarios.
Each JSON object is now broken across multiple single-quoted lines
that k3po concatenates without newlines, so the observed wire bytes
are unchanged while the scripts are easier to read.
Equivalence verified by temporarily reverting one side of a pair
to its single-line form and confirming the test still passes.
Also drop the tools.call.toolkit.elicit network scenario entirely:
toolkit prefix manipulation is purely a proxy binding (app-to-app)
concern and never appears on the wire, so the network-level scenario
was misleading. The application-level tools.call.toolkit.elicit
scenario remains and continues to cover the proxy behaviour.
…1739) - Rename authorization URL host from provider.example to server.example.com to align with the standard reserved example domain (RFC 2606) used elsewhere in the project. - URL-escape the redirect_uri parameter value in elicitation URLs as required when embedding a URL inside another URL's query string. The CALLBACK_PLACEHOLDER (https://replace.me/callback) and the substituted callback URI (https://localhost:8080/mcp/auth/callback) are both now encoded as percent-escaped values. Wire bytes change is intentional and observable: scripts and McpFunctions test fixtures updated together. Verified with full reactor build: runtime/binding-mcp ITs all green (McpClientIT, McpServerIT, McpProxyIT, ApplicationIT, NetworkIT).
Add config plumbing for the upcoming elicitation flow in the mcp server kind: - Schema patch: new options.elicitation block with callback path property (default: auth/callback). - McpElicitationConfig + builder pojos. - McpOptionsConfig and builder updated to carry elicitation. - McpOptionsConfigAdapter reads/writes the new block. No behavior change yet — McpServerFactory does not consume the config. Subsequent commits will wire the callback path recognition, CHALLENGE elicitCreate interception, and FLUSH elicitComplete forwarding. Verified: full reactor build clean; 50 ApplicationIT, 54 NetworkIT, 128 runtime/binding-mcp tests all pass.
Add path-based dispatch so the mcp server kind responds to inbound GET requests at the configured elicitation callback path with a 200 text/plain "Authorization complete..." body, suitable for the browser that completed the OAuth dance. Implementation: - New :path header lookup in newStream dispatch. - isAuthCallbackPath helper that matches when the request path (excluding query) ends with "/" + options.elicitation.callback, defaulting to "auth/callback". - New McpAuthCallbackHandler stream handler that emits a 200 reply with the success body once the inbound request closes. - New STATUS_410 and CONTENT_TYPE_TEXT_PLAIN constants for upcoming reject/unknown-elicitation handling. This step unconditionally returns 200; full elicitationId tracking and 410 Gone for unknown elicitations land in the next commit alongside CHALLENGE elicitCreate handling. Verified: McpServerIT#shouldReceiveAuthCallback passes; 50 McpServerIT total (+1), 129 runtime/binding-mcp tests all pass.
…1739) Resolve the auth callback's `state` query param against the per-session `elicitations` map; respond 200 OK with the configured success body when a suspended request stream is found, otherwise 410 Gone with an "expired or unknown" body. Replace the placeholder `auth.callback` scenario with the explicit `reject.auth.callback.unknown.elicitation` IT (the placeholder's success path now requires the full elicit-create flow, which lands in a follow-up commit). Also adds the per-session `elicitations` map and the request-stream `doAppFlushElicitCallback` writer, used by the success branch once the CHALLENGE elicitCreate flow lands. https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
…te (#1739) On the request stream's reply path, parse a `McpChallengeEx` extension on inbound CHALLENGE; if `elicitCreate`, prepend `<sessionId>.` to the URL's `state` parameter, substitute `redirect_uri` with a synthesized `https://<authority><path>/<callback>` derived from the inbound POST headers and the binding's elicitation config, register the request stream under the original `state` token in the per-session `elicitations` map, upgrade the response to `text/event-stream`, and emit two SSE events (an empty `id: <reqId>:0` init marker, then `id: <reqId>:1` carrying the `elicitation/create` JSON-RPC envelope). On the request stream's reply FLUSH path, recognize `elicitComplete` extension and emit a matching `id: <reqId>:N` SSE event with the `elicitation/complete` envelope, then deregister the state token. Pre-existing flush kinds (resumable / progress / suspend) and the non-elicit CHALLENGE fast-path remain on their original code paths. Per-instance state on `McpServer`: `elicitMode` flag, `elicitSeq` counter for SSE event ids, plus `redirectUri` set at construction. `McpRequestStream` carries a `stateToken` back-pointer so `elicitComplete` can deregister deterministically. The end-to-end success scenario (`shouldCallToolElicitCompleted`) is not yet exercisable from `McpServerIT` because the flow needs zilla to defer forwarding the network END to the app stream until after the elicitation completes (the deferred-END convention noted in the issue plan but not yet implemented). The peer-to-peer ApplicationIT covers the same scenario and continues to pass. https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
#1739) Add `resolveRedirectUri(HttpBeginExFW)` to `McpBindingConfig`. It extracts `:authority` and `:path` from the inbound BEGIN extension and combines them with the binding's own `options.elicitation.callback` to form `https://<authority><path>/<callback>`. `newStream` no longer performs header lookup or URL synthesis for the redirect URI — it just calls `binding.resolveRedirectUri(httpBeginEx)` and passes the result to the McpServer constructor. https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
…cpServerIT (#1739) Add `shouldCallToolElicitCompleted` to McpServerIT and the parallel auth-callback connect/accept block to the network elicit-completed scripts so the scenario drives a second connection that resolves the elicitation. Annotated `@Ignore` on the IT method: peer-to-peer ApplicationIT and NetworkIT pass with the existing CHALLENGE elicitCreate handler, but through the zilla k3po driver `write advised zilla:challenge` does not emit a wire CHALLENGE today, so the app-side script cannot trigger SSE upgrade for elicit-create from a Zilla-mediated test. The same limitation already exists for `tools.call.with.progress.suspended`; that test passes only because it independently triggers SSE upgrade via a `progressToken` field on the request. Re-enabling this IT requires either a k3po-driver-zilla fix to emit CHALLENGE frames on write-advised, or moving elicitCreate from CHALLENGE onto a FLUSH extension (which does flow through the driver). Either approach is bigger than Phase 1 close-out; tracking as a follow-up. https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
#1739) CHALLENGE flows target -> initiator on the initial direction, like WINDOW and RESET, so the k3po script verbs are inverted from FLUSH: - target side (accepted server) uses `read advise zilla:challenge` with a `challengeEx()` builder to SEND the CHALLENGE on the read direction's reverse - initiator side (connect client) uses `write advised zilla:challenge` with a `matchChallengeEx()` matcher to CONFIRM RECEIPT on the write direction's context The four elicit scenarios were authored with the inverted form; flip them so the script semantics match the wire-level direction. All peer-to-peer ApplicationIT (50/50) and NetworkIT (53/53) remain green. Update the `shouldCallToolElicitCompleted` ignore reason to point at the actual blocker: zilla forwards the network END to the app stream eagerly (in `McpServer.onNetEnd`), closing initial-forward before the app's `read advise zilla:challenge` can fire to send CHALLENGE back. A targeted deferred-END change in zilla is needed to unblock this; a naive blanket deferral broke other request-stream tests that read END before writing reply. https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
Bump k3po dependency to 3.4.0 (which permits advised statements ahead of `connected` in scripts) and defer the accepted-side WINDOW emission in the zilla k3po transport so it follows `fireChannelConnected`. Previously `ZillaStreamFactory.Stream.onBegin` emitted WINDOW synchronously on stream-open, before the script's `connected` block could process any leading advised statements. Now: - `Stream.onBegin` only emits WINDOW for connect-side (initiator) channels (`channel.getParent() == null`); accept-side child channels skip the eager emission. - `ZillaPartition.handleBeginInitial`'s `windowFuture` listener flushes the WINDOW after `fireChannelConnected(childChannel, ...)` returns, when the channel's update mode is `HANDSHAKE` or `STREAM`. For accept-side scripts that currently place `read advise zilla:challenge` after `connected`, observable behaviour is unchanged: the WINDOW still fires before any further BEGIN/DATA arrives on the wire, just one tick later via the existing `windowFuture` listener path. The change unlocks scripts that place advised statements ahead of `connected` (per k3po 3.4.0). All existing engine ITs (DuplexIT, HalfDuplexIT, SimplexIT, EngineIT) remain green; binding-mcp ITs (McpServerIT, McpClientIT, McpProxyIT) also remain green. https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
Continuation of cc83e5e. The accepted-side WINDOW emission was deferred to the windowFuture listener but still ran synchronously, so for SIMPLEX and HALF_DUPLEX the WINDOW landed in the streamsBuffer before any CHALLENGE that the script's `read advise` queued via the (already-async) AdviseInputTask path. That left CHALLENGE arriving after WINDOW on the wire and broke the new `read advise` before `connected` ordering. Submit the WINDOW write through the engine task queue, and queue it before invoking fireChannelConnected. That places WINDOW after any task queued before windowFuture.setSuccess (e.g. an AdviseInputTask from a `read advise` placed ahead of `connected`) and ahead of tasks queued by statements that follow `connected` (e.g. AbortInputTask from a trailing `read abort`), preserving the existing wire order for non-challenge scripts. Reorder the six driver CHALLENGE scripts (duplex, duplex.ext, half.duplex, half.duplex.ext, simplex, simplex.ext) to place `read advise zilla:challenge` before `connected` on the accept side and `write advised zilla:challenge` before `connected` on the connect side, exercising the new ordering. DuplexIT 72/72, HalfDuplexIT 64/64, SimplexIT 35/35, EngineIT 14/14 with 7 expected skips.
…ts (#1739) Reorder the four MCP elicit application scripts (tools.call.elicit.completed, tools.call.elicit.declined, tools.call.elicit.timeout, tools.call.toolkit.elicit) to place `read advise zilla:challenge ${... .elicitCreate() ...}` on the accept side and the matching `write advised zilla:challenge` on the connect side before the (second) `connected`, matching the ordering exercised by the driver-level CHALLENGE tests in 484fb4e. Peer-to-peer ApplicationIT 50/50 and NetworkIT 53/53 pass against the reordered scripts. Update the @ignore note on McpServerIT.shouldCallToolElicitCompleted: the original blocker (zilla forwarding END before the app's read-advise could fire) is resolved by the driver-side WINDOW deferral. Through-Zilla still fails because the binding's synthesized redirect_uri produces a malformed URL (missing host, doubled path prefix) — only visible now that the app's elicitCreate actually reaches the binding. That is a separate binding-mcp issue, tracked outside this commit.
Three fixes that together let the through-Zilla elicit-completed scenario pass for the first time: 1. McpBindingConfig.resolveRedirectUri: Array32FW.matchFirst returns the shared item flyweight, so a second matchFirst call re-wraps it onto the next match and authorityHeader.value() reads the path. Fix by extracting each header's value out before issuing the next matchFirst, so the synthesized redirect_uri is `https://localhost:8080/mcp/auth/callback` instead of the broken `https:///mcp/mcp/auth/callback`. 2. McpServer.onNetEnd: defer the network END forwarding to the upstream while the McpServer is in elicitMode (i.e. an upstream CHALLENGE elicitCreate was received). Pair this with the auth-callback handler now invoking `resolved.doAppEnd(...)` and the new `doDeferredCloseInitial()` after dispatching the elicitCallback FLUSH, so the upstream sees BEGIN, body, FLUSH, END (matching the spec ordering) instead of BEGIN, body, END, (later) FLUSH. 3. McpAuthCallbackHandler now builds the elicitCallback FLUSH URL inline from the request's `<:scheme>://<:authority><:path>` and passes that verbatim to the upstream. This matches what the user actually navigated to, with no separate buildCallbackUrl helper and no state stripping — the upstream sees the same `state` value the auth provider returned. Update the four MCP elicit application scripts (and the network elicit.completed scripts) to expect the new URL with `http://` and sessionId-prefixed state. McpServerIT: un-@ignore shouldCallToolElicitCompleted; add shouldCallToolElicitDeclined and shouldCallToolElicitTimeout @ignore'd with notes — those scenarios need additional translation logic from the binding (cancelled/declined elicitComplete -> JSON-RPC error event) which is tracked as a follow-up. McpServerIT 51 + 2 ignored, McpClientIT 42/42, McpProxyIT 37/37, ApplicationIT 50/50, NetworkIT 53/53.
…icit (#1739) Translate an upstream elicitComplete with status=DECLINED or CANCELLED into a JSON-RPC error event on the network SSE response, completing the last two scenarios from issue #1739 Phase 1: - DECLINED -> id: 2:3 + data: { code: -32000, message: "Authorization declined" }, then close the response. - CANCELLED -> id: 2:3 + data: { code: -32000, message: "Authorization timed out" }, then close. doEncodeElicitErrorEvent reuses the existing JSON_RPC_ERROR_* constants and the SSE id-line + data: + terminator pattern of the other doEncodeElicit* helpers; doEncodeResponseEnd is invoked after the error event so the network sees the SSE stream close once the elicitation resolves negatively. The declined network scripts were missing the auth-callback connect that simulates the OAuth provider's redirect with error=access_denied. Add it to both client.rpt and server.rpt so the binding actually receives the callback that drives the elicitCallback FLUSH. McpServerIT 53/53, McpClientIT 42/42, McpProxyIT 37/37, ApplicationIT 50/50, NetworkIT 53/53.
jfallows
commented
May 7, 2026
Comment on lines
+138
to
+143
| final HttpHeaderFW authorityHeader = httpBeginEx.headers() | ||
| .matchFirst(h -> HTTP_HEADER_AUTHORITY.equals(h.name().asString())); | ||
| // matchFirst returns a shared flyweight; copy its value out before | ||
| // calling matchFirst again, otherwise the next call re-wraps the | ||
| // shared instance and authorityHeader.value() reads the wrong header | ||
| final String authority = authorityHeader != null ? authorityHeader.value().asString() : null; |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| final HttpHeaderFW authorityHeader = httpBeginEx.headers() | |
| .matchFirst(h -> HTTP_HEADER_AUTHORITY.equals(h.name().asString())); | |
| // matchFirst returns a shared flyweight; copy its value out before | |
| // calling matchFirst again, otherwise the next call re-wraps the | |
| // shared instance and authorityHeader.value() reads the wrong header | |
| final String authority = authorityHeader != null ? authorityHeader.value().asString() : null; | |
| final String authority = Optional.ofNullable(httpBeginEx.headers() | |
| .matchFirst(h -> HTTP_HEADER_AUTHORITY.equals(h.name().asString()))) | |
| .map(h -> h.value().asString()) | |
| .orElse(null); |
Same pattern for path.
Comment on lines
+151
to
+154
| final McpElicitationConfig elicitation = options != null ? options.elicitation : null; | ||
| final String callback = elicitation != null | ||
| ? elicitation.callback | ||
| : McpElicitationConfig.DEFAULT_CALLBACK_PATH; |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| final McpElicitationConfig elicitation = options != null ? options.elicitation : null; | |
| final String callback = elicitation != null | |
| ? elicitation.callback | |
| : McpElicitationConfig.DEFAULT_CALLBACK_PATH; | |
| final String callback = Optional.ofNullable(options) | |
| .map(o -> o.elicitation) | |
| .map(e -> e.callback) | |
| .orElse(DEFAULT_CALLBACK_PATH); |
Use static import for McpElicitationConfig.DEFAULT_CALLBACK_PATH.
| .typeId(zilla:id("mcp")) | ||
| .elicitCreate() | ||
| .id("elicit-1") | ||
| .url("https://server.example.com/authorize?state=7f3a9b1c&redirect_uri=https%3A%2F%2Freplace.me%2Fcallback") |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| .url("https://server.example.com/authorize?state=7f3a9b1c&redirect_uri=https%3A%2F%2Freplace.me%2Fcallback") | |
| .url("https://server.example.com/authorize?state=7f3a9b1c&redirect_uri=%s".formatted(http:encodeQuery("https://replace.me/callback")) |
Does this seem more readable?
If so we can add encodeQuery to HttpFunctions and call from here to get the same final output via:
URLEncoder.encode(rawValue, StandardCharsets.UTF_8).replace("+", "%20");
Comment on lines
309
to
318
Contributor
Author
There was a problem hiding this comment.
Combine this using Optional.ofNullable with .map and .orElse(null);
Comment on lines
320
to
323
Contributor
Author
There was a problem hiding this comment.
Combine this using Optional.ofNullable with .map and .orElse(null);
Comment on lines
+358
to
+360
| final HttpHeaderFW schemeHeader = httpBeginEx.headers() | ||
| .matchFirst(h -> HTTP_HEADER_SCHEME.equals(h.name().asString())); | ||
| final String scheme = schemeHeader != null ? schemeHeader.value().asString() : "https"; |
Contributor
Author
There was a problem hiding this comment.
Combine this using Optional.ofNullable with .map and .orElse(null);
Comment on lines
+361
to
+363
| final HttpHeaderFW authorityHeader = httpBeginEx.headers() | ||
| .matchFirst(h -> HTTP_HEADER_AUTHORITY.equals(h.name().asString())); | ||
| final String authority = authorityHeader != null ? authorityHeader.value().asString() : ""; |
Contributor
Author
There was a problem hiding this comment.
Combine this using Optional.ofNullable with .map and .orElse(null);
| this.session = session; | ||
| this.decoder = decodeJsonRpc; | ||
| this.initialMax = decodeMax; | ||
| this.redirectUri = redirectUri; |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| this.redirectUri = redirectUri; | |
| this.redirectURI = redirectURI; |
| { | ||
| if (elicitMode) | ||
| { | ||
| codecLimit += encodeSseIdLine(codecBuffer, codecLimit, decodedId, ++elicitSeq); |
Contributor
Author
There was a problem hiding this comment.
For the final response, there is no need to recover, so no preamble event id needed, same for error cases.
Suggest removal of elicitSeq.
| private McpRequestStream stream; | ||
|
|
||
| private final String redirectUri; | ||
| private boolean elicitMode; |
Contributor
Author
There was a problem hiding this comment.
Can we avoid this boolean flag? Is the state it represents captureable or captured elsewhere?
Apply review comments from #1752: - McpBindingConfig.resolveRedirectUri: collapse the matchFirst + null guards into Optional.ofNullable().map().orElse(null) chains; static import McpElicitationConfig.DEFAULT_CALLBACK_PATH and pull the callback through the same Optional chain via options.elicitation.callback. - McpServerFactory.newStream: same Optional.ofNullable pattern for every HTTP header lookup (session/method/accept/path on POST; scheme/authority on GET auth-callback). Drop resolvedSession (just use session) and the redundant `binding != null` guard before resolveRedirectUri (already implied by route != null). Rename redirectUri/callbackUrl to redirectURI/callbackURL (URI/URL acronyms uppercased). - HttpFunctions: add @function String encodeQuery(String) that returns URLEncoder.encode(value, UTF_8).replace("+", "%20"). Use it in the four application elicit scripts so the redirect_uri value reads as "https://replace.me/callback" instead of the URL-encoded literal. - McpServer encoder cleanup: drop the elicitSeq field and the `id:` line from doEncodeResponsePreamble and doEncodeElicitErrorEvent — the final response and error events are terminal and don't need a Last-Event-ID for resumption. The remaining init/create/complete events use fixed ELICIT_INIT_SEQ/ELICIT_CREATE_SEQ/ELICIT_COMPLETE_SEQ constants. Update the network elicit.{completed,declined,timeout} scripts to drop the matching `id: 2:3` reads/writes. - Drop the elicitMode boolean: the END-deferral gate is now `stream != null && stream.stateToken == null`, which captures the same state (CHALLENGE elicitCreate received and elicitation pending) without an additional one-way flag. McpServerIT 53/53, McpClientIT 42/42, McpProxyIT 37/37, ApplicationIT 50/50, NetworkIT 53/53.
jfallows
commented
May 7, 2026
| public String resolveRedirectUri( | ||
| HttpBeginExFW httpBeginEx) | ||
| { | ||
| String redirectUri = null; |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| String redirectUri = null; | |
| String redirectURI = null; |
| return result; | ||
| } | ||
|
|
||
| public String resolveRedirectUri( |
Contributor
Author
There was a problem hiding this comment.
Suggested change
| public String resolveRedirectUri( | |
| public String resolveRedirectURI( |
Comment on lines
+4724
to
+4728
| private static String manipulateElicitUrl( | ||
| String originalUrl, | ||
| String sessionId, | ||
| String redirectURI) | ||
| { |
Contributor
Author
There was a problem hiding this comment.
Can we use Pattern, Matcher and replace via regex here instead of verbose parsing?
Comment on lines
+118
to
+120
| private static final int ELICIT_INIT_SEQ = 0; | ||
| private static final int ELICIT_CREATE_SEQ = 1; | ||
| private static final int ELICIT_COMPLETE_SEQ = 2; |
Contributor
Author
There was a problem hiding this comment.
These should not be needed.
The id field from the initial challenge ex and reply flush ex frames is the SSE event id for elicitation, same approach as other similar frames unrelated to elicitation.
- McpBindingConfig: rename `resolveRedirectUri` -> `resolveRedirectURI` (method + local variable) for URI acronym consistency. - McpServerFactory.manipulateElicitUrl: replace the verbose query parser with two Pattern.replaceFirst calls — STATE_PARAM_PATTERN prefixes the state value with `<sessionId>.` and REDIRECT_URI_PARAM_PATTERN replaces the redirect_uri value with the URL-encoded effective callback URL. McpServerIT 53/53, McpClientIT 42/42, McpProxyIT 37/37, ApplicationIT 50/50, NetworkIT 53/53. Note: a second batch of review feedback (drop ELICIT_*_SEQ constants and use the elicit frame extension `id` field as the SSE event id) is being investigated alongside a binding-http CI failure caused by 484fb4e's deferred-WINDOW-via-task-queue change; will follow in a separate commit.
…1739) Add `read "This is some data"` before `read abort` in the backend script. Without it, the backend's read-abort fires immediately after `connected`, racing the proxy's data write — under faster RESET timing the proxy's WriteTask runs after the channel is write-aborted and its write-handler future never completes, hanging the script. Adding the explicit `read` makes the backend wait for the proxy's data to arrive before aborting, forcing the deterministic ordering WINDOW -> data -> RESET. The change has no effect on the test's intent (verifying the proxy doesn't retry non-idempotent requests). Found while exercising the engine k3po driver's deferred-WINDOW path (484fb4e): with WINDOW emitted via the task queue the original script race surfaced; this script update removes the race and the engine path needs no further change.
Address review feedback: drop the hardcoded ELICIT_INIT_SEQ /
ELICIT_CREATE_SEQ / ELICIT_COMPLETE_SEQ constants and use the `id` field
from the elicitCreate (CHALLENGE ext) and elicitComplete (FLUSH ext)
frames as the SSE event id, mirroring the existing pattern in
encodeSseNotifyEvent / encodeSseProgressEvent (where the SSE id is the
notification / progress id from the corresponding frame ext).
- Drop doEncodeElicitInitEvent and the empty `id: <reqId>:0\n\n`
preamble it emitted on the network response. The init event served no
purpose: it was an SSE event with no `data:` field (so clients did not
dispatch it) and the binding has no logic that resumes from a
Last-Event-ID anchor pointed at it.
- doEncodeElicitCreateDataEvent now emits `id: <reqId>:<elicitationId>`
using the elicitCreate ext's `id` field.
- doEncodeElicitCompleteDataEvent emits the same SSE id using the
elicitComplete ext's `id` field (same elicitation id, since both
frames refer to the same elicitation cycle).
- Replace encodeSseIdLine(int seq) helper with encodeSseElicitIdLine
(String elicitationId).
- Update the network elicit.{completed,declined,timeout} scripts to
match: drop the `id: 2:0\n` + empty-event reads/writes, change the
create / complete events from `id: 2:1\n` / `id: 2:2\n` to
`id: 2:elicit-1\n`.
McpServerIT 53/53, McpClientIT 42/42, McpProxyIT 37/37,
ApplicationIT 50/50, NetworkIT 53/53.
…vent id - Add ElicitationIdSupplier to McpConfiguration (UUID by default), mirroring SessionIdSupplier so ITs can override deterministically - Synthesise opaque elicitationId on CHALLENGE arrival; use challenge.id() as the SSE event-id sequence component (consistent with progress/resumable form) - Replace state=<sessionId>.<orig> URL prefix with state=<elicitationId>.<orig> - Move per-session elicitations map to factory-wide map keyed by elicitationId - Override supplier to return 'elicit-1' in McpServerIT for deterministic scripts - Add shouldGenerateElicitCreateChallengeEx unit test for builder coverage
…e prefix - Restore per-session elicitations map; drop factory-wide map - State format becomes <sessionId>.<elicitationId>.<originalState> so HTTP affinity (#1744) can extract sessionId via prefix-match to route the callback to the worker that owns the session, then look up the per-session elicitations map by elicitationId - resolveElicitation parses both segments and looks up sessions[sessionId] then session.elicitations[elicitationId] - Default ElicitationIdSupplier returns 8 random hex chars (32 bits, plenty for session-scoped uniqueness) instead of full UUID
… app scripts Variants of the existing elicit scenarios with the FLUSH elicitCallback step removed, modelling the upstream-driven elicit flow where the auth callback is resolved by the upstream MCP server and never traverses the application stream between mcp-client and the application. These pair with the existing network elicit scripts as the upstream behavior when verifying mcp-client elicitation passthrough.
Stub out shouldCallToolElicit{Completed,Declined}Proxied + shouldCallToolElicitTimeout
on McpClientIT, marked @ignore until elicit decode is implemented in McpClientFactory.
The proxied variants pair existing network elicit scripts (upstream behaviour)
with the new .proxied application scripts (app side, no FLUSH elicitCallback);
the timeout case reuses the existing application script as-is since it already
omits the elicitCallback step.
…s on client kind
Adds the decode-side scaffolding so an mcp-client binding observing
elicitation/create and elicitation/complete on an upstream SSE stream
can translate them into MCP CHALLENGE elicitCreate and FLUSH
elicitComplete frames on the application stream. New JSON path
includes (/params/elicitationId, /params/url, /params/mode,
/params/status) feed dedicated decode states; finalizeSseEvent
dispatches to new onDecodeElicit{Create,Complete} hooks on McpStream;
McpRequestStream overrides emit the corresponding extension frames.
This is decoder-only — no upstream POST currently triggers these
paths in the existing tests. The McpClientIT proxied stubs remain
@ignore'd; they will be exercised by guard-driven flows in a
follow-up commit.
… option Adds an optional `preauthorize` URL field to the TestGuard config. When set, the guard returns NEEDS_PREAUTHORIZE from reauthorize(null) and returns the configured URL from preauthorize(...). reauthorize(<url>) treats credentials containing `code=` as a successful OAuth callback and returns a positive session id; anything else continues to fall through to NOT_AUTHORIZED. The default (no preauthorize) preserves the existing static-credentials behaviour. Required by binding-mcp's client-kind elicitation flow, where the binding intercepts a tools/call BEGIN, detects NEEDS_PREAUTHORIZE, emits a CHALLENGE elicitCreate frame to the app, and resumes once the app supplies the OAuth callback URL.
…esolution Adds McpAuthorizationConfig (single field: name) plus its builder and JSON adapter wiring on top of the existing McpOptionsConfig. The schema patch surfaces the authorization block on every kind so the binding-mcp client can name a guard for outbound credentials. McpBindingConfig grows a second constructor that accepts a guard supplier and resolves the named guard once at attach time, making the GuardHandler available on the binding for runtime use. McpClientFactory captures supplyGuard from the EngineContext and threads it through attach. No tools/call wiring yet - this is purely config/resolution scaffolding so the next commit can branch on guard.reauthorize without further config plumbing.
…-net guards
Opens the door for a per-request-stream guard-driven flow on the
mcp-client kind by promoting the McpStream onAppBegin / onAppData /
onAppEnd / onAppFlush hooks from private to package-private and
factoring out three subclass override points:
proceedWithRequest(traceId, authorization, beginEx) — defaults to
true; a subclass that needs to defer the upstream POST (for
example, a tools/call stream awaiting an OAuth pre-authorization
callback) returns false to skip http.doEncodeRequestBegin /
onAppBeginImpl in onAppBegin while still letting the parent emit
the WINDOW.
bufferAppData(traceId, authorization, payload) — defaults to false;
subclasses that need to hold app DATA until later (after a guard
pre-authorization completes) return true to suppress
http.doEncodeRequestData and absorb the bytes locally.
deferAppEnd(traceId, authorization) — defaults to false; subclasses
return true to suppress http.doEncodeRequestEnd from the parent
onAppEnd, replaying the END themselves once the deferred upstream
request finally opens.
McpRequestStream.onNetBegin now skips its automatic doAppBegin when
the reply state is already opening, so a subclass that has emitted
its own reply BEGIN early (during pre-authorization) won't get a
duplicate when the upstream HTTP stream finally connects.
HttpStream.doNetEnd / doNetReset / doNetAbort and flushNetWindow now
short-circuit on net == null, so a stream whose upstream POST never
opened (e.g. an aborted pre-authorization) can be torn down without a
NullPointerException on outbound throttle frames.
McpStream gains a `credentials` field (assigned by a subclass after
guard authorization completes) which HttpToolsCallStream reads to
emit an `authorization: Bearer <credentials>` header on the upstream
POST when present.
These are scaffolding-only changes; no production stream subclass
overrides the new hooks yet, so existing behaviour is unchanged. All
42 previously-passing McpClientIT tests still pass; McpServerIT and
McpProxyIT also unchanged.
When the binding's configured guard returns NEEDS_PREAUTHORIZE on a tools/call BEGIN, defer the upstream POST and surface the authorization URL to the app via a CHALLENGE elicitCreate. Once the app sends back the elicitCallback FLUSH, re-authorize against the guard with the callback URL. On COMPLETED, open the upstream POST with the bearer credentials returned by the guard and replay the buffered request body. On DECLINED or CANCELLED (inactivity timeout), emit elicitComplete with the corresponding status and abort the app reply. Override doAppChallenge on the McpClientFactory McpStream to write on initialId. The K3po extension registers the channel throttle handler at initialId for client-side connect channels (ZillaTarget.connectClient), so a CHALLENGE on replyId is silently dropped before reaching fireOutputAdvised. McpServerFactory continues to write challenges on replyId since its app-side scripts use accept channels. Add 3 ITs: shouldCallToolElicitCompletedGuarded, shouldCallToolElicitDeclinedGuarded, shouldCallToolElicitTimeoutGuarded. Each binds a test guard whose preauthorize URL is a known string the app script matches byte-exact. https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
…flight
Per the unified contract for elicit-aware MCP request streams, the app
must not send initial END until the elicit dance has completed
(COMPLETED / DECLINED / CANCELLED). An END arriving while pendingAuth
is true is a protocol error — mcp-client now resets the initial
direction and aborts the reply instead of buffering and replaying.
State machine simplification: drop endBuffered + aborted in
McpToolsCallStream and replace with a single requestSent flag set on
any terminal elicit transition. deferAppEnd returns requestSent for
the post-elicit END (suppresses redundant http.doEncodeRequestEnd) and
on the protocol-error path resets/aborts and returns true.
In the COMPLETED path, doEncodeRequestEnd is now always emitted right
after the buffered body — the elicitCallback FLUSH timing under the
unified contract is the cue that the buffered body is complete, so the
upstream POST can be closed immediately. (Previously gated on
endBuffered, which was only true under the now-obsolete write-close-
in-middle script ordering.)
Add dedicated tools.call.elicit.{completed,declined,timeout}.guarded
scripts shaped for the new contract (write close at end of script,
after read closed of the reply). The McpClientIT 2b ITs and the
ApplicationIT peer-to-peer ITs use these new scenarios. Phase 1's
non-guarded scripts are unchanged in this commit and will be reshaped
together with the mcp-server JSON-parse-driven END deferral in a
follow-up commit.
Tests: McpClientIT 48/48, McpServerIT 53/53, McpProxyIT 37/37,
ApplicationIT 14/14, NetworkIT 17/17 — no regressions.
https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
Add doClientFlush on McpProxyFactory.McpClient and wire McpServer's
onServerFlush to call it. Previously onServerFlush was a no-op
("pass-through flush — no action required"), which would silently drop
any initial-direction FLUSH (e.g. an elicitCallback FLUSH from app
to upstream) when an mcp-server → mcp-proxy → mcp-client pipeline
needs to forward it. Symmetric with the existing
McpClient.onClientFlush → server.doServerFlush relay on the reply
direction.
No script or test additions in this commit — the relay is dormant
until an upstream binding emits an initial-direction FLUSH on the
proxy's app stream. McpProxyIT 37/37, McpServerIT 53/53, McpClientIT
48/48 (3 skipped @ignore'd stubs) — no regressions.
Prep for the stage-4 pipeline IT that exercises an elicit dance
through the mcp-server → mcp-proxy → mcp-client chain.
https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
…tion
Add a streaming brace-depth tracker to McpToolsCallStream that observes
app DATA bytes as they flow to net and proactively calls
http.doEncodeRequestEnd once the params JSON object closes (top-level
'}' at depth 1). This is the cycle-breaker for the 2a (upstream-driven
elicitation) flow: under the unified contract the app cannot send
initial END until the elicit dance completes, but the upstream HTTP
server cannot respond until the request is fully ended — so the
binding has to drive net END from JSON payload completion, not from
the app's END.
The tracker handles JSON strings (skip braces inside "..."), escaped
characters ('\\X'), and is multibyte-UTF-8-safe (continuation bytes
'>= 0x80' don't collide with the structural ASCII characters).
Also benign for the 2b path (gated on '!pendingAuth' so it doesn't
fire while DATA is being buffered for guard auth) and for the
no-elicit path (doEncodeRequestEnd would have been called by the
parent's onAppEnd anyway; with requestSent set the parent skips it,
so the END just comes a frame earlier).
Tests: McpClientIT 48/48 (3 skipped @ignore'd stubs), McpServerIT
53/53, McpProxyIT 37/37 — no regressions. The 2a stubs still need
their scripts reshaped (stage 4) before they can be unignored.
https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
Realize the 2a (proxied) elicitation flow on the mcp-client kind: when
a guard is not configured, the upstream MCP server itself emits the
elicit dance over its SSE response (elicitation/create →
elicitation/complete) and mcp-client decodes those events and relays
them to the app channel as CHALLENGE elicitCreate / FLUSH
elicitComplete. The brace-depth tracker added in stage 3 already drives
proactive net END so the upstream HTTP request completes regardless of
when the app sends its initial END.
Move doAppBegin to the top of McpToolsCallStream.proceedWithRequest so
the app's reply BEGIN is emitted regardless of guard outcome. Lets K3po
'connected' fire predictably for both 2a (no guard) and 2b (guard +
NEEDS_PREAUTHORIZE) paths — onNetBegin still skips its own doAppBegin
via the existing replyOpening guard.
Reshape tools.call.elicit.{completed,declined}.proxied app scripts to
the realistic 2a wire ordering (CHALLENGE arrives after the request
body has flowed and upstream has responded), update the URL in
matchers to match what upstream emits, and add a new
tools.call.elicit.timeout.proxied scenario for the upstream-driven
timeout case.
Add proxied network scripts (tools.call.elicit.{completed,declined}.proxied
under streams/network/) — same as existing elicit network scripts but
without the Phase 1 server-kind auth-callback connection (mcp-client
does not make a callback HTTP request; the upstream MCP server owns
the OAuth callback path in 2a).
Unignore shouldCallToolElicit{Completed,Declined}Proxied and rename
shouldCallToolElicitTimeout to shouldCallToolElicitTimeoutProxied (now
paired with the new app scenario). Add ApplicationIT
shouldCallToolElicitTimeoutProxied and NetworkIT
shouldCallToolElicit{Completed,Declined}Proxied peer-to-peer tests.
Tests: McpClientIT 48/48 (was 45/48 with 3 ignored), McpServerIT 53/53,
McpProxyIT 37/37. Spec ApplicationIT and NetworkIT cover all new
scenarios peer-to-peer.
https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
Phase 1's unconditional task-queue defer of the accepted-side WINDOW (commits cc83e5e / 484fb4e) was needed for scripts that place a `read advise` ahead of `connected` — to ensure the AdviseInputTask queues its frame before the WINDOW write reaches the binding. But the same defer regresses other accept-side scripts that don't use the wire-order trick (e.g. binding-http's AdvisoryIT) because it changes the relative ordering of WINDOW and the script's post-`connected` listener registrations. Make the defer conditional via a per-channel `windowNeedsTask` flag set by `ZillaEngine.adviseInput` (the entry point for `read advise` statements). When the flag is set, take the existing deferred path: queue WINDOW as a task before invoking `fireChannelConnected`. When the flag is clear, take the pre-Phase-1 path: invoke `fireChannelConnected` synchronously and emit WINDOW direct, so the script's post-`connected` listeners are registered before WINDOW hits the wire. The flag is one-shot relevant to the windowFuture listener; if a `read advise` runs after `connected`, the flag-set is harmless because the listener has already chosen the direct path on its single invocation. Tests: engine DuplexIT 72/72, HalfDuplexIT 64/64, SimplexIT 35/35 (wire-order trick still works); binding-mcp 138/138 (no regression); binding-http server.AdvisoryIT.shouldSendRequestAndFlush restored. binding-http client.AdvisoryIT.shouldSendRequestAndFlush remains failing — a separate Phase 1 regression in the client-kind flush relay path that does not use the wire-order trick and needs further investigation. https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
Replace the conditional task-queue-defer approach (commit bcf7b03 + Phase 1 cc83e5e/484fb4eb) with a simpler in-line task drain. The accepted-side WINDOW is once again emitted eagerly inside ZillaStreamFactory.Stream.onBegin (pre-Phase-1 behaviour). For accept-side child channels, ZillaEngine.drainTasks() is called between beginInputFuture.setSuccess() and the eager doWindow so any tasks queued during fireChannelBound's ACCEPTED-event chain (e.g., an AdviseInputTask from a `read advise` placed before `connected`) run synchronously and write their frames to streamsBuffer ahead of the WINDOW. Wire order for trick scripts: ACCEPTED → script statements (queue AdviseInputTask) → drainTasks (CHALLENGE written) → eager WINDOW written. Same ordering as Phase 1's intended CHALLENGE→WINDOW; no flag, no submitTask gymnastics. Wire order for non-trick scripts (e.g., binding-http AdvisoryIT): same as pre-Phase-1 — no tasks queued before drain runs (drain is a no-op), eager WINDOW written immediately. Removes: - ZillaChannel.windowNeedsTask field + setWindowNeedsTask/windowNeedsTask methods - ZillaEngine.adviseInput's flag-set call - ZillaPartition windowFuture listener's doWindow + submitTask logic (reverts listener to pre-Phase-1: just fireChannelConnected) Adds: - ZillaEngine.drainTasks() — package-private, delegates to executeTasks Tests: engine driver ITs (DuplexIT 72/72, HalfDuplexIT 64/64, SimplexIT 35/35, EngineIT 14/14); binding-http 374/374 (both client.AdvisoryIT.shouldSendRequestAndFlush and server.AdvisoryIT.shouldSendRequestAndFlush restored); binding-mcp 138/138; binding-mcp.spec ApplicationIT/NetworkIT all green. https://claude.ai/code/session_01Sm5BSDDaRNueem3xUeM2Li
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 1 of #1739: MCP elicitation support on the
mcpserver-kind binding. Lets an upstream tool implementation suspend atools/callrequest to ask the human user to authorize via OAuth, then resume the request once the user completes (or declines, or the auth times out).The feature spans the binding (request/elicitation state machine, OAuth callback dispatch, SSE event encoding) and the engine's k3po test transport (wire ordering of CHALLENGE elicitCreate before WINDOW so the upstream can advise the binding before END is forwarded).
Protocol shape
CHALLENGEcarryingmcp:elicitCreate(id, OAuth URL); the binding later flushes amcp:elicitCallback(carrying the user's effective callback URL) on the same initial direction; the upstream replies with amcp:elicitCompleteflush (statusCOMPLETED/DECLINED/CANCELLED).tools/callis upgraded to surface JSON-RPC elicitation events —id: <reqId>:1 + data: { method: "elicitation/create", params: { url, elicitationId } }, then…:2 + elicitation/complete, then either…:3 + result(completed) or…:3 + error(declined / cancelled).options.elicitation.callback, defaultauth/callback); onGET <:scheme>://<:authority>/<callback>?…&state=<sessionId>.<original-state>the binding looks the elicitation up by session, dispatchesmcp:elicitCallbackto the upstream with<:scheme>://<:authority><:path>verbatim, and replies200 OK.What's in this branch
binding-mcpruntimeMcpElicitationConfigunderoptions(callbackpath, defaultauth/callback).McpServerFactoryrecognises the configured callback path onGETand routes to a newMcpAuthCallbackHandler; per-sessionelicitationsmap onMcpLifecycleStreamis keyed by the original (upstream-issued) state token.McpRequestStream.onAppChallengeparsesmcp:elicitCreate, stores the elicitation, manipulates the upstream URL (sessionId-prefixedstate, redirect_uri replaced with the binding's effective callback URL), and emits the SSE init +elicitation/createevents.McpRequestStream.onAppFlush(elicitComplete)emits the SSEelicitation/completeevent; forDECLINED/CANCELLEDit also emits a JSON-RPC error (-32000 Authorization declined/Authorization timed out) and closes the response.McpServer.onNetEnddefers forwarding the network END to the upstream while inelicitMode, paired with the auth-callback handler invokingdoAppEndafter the elicitCallback flush — the upstream seesBEGIN → body → FLUSH → END(matching spec ordering) instead ofBEGIN → body → END → (later) FLUSH.McpBindingConfig.resolveRedirectUrisynthesiseshttps://<:authority><path>/<callback>from the request headers; anArray32FW.matchFirstflyweight-reuse fix copies each header's value out before the nextmatchFirstcall so the synthesised URL is well-formed (no missing host, no doubled path prefix).mcp:elicitCreate,mcp:elicitCallback,mcp:elicitComplete) in the binding IDL.enginek3po test transportread advise/write advisedahead ofconnected).WINDOWso a script's advised statements placed beforeconnectedget to write their frames first; submit theWINDOWwrite through the engine task queue before invokingfireChannelConnectedso the relative order vs. tasks queued by statements followingconnected(e.g. anAbortInputTaskfrom a trailingread abort) is preserved. Wire order becomesBEGIN → CHALLENGE → WINDOW → DATA → ENDfor scripts that opt in, and staysBEGIN → WINDOW → …for everything else.duplex,duplex.ext,half.duplex,half.duplex.ext,simplex,simplex.ext) reordered to exercise the new ordering.binding-mcp.specscripts and ITstools.call.elicit.{completed,declined,timeout},tools.call.toolkit.elicit,reject.auth.callback.unknown.elicitation, plus the existingtools.call.*set adapted for elicitation flow.read advise zilla:challenge ${mcp:challengeEx().elicitCreate()…}ahead ofconnected(matching the new wire ordering).McpFunctionsbuilders/matchers for the three new extension kinds;McpFunctionsTestcovers them.ApplicationITandNetworkITfor each scenario.Test plan
./mvnw install -pl runtime/binding-mcp --also-make -Djacoco.skip=trueMcpServerIT53/53 (zero skips) — includingshouldCallToolElicit{Completed,Declined,Timeout}andshouldCallToolToolkitElicit.McpClientIT42/42,McpProxyIT37/37 (no behavioural change to those kinds; protocol regressions ruled out).binding-mcp.specpeer-to-peer:ApplicationIT50/50,NetworkIT53/53.enginedriver tests with the reordered CHALLENGE scripts:DuplexIT72/72,HalfDuplexIT64/64,SimplexIT35/35,EngineIT14/14.Out of scope
mcp-clientkind) and Phase 3 (mcp-proxykind) elicitation — tracked as follow-up phases onbinding-mcpsupport elicitation #1739.binding-http: per-route affinity extraction #1744.Closes #1739 (Phase 1 — server kind only).
https://claude.ai/code/session_0172bbrihatvupUXDAn5ddvC
Generated by Claude Code