[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: 96daba1

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: 96daba1

[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.