Skip to content

fix: handle cancellation for request id zero - #2138

Closed
he-yufeng wants to merge 1 commit into
modelcontextprotocol:mainfrom
he-yufeng:fix/cancel-request-id-zero
Closed

fix: handle cancellation for request id zero#2138
he-yufeng wants to merge 1 commit into
modelcontextprotocol:mainfrom
he-yufeng:fix/cancel-request-id-zero

Conversation

@he-yufeng

Copy link
Copy Markdown

Summary

  • preserve requestId: 0 when handling notifications/cancelled
  • add a regression test that cancels an inbound request whose JSON-RPC id is 0

Fixes #2115.

Validation

  • pnpm --filter @modelcontextprotocol/core exec vitest run test/shared/protocol.test.ts -t "notifications/cancelled behavior"
  • pnpm --filter @modelcontextprotocol/core exec vitest run test/shared/protocol.test.ts
  • pnpm --filter @modelcontextprotocol/core run typecheck
  • pnpm --filter @modelcontextprotocol/core run lint
  • pre-push hook: pnpm -r typecheck, pnpm -r build, pnpm sync:snippets --check, pnpm -r lint
  • git diff --check

@he-yufeng
he-yufeng requested a review from a team as a code owner May 21, 2026 07:47
@changeset-bot

changeset-bot Bot commented May 21, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c640656

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

This PR includes changesets to release 1 package
Name Type
@modelcontextprotocol/core 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 May 21, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2138

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2138

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2138

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server-legacy@2138

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2138

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2138

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2138

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2138

commit: c640656

@he-yufeng
he-yufeng force-pushed the fix/cancel-request-id-zero branch from 0d326bf to a3d7042 Compare May 24, 2026 10:34
@he-yufeng

Copy link
Copy Markdown
Author

Rebased this on the latest main and re-ran the focused/core checks locally:

  • pnpm --filter @modelcontextprotocol/core exec vitest run test/shared/protocol.test.ts -t "requestId 0"
  • pnpm --filter @modelcontextprotocol/core exec vitest run test/shared/protocol.test.ts
  • pnpm --filter @modelcontextprotocol/core run typecheck
  • pnpm --filter @modelcontextprotocol/core run lint
  • git diff --check upstream/main..HEAD
  • pre-push hook: pnpm -r typecheck, pnpm -r build, pnpm sync:snippets --check, pnpm -r lint

@he-yufeng
he-yufeng force-pushed the fix/cancel-request-id-zero branch from a3d7042 to 553b6b6 Compare May 27, 2026 10:45
@he-yufeng

Copy link
Copy Markdown
Author

Added the @modelcontextprotocol/core patch changeset and force-pushed 553b6b6. Local validation: changeset status shows @modelcontextprotocol/core patch; focused requestId 0 test passed; full core protocol.test.ts passed with 151 tests; core typecheck and lint passed; git diff --check passed. Pre-push typecheck, build, and lint also passed.

@he-yufeng
he-yufeng force-pushed the fix/cancel-request-id-zero branch from 553b6b6 to c640656 Compare June 6, 2026 21:07
@he-yufeng

Copy link
Copy Markdown
Author

Rebased on current main, resolved the protocol test conflict caused by the task-era test removal, and force-pushed the branch.

The resolved diff is still only the intended request-id-zero cancellation fix: _oncancel now only ignores an absent requestId, and the regression test covers requestId: 0.

Validated locally:

  • pnpm changeset status --since origin/main (@modelcontextprotocol/core patch)
  • pnpm --filter @modelcontextprotocol/core exec vitest run test/shared/protocol.test.ts -t "requestId 0" (1 passed)
  • pnpm --filter @modelcontextprotocol/core exec vitest run test/shared/protocol.test.ts (31 passed)
  • pnpm --filter @modelcontextprotocol/core typecheck
  • pnpm --filter @modelcontextprotocol/core lint
  • git diff --check origin/main..HEAD

The pre-push hook also completed the repo-wide build, typecheck, and lint steps successfully.

@he-yufeng

Copy link
Copy Markdown
Author

Closing this during backlog cleanup. This queue has many review-required MCP TypeScript SDK PRs, and this item is lower priority than the remaining transport/protocol fixes with clearer current value. I can reopen or refresh it if maintainers want this exact change.

@he-yufeng he-yufeng closed this Jun 10, 2026
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.

requestId 0 is silently dropped by _oncancel, making the first request from every Protocol uncancellable

1 participant