Skip to content

Commit 530e4ba

Browse files
jfallowsclaude
andauthored
feat(binding-mcp): propagate upstream Bearer auth challenges and defer per-request app stream end (#1796)
* feat(binding-mcp): propagate upstream Bearer auth challenges via McpResetEx Adds a new McpResetEx union extension carrying parsed Bearer challenge fields (realm, scopes, error). On a 401/403 upstream response with a Bearer WWW-Authenticate header, mcp(client) parses the challenge per RFC 6750, builds an McpResetEx.bearer, and emits RESET-with-extension on the application initial stream. Error is taken from the upstream challenge when present, otherwise defaulted to insufficient_scope for 403 and invalid_token for 401. McpFunctions resetEx() / matchResetEx() builders mirror the existing challengeEx pattern with a bearer sub-builder. Tests: - McpFunctionsTest: generate + match + mismatch cases for each field - ApplicationIT/NetworkIT: peer-to-peer lifecycle.initialize.reject.bearer - McpClientIT: shouldRejectLifecycleInitializeOnUpstreamBearerChallenge Tracking #1795. * feat(binding-mcp): hold per-request stream open through response cycle For mcp(server) to propagate an upstream auth-challenge RESET (or any upstream-side rejection) back to the inbound peer cleanly, the per-request stream's app-initial must stay open until the response cycle has terminated. Previously mcp(server) closed app-initial as soon as the inbound HTTP body was consumed, which forced upstream into half-closed state before it could respond with anything other than a normal reply. Server-side change: defer stream.doAppEnd out of McpServer.onNetEnd and decodeNet end-of-buffer. Close app-initial only when upstream's reply has terminated (McpRequestStream.onAppEnd or onAppAbort). Mirror on the client side: the proactive HTTP request pattern that McpToolsCallStream had (JSON brace-counting + immediate END for empty bodies) is lifted to McpRequestStream so every per-method stream is proactive. McpToolsListStream, McpPromptsListStream, McpResourcesListStream override onAppBeginImpl to send HTTP request End immediately on app BEGIN (zero-arg methods). McpPromptsGetStream and McpResourcesReadStream override onAppData to drive brace counting via tryCompleteRequestBody. Mirror on the proxy: McpProxyListFactory's onNextClient now sends doClientEnd unconditionally after doClientBegin (list methods have no body), so each outbound route closes its initial without waiting for the inbound to close. Scripts: the application-side scenarios for tools.call, tools.list, prompts.get, prompts.list, resources.read, resources.list (plus the .10k, .100k, .with.progress, .elicit.* variants and lifecycle.shutdown) move `write close` (client) to AFTER `read closed` and `read closed` (server) to AFTER `write close`. This locks in the new invariants: proactivity on mcp(client) and hold-open on mcp(server). For the .aborted variants, the post-abort close handshake is dropped entirely because k3po cannot model frame exchange after a peer-to-peer ABORT. Tests: - ApplicationIT, NetworkIT, McpProxyCacheIT, McpProxyLifecycleIT: green - McpClientIT: 1 known failure (shouldCallToolWithProgressResume) - McpProxyIT: 1 known failure (shouldListToolsThenCancel) - McpServerIT: 2 known failures (shouldListToolsThenCancel, shouldShutdownLifecycleRequests) The 3 known failures involve cancel/shutdown/resume semantics that intersect this refactor in non-trivial ways and need separate follow-up. Tracking #1795. * fix(binding-mcp): close mcp(server) app-initial on cancel and align canceled scripts In mcp(server), the per-request stream's hold-open invariant means app-initial isn't closed on inbound HTTP body completion. On a cancel signal (notifications/cancelled), doAppCancel only sent RESET on app-reply, leaving app-initial open indefinitely. Now also sends END on app-initial after the RESET. The tools.list.canceled scripts had been swapped during the hold-open refactor to put `write aborted` before `read closed`, but mcp(proxy) list factory keeps its eager doClientEnd in onNextClient (zero-arg list methods have no body to wait for). The proxy emits END first, then RESET arrives later when the inbound cancels. Reverting the canceled scripts to the pre-refactor order (`read closed` first, `write aborted` second) matches both bindings: mcp(server)'s doAppCancel order (END first, RESET second) and mcp(proxy)'s eager-END flow. Also restore `read closed` and corresponding `write close` in the lifecycle.shutdown.requests scripts so that the upstream simulator sees the END that doAppCancel now emits. Test status: - McpServerIT 54/54 (was 52/54) - McpProxyIT 43/43 (was 42/43) - McpClientIT 48/49 (shouldCallToolWithProgressResume still failing) - ApplicationIT 91/91, NetworkIT 57/57, ProxyCacheIT 19/19 Tracking #1795. https://claude.ai/code/session_01BYNJLhjKpXGDVtuNgYysgy * refactor(binding-mcp): apply deferred-request-end invariant to all per-request app scripts Invariant: For per-request MCP application streams (toolsList, toolsCall, promptsGet, promptsList, resourcesRead, resourcesList), the request END (initial close) is not sent proactively but follows reply close. Scripts (specs/binding-mcp.spec): - client.rpt: move `write close` from before reads to AFTER the last `read closed`/`read abort`/`read aborted` in each per-request block. - server.rpt: move `read closed` from before writes to AFTER the last `write close`/`write aborted` in each per-request block. Applied to 102 .rpt files across cache.hydrate*, cache.notify*, cache.refresh*, cache.serve*, lifecycle.{client,server}.{read,write}.{abort,close}, lifecycle.notify.tools.list.changed.toolkit.multi*, lifecycle.shutdown.requests, prompts.*.toolkit*, resources.*.toolkit*, tools.*.toolkit*, and tools.list.canceled. Lifecycle blocks (.lifecycle()) are excluded since they have their own close semantics. Runtime: - McpServerFactory.doAppCancel: send doAppReset BEFORE doAppEnd (RESET on reply before END on initial) to match the new invariant order in canceled/shutdown scripts. - McpProxyListFactory: stop sending doClientEnd eagerly in onNextClient; send doClientEnd from McpListClient.onClientEnd (after upstream reply ends) and from McpListServer.onServerReset (after doClientReset) for cancel propagation. - McpProxyCacheHydrater: stop transitioning to closingInitial in doListHydrateBegin; send doListHydrateEnd from onListHydrateEnd (after upstream reply ends). Test status: - specs/binding-mcp.spec: 167/167 (ApplicationIT 91, NetworkIT 57, ProxyCacheIT 19, SchemaTest 6, McpFunctionsTest 58) - runtime/binding-mcp: 174/175 - McpClientIT: 48/49 (shouldCallToolWithProgressResume — pre-existing) - McpServerIT: 54/54 - McpProxyIT: 43/43 - McpProxyCacheIT: 23/23 (10k/100k pre-existing flaky in suite ordering) - McpProxyLifecycleIT: 6/6 Tracking #1795. https://claude.ai/code/session_01BYNJLhjKpXGDVtuNgYysgy * fix(binding-mcp): use abort (not end) to close held-open initial on abort/reset paths In handlers reacting to a reply termination, the proactive close of the held-open initial must match the termination severity: - McpServerFactory.McpRequestStream.onAppAbort: doAppEnd -> doAppAbort - McpProxyListFactory.McpListClient.onClientAbort: add proactive doClientAbort to close the held-open client-initial (symmetric to onClientEnd, which closes it via doClientEnd) - McpProxyListFactory.McpListServer.onServerReset: drop the extra client.doClientEnd; doClientReset already provides 1-1 propagation * fix(binding-mcp): proactively abort held-open client-initial on inbound reset For artificially held-open streams in McpProxyListFactory, the deferred close has no natural propagation source -- it must be initiated by the handler. Symmetric to onClientAbort which now drives doClientAbort, the cancel-from-reply case in onServerReset must also drive doClientAbort alongside the 1-1 doClientReset propagation. * fix(binding-mcp): abort held-open app-initial on cancel coordinator doAppCancel coordinates a user-initiated cancel for an artificially held-open per-request app stream. Per the same held-open-needs-abort rule applied in the handler paths, switch the proactive initial-side close from doAppEnd to doAppAbort. Align the application/tools.list.canceled scripts: read closed -> read aborted on the upstream peer side, and write close -> write abort on the binding-app peer side. * refactor(binding-mcp): model bearer error as enum, consolidate challenge parsing Address review feedback on bearer challenge propagation: - mcp.idl: replace string16 error with McpBearerError enum modeling the 3 RFC 6750 codes (INVALID_REQUEST, INVALID_TOKEN, INSUFFICIENT_SCOPE); ignore non-standard error values - McpClientFactory: collapse three find-patterns into a single regex with optional named groups (realm, scope, error) in any order; hold a reusable Matcher on the factory and drive it via reset(input).matches(); default error to INVALID_TOKEN (401) / INSUFFICIENT_SCOPE (403) when absent - McpClientFactory: drop redundant ext.sizeof() guard before tryWrap; align header lookup with Optional.ofNullable + matchFirst + map style - McpClientIT: rename test to shouldRejectLifecycleInitializeWithBearerChallenge and remove extra blank line - McpFunctions + tests + bearer scripts updated to use enum value names * refactor(binding-mcp): track HTTP request body completion via McpState Replace the per-request requestSent boolean with the existing state field, folding the concept into McpState.initialOpened. McpStream.onAppBegin now sets only openingInitial; each stream sets openedInitial when its HTTP request body has been fully forwarded (immediately for lifecycle and zero-arg request streams, after brace-counted body completion for the parametrized streams, and on the elicitation abort/timeout/failure paths in toolsCall). deferAppEnd queries the bit instead of the field. * test(binding-mcp): align aborted/shutdown scripts with held-open-needs-abort Per the architectural invariant that artificially held-open initials must be closed via ABORT when the reply terminates abortively, update the peer-to-peer app-side scripts: - tools.list.aborted, prompts.list.aborted, resources.list.aborted: client.rpt write close -> write abort; server.rpt read closed -> read aborted - lifecycle.shutdown.requests (per-request promptsGet stream block): same flip for the cancel-on-shutdown path This matches the existing tools.list.canceled alignment and brings the McpServerIT and McpProxyIT abort scenarios green. * fix(binding-mcp): restore proactive doAppEnd in onNetEnd with replyClosed guard Per review feedback: - McpServerFactory.onNetEnd: restore proactive stream.doAppEnd alongside the closedInitial transition, so an early-completing upstream reply can unblock the held-open initial as soon as the inbound HTTP request body END arrives. - McpServerFactory.decodeNet end-of-buffer: same proactive call, using the same condition shape as onNetEnd. - McpRequestStream.doAppEnd: gate the close on replyClosed so the proactive call is a no-op until the natural reply-end path (McpRequestStream.onAppEnd, which now sets closedReply before calling doAppEnd) drives the close. doAppAbort stays unguarded -- the unclean close path matches the unclean termination signal. Aligns tools.call.elicit.declined scripts with the matching ABORT termination path: server.rpt read closed -> read aborted; client.rpt write close -> write abort. * fix(binding-mcp): gate McpProxyListFactory.doClientEnd on replyClosed Apply the same race-safety guard pattern as McpServerFactory.doAppEnd to the proxy's per-route held-open client-initial close. The proactive caller in onClientEnd already sets closedReply before invoking doClientEnd, so the guard passes trivially there; the McpListServer. onServerEnd 1-1 propagation path now defers the route's initial close until that route's reply has terminated, mirroring mcp(server). * refactor(binding-mcp): use StreamingJsonParser for request body completion Replace the hand-rolled byte-level brace counter in McpRequestStream (advanceParamsBraceDepth, paramsBraceDepth/paramsInString/paramsEscaped) with a JsonParser tracking structural depth via START_OBJECT/END_OBJECT and START_ARRAY/END_ARRAY events. Each request stream lazily creates a JsonParser from a dedicated factory (no path includes that match anything, so all VALUE events are non-readable -- this lets the tokenizer's internal resumeOp carry parse state across DATA frame boundaries without needing the InputStream to buffer unread bytes across wrap() calls). A shared per-worker DirectBufferInputStreamEx is wrapped fresh on each onAppData with the incoming payload. Brace counting was correct but duplicated logic the StreamingJsonParser already implements; this aligns the request-body completion detection with the response-decode pattern already used in McpHttpStream. * test(binding-mcp): consume bidirectional SUSPENDED challenge on resume scripts mcp(client) spontaneously emits a SUSPENDED challenge on the app-initial throttle when the upstream SSE connection drops -- in addition to whatever SUSPENDED the app peer issues. With only one ISSUE+OBSERVE pair on the SUSPENDED step, the binding-emitted challenge remained unconsumed and later blocked the FLUSH reads on the resume path with an unmatched write-advised event. Add a second SUSPENDED pair to both client.rpt and server.rpt so each side issues once and observes once: - client.rpt: read advise (issue) + write advised (observe) - server.rpt: write advised (observe) + read advise (issue) This keeps ApplicationIT peer-to-peer consistent (each issue matches the other side's observation) and lets McpClientIT consume the binding's spontaneous emission alongside the script's own issuance, fixing shouldCallToolWithProgressResume against the binding. * refactor(binding-mcp): split parser factories by direction in McpClientFactory Rename the shared JsonParserFactory and DirectBufferInputStreamEx fields to reflect which JSON-RPC direction they serve: - inputRO -> responseInputRO - parserFactory -> responseParserFactory (for inbound JSON-RPC response decode, wired with CLIENT_JSON_PATH_INCLUDES to make selected response paths readable) - paramsParserFactory -> requestParserFactory + paramsInputRO -> requestInputRO (for outbound JSON-RPC request body completion detection, wired with a non-matching PATH_INCLUDES so every value is non-readable and the tokenizer's internal resumeOp carries parse state across DATA frame boundaries) * refactor(common-json): null PATH_INCLUDES means include all, empty means include none Previously, an empty list (or omitted PATH_INCLUDES) both meant "include all values as readable". Distinguish the two so consumers can express "include nothing" cleanly without resorting to a non-matching sentinel path: - null pathIncludes (e.g. PATH_INCLUDES omitted from config) → include all - empty pathIncludes (e.g. PATH_INCLUDES = List.of()) → include none - non-empty list → restrict to matching paths StreamingJsonParser.pathList now lets null propagate; compilePaths returns null for null input; currentPathReadable treats null as include-all. binding-mcp McpClientFactory requestParserFactory switches from the List.of("$.NEVER_MATCH") sentinel to List.of() under the new semantics, and adds a blank line between static and non-static field groups. * refactor(binding-mcp): drop redundant state guards in McpProxyListFactory onXxx The engine guarantees that frames are not delivered on a stream direction after that direction has closed, so the !McpState.replyClosed(state) guards in onClientEnd, onClientAbort, and onClientReset are unreachable in practice. Removed. Also reorder doClientEnd condition to put the primary !initialClosed check first for readability, matching the response-decoder convention. * refactor(binding-mcp): adopt HttpRequestDecoder pattern for request body Per review feedback, model request body completion via the same decoder strategy pattern used for HTTP response decode rather than an ad-hoc method, so future request-side states can be added consistently. - Add @FunctionalInterface HttpRequestDecoder mirroring HttpResponseDecoder - Add decodeJsonRpcParamsBody (depth-tracking via the params parser) and decodeRequestEnd (terminal state) as decode method references on the factory - Add requestDecoder field on McpRequestStream; parametrised subclasses (McpToolsCallStream, McpPromptsGetStream, McpResourcesReadStream) initialise it to decodeJsonRpcParamsBody in their constructors; zero-arg request streams leave it null - Replace tryCompleteRequestBody / advanceParamsParser with a thin decodeRequestBody driver on McpRequestStream that delegates to the current decoder. On body completion the decoder transitions to decodeRequestEnd, sets state.openedInitial, and emits the HTTP request end exactly as before. * refactor(binding-mcp): drop redundant ext.sizeof() guards before tryWrap tryWrap returns null for empty/short extension buffers, so the explicit if (ext.sizeof() > 0) guard before calling it is dead weight. Removed from McpClientFactory's HttpInitializeStream onNetBegin, HttpEventStream onNetBegin, and the two McpStream onAppChallenge handlers. Same shape as the bearer-challenge path in r3299368553. * refactor(binding-mcp): fold deferAppEnd into HttpRequestDecoder state Lift requestDecoder up from McpRequestStream to McpStream and reduce deferAppEnd to a single base implementation that compares the field against the terminal decoder: requestDecoder == decodeRequestEnd. The McpRequestStream and McpToolsCallStream overrides go away. - McpStream.deferAppEnd: one-liner reference comparison - McpStream.onAppEnd: marks requestDecoder = decodeRequestEnd after the synchronous doEncodeRequestEnd so subsequent state queries are consistent - All synchronous request-end emitters (McpLifecycleStream and the zero-arg request streams' onAppBeginImpl, the authorised elicit path in McpToolsCallStream.onElicitCompleted) now transition the decoder to decodeRequestEnd alongside doEncodeRequestEnd - McpToolsCallStream replaces its deferAppEnd override with an onAppEnd override that handles the pendingAuth cleanup (cancel timeout, reset + abort), transitions requestDecoder, then calls super.onAppEnd; the elicit timeout signal and onElicitFailed paths likewise transition the decoder so super.onAppEnd's deferAppEnd skips a second request end Per review feedback, option (c): "fold 'request body sent' into the existing decoder-state transition and reduce deferAppEnd to a one-liner without overrides". * refactor: apply review naming/style feedback across binding-mcp + common-json binding-mcp/McpClientFactory: - Inline deferAppEnd() check at the McpStream.onAppEnd call site (single use, no remaining overrides since f11bb5a) - Rename @FunctionalInterface HttpRequestDecoder -> McpRequestDecoder - Rename the McpStream `requestDecoder` field to `decoder` for symmetry with the existing McpHttpStream.decoder field - Rename the SSE event-stream field `eventStream` -> `sse` as a parallel of `http`; rename the eventStreamRef() accessor -> sseRef() - Rewrite the HttpInitializeStream session-header lookup using the Optional.ofNullable(...).map(...).map(...).orElse(...) pattern that the bearer-challenge headers already use binding-mcp/McpProxyListFactory: - Break the doClientEnd guard across two lines after the && per style common-json/StreamingJsonTokenizer: - Refactor compilePaths via Optional.ofNullable(...).map(...).orElse(null) with helper compilePathList and compilePath methods using stream().map().toList() - Expose StreamingJsonTokenizer.INCLUDE_ALL as a public sentinel constant so test sites can read clearly as "include all" common-json/StreamingJsonTokenizerPathTest: - Use StreamingJsonTokenizer.INCLUDE_ALL in place of null literal * refactor(binding-mcp): apply naming/style review feedback McpProxyListFactory: - Rename per-item helper methods for consistency with sibling doEncodeBeginItems / doEncodeEndItems: streamItemBegin -> doEncodeBeginItem streamItemChunk -> doEncodeItemChunk streamItemEnd -> doEncodeEndItem McpServerFactory: - Rename eventStream field/method -> sse / sseRef as a parallel of http (same as McpClientFactory in 0070c60) - Break the doAppEnd guard across two lines after && for style symmetry with McpProxyListFactory.doClientEnd McpClientFactory.decodeJsonRpcParamsBody: - Replace local `done` boolean with a loop guard on the decoder state: `stream.decoder != decodeRequestEnd`. The decoder transition is already the terminal signal; using it directly removes the extra flag. * fix(binding-mcp): grant initial reply window for proxy list streams Two changes to McpProxyListFactory eliminate a race where the proxy could send a reply WINDOW with maximum=0 to upstream and never recover: - McpListClient.onClientBegin now propagates bufferPool.slotCapacity() as the minimum reply max instead of 0, so the first WINDOW back to upstream carries non-zero credit regardless of whether the hydrater has yet granted credit via the server propagation chain. - McpListServer constructor initializes replyMax to bufferPool.slotCapacity(), mirroring how McpListHydrateStream sets its own replyMax. This lets doEncodeItemChunk emit bytes to the hydrater before the hydrater's first WINDOW arrives at the server, which is consistent with the cache-side stream's assumption that the slot capacity is the agreed-on credit. Probes confirmed the first reply WINDOW upstream sees with this change is max=8192 (slot capacity) instead of max=0. Reduces but does not eliminate the McpProxyCacheIT.shouldHydrate100k flake; a separate race remains under investigation. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * fix(binding-mcp): emit decoded item bytes before breaking on parser EOF The decoder in McpProxyListFactory.decodeItemBody only emitted bytes to the server at the top of each loop iteration, before parser.hasNext(). When parser.hasNext() consumed bytes that did not yield a complete event (e.g., mid-string content in a long JSON value), the loop broke without emitting those consumed bytes — they remained in the client reply slot. In failing runs of shouldHydrate100k, the upstream flushed the 100k description value as a single 8192-byte DATA frame. The decoder advanced the parser through all 8192 bytes but emitted nothing, so the server never forwarded data to the hydrater, the hydrater never sent a WINDOW back, and the client never propagated a WINDOW upstream. Upstream's writableBudget stayed at 0 and the test timed out. In passing runs the same chunk arrived fragmented (8176 + 14 + 2 bytes across several frames), so a subsequent onClientData → decode() call emitted the parser-advanced bytes at the top of the next iteration. Emitting on the EOF break path makes the data path progress on the same decode() call regardless of how upstream fragments the payload. After this fix: 30/30 runs pass for McpProxyCacheIT.shouldHydrate100k (was ~50% pre-fix, ~60% after the earlier window-init fix in a92a482). https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * fix(binding-mcp): revert preset McpListServer.replyMax to avoid invariant violation The earlier change in a92a482 pre-set McpListServer.replyMax to bufferPool.slotCapacity() at construction. This violated the WINDOW invariant `maximum + acknowledge >= replyMax + replyAck` in onServerWindow whenever the downstream peer's first WINDOW carried a smaller maximum than slotCapacity — which is the case for non-cache McpProxyIT scenarios. The assertion failure terminated the engine worker: java.lang.AssertionError at McpProxyListFactory$McpListServer.onServerWindow(McpProxyListFactory.java:1180) The decoder-emit-on-EOF fix in 557e7b5 is sufficient on its own to make McpProxyCacheIT.shouldHydrate100k pass reliably (20/20 confirmed after this revert). The preset was unnecessary and only added to address what turned out to be a misdiagnosis. The McpListClient.onClientBegin fix from a92a482 is preserved — it addresses the genuine 0/0 reply window race on upstream first WINDOW. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * test(binding-mcp): @ignore flaky proxy abort tests under held-open invariant The script change in 86a4823 aligned per-request app-side abort scripts with the held-open-needs-abort invariant introduced for McpServer/McpClient streams (`write abort` → `read aborted` instead of `write close` → `read closed`). McpServerIT and McpClientIT exercise this cleanly because their bindings own the held-open initial directly. For McpProxyIT, the same script flows through the proxy, where abort propagation across the held-open per-request streams is timing-sensitive under the new invariant. The McpProxyListFactory.onClientAbort path proactively closes the upstream initial with doClientAbort, but in intermittent runs k3po observes the upstream stream's write-aborted event firing before the propagated read-aborted, presumably due to a synthetic reset firing on the throttle during channel close. Result: ~1 in 5 McpProxyIT full-suite runs fails on one of: - shouldAbortToolsList - shouldAbortListPrompts - shouldAbortListResources Other proxy abort tests (shouldAbortCallTool, shouldAbortGetPrompt, shouldAbortReadResource) are not affected because their factories use different stream models. @ignore these three to unblock CI; the underlying proxy abort propagation through held-open list streams is tracked for follow-up. The McpServerIT, McpClientIT, and ApplicationIT peer-to-peer variants of the same scripts continue to pass and exercise the invariant end-to-end. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * wip(binding-mcp): probe instrumentation for abort flake investigation Adds ProbeLog (buffered ring log in engine module) and probe calls in McpProxyListFactory + k3po ZillaTarget/ZillaStreamFactory to capture frame-level traces for shouldAbortListPrompts diff analysis. Investigation finding: McpListServer.onServerBegin emits the 12-byte JSON envelope prelude to the client before the upstream is opened, racing against the upstream-driven ABORT. When the prelude DATA wins, the client's `read abort` rejects DATA -> RESET on reply -> server script's `read aborted` observes [write] aborted instead. Revert or replace with a real fix that defers doEncodeBeginItems until the upstream has produced its BEGIN/DATA/FLUSH/END. * fix(binding-mcp): gate proxy list envelope on reply window via encoder strategy McpProxyListFactory's McpListServer used to emit the JSON envelope prelude ({"prompts":[ etc), per-item separators, and the postlude (]}) directly via doServerData without consulting the reply window. With replyMax=0 (which is the proxy's initial state before the client's first WINDOW frame arrives), those bytes were sent in violation of the flow-control invariant. Mirror the decoder strategy pattern with an encoder counterpart: - Add a private McpListServerEncoder @FunctionalInterface alongside McpListClientDecoder, with one state per output phase: encodePrelude, encodeItems, encodeSeparator, encodePostlude, encodeEnd, encodeIgnore. - The encoder state methods are private methods on the factory; the encoder field, the encode() loop, and the prelude/separator/postlude progress cursors all live on McpListServer so the symmetry with decode() on McpListClient is exact. - Each emit state pushes up to replyWindow bytes via doServerData and transitions when its constant buffer is fully flushed. encodeItems is a no-op that just gates item emission until prelude is done and transitions to encodePostlude when endItemsPending is set. - doEncodeItemChunk now refuses to emit while encoder != encodeItems so item bytes never overtake a pending separator. The existing JsonParser decoder backpressures naturally because doEncodeItemChunk already returns the bytes actually emitted. - onServerBegin no longer eagerly emits the prelude; onServerWindow drives encode(traceId) before pumping the decoder so prelude/separator/postlude flush as window credit arrives. - Reply-side termination paths (onServerReset, doServerAbort) flip the encoder to encodeIgnore so encode() becomes a no-op after the stream is closed. This eliminates one class of races where the speculative envelope bytes beat real upstream-driven frames to the client. The held-open abort tests remain @ignore'd; their flakiness is a separate coordination issue around RESET propagation order, not the flow-control violation fixed here. Also removes the probe instrumentation (ProbeLog + k3po extension overrides) that was committed earlier for analysis. * wip(binding-mcp): probe scaffolding for abort race investigation Re-introduces ProbeLog plus probe calls in McpProxyListFactory and the k3po test extension (ZillaTarget, ZillaStreamFactory) to capture frame-level events from both the proxy JVM and the k3po driver JVM. Used to identify why shouldAbortListPrompts still flakes with the encoder fix in place: proxy-side trace is deterministic, the race is on the k3po server side (server script's own write abort vs. the proxy-forwarded RESET reaching its reply/write side). Investigation only — revert or replace with the real fix. * wip(binding-mcp): restore @ignore after 1:1 propagation experiment The 1:1 per-direction propagation experiment (dropping cross-direction doClientAbort from onServerReset and onClientAbort) made the abort tests strictly worse (0/15) while cancel stayed 15/15, disproving the fan-out hypothesis. Reverted the McpProxyListFactory changes and restored the @ignore on the three abort tests; removed the trace-dump wrapper on shouldAbortListPrompts. Investigation scaffolding (ProbeLog + probes) still present. * fix(binding-mcp): propagate proxy list teardown per-direction; lazy envelope Make the proxy list teardown deterministic by propagating abort/reset on each direction independently instead of fanning a single inbound frame into both directions, and defer the JSON envelope until there is actually something to send. Changes in McpProxyListFactory (McpListServer / McpListClient): - onServerReset (client RESET on reply) now only forwards RESET on the upstream reply; it no longer also aborts the upstream initial. The initial teardown is driven by the client's own write abort propagating through onServerAbort. - onClientAbort (server ABORT on reply) now only forwards the abort downstream; it no longer injects an abort on the upstream initial. - The upstream client reference is detached only once McpState.closed(state) is true (both downstream directions closed), via detachClientIfClosed(), rather than being nulled on the first direction to close. Nulling early was severing the still-open initial so the client's write abort could not reach the server, which the injected cross-direction abort had been papering over. - Envelope emission is now lazy: the encoder starts in a new encodeWait state and only emits the "{"<key>":[" prelude when the first item is ready (doEncodeBeginItem) or on graceful end of an empty list (doEncodeEndItems). An upstream abort before any item therefore emits no envelope bytes — the client sees a clean abort instead of a partial envelope followed by abort. Spec scripts: the three list *.aborted application client scripts now use passive `read aborted` instead of active `read abort`, so each direction has exactly one active aborter and one passive observer (server-initiated reply abort, client-initiated initial abort). This removes the prior both-sides-active-on-reply conflict. Re-enables shouldAbortListPrompts, shouldAbortToolsList and shouldAbortListResources (previously @ignore'd as flaky); all four abort/cancel proxy scenarios are now deterministic. Also removes the ProbeLog investigation scaffolding. * refactor(binding-mcp): set encodeIgnore on reply close, drop encode() guard Address review feedback: every transition to replyClosed in McpListServer now sets encoder = encodeIgnore (added to doServerEnd; already present in doServerAbort and onServerReset), so the McpListServer.encode() loop reaches the terminal encodeIgnore state and emits nothing once the reply is closed. The explicit `if (McpState.replyClosed(state)) return;` guard in encode() is therefore redundant and removed. * feat(binding-mcp): render McpResetEx.bearer as HTTP 401/403 WWW-Authenticate Complete the bidirectional McpResetEx.bearer support: the MCP server now renders an inbound application RESET carrying McpResetEx.bearer back to its inbound HTTP peer as an HTTP 401/403 response with a WWW-Authenticate: Bearer header (RFC 6750) — the mirror of the MCP client's existing parse of an upstream 401/403 challenge into a RESET + McpResetEx.bearer. McpServerFactory: - doNetRejectBearer tryWraps reset.extension() as McpResetExFW; on KIND_BEARER it reads realm/scopes/error and renders via doNetBeginRejectedBearer (HTTP response BEGIN with :status + www-authenticate, then END), falling back to doNetReset otherwise. - bearerChallengeStatus maps INSUFFICIENT_SCOPE->403, else 401; bearerChallengeHeader formats Bearer realm/scope/error (lowercase RFC token). - McpRequestStream.onAppReset (per-request, e.g. tools/call) and McpLifecycleStream.onAppReset (initialize) both route through it. - Defer doEncodeInitialize until the lifecycle app reply BEGIN (initializePending flag) so an app RESET on initialize can render the challenge instead of the eagerly-sent 200. Specs: WWW-Authenticate header now carries error= for symmetry (server renders what the client parses). New tools.call.reject.bearer scenario (401/invalid_token) and updated lifecycle.initialize.reject.bearer (403/insufficient_scope) peer scripts are reused across client- and server-kind ITs. Tests: McpServerIT +2 (shouldRejectLifecycleInitializeWithBearerChallenge, shouldRejectToolsCallWithBearerChallenge), McpClientIT +1 (shouldRejectToolsCallWithBearerChallenge), NetworkIT/ApplicationIT +1 each. binding-mcp 177/177 and spec 150/150 green; checkstyle clean. * refactor(binding-mcp): drive initialize encode from onAppBegin via reply state Replace the McpLifecycleStream.initializePending boolean with the existing HTTP reply state. Since the MCP server net side is HTTP (one stream per exchange), the reply direction uniquely represents a single request's response: - doEncodeInitialize now fires unconditionally from McpLifecycleStream.onAppBegin (the app accepting the session is the trigger), instead of being gated by a flag set at request time. - McpLifecycleStream.onAppReset renders the bearer challenge / net reset only when no HTTP response has begun, guarded by !McpState.replyOpening(server.state) instead of the flag. No behavior change: replyOpening(server.state) is false exactly while the initialize 200 is still pending, matching the prior flag. McpServerIT 56/56, McpClientIT 50/50, checkstyle clean. * feat(binding-mcp): open newly-configured toolkits on aggregate event resume On aggregate event-id resume, open a lifecycle substream for every currently-configured toolkit route rather than only those named in the resume token. Orphaned token prefixes (toolkits removed from config since the id was minted) are dropped, while toolkits added since are opened fresh with no resume id so their notifications immediately join the aggregate cursor. Capabilities are already advertised as the union across all routes, so a resuming client otherwise believes a newly-added toolkit's tools exist yet never receives its list-changed notifications. Rework lifecycle.events.resume.partial.prefixed to a multi-backend server (bluesky resumes S=100->101; quartz opens fresh and emits 2=200) and assert the post-resume aggregate cursor becomes 2=200;S=101. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * feat(binding-mcp): breadth-first two-mode bearer challenge propagation Propagate an upstream McpResetEx bearer challenge to the client in both proxy modes. Without cache (MODE A), establish all configured toolkit lifecycle clients up front and withhold the aggregate BEGIN until every client replies; the first client to RESET with a bearer ext is relayed downstream as a RESET carrying the ext (rendered as HTTP 401/403) and the siblings are aborted. With cache (MODE B), upstream sessions stay deferred and the bearer challenge surfaces on the first request that forces an upstream session (e.g. tools/call). https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * feat(binding-mcp): render bearer reset on pending lifecycle SSE resume When a standalone lifecycle GET resumes with Last-Event-ID and the upstream app RESETs with a bearer extension, route the bearer challenge to the pending SSE GET reply (401/403 WWW-Authenticate) instead of the already-closed initialize stream. Open a window on the GET so the client reaches connected before reading the rejection. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): stage proxy list reply through a single encode slot Fold the prelude, item bytes, id-prefix injection, separators, and postlude of the mcp(proxy) list reply into one BufferPool encode slot on McpListServer, drained to the wire per reply window. The JSON decoder and framing encoders now append into the slot via encodeSlotAppend rather than writing to the wire directly, so drainEncodeSlot is the sole producer of reply DATA. Item back-pressure pauses on slot space (resumed on window via the existing decodedItemProgress cursor) instead of inline reply-window gating, preserving streaming of items larger than the slot capacity. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): fold proxy list encode into write-or-stash doServerData Replace the separate append/drain helpers on McpListServer with a single write-or-stash doServerData (mirroring HttpServerFactory.doNetworkData): emit what the reply window allows directly, stash the remainder into the encode slot. flushEncodeSlot re-flushes the slot on a new reply window, mirroring the decode(traceId) re-drive. doServerData returns the bytes accepted so the pull decoder pauses when the slot is full, since the id-prefix injection makes output exceed input. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): collapse proxy list encoder state machine into encode slot Now that the encode slot owns all reply back-pressure, the McpListServerEncoder state machine is redundant. Replace the functional interface, the encoder field, the encode(traceId) dispatch loop, and the seven encode* methods with inline sequencing: encodeFraming stages the lazy prelude and a pending client-boundary separator, doEncodeItemChunk emits item bytes only once framing is complete, and encodeEnd appends the postlude then closes once the slot drains. The replyClosed guard in encodeFraming replaces encodeIgnore. Behavior-preserving; reuses the existing prelude/separator/postlude progress cursors for partial-stash resume. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): rename encodeFraming to doEncodeFraming Consistent with the do* naming used for the other encode helpers. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): apply review naming feedback - resolveAllLifecycle → resolveAll - onResumeConfiguredRoutes → onServerResumeRoutes, hoist routedId/client locals - clearEventStream → clearSse; sse field private (was package-private with accessor) - decodeMax factory field replaces bufferPool.slotCapacity() at the upstream reply-window grant - flushEncodeSlot → encode; per-write local renamed emit → length, parameter renamed length → maxLength to match the HTTP convention https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): apply review feedback for proxy fan-out teardown - Wrap decoder→server dispatch through onDecodedItemBegin / onDecodedItemChunk / onDecodedItemEnd on McpListClient, instead of chasing client.server.doEncodeX pointers from the factory-level decode methods. - McpProxyListFactory.onClientReset: send a real RESET frame on the client reply via doClientReset instead of mutating closedReply state directly; the upstream reply direction is not implicitly closed by an inbound RESET on initial. - McpProxyLifecycleFactory.onClientBearerReset: drop the redundant !bearerRelayed guard on the relay path — doServerReset already short-circuits on initialClosed, so the outer flag is dead defense. The flag itself stays for the success-path doServerBeginDeferred gate. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): remove bearerRelayed flag, gate success path on state doServerBeginDeferred is now gated on !initialClosed && !replyClosed, which captures exactly the post-bearer-relay state (doServerReset sets closedInitial) without needing a separate flag. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ * refactor(binding-mcp): inline onClientClosed at each McpListServer state-close site Drop the detachClientIfClosed helper and inline the check at the state mutation that produces the second-direction close. After each state = McpState.closedInitial(state) (onServerEnd, onServerAbort) or state = McpState.closedReply(state) (onServerReset, doServerEnd, doServerAbort), fire onClientClosed(traceId) when McpState.closed(state) holds. onClientError no longer needs the explicit detach since the doServerAbort it invokes now carries the check inline. Matches the pattern used by other bindings. https://claude.ai/code/session_01UMAqxehMetkX17zs3NGRwQ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9b2aef8 commit 530e4ba

197 files changed

Lines changed: 3051 additions & 774 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

runtime/binding-mcp/src/main/java/io/aklivity/zilla/runtime/binding/mcp/internal/config/McpBindingConfig.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,20 @@ else if (identifier != null)
156156
return resolved;
157157
}
158158

159+
public List<Long> resolveAll(
160+
long authorization)
161+
{
162+
final List<Long> result = new ArrayList<>();
163+
for (McpRouteConfig route : routes)
164+
{
165+
if (route.authorized(authorization))
166+
{
167+
result.add(route.id);
168+
}
169+
}
170+
return result;
171+
}
172+
159173
public List<McpRouteConfig> resolveAll(
160174
McpBeginExFW beginEx,
161175
long authorization)

0 commit comments

Comments
 (0)