Skip to content

feat(binding-mcp): propagate upstream Bearer auth challenges and defer per-request app stream end#1796

Merged
jfallows merged 45 commits into
developfrom
feature/1795-mcp-bearer-reset
May 28, 2026
Merged

feat(binding-mcp): propagate upstream Bearer auth challenges and defer per-request app stream end#1796
jfallows merged 45 commits into
developfrom
feature/1795-mcp-bearer-reset

Conversation

@jfallows
Copy link
Copy Markdown
Contributor

@jfallows jfallows commented May 24, 2026

Summary

Closes #1795.

  • Propagate upstream HTTP Bearer auth challenges (401/403 with WWW-Authenticate: Bearer) from mcp(client) back to the inbound peer as a RESET with McpResetEx.bearer (realm, scopes, error per RFC 6750), and render that same McpResetEx.bearer at mcp(server) as an HTTP 401/403 WWW-Authenticate: Bearer response — bidirectional support.
  • Establish the architecture invariant that per-request MCP application streams hold initial open until the reply terminates, with end deferred (rather than sent proactively) on cancel/shutdown/normal close.

Architecture invariants

  • mcp(client) is proactive: HTTP request BEGIN is sent on app BEGIN; body is streamed as DATA arrives; HTTP request End is sent based on body-completion detection (JSON brace counting for variable-arg methods, immediate for zero-arg methods) — never waits for app END.
  • mcp(server) holds initial open: app-initial stays open until upstream's reply terminates (END, ABORT, or RESET); doesn't close just because inbound HTTP body is done.
  • mcp(proxy) holds initial open: fan-out routes defer doClientEnd until upstream's reply ends, mirroring mcp(server). For cancel: doClientReset precedes doClientEnd on each route.
  • Both directions independent: response can flow back before request body finishes; RESET-with-extension can flow back at any time while initial is open. This is what allows the bearer auth-challenge RESET to propagate cleanly from upstream through mcp(client) / mcp(proxy) to the inbound peer.

Changes

Bearer challenge propagation (commit 1)

  • mcp.idl: add McpResetEx union with McpBearerResetEx { realm, scopes, error }.
  • McpFunctions: builder/matcher for bearer reset ex.
  • McpClientFactory.McpHttpStream.onNetBegin: on 401/403 with Bearer WWW-Authenticate, parse realm/scope/error per RFC 6750 and emit RESET + McpResetEx.bearer on the app initial; default error to insufficient_scope for 403 and invalid_token for 401 when upstream doesn't supply it.
  • New lifecycle.initialize.reject.bearer scripts (network + application) and matching NetworkIT/ApplicationIT/McpClientIT entries.

Hold-open / proactive refactor (commit 2)

  • McpServerFactory: defer stream.doAppEnd out of onNetEnd / decodeNet end-of-buffer; close app-initial only when upstream's reply has terminated (McpRequestStream.onAppEnd / onAppAbort).
  • McpClientFactory: lift requestSent / paramsBraceDepth / paramsInString / paramsEscaped and tryCompleteRequestBody(data) from McpToolsCallStream to McpRequestStream. McpToolsListStream / McpPromptsListStream / McpResourcesListStream override onAppBeginImpl to send HTTP request End immediately (zero-arg). McpPromptsGetStream / McpResourcesReadStream override onAppData to drive brace counting via tryCompleteRequestBody.
  • McpProxyListFactory.onNextClient: remove the if (McpState.initialClosed(state)) guard around doClientEnd.
  • Move write close (client) to AFTER read closed and read closed (server) to AFTER write close for tools.call, tools.list, prompts.get, prompts.list, resources.read, resources.list (plus .10k, .100k, .with.progress, .elicit.* variants and lifecycle.shutdown). .aborted variants drop the post-abort close handshake.

Cancel cleanup (commit 3)

  • McpServerFactory.doAppCancel: send doAppReset (RESET on app-reply) BEFORE doAppEnd (END on app-initial). Previously doAppCancel only sent RESET, leaving app-initial dangling after the hold-open refactor.

Universal invariant across all per-request scripts (commit 4)

  • 102 .rpt files transformed. Per-request blocks identified by .toolsList(), .toolsCall(), .promptsGet(), .promptsList(), .resourcesRead(), .resourcesList() BEGIN extension. Lifecycle blocks (.lifecycle()) excluded.
    • client.rpt: write close moved from before reads to AFTER read closed/read abort/read aborted.
    • server.rpt: read closed moved from before writes to AFTER write close/write aborted.
  • 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).

