mcp-client: stateful MCP server restart leaves every tool call failing until DSH restarts

[gkinter]

Symptom

After a stateful streamable-HTTP MCP server restarts, every tool call from a DSH instance that was connected before the restart fails until the DSH instance itself restarts:

Error POSTing to endpoint: {"jsonrpc":"2.0","error":{"code":-32000,"message":"Bad Request: No valid session ID provided"},"id":null}

Cause

The restarted server does not know the client's mcp-session-id. The MCP spec answers with HTTP 404 and requires the client to start a new session; common servers answer 400 "No valid session ID provided". Neither closes the client transport: the SDK throws a generic non-ok-POST error for the request and reports the dead GET stream through onerror. dsh-mcp-client reconnects only from generation.onclose, so no reconnect ever runs and every tool closure stays bound to the dead generation.

Proposal

Detect session loss narrowly (HTTP 404 when the request carried a session id, or a 400 whose body says the session id is missing, invalid, unknown or expired; any other 400 is the request's own fault and surfaces unchanged). On a hit, close the current generation through the supervisor's own close so the existing onclose to reconnect path builds the next one, wait (bounded) for the next connected and re-synced generation, and retry the failed request once. Concurrent failures share one recycle, aborts are never retried, and the final error names the lost session and the reconnect outcome.

We run this as an out-of-tree patch against the built lib/index.js with a pure detector plus a test harness around the supervisor; happy to port it to packages/mcp/mcp-client/src with specs in your style if the design is acceptable. I have not opened a branch yet because it touches the connection supervisor and I would like to confirm the direction first.

[MaxHou-infinity]

Independent corroboration from a separate reproduction, plus three data points that may help whoever implements this.

1. The newest published build does not contain a fix. @deepseek-ai/dsh-mcp-client@0.2.0-rc.2 and @deepseek-ai/dsh-mcp-client@0.2.1-alpha.4 ship a byte-identical lib/index.js:

sha256  758d8fc473b700b5d351158d06ce9f9e92c16e8b25497b08bb7bd2122a87f5dd
$ grep -c onerror lib/index.js
0

So the gap is present in the newest published build, not only the rc.

2. A discriminator confirming the client retains a dead id rather than omitting it. Against the same endpoint after the server restarted — for the server I tested:

request	response
POST, no mcp-session-id header	422 Unexpected message, expect initialize request
POST, stale mcp-session-id	404 Not Found: Session not found
POST after a fresh initialize	200 — tools list and calls work

Relevant to the detector: on this server a 404 is only ever produced when a session id was sent, so the 404 arm does not need to inspect the outgoing request to tell "session lost" from "client never had a session". (Your 400 arm still needs the body check, since that status is shared with ordinary bad requests.)

3. An asymmetry in the supervisor worth stating explicitly in whatever lands. A failed connect attempt does reach the supervisor — settleFailedGeneration → generationDown → scheduleReconnect — so a server that is unreachable at startup keeps being retried up to the attempt budget. A session invalidated on an already-established connection takes a completely different path and never reaches it. That contrast is the whole bug in one line, and it is why this cannot be addressed by tuning the existing retry policy.

4. Impact note. The MCP bridge is process-scoped — one plugin instance per Host, shared by every conversation — so starting a new conversation does not recover; only restarting the Host does. Worth reflecting in the final error text, since a per-call failure currently reads as "the server is broken" when the server is healthy.

Thanks for the write-up — the proposal is more complete than anything I had. If a ported patch lands, I am happy to exercise it against a long-running Host with a server that restarts underneath it.

[MaxHou-infinity]

Following up with a working implementation, published as an out-of-tree patch since this repo does not take PRs:

https://github.com/MaxHou-infinity/dsh-mcp-session-recovery

Two things that may be worth having before this lands natively.

An A/B control, rather than a test that merely passes. The harness mounts the real dsh-mcp-client against a fake Streamable HTTP server that clears its session table mid-flight, then asserts whether the client recovers on its own. Same test, two targets — pristine and patched:

	pristine	patched
before the restart	ok (gen1)	ok (gen1)
first call after	404 Session not found	404 Session not found
call again	404 — stuck	ok (gen2)
handshakes	1 (never reconnected)	2
reconnect log	none	connection lost; reconnecting → reconnected and re-synced tools

The control is what turns it into evidence: the pristine run reproduces the exact symptom, and the patch is the only variable.

The fix is a missing subscription, not missing machinery. I added no retry logic — generationDown() already does everything required (backoff, fresh handshake, enqueueSync re-registration). Only the trigger was absent:

generation.onerror = (error) => {
  if (attemptSettled && isSessionLostError(error)) generationDown(generation);
};

with isSessionLostError deliberately narrow — 404, plus 400 whose body names the session — because a false positive recycles a healthy generation. Every other failure surfaces unchanged, exactly as today.

One point for the design, since it changes what "fixing this" has to cover: a failed connect attempt already reaches the supervisor (settleFailedGeneration → generationDown → scheduleReconnect). It is specifically a session invalidated on an already-established connection that falls through. That is why no tuning of the existing retry policy can address it — the event never arrives.

The patch applies cleanly to both 0.2.0-rc.2 and 0.2.1-alpha.4 (byte-identical lib/index.js). Glad to hand it over, drop it, or test a native implementation — whichever is useful.

[MaxHou-infinity]

A correction to my earlier comments, plus the end-to-end result — posting because it changes who actually hits this.

The trigger is not necessarily a server restart. I framed this as "a stateful server restarts and drops its in-memory sessions". Measuring against a real server (Nowledge Mem's local backend) shows that is not the general case:

• sessions there are persisted to disk and restored on restart. I created a session, restarted the server twice, and it still answered 200 afterwards.

• what actually invalidates a session is an idle timeout. The server logs:

  MCP session idle timeout and restart restore store configured
    seconds=43200                                    # 12 hours
  

So the practical trigger is "DSH was left running for >12h without calling that server" — no crash, no restart, nothing visible. That is far easier to hit than a server restart, which means more people are affected than the restart framing suggests. The onerror gap is identical either way; only the trigger differs, and it is worth stating both wherever this gets documented.

End-to-end confirmation on a real host. With the patch installed and DSH left running, I invalidated the session the way that server actually does — aged the store entry past 43200s, restarted the server so it would prune on load:

step	observed
first tool call	Error POSTing to endpoint: Not Found: Session not found
server session store, ~8s later	a new dsh-mcp-client session id, unprompted
second tool call	succeeded

No DSH restart in between. A new session id appearing on its own is decent evidence: only a fresh initialize handshake produces one, and the reconnected generation is the only thing that runs one.

Two notes that follow from this:

• @gkinter — a 404-only detector covers the real-world case, so it is the right primary signal; the 400 arm remains worth keeping for servers that answer that way. Also: SdkHttpError exposes status as a getter over data.status, and the body text lands on error.data.text (it is also in message), which is what a body-sniffing arm needs.
• For anyone reproducing this: mutating a live session store does nothing, since the running server holds the session in memory. Stop the server first.

Corrected framing and the harness are in the repo: https://github.com/MaxHou-infinity/dsh-mcp-session-recovery

[MaxHou-infinity]

Proposing we consolidate into one implementation. Two independent patches for the same gap make the first question "which one" rather than "should this be fixed", and that is time better spent on the fix itself.

@gkinter — your design is the more complete base and I would rather not see it split. The 400 arm, retry-once semantics, concurrent-failure recycle sharing and bounded wait are all things mine does not do. Mine is deliberately narrower (a 404, or a 400 whose body names the session → the existing generationDown), which makes it easy to verify and equally easy to outgrow.

What I would rather contribute than compete with:

An A/B harness. test/run.mjs in the repo mounts the real dsh-mcp-client against a fake streamable-HTTP server that clears its session table mid-flight, and runs the identical test against pristine and patched targets. The pristine run is what turns the patched run into evidence — it reproduces the exact symptom with the patch as the only variable. Ported to your design it would give the upstream port a control.

The corrected trigger, which widens the scope. Sessions are not necessarily dropped on restart: the server I measured persists them and survives restarts intact, but prunes them on a 43200s idle timeout. So the failure needs no crash — just a host left running past the timeout. Under a restart-only framing, transparent recovery looks like polish; under the real trigger it is the main case, because there is no user-visible event to explain the failure.

Production confirmation. On a real host, invalidating a session the way that server actually does: the first call 404s, a new dsh-mcp-client session id appears server-side about 8s later, and the second call succeeds — no DSH restart. Happy to repeat this against a ported implementation.

Offered as: pick whichever base is easier to port into packages/mcp/mcp-client/src, and I will fold in the harness and re-run the end-to-end check against it. If the direction the maintainers confirm differs from both, that is the better outcome and this retires quietly.