Serve repeat initialize from a cached result

[amirejaz]

Summary

A stdio MCP server is one process and therefore exactly one MCP session, while the streamable proxy fans an arbitrary number of HTTP client sessions onto it. Every client's initialize was forwarded to that single session, so only the first could ever succeed — go-sdk v1.7+ rejects a second handshake with duplicate "initialize" received.

Behind vMCP this is worse than the reported symptom. #5890 reports the health check failing every 30s, but every vMCP call verb builds a fresh client and handshakes (legacyCallTool at pkg/vmcp/client/client.go:1681-1692, and the resource/prompt paths at :1890, :2008, :2149). Once the aggregator claims the single handshake at startup, tool calls fail too — so the ping-based health check suggested in #5890 would have made the backend report healthy while remaining unusable.

• Cache the InitializeResult from the first handshake and answer later ones locally. This restores what go-sdk v1.6.1 and earlier did when replaying a repeated initialize, but places it in the component that actually owns the multiplexing rather than asking every MCP server to tolerate duplicate handshakes.
• Single-flight the first handshake. The cache lock is held across the first upstream round trip, so concurrent clients wait and then read the cache instead of racing a second initialize to the backend. It is bounded by forwardUpstream's requestTimeout, the same reasoning uriLocks relies on for resources/subscribe.
• Never cache errors, so a transient backend failure cannot pin every later client to one bad response.
• Swallow notifications/initialized after the first. This is not defensive guesswork: go-sdk rejects a repeated initialized exactly as it rejects a repeated initialize (mcp/server.go:1424 vs :1978). Later ones are still acknowledged with 202, which is all a notification sender expects.

Interception hooks into the existing interceptSessionScopedRequest switch, alongside the resources/subscribe ref-counting and logging/setLevel reconciliation that already do this shape of fan-in mediation, and reuses writeInterceptedResponse so a cached answer is indistinguishable from a forwarded one (session header included).

Closes #5890
Closes #1982

Type of change

• Bug fix
• New feature
• Refactoring (no behavior change)
• Dependency update
• Documentation
• Other (describe):

Test plan

• Unit tests (task test)
• Linting (task lint-fix)
• Manual testing (described below)

Test	Without the fix
TestEveryClientHandshakeSucceedsAgainstStrictBackend	Fails with duplicate "initialize" received on clients 2 and 3
TestConcurrentInitializeReachesBackendOnce	Fails with the same error on 4 of 5 concurrent clients
TestDuplicateInitializedNotificationIsNotForwarded	Fails — 3 notifications reach the backend instead of 1
TestFailedInitializeIsNotCached	Passes either way — guard test, see below

The fake backend reproduces go-sdk v1.7+ session semantics rather than accepting everything, so these assert the outcome that matters -- a later client's handshake succeeds -- and fail with the literal error from #5890 when the cache is removed, not merely with a mismatched message count. I verified that by reverting the production change and re-running.

TestFailedInitializeIsNotCached is deliberately a guard rather than a regression test: it exercises a backend that fails the first handshake and succeeds on the second, which passes with or without a cache. It exists to pin the "errors are not cached" invariant against future changes.

All existing pkg/transport/... tests pass with -race, including the streamable proxy's MCP client integration test which performs a real handshake through this path. task lint-fix is clean on the changed files; the one remaining finding (cmd/thv/app/upgrade.go gosec G115) is pre-existing and untouched here.

Manual verification against the reported backend

Built thv from this branch and from main, and ran the exact setup from #5890 -- thv run github, which resolves to ghcr.io/github/github-mcp-server:v1.6.0 with transport: stdio. Three independent client handshakes, none carrying a session ID:

	client 1	client 2	client 3
main (v0.41.0-9-g33e06f4f5)	OK	duplicate "initialize" received	duplicate "initialize" received
this branch (v0.41.0-11-ga5303d5d3)	OK	OK	OK

The main run reproduces the reported failure verbatim, so the passing result is attributable to this change rather than to v1.6.0 being tolerant of repeat handshakes.

Also confirmed the cached handshake yields a genuinely usable session rather than just a well-formed response: client 2 sent notifications/initialized (HTTP 202) and then tools/list, which returned all 44 tools. The result payload is byte-identical (4238 bytes, valid JSON) across all three of main-forwarded, branch-forwarded and branch-cached.

Does this introduce a user-facing change?

Yes. Multiple clients can now connect to a single stdio MCP server through ToolHive without the second and subsequent handshakes failing, verified against github-mcp-server v1.6.0.

Yes. Multiple clients can now connect to a single stdio MCP server through ToolHive without the second and subsequent handshakes failing, and vMCP can aggregate stdio backends that reject repeat handshakes. Verified end to end against the exact stack from #5890 -- vMCP v0.40.1 with github-mcp-server v1.6.0 over stdio -- where the backend previously went unhealthy and degraded the whole gateway.

Special notes for reviewers

What is and isn't shared. Worth being explicit, since this is the design question that kept #1982 open:

• Server capabilities in the replayed result don't vary by client — replaying them is simply correct.
• Client capabilities (roots, sampling, elicitation) are frozen to the first client's, but inert: server-initiated requests are already rejected outright by this proxy (dispatcher.go:179), so nothing observable depends on them.
• The negotiated protocol version is the one genuinely shared, client-visible value. A client that asked for a different version receives the first client's, and per the MCP lifecycle spec should disconnect if it can't speak it. This is inherent to collapsing N client sessions onto one backend session.