Server-side bearer rendering (later commits)

  • McpServerFactory: render an inbound app RESET carrying McpResetEx.bearer as an HTTP 401/403 response with WWW-Authenticate: Bearer (doNetRejectBearer / doNetBeginRejectedBearer); status from McpBearerError (403 insufficient_scope, else 401). The initialize 200 is encoded from McpLifecycleStream.onAppBegin (app accept), and the reject challenge is rendered from onAppReset gated on !McpState.replyOpening(server.state) (HTTP reply not yet begun). New tools.call.reject.bearer (401/invalid_token) scenario; lifecycle.initialize.reject.bearer header now carries error= for symmetry; peer scripts reused across client- and server-kind ITs (McpServerIT, McpClientIT, NetworkIT, ApplicationIT).
  • McpProxyLifecycleFactory: relay an upstream bearer RESET across fan-out routes — onClientReset forwards reset.extension() via onClientBearerResetdoServerReset(traceId, extension) (first bearer wins by !McpState.replyOpened(state), sibling routes aborted), so a bearer challenge traverses a proxy (5d1283d1).
  • McpServerFactory (McpEventStream): when a standalone lifecycle SSE GET resumes with Last-Event-ID and the upstream app RESETs with McpResetEx.bearer, render the 401/403 WWW-Authenticate on the pending SSE GET reply (McpEventStream.doNetRejectBearer, opening a window first so the client reaches connected) rather than the already-closed initialize stream; onAppReset routes to the pending sse when the main reply has already opened. New lifecycle.events.resume.reject.bearer scenario (McpServerIT + NetworkIT/ApplicationIT).

Proxy list reply path (later commits)

  • McpProxyListFactory: per-direction abort/cancel teardown (no cross-direction fan-out). The upstream client is detached only once McpState.closed(state), via an inline if (McpState.closed(state)) onClientClosed(traceId); at each state-close site in McpListServer (onServerEnd/onServerAbort/onServerReset/doServerEnd/doServerAbort). Reply envelope is lazy — doEncodeFraming only stages the prelude once items exist or a graceful empty end, so an abort-before-items emits no partial envelope. The three previously-@Ignored held-open *.aborted proxy scenarios are now deterministic; *.aborted client scripts use passive read aborted.
  • Reply DATA flows through a single BufferPool encode slot on McpListServer. doServerData is a write-or-stash emit (mirroring HttpServerFactory.doNetworkData): writes what the reply window allows directly, stashes the remainder, returns the bytes accepted. encode(traceId) is the slot drain (mirroring decode(traceId) from the decode side), called from onServerWindow. The prelude, per-client separator, postlude, id-prefix injection, and item bytes all flow through doServerData, so the drain is the sole producer of reply DATA. The decoder pauses on slot space (retaining pending bytes via the existing decodedItemProgress cursor in the decode slot), preserving streaming of items larger than the slot capacity.
  • Decoder→server dispatch goes through McpListClient.onDecodedItemBegin / onDecodedItemChunk / onDecodedItemEnd rather than reaching through client.server.doEncodeX pointers.

Test plan

  • specs/binding-mcp.spec — peer-to-peer scripts consistent (NetworkIT 59, ApplicationIT 94, plus ProxyCacheIT, SchemaTest, McpFunctionsTest)
  • runtime/binding-mcp — all green
    • McpServerIT 57/57 ✓ (incl. server-side bearer rendering + lifecycle SSE resume bearer)
    • McpClientIT 50/50 ✓ (shouldCallToolWithProgressResume now passing 6/6)
    • McpProxyIT 44/44 ✓ (held-open abort/cancel scenarios deterministic; previously-@Ignored abort tests re-enabled; encode-slot refactor behavior-preserving)
    • McpProxyCacheIT 24/24 ✓
    • McpProxyLifecycleIT 7/7 ✓

https://claude.ai/code/session_01BYNJLhjKpXGDVtuNgYysgy


Generated by Claude Code

claude and others added 7 commits May 24, 2026 00:09
…esetEx

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.
 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.
…anceled 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
…r-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
…bort/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
…nd 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.
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.
claude added 8 commits May 25, 2026 18:15
…nge 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
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.
…s-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.
…osed 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.
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).
…etion

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.
…e 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.
…ntFactory

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)
…ans 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.
claude added 4 commits May 25, 2026 22:43
…tory 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.
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.
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.
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".
…mon-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
}
}

