[JQ100]

Fixes #3045.

Before: protocol:timeout:max-total switched to fake timers, started tools/call, and advanced the clock straight away. Over SSE the request is a real HTTP POST that fake timers don't wait for, so when it landed late the 500ms per-request timeout fired before any progress was sent. The SSE variants then failed intermittently with expected 0 to be greater than or equal to 3, and when they did, the test never exercised maxTotalTimeout.

After: the tool handler resolves a promise as soon as it starts, and the test awaits it before advancing the clock. Awaiting a promise doesn't move the fake clock, unlike vi.waitFor, which advances fake timers while it polls. Test-only change, 9 lines, no changeset.

Motivation and Context

On a clean clone on macOS, pnpm test:all intermittently fails on this test, while CI on main is green. Details and the original investigation are in #3045.

One correction to that issue: it says the failure is followed by an unhandled SdkError: Request timed out. max-total attaches call.catch(() => {}), so that rejection isn't from this test. It comes from protocol:timeout:reset-on-progress (data: { timeout: 200 }), which leaves 1–2 unhandled rejections on every run on macOS, with or without this change. That's a separate problem and I've left it out of this PR; happy to open an issue for it.

How Has This Been Tested?

On macOS (Darwin 27), Node 24.16.0, from main @ 39ae8e61:

• Measured: pnpm vitest run scenarios/protocol.test.ts -t "timeout:basic|timeout:max-total" in test/e2e, repeated. Before: 5 of 20 SSE variants failed (10 runs). After: 0 of 40 (20 runs).
• Forced: a temporary patch delayed only the tools/call POST by 50ms of real time (via a setTimeout captured before vi.useFakeTimers()). Without this change both SSE variants failed on 3 of 3 runs; with it all 12 variants passed on 3 of 3 runs. The patch isn't part of this PR.
• pnpm lint:all and pnpm --filter @modelcontextprotocol/test-e2e typecheck pass. In pnpm test:all every test passes; vitest still exits 1 because of the reset-on-progress unhandled rejections described above.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1811953

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: 1811953

[arhancanli]

Read 61396ea (not run). The reasoning holds: the client-side timers are armed while the clock is frozen, so awaiting callReceived can only let the POST land, and unlike vi.waitFor it does not advance fake time.

One failure mode: callReceived has no timeout and no reject path. If the call never reaches the handler (schema rejection of delayMs/steps, a transport bug, a session that fails to open), the test hangs on a promise that never settles with fake timers on, so it dies on the suite-level timeout with no hint of why. A cheap guard is await Promise.race([callReceived, call.then(() => { throw new Error('call settled before the handler ran'); })]), which turns that into an immediate named failure (call already has a catch attached, so use a separate handle for the race).

Question: markCallReceived is also invoked on the stdio and in-memory variants where the ordering was never a problem. Do you want to keep it unconditional so all variants share one code path (my preference), or gate it to SSE?

[JQ100]

Thanks for the careful read, and sorry for the slow reply.

Guard: added in 1811953, along the lines you suggested:

const settledFirst = call.then(() => {
    throw new Error('tools/call settled before the tool handler ran');
});
settledFirst.catch(() => {});
await Promise.race([callReceived, settledFirst]);

The extra catch is on the guard promise itself: once callReceived wins, call later rejects with the expected RequestTimeout, and that rejection flows through settledFirst, so it needs its own handler or it shows up as unhandled.

To check that it fires, I temporarily sent delayMs: 'bad'. Without the guard, all 12 variants hang to the 15s test timeout and report only Test timed out (about 3 minutes in total). With it, they fail in under 2s with tools/call settled before the tool handler ran. With valid arguments, all 12 still pass (3 runs), with no unhandled rejections.

One limit worth stating: this only catches a call that settles. If the request is lost and nothing ever comes back, the per-request timeout can't fire either, since the fake clock is frozen at that point. That case still ends on the test timeout. I think that's acceptable, but happy to add a real-time bound if you'd rather.

Unconditional vs. SSE-only: agreed, keeping it unconditional. On the other transports the wait is harmless: it resolves as soon as the call arrives and never moves the clock. One code path is also easier to read than a transport check.