Known limitation — a backend process replaced underneath the proxy. The cache is not invalidated on container re-attach. That path (pkg/transport/stdio.go) only fires when the container is still running, i.e. the same live process whose session is still valid — clearing the cache there would break the common case by sending a second initialize to an already-initialized session. Distinguishing a genuinely new process needs a signal the proxy doesn't currently have (container start time or restart count), so a container restarted underneath a running proxy still requires a workload restart. Called out rather than papered over; happy to file a follow-up.

Verified against the reported stack. #5890 was reproduced exactly and then confirmed fixed, using vMCP v0.40.1 (the reporter's version) against github-mcp-server v1.6.0 over stdio:

	unfixed proxy	fixed proxy
duplicate "initialize" received	12	0
health checks succeeded	0	8
health checks failed	8	0
gateway phase	Failed	Ready

Note for anyone reproducing on current main. #5890 does not reproduce with a vMCP built from main: the dual-era revision probing added since v0.40.1 changes the call path so the duplicate handshake never occurs. On main the same backend instead fails roughly half its health checks with failed to modern discover ... backend returned a Legacy-shaped body, flip-flopping the cached revision between 2026-07-28 and 2025-11-25. That is a separate bug, unaffected by this change, filed separately. Reproducing #5890 requires vMCP v0.40.1.

Scope.
Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.60%. Comparing base (a48448d) to head (eab13ac).

Files with missing lines	Patch %	Lines
pkg/transport/proxy/streamable/initialize_cache.go	80.00%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6153      +/-   ##
==========================================
+ Coverage   72.59%   72.60%   +0.01%     
==========================================
  Files         736      737       +1     
  Lines       76359    76395      +36     
==========================================
+ Hits        55435    55470      +35     
+ Misses      16981    16973       -8     
- Partials     3943     3952       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jerm-dro]

Apologies — I should have elaborated more on my initial review rather than leaving this to a second pass.

One structural suggestion inline on initialize_cache.go: the synchronization can be simpler than singleflight-plus-mutex, which also collapses the flight's return type from three to one and removes the id-rewriting hazard by construction. Behaviour should be unchanged.

Separately, the description's "the cache lock is held across the first upstream round trip, so concurrent clients wait and then read the cache" bullet describes the pre-singleflight revision and now contradicts the comment at initialize_cache.go:49. TestConcurrentInitializeReachesBackendOnce's doc comment has the same drift.

[jerm-dro]

Suggestion: drop the mutex and let the callback be the only place that touches the cache.

result needs a lock only because it's also read on the fast path above, outside the flight. singleflight runs at most one callback per key at a time and hands that callback's return value to every waiter, so a field touched only inside the callback never has overlapping accesses — the re-check you already have becomes the only cache read:

v, err, _ := c.flight.Do(initializeFlightKey, func() (any, error) {
	// A handshake already succeeded: replay it, no backend round trip.
	if c.result != nil {
		return handshake{result: c.result}, nil
	}

	// Detached from the caller's context: this result belongs to every waiter,
	// so whichever caller performs the forward must not cancel the handshake
	// for the others by disconnecting. forwardUpstream still bounds it with
	// p.requestTimeout.
	msg, ferr := p.forwardUpstream(context.WithoutCancel(ctx), sessID, req)
	if ferr != nil {
		return nil, ferr // nothing cached; the next client retries for real
	}

	resp, ok := msg.(*jsonrpc2.Response)
	if !ok {
		return nil, errUnexpectedInitializeReply
	}
	if resp.Error != nil {
		// Shared with this flight's waiters but deliberately not cached, so a
		// transient failure cannot pin every future client to one response.
		return handshake{rpcErr: resp.Error}, nil
	}
	if len(resp.Result) == 0 {
		return nil, errUnexpectedInitializeReply // no result and no error
	}

	c.result = resp.Result
	return handshake{result: resp.Result}, nil
})
if err != nil {
	return upstreamErrorResponse(req.ID, err)
}
h := v.(handshake) // the callback returns a handshake or a non-nil error
return &jsonrpc2.Response{ID: req.ID, Result: h.result, Error: h.rpcErr}

handshake is just struct{ result json.RawMessage; rpcErr error } — the id-free half of the outcome. Sharing that instead of a whole *jsonrpc2.Response means each caller builds its own reply, so switch out := v.(type) and its default: branch both go: a waiter can no longer receive the forwarding caller's JSON-RPC id, by construction rather than by a branch catching it.

That leaves mu guarding nothing, so it plus cachedResult() and storeResult() can go. initializedForwarded wants atomic.Bool with CompareAndSwap(false, true) rather than the flight — singleflight de-duplicates rather than serializes, so two concurrent claims would share one callback's answer and both forward. Worth a comment on result stating that only the callback may touch it: that rule is what replaces the lock, and a leftover accessor is what a future caller would reach for from outside.

Behaviour should be unchanged and the existing tests should stay green as written — worth a -race run to confirm. One intentional exception: a reply with no error and an empty result currently forwards verbatim and would now become an internal error. That's malformed for initialize, so I think erroring is better, but it is a change.