[liangqianxing]

Summary

Fixes #3075.

When ClientResponseCache.resetForReconnect() clears _evictionGeneration, an in-flight request from before the reset whose generation snapshot was 0 previously passed the write() guard ((this._evictionGeneration.get(gk) ?? 0) !== capturedGen) in two cases:

1. Before any post-reset request for the same key captured a generation (_evictionGeneration.get(gk) was undefined, falling back via ?? 0 to 0 === 0).
2. After a post-reset request for the same key captured generation 0 (both snapshots shared the same numeric token).

Changes

• packages/client/src/client/responseCache.ts:
  • Require this._evictionGeneration.get(gk) === capturedGen in write() without the ?? 0 fallback so writes for keys that have not captured a generation in the current connection epoch are rejected.
  • Maintain a monotonic _generationBase (starting at 0 and advancing to max(this._generationBase, ...this._evictionGeneration.values()) + 1 when resetForReconnect() clears a non-empty _evictionGeneration map) and use ?? this._generationBase in captureGeneration() and evict(), ensuring post-reset tokens are strictly greater than any pre-reset token while still clearing _evictionGeneration across reconnects.
• packages/client/test/client/responseCache.test.ts:
  • Update the resetForReconnect unit test to verify that _evictionGeneration is emptied and that the next capture advances past the pre-reset counter.
  • Add parameterized unit tests covering private/public scopes × changeServer (false/true) × ordering (none, capture-write-stale, capture-stale-write, write-recapture-stale, concurrent-post-reset, evicted-pre-reset-and-post-reset).
• .changeset/reject-pre-reset-cache-writes.md:
  • Add patch changeset for @modelcontextprotocol/client.

Verification

• pnpm --filter @modelcontextprotocol/client test (1,100 tests passed)
• pnpm --filter @modelcontextprotocol/client run typecheck
• pnpm --filter @modelcontextprotocol/client run lint

🤖 Generated with Claude Code

[changeset-bot]

🦋 Changeset detected

Latest commit: a49eee6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@modelcontextprotocol/client	Patch
@modelcontextprotocol/codemod	Patch
@modelcontextprotocol/core	Patch
@modelcontextprotocol/server-legacy	Patch
@modelcontextprotocol/server	Patch
@modelcontextprotocol/core-internal	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@3092

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@3092

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@3092

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@3092

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@3092

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@3092

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@3092

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@3092

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@3092

commit: a49eee6

[arhancanli]

Read the diff of responseCache.ts and the new test; I did not run it. I think two orderings still let a pre-reset response through, because a pre-reset generation-0 token and a post-reset generation-0 token are the same number, and the new flags only count writes:

1. Stale write between a post-reset capture and its write: reset, then request B calls captureGeneration (map entry is now 0), then the delayed pre-reset response calls write(..., 0). get(gk) === 0 passes, _preResetZeroKeys has the key, _postResetZeroWrittenKeys does not, so the stale response is written and marks the key as redeemed. B's own fresh response is then dropped. The new test only covers capture-write-stale, not capture-stale-write.

2. captureGeneration now deletes _postResetZeroWrittenKeys for the key. After A captures and writes (flag set), any later request B for the same key clears the flag, so a stale pre-reset write arriving after that overwrites A's fresh entry. Test order: reset, A capture+write, B capture, stale write.

A side effect of the one-write rule: two legitimate post-reset requests that both captured 0 (a concurrent tools/list) lose the second write, which is only a missed cache fill.

The root cause is that resetForReconnect() clears the map, so generations restart at 0. If the cache kept a monotonic epoch (bumped in resetForReconnect, compared in write, or the generation map left in place with a bump per key), a pre-reset token could never equal a post-reset one and _preResetZeroKeys / _postResetZeroWrittenKeys would not be needed. Would that fit with the ?? 0 handling that other call sites rely on?

[liangqianxing]

Thanks for catching those orderings — you're right that as long as captureGeneration() can return the same token before and after resetForReconnect(), write() cannot distinguish a delayed pre-reset write from an in-flight post-reset one.

