Skip to content

Revert MCP POST routing and restore server examples#1532

Merged
mattzcarey merged 2 commits into
mainfrom
revert-mcp-fix
May 15, 2026
Merged

Revert MCP POST routing and restore server examples#1532
mattzcarey merged 2 commits into
mainfrom
revert-mcp-fix

Conversation

@threepointone
Copy link
Copy Markdown
Contributor

@threepointone threepointone commented May 15, 2026

Summary

Test plan

  • npx tsc --noEmit -p examples/mcp-elicitation/tsconfig.json && npx tsc --noEmit -p examples/mcp-server/tsconfig.json
  • npx oxlint examples/mcp-elicitation/src/index.ts examples/mcp-server/src/index.ts
  • npx oxfmt --check README.md examples/AGENTS.md examples/TODO.md examples/mcp-elicitation/README.md examples/mcp-elicitation/package.json examples/mcp-elicitation/src/index.ts examples/mcp-elicitation/tsconfig.json examples/mcp-elicitation/wrangler.jsonc examples/mcp-server/README.md examples/mcp-server/package.json examples/mcp-server/src/index.ts examples/mcp-server/tsconfig.json examples/mcp-server/wrangler.jsonc package-lock.json .changeset/bright-buses-route.md

Note: local npm run check is currently blocked by unrelated local workspace state: sherif warns about examples/think-slide-deck/package.json, and oxfmt --check . reports formatting issues in packages/codemode/CHANGELOG.md and packages/think/CHANGELOG.md.

Made with Cursor


Open in Devin Review

threepointone and others added 2 commits May 15, 2026 12:36
Bring back the server-only MCP elicitation and raw transport examples that were lost during the full-stack example migration. Also document that focused MCP protocol examples may stay Wrangler-only, refresh the workspace lockfile for the restored mcp-server workspace, and add a patch changeset noting the PR #1514 routing revert.

Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 15, 2026

🦋 Changeset detected

Latest commit: d335334

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

This PR includes changesets to release 1 package
Name Type
agents 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

@threepointone threepointone requested a review from mattzcarey May 15, 2026 12:10
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines 353 to +355
const connection = Array.from(
agent.getConnections<{ requestIds?: RequestId[] }>()
).find((conn) => conn.state?.requestIds?.includes(requestId));
agent.getConnections<{ requestIds?: number[] }>()
).find((conn) => conn.state?.requestIds?.includes(requestId as number));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 RequestId narrowed to number[] breaks string request IDs

At line 354-355, the code types requestIds as number[] and casts requestId as number. However, RequestId in the MCP SDK (and JSON-RPC 2.0 spec) is string | number. If a client sends a request with a string ID (e.g. "req-1"), Array.prototype.includes() uses strict equality (===), so ["req-1"].includes("req-1" as number) would never match—it compares a string against the number type expectation. This causes a runtime error: "No connection established for request ID: req-1". The old code correctly used RequestId[] without any cast (packages/agents/src/mcp/transport.ts:354-355 in the base).

Suggested change
const connection = Array.from(
agent.getConnections<{ requestIds?: RequestId[] }>()
).find((conn) => conn.state?.requestIds?.includes(requestId));
agent.getConnections<{ requestIds?: number[] }>()
).find((conn) => conn.state?.requestIds?.includes(requestId as number));
const connection = Array.from(
agent.getConnections<{ requestIds?: RequestId[] }>()
).find((conn) => conn.state?.requestIds?.includes(requestId));
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 378 to 381
for (const id of relatedIds) {
this._requestResponseMap.delete(id);
}
connection.setState({ requestIds: [] });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Removing setState({ requestIds: [] }) leaves stale request IDs on closed connections

When all responses for a batched POST have been sent (shouldClose = true), the old code cleared the connection's request IDs via connection.setState({ requestIds: [] }). This cleanup was removed at the end of the send() method. Without it, a connection that has been signaled to close still retains its old requestIds in its state. If a subsequent send() call races before the connection is fully torn down, the stale requestIds could cause the lookup at line 353-355 to match the wrong (closing) connection, routing a message to a stream that's about to close.

(Refers to lines 376-381)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@@ -1,25 +1,25 @@
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Missing $schema in mcp-elicitation wrangler.jsonc violates examples/AGENTS.md convention

The examples/AGENTS.md convention states wrangler.jsonc must include "$schema": "../../node_modules/wrangler/config-schema.json". The mcp-elicitation example's wrangler.jsonc had this field in the old version but it was removed in this PR.

Suggested change
{
{
"$schema": "../../node_modules/wrangler/config-schema.json",
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@@ -0,0 +1,11 @@
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Missing $schema in mcp-server wrangler.jsonc violates examples/AGENTS.md convention

The examples/AGENTS.md convention states wrangler.jsonc must include "$schema": "../../node_modules/wrangler/config-schema.json". The new mcp-server example's wrangler.jsonc is missing this field.

Suggested change
{
{
"$schema": "../../node_modules/wrangler/config-schema.json",
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 15, 2026

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@1532

@cloudflare/ai-chat

npm i https://pkg.pr.new/@cloudflare/ai-chat@1532

@cloudflare/codemode

npm i https://pkg.pr.new/@cloudflare/codemode@1532

hono-agents

npm i https://pkg.pr.new/hono-agents@1532

@cloudflare/shell

npm i https://pkg.pr.new/@cloudflare/shell@1532

@cloudflare/think

npm i https://pkg.pr.new/@cloudflare/think@1532

@cloudflare/voice

npm i https://pkg.pr.new/@cloudflare/voice@1532

@cloudflare/worker-bundler

npm i https://pkg.pr.new/@cloudflare/worker-bundler@1532

commit: d335334

@mattzcarey mattzcarey merged commit 0a2d6df into main May 15, 2026
8 checks passed
@mattzcarey mattzcarey deleted the revert-mcp-fix branch May 15, 2026 12:23
@github-actions github-actions Bot mentioned this pull request May 15, 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.

2 participants