cloudflare / cloudflare/agents
addMcpServer: default callback URL silently breaks when callers don't set `callbackPath` + `sendIdentityOnConnect: true`
- Dominant language
- TypeScript
- Stars
- 5.6k
- Forks
- 711
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 53
Description
## Summary
`addMcpServer(name, url)` with no explicit `callbackPath` falls back to a default callback URL of the form:
```
${host}/${agentsPrefix}/${kebab(ParentClass.name)}/${this.name}/callback
```
e.g. `https://example.com/agents/my-assistant/octocat/callback`.
There is an existing guard in [`packages/agents/src/index.ts`](https://github.com/cloudflare/agents/blob/main/packages/agents/src/index.ts) (~line 5495) that refuses this default and throws a helpful error:
```ts
if (
!this._resolvedOptions.sendIdentityOnConnect &&
resolvedCallbackHost &&
!resolvedCallbackPath
) {
throw new Error(
"callbackPath is required in addMcpServer options when sendIdentityOnConnect is false — " +
"the default callback URL would expose the instance name. " +
"Provide a callbackPath and route the callback request to this agent via getAgentByName."
);
}
```
But the guard is bypassed whenever `sendIdentityOnConnect: true` is set. That option is about something different (broadcasting the DO's identity over the WS handshake), and yet turning it on silently disables a piece of safety scaffolding that has nothing to do with identity broadcasting.
## Why it bit us
Discovered while reviewing #1374. In `examples/assistant`:
- `MyAssistant` sets `static options = { sendIdentityOnConnect: true }` so the browser can learn the server-assigned DO name.
- The Worker narrows `run_worker_first` to `/auth/*` and `/chat*` (so random clients can't reach `/agents/my-assistant/` directly — an auth-bypass fix).
- The example's `addServer` RPC calls `this.addMcpServer(name, url)` with no options.
Because `sendIdentityOnConnect` is true, no error is thrown. The default callback URL `https://.../agents/my-assistant//callback` is silently generated and persisted. When the user goes through OAuth:
1. The provider redirects the browser to that URL.
2. `/agents/*` is not in `run_worker_first`, so the SPA asset handler serves `index.html` with a 200.
3. The Durable Object never sees the `code`.
4. The MCP server hangs in \`AUTHENTICATING\` forever, with no server-side error.
The fix on the example side was trivial (one option: `callbackPath: \"chat/mcp-callback\"`), but the reason it was even possible to misconfigure without a warning is the guard bypass above.
## Proposal
Make \`sendIdentityOnConnect: true\` not disable the callback-path enforcement. Options, in order of least-disruptive first:
1. **Always throw/warn when the default callback URL is used**, regardless of \`sendIdentityOnConnect\`. The default URL leaks \`this.name\` into the path either way, and it's a foot-gun for any Worker that doesn't route \`/agents/*\` to the framework. If it's considered a breaking change, start with a \`console.warn\`.
2. Require an explicit opt-in (e.g. \`allowDefaultCallbackUrl: true\`) when the caller actually wants the legacy behavior.
3. If there's a reason \`sendIdentityOnConnect\` was coupled to the guard, document it prominently — right now the coupling is non-obvious and the failure mode is silent.
## Acceptance criteria
- Calling \`addMcpServer(name, url)\` with no explicit \`callbackPath\` in a \`sendIdentityOnConnect: true\` agent either throws the existing helpful error or logs a \`console.warn\` that surfaces the misconfiguration at development time.
- The existing tests in \`packages/agents/src/tests/mcp/add-mcp-server.test.ts\` are extended to cover the \`sendIdentityOnConnect: true\` branch.
- \`addMcpServer\` docs call out that the default callback URL only works if you route \`/agents/*\` to the framework's \`routeAgentRequest\`.
## Workaround (today)
Always pass an explicit \`callbackPath\` that lands on a Worker route you control, and route OAuth callbacks through the same authenticated path as the rest of your app:
\`\`\`ts
await this.addMcpServer(name, url, { callbackPath: "chat/mcp-callback" });
\`\`\`
## Related
- #1374 (context / reproduction)
- \`packages/agents/src/index.ts\` around line 5495 (the guard) and 5520-5527 (the default URL construction)
Contributor guide
Assessment
This issue has not been assessed yet.