I've replaced _preResetZeroKeys / _postResetZeroWrittenKeys in 96daba1 with a monotonic _generationBase (starting at 0 and advancing to max(this._generationBase, ...this._evictionGeneration.values()) + 1 when resetForReconnect() clears a non-empty _evictionGeneration map), with captureGeneration() and evict() falling back to ?? this._generationBase instead of ?? 0 and write() checking this._evictionGeneration.get(gk) === capturedGen.

That keeps _evictionGeneration cleared across reconnects, preserves generation 0 on the initial connection, ensures post-reset tokens are strictly greater than any pre-reset token (including keys evicted before reset), and lets concurrent post-reset captures write normally.

I also updated the resetForReconnect unit test to verify both that _evictionGeneration is emptied and that the next capture advances past the pre-reset counter, and expanded the reconnect write matrix to cover capture-stale-write, write-recapture-stale, concurrent post-reset writes, and pre/post-reset evictions.

[arhancanli]

Read 96daba1 (diff only, I have not run the suite). The monotonic base closes both orderings from my earlier comment. A pre-reset token is at most the old max, and the base moves to max + 1, so it can never equal a post-reset capture. That holds for tokens from before an eviction too, and it makes the one-write rule unnecessary. Concurrent post-reset captures also both write now, which was the cache-fill loss I mentioned.

One case I would add to the matrix: two resets in a row with nothing captured in between, then a stale write using a token from before the first reset. The size > 0 guard means the second reset leaves _generationBase alone. I believe that is correct, because any token from the first connection is already below the base. A test would pin it so a later change to the guard cannot reopen the hole.

Question: does anything outside ClientResponseCache (a user-supplied store, or a persisted generation) ever hold a token across resetForReconnect()? If so, the base would need to survive that boundary as well.

[liangqianxing]

Added the double-reset case (two consecutive resetForReconnect() calls with no captures in between, followed by a post-reset capture/write and a stale pre-reset write) to the parameterized matrix in 7023423.

Regarding your question on token lifetime: nothing outside ClientResponseCache holds or persists a generation token. captureGeneration() is an internal method on ClientResponseCache whose return value only lives as a local stack variable across the await inside Client._listAllPages and Client.readResource before being passed straight back to this._responseCache.write(...). The external ResponseCacheStore interface only sees CachedEntry = { value, expiresAt, stamp } (stamp is the opaque invalidation tag, separate from generation), so keeping _generationBase in-memory on the ClientResponseCache instance covers the entire token boundary.

[arhancanli]

Read 7023423 (diff only, not run). The double-reset case does what I wanted: the second reset sees an empty map, leaves the base alone, and the pre-reset write still loses. Thanks for confirming that nothing outside ClientResponseCache holds a token, that closes my question.

One more ordering for the matrix: a capture that is in flight while two resets happen. Request A captures at base N, reset 1 runs with A's key in the map so the base moves to N+1, a post-reset capture adds a key, and reset 2 runs and moves it to N+2 (the map is non-empty again). A then writes with its N token. By the same argument it should be rejected, but it is the only path where both resets take the size > 0 branch, so it is worth one parameterized row. Does write() compare against the per-key entry or the base when the key was cleared by the reset?

[liangqianxing]

Added reset-capture-reset-stale to the matrix in 14e1320 (covering a capture before reset 1, an intervening capture between reset 1 and reset 2 so both resets take the size > 0 branch and advance _generationBase from N to N + 1 to N + 2, and verifying that stale writes from both pre-reset epochs are rejected before and after a fresh post-reset2 write).

On the write() check: write() compares strictly against the per-key map entry (this._evictionGeneration.get(gk) !== capturedGen, without a ?? fallback):

• When the key was cleared by resetForReconnect() and has not been re-captured on the current connection yet, this._evictionGeneration.get(gk) is undefined, which never equals any numeric capturedGen, so the write is rejected immediately.
• Once a post-reset request calls captureGeneration(), the key is recorded in _evictionGeneration at >= _generationBase (N + 2 after the second reset), which is strictly greater than any pre-reset token (N or N + 1), so the stale write still fails the equality check.

[arhancanli]