private void streamItemBegin(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check naming for consistency, should this be doEncodeBeginItem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed in c8296d4f to doEncodeBeginItem.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check naming for consistency, should this be doEncodeItemChunk?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed in c8296d4f to doEncodeItemChunk.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check naming for consistency, should this be doEncodeEndItem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed in c8296d4f to doEncodeEndItem.


Generated by Claude Code

long authorization)
{
if (!McpState.initialClosed(state))
if (McpState.replyClosed(state) && !McpState.initialClosed(state))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (McpState.replyClosed(state) && !McpState.initialClosed(state))
if (!McpState.initialClosed(state) &&
McpState.replyClosed(state))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c8296d4f.


Generated by Claude Code


state = McpState.closedReply(state);

if (eventStream != null)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (eventStream != null)
if (sse != null)

Same feedback for other classes, rename eventStream to sse.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed eventStreamsse and eventStreamRef()sseRef() throughout McpServerFactory in c8296d4f.


Generated by Claude Code

stream.paramsParser = requestParserFactory.createParser(input);
}
final JsonParser parser = stream.paramsParser;
boolean done = false;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to mark the stream as "closing" via McpState here, instead of the done flag?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c8296d4f. Replaced the local done boolean with a guard on the decoder transition itself: the loop now reads while (stream.decoder != decodeRequestEnd && parser.hasNext()). The terminal stream.decoder = decodeRequestEnd assignment inside the END_OBJECT/END_ARRAY branch is what exits the loop, so the extra flag is gone.


Generated by Claude Code

claude added 4 commits May 26, 2026 00:08
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.
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
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
…iant 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
claude added 6 commits May 26, 2026 06:42
…variant

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
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.
…r 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.
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.
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.
…nvelope

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.
Comment on lines +1295 to +1302
if (encoder != encodeItems)
{
encode(traceId);
if (encoder != encodeItems)
{
return 0;
}
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests we need to introduce the encoder slot, so we can stage the bytes into the slot buffer, up to encodeMax (= bufferPool.slotCapacity()) and then write out based on available client window, instead of trying to have both an encoder abstraction and also write out these items directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed this is the right end state — fold prelude/items/separator/postlude into a single encode-slot (staged up to bufferPool.slotCapacity(), drained to the wire per client window) so the encoder owns all output instead of mixing the state machine with direct doEncodeItemChunk writes. Deferring it to a follow-up rather than bundling into this PR, since it's a sizable refactor of the decode→encode item path (the JSON decoder would write items into the slot instead of straight to the wire) and warrants its own review. The current per-direction teardown + lazy-prelude fix here is complete and the abort/cancel scenarios are deterministic; the slot refactor can land on top without changing observable behavior.


Generated by Claude Code

claude added 10 commits May 26, 2026 21:45
… 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.
…nticate

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.
…ply 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.
…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
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
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
… 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
…erverData

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
… 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
return resolved;
}

