Skip to content

fix(core): reset request timeout on progress without an onprogress handler - #2224

Open
minglong51 wants to merge 1 commit into
modelcontextprotocol:mainfrom
minglong51:fix/reset-timeout-without-onprogress-2076
Open

fix(core): reset request timeout on progress without an onprogress handler#2224
minglong51 wants to merge 1 commit into
modelcontextprotocol:mainfrom
minglong51:fix/reset-timeout-without-onprogress-2076

Conversation

@minglong51

@minglong51 minglong51 commented Jun 1, 2026

Copy link
Copy Markdown

What

resetTimeoutOnProgress: true is documented to keep a long-running request alive as progress notifications arrive. In practice it only worked when an onprogress callback was also passed on the same request() call.

Without onprogress, the request did not advertise _meta.progressToken, and _onprogress treated any manually delivered progress as an unknown token before the timeout-reset code could run. (#2076)

Fix

  • Advertise _meta.progressToken when either onprogress or resetTimeoutOnProgress is enabled.
  • Treat a progress token as known when it belongs to either a registered progress handler or an in-flight request.
  • Reset the timeout for the in-flight request even when no progress callback is registered.
  • Invoke the progress callback only when one exists.

This preserves the existing task-progress path, where a progress handler may remain after the response handler is gone, and still reports genuinely unknown tokens.

Test plan

  • Added a public Protocol.request() regression covering resetTimeoutOnProgress: true without onprogress.
  • The regression asserts that the outbound request advertises a progress token, progress extends the timer beyond its original deadline, the request resolves successfully, and no spurious unknown-token error is emitted.
  • pnpm exec vitest run test/shared/protocol.test.ts: 40 passed.
  • pnpm --filter @modelcontextprotocol/core-internal test: 1,434 passed.
  • pnpm --filter @modelcontextprotocol/core-internal check: passed.
  • Pre-push repository hooks: build, typecheck, and lint passed across the workspace.

Scope

This PR targets main (SDK v2). The stable v1.x branch requires a separate backport PR if maintainers want the same fix there.

Fixes #2076

@minglong51
minglong51 requested a review from a team as a code owner June 1, 2026 03:22
@changeset-bot

changeset-bot Bot commented Jun 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: aa47d06

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/server Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch
@modelcontextprotocol/codemod 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

pkg-pr-new Bot commented Jun 1, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: aa47d06

@minglong51

Copy link
Copy Markdown
Author

Gentle ping on this when you have review bandwidth. CI is green, the regression test covers #2076, and the change is scoped to preserving resetTimeoutOnProgress behavior even when no onprogress callback is supplied.

Happy to adjust if you prefer the timeout-reset ordering handled differently.

@minglong51
minglong51 force-pushed the fix/reset-timeout-without-onprogress-2076 branch from 57a2c9a to aa47d06 Compare August 1, 2026 22:52
@minglong51

Copy link
Copy Markdown
Author

Revived and rebased onto current main (aa47d066). The refresh also closes the outbound half of #2076: resetTimeoutOnProgress now advertises _meta.progressToken even without onprogress, so a peer can actually address progress to the request. The public Protocol.request() regression covers token advertisement, timeout extension, successful completion, and absence of the spurious unknown-token error. Workspace build, typecheck, and lint passed in the pre-push hook; the core-internal suite passed 1,434 tests. Ready for re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

resetTimeoutOnProgress on low-level request() only works when onprogress is explicitly provided

1 participant