Read 14e1320 (diff only, not run) and the write() answer. It matches the code at the head: line 538 compares _evictionGeneration.get(...) strictly against capturedGen with no ??, so a cleared key gives undefined !== N and the write is dropped. The new reset-capture-reset-stale row covers the one path I was worried about, thanks.

One last ordering, on evict() rather than write(). evict(method) now does (get(method) ?? _generationBase) + 1, so on a list key that was never captured on this connection it creates an entry at base + 1 with no capture. A request that then calls captureGeneration() returns base + 1, which looks fine. But what about a request that captured before the evict and writes after it: capture returns base, evict moves the key to base + 1, write sees base + 1 !== base and skips, as intended. Is that interleaving (capture, evict, write, all after a reset) in the matrix, or is it only covered by the older pre-reset eviction rows?

[liangqianxing]

Previously capture -> evict -> write was only tested on the initial connection (_generationBase === 0, in the earlier write skips when the captured generation moved test), while evicted-pre-reset-and-post-reset ran evict -> capture -> write after the reset.

Added post-reset-capture-evict-write to the matrix in 0efcbb3 so capture (base) -> evict (base + 1) -> stale write (base) -> fresh capture (base + 1) -> fresh write (base + 1) is explicitly covered after resetForReconnect() as well.

[arhancanli]

Read 0efcbb3 (diff only, not run). The new row has the right order: capture at base, evict to base + 1, then the stale write, and a fresh capture and write at base + 1.

One gap in what it can catch. The final assertion is only the hit after the last write. In the new branch the first two stale writes (postResetBeforeEvict and oldGeneration) happen while the entry is empty, and TOOL_B is written after them. If write() wrongly accepted either of those, TOOL_A would land and then TOOL_B would overwrite it, so the row still ends on TOOL_B and passes. Only the last two writes, after TOOL_B, are protected by the final check.

Reading cache.read('tools/list') right after the two early stale writes and asserting a miss would close it. Do the other rows in the matrix have the same shape, with the stale write before the fresh one? If so, one intermediate read helper would cover all of them.

[liangqianxing]

Good point — reset-capture-reset-stale, capture-stale-write, concurrent-post-reset, and post-reset-capture-evict-write all issue stale writes before the fresh TOOL_B write, so without an intermediate check a stale TOOL_A write that landed early would be masked when TOOL_B overwrote it.

Added an expectCacheMiss() helper in 6696ea1 (checking both read('tools/list') and toolDefinition('a')) immediately after the early stale writes across all rows where stale writes precede the fresh write (double-reset, reset-capture-reset-stale, capture-stale-write, concurrent-post-reset, evicted-pre-reset-and-post-reset, and post-reset-capture-evict-write), and also verified that the first concurrent post-reset write (gen1) lands before the second (gen2) overwrites it.

[arhancanli]

Read 6696ea1 (diff only, not run). expectCacheMiss() checking both read('tools/list') and toolDefinition('a') is the right shape, and it now sits after the early stale writes in every row that has them. Asserting gen1 lands before gen2 overwrites it in concurrent-post-reset also closes the case where gen1 was silently dropped.

One ordering that row still skips: a stale write arriving after gen1 has landed but before gen2 writes. In concurrent-post-reset the oldGeneration write happens before gen1, so it only shows the guard when the entry is empty. If the guard compared against the entry's current generation instead of the captured base, an oldGeneration write between the gen1 and gen2 writes could slip through, and the final TOOL_B write would hide it again.

Would adding a stale write with a distinct payload (say { tools: [TOOL_C] }, or a differently shaped TOOL_A, since gen1 already wrote TOOL_A and an identical stale write would be invisible) between those two writes, then asserting the read still equals what gen1 wrote, be cheap to fit into that row?

[liangqianxing]

Added a stale write with a distinct payload ({ tools: [toolC] }) between the gen1 and gen2 writes (and again after gen2) in concurrent-post-reset in 743faf0, asserting that read('tools/list') still returns { tools: [TOOL_A] } and toolDefinition('c') remains undefined before gen2 writes TOOL_B.

[arhancanli]

Read 743faf0 (diff only, not run). The toolC write between gen1 and gen2 with the toolDefinition('c') check closes the current-generation-compare case, and the post-gen2 one is hidden by the final TOOL_B hit only if read is also asserted, which the closing block does.