public List<Long> resolveAllLifecycle(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public List<Long> resolveAllLifecycle(
public List<Long> resolveAll(

Comment on lines +274 to +277
if (!clients.containsKey(route.routedId()))
{
supplyClient(route.routedId()).doClientBegin(traceId);
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!clients.containsKey(route.routedId()))
{
supplyClient(route.routedId()).doClientBegin(traceId);
}
final long routedId = route.routedId();
if (!clients.containsKey(routedId))
{
final McpLifecycleClient client = supplyClient(routedId);
client.doClientBegin(traceId);
}

}
}

private void onResumeConfiguredRoutes(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private void onResumeConfiguredRoutes(
private void onServerResumeRoutes(

{
if (!bearerRelayed && !McpState.replyOpened(state))
{
bearerRelayed = true;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need this flag?
If we abort the initial stream of each client, then we cannot get a client reset on the initial stream of any client.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this flag, and for the success case verify that initial is still opened / reply is not closed before sending the begin?

return accepted;
}

private void flushEncodeSlot(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private void flushEncodeSlot(
private void encode(

case START_OBJECT:
client.decodedItemProgress = decodedItemProgress - 1;
client.server.streamItemBegin(traceId);
client.server.doEncodeBeginItem(traceId);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we don't chase pointers like this.
Instead we call an onDecodedXxx method on client, and client then calls server.doXxx in reaction.

Same for encode item chunk etc.

{
client.doClientEnd(traceId);
}
detachClientIfClosed();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels wrong.

Instead each time we update the replyClosed or initialClosed state, we check for if closed and then call onClientClosed, which can do the client detach as needed without this special case.

if (encodeSlot == NO_SLOT)
{
final int replyWin = replyMax - (int) (replySeq - replyAck) - replyPad;
final int emit = Math.min(Math.max(replyWin, 0), length);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, we use length for this typically.
Replace similar occurrences of emit with length in this and other map factory classes.

}

@Override
void clearEventStream()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void clearEventStream()
void clearSse()

final McpLifecycleStream session;
final int requestId;
HttpEventStream eventStream;
HttpEventStream sse;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it both package private and have an accessor?

claude added 4 commits May 28, 2026 02:47
- 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
- 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
…n 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
…ate-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
Copy link
Copy Markdown
Contributor Author

@jfallows jfallows left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jfallows jfallows merged commit 530e4ba into develop May 28, 2026
143 of 146 checks passed
@jfallows jfallows self-assigned this May 28, 2026
jfallows pushed a commit that referenced this pull request May 28, 2026
…er TLS

Drops the local github-mcp-server docker service in favor of GitHub's
public Streamable HTTP endpoint at https://api.githubcopilot.com/mcp/.
Adds a dedicated http→tls→tcp chain for the github route (TLS sni
api.githubcopilot.com, trustcacerts for public CA verification, ALPN
http/1.1, TCP 443). The everything and time routes keep their existing
plain http→tcp path unchanged.

Auth-challenge pass-through still works: an unauthenticated call returns
GitHub's HTTP 401 / WWW-Authenticate: Bearer through Zilla's mcp(server)
which renders the same 4xx + WWW-Authenticate back to the inbound client,
courtesy of the McpResetEx bearer propagation that landed in #1796.
jfallows pushed a commit that referenced this pull request May 28, 2026
…er TLS

Drops the local github-mcp-server docker service in favor of GitHub's
public Streamable HTTP endpoint at https://api.githubcopilot.com/mcp/.
Adds a dedicated http→tls→tcp chain for the github route (TLS sni
api.githubcopilot.com, trustcacerts for public CA verification, ALPN
http/1.1, TCP 443). The everything and time routes keep their existing
plain http→tcp path unchanged.

Auth-challenge pass-through still works: an unauthenticated call returns
GitHub's HTTP 401 / WWW-Authenticate: Bearer through Zilla's mcp(server)
which renders the same 4xx + WWW-Authenticate back to the inbound client,
courtesy of the McpResetEx bearer propagation that landed in #1796.
jfallows pushed a commit that referenced this pull request May 28, 2026
…er TLS

Drops the local github-mcp-server docker service in favor of GitHub's
public Streamable HTTP endpoint at https://api.githubcopilot.com/mcp/.
Adds a dedicated http→tls→tcp chain for the github route (TLS sni
api.githubcopilot.com, trustcacerts for public CA verification, ALPN
http/1.1, TCP 443). The everything and time routes keep their existing
plain http→tcp path unchanged.

Auth-challenge pass-through still works: an unauthenticated call returns
GitHub's HTTP 401 / WWW-Authenticate: Bearer through Zilla's mcp(server)
which renders the same 4xx + WWW-Authenticate back to the inbound client,
courtesy of the McpResetEx bearer propagation that landed in #1796.
jfallows pushed a commit that referenced this pull request May 28, 2026
…er TLS

Drops the local github-mcp-server docker service in favor of GitHub's
public Streamable HTTP endpoint at https://api.githubcopilot.com/mcp/.
Adds a dedicated http→tls→tcp chain for the github route (TLS sni
api.githubcopilot.com, trustcacerts for public CA verification, ALPN
http/1.1, TCP 443). The everything and time routes keep their existing
plain http→tcp path unchanged.

Auth-challenge pass-through still works: an unauthenticated call returns
GitHub's HTTP 401 / WWW-Authenticate: Bearer through Zilla's mcp(server)
which renders the same 4xx + WWW-Authenticate back to the inbound client,
courtesy of the McpResetEx bearer propagation that landed in #1796.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

binding-mcp: propagate upstream auth challenges to inbound client

2 participants