Both stale writes use oldGeneration, the one captured before the reset. The more interesting stale token in this row is gen1: it was valid when captured and TOOL_A landed under it, so after gen2 writes TOOL_B a late write with gen1 is the case where a guard that only rejects pre-reset tokens would pass it. Is a { tools: [toolC] } write with gen1 after the gen2 write, asserting the read stays TOOL_B, in the matrix, or would it duplicate another row?

[liangqianxing]

In concurrent-post-reset, gen1 and gen2 are actually the same numeric value (gen1 === gen2 === _generationBase): captureGeneration() snapshots the current eviction generation without incrementing it (this._evictionGeneration.get(gk) ?? this._generationBase), and write() does not bump _evictionGeneration either — only evict() / evictKey() (and resetForReconnect()) advance the generation.

That's by design so two concurrent requests in the same valid epoch (with no intervening list_changed / resources/updated eviction) both have their writes accepted rather than one invalidating the other; each caller in Client._listAllPages / Client._readResource invokes write() at most once with its captured snapshot.

When an eviction does happen after a post-reset capture (gen1 at base, then evict() bumps to base + 1, then gen2 captures base + 1 and writes TOOL_B), a late write using the superseded post-reset token (postResetBeforeEvict) after TOOL_B is already covered in the post-reset-capture-evict-write row (line 1447: await cache.write('tools/list', { tools: [TOOL_A] }, postResetBeforeEvict, freshness) after postEvictGeneration wrote TOOL_B).

[arhancanli]

That corrects me: if captureGeneration() does not bump, gen1 and gen2 are the same token in that row, so a late gen1 write after gen2 is just a second same-epoch write and not a stale one. Re-reading the diff, the superseded-token case is the postResetBeforeEvict write after TOOL_B in the last branch, so nothing is missing from the matrix on that point.

One thing I could not tell from the diff alone: in concurrent-post-reset, gen1 and gen2 are captured before any write, so two readers in the same epoch both land. Is there a row where a write under the shared token lands after an evict() that happened between the captures and the write (capture, capture, evict, write)? That is the ordering where "same epoch" stops being true while both callers still hold a token they believe is current.

[liangqianxing]

Updated post-reset-capture-evict-write in 42f64e6 to take two concurrent post-reset captures (postResetBeforeEvict1 and postResetBeforeEvict2) before evict('tools/list') runs, and verified that writes from both callers are rejected both before (expectCacheMiss()) and after the post-eviction capture (postEvictGeneration) writes TOOL_B.

[arhancanli]

Read 42f64e6 (diff only, not run). Taking two captures before evict() and writing both afterwards, once before and once after the postEvictGeneration write of TOOL_B, covers the case I asked about: both tokens predate the evict, so both writes are rejected, and the final read still sees only TOOL_B.

One thing left in that row: the two writes use TOOL_A, which is also what the cache held before the evict. If the guard failed and let a write through, would expectCacheMiss() still tell it apart from a legitimate TOOL_A entry, or would a distinct { tools: [toolStale] } payload make the failure unambiguous? Does the same row exist for resources/read, where _readResource captures per URI?

[liangqianxing]

Updated in 98c1e4a:

1. In post-reset-capture-evict-write, the two pre-evict post-reset writes now carry distinct { tools: [toolStale1] } and { tools: [toolStale2] } payloads (separate from oldGeneration's TOOL_A), and both toolDefinition('stale1') and toolDefinition('stale2') are asserted to be undefined both before and after postEvictGeneration writes TOOL_B.
2. Added a parameterized resources/read post-reset test (private/public × changeServer) covering captureGeneration('resources/read', 'res://a') before reset, two concurrent post-reset captures followed by evictKey('resources/read', 'res://a'), distinct stale payloads (old-a, stale-a1, stale-a2) rejected before and after the post-eviction write (fresh-a), and verifying that a sibling URI (res://b) captured before evictKey('resources/read', 'res://a') still writes through unaffected.

[arhancanli]

Read 98c1e4a (diff only, not run). The distinct stale1/stale2 payloads make a leaked write unambiguous now, and the resources/read row covers both captures of res://a ahead of evictKey.

One thing about the sibling check: siblingGenB is captured after resetForReconnect(), so fresh-b landing only shows that evictKey('res://a') doesn't poison a post-reset token for res://b. It does not show that a res://b token captured before the reset is still rejected, and that is the case the reset guard exists for. Would it be cheap to capture oldGenB next to oldGen and assert that cache.write(..., oldGenB, freshnessB) leaves res://b unread before fresh-b is written?

[liangqianxing]

Added oldGenB (captured for res://b before resetForReconnect() alongside oldGen for res://a) in 8eb4040, verifying that cache.write('resources/read', { contents: [{ text: 'old-b' }] }, oldGenB, freshnessB) is rejected before siblingGenB is captured, after siblingGenB is captured (and evictKey('resources/read', 'res://a') runs) while res://b is still unread, and after fresh-b is written.

[arhancanli]

Read 8eb4040 (diff only, not run). That closes the gap: oldGenB is captured before resetForReconnect(), and the pre-reset old-b write is now checked at three points: before siblingGenB exists, after the evictKey on res://a, and after fresh-b lands. The last check is the most useful one, because it shows a stale token cannot overwrite a fresh entry.

One related case I could not find in the tests: captureGeneration for a key that has never been written or evicted before the reset. Does that path return a token that write still rejects after resetForReconnect(), or does an unseen key start at a fresh generation? A one-line assertion on a never-seen res://c would pin it down either way.

[liangqianxing]

In a49eee6:

• Because resetForReconnect() clears _evictionGeneration, any pre-reset token captured for an unseen key has no entry in _evictionGeneration after reset (this._evictionGeneration.get(gk) is undefined), so write() rejects it (undefined !== capturedGen). And when a never-seen key is first captured after resetForReconnect(), captureGeneration() initializes it to _generationBase (max(preResetGenerations) + 1).
• Added an assertion on an unseen method key (expect(userCache.captureGeneration('resources/list')).toBe(2)) in the resetForReconnect unit test (packages/client/test/client/responseCache.test.ts:178) and an unseen URI (res://c) capture + write check (expect(unseenGenC).toBe(1) followed by write + read) in the parameterized resources/read reconnect test (packages/client/test/client/responseCache.test.ts:1494).

[arhancanli]

Read a49eee6 (diff only, not run). Both answers check out against the code: captureGeneration records the key, resetForReconnect() clears _evictionGeneration, so a pre-reset token for res://c meets undefined in write() and is rejected, while the first post-reset capture starts at _generationBase (max of the old generations plus one). The toBe(2) assertion on resources/list and the unseenGenC check in the parameterized resources/read test pin exactly that.

One path I could not see in the tests: _generationBase starts at 0, so before any reset an unseen key reads as 0 through the ?? this._generationBase fallback. Does write() ever get called with a token that did not come from captureGeneration (a literal 0, say)? Before a reset that would still pass, after one it is rejected, which looks right, but it depends on every caller going through the capture step. Is that guaranteed by the call sites in client.ts?

[liangqianxing]

Yes — ClientResponseCache is @internal to @modelcontextprotocol/client and only has two .write() call sites in packages/client/src/client/client.ts (_listAllPages at lines 1749/1767 and readResource at lines 1937/1942), both of which pair const generation = this._cache.captureGeneration(...) immediately before the request with await this._cache.write(..., generation, ...) upon completion. Moreover, because write() checks this._evictionGeneration.get(gk) !== capturedGen without a ?? this._generationBase fallback, even a literal 0 for an uncaptured key is rejected (undefined !== 0) before or after a reset unless captureGeneration() was actually called for that key on the initial connection.

[arhancanli]

That settles it, and it corrects what I wrote: with no ?? this._generationBase fallback in write(), a literal 0 for a key that was never captured fails undefined !== 0 even before any reset, so I was wrong that it would pass on the first connection. Both call sites pair capture and write, and @internal keeps outside callers out.

Since that guarantee lives in the call sites rather than in write() itself, would a one-line test that calls write() with a literal 0 for an uncaptured key and expects a no-op be worth adding? It would fail loudly if someone later adds a fallback to write() or a third call site that skips the capture.