addMcpServer: default callback URL silently breaks when callers don't set `callbackPath` + `sendIdentityOnConnect: true` · Issue #1378 · cloudflare/agents · GitHub
Skip to content

addMcpServer: default callback URL silently breaks when callers don't set callbackPath + sendIdentityOnConnect: true #1378

Open

Description

@threepointone

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 (~line 5495) that refuses this default and throws a helpful error:

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/<login> 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/<login>/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

Activity

  1. added
    bugSomething isn't working
    mcpIssues related to MCP functionality
    on May 5, 2026
  2. omribz156 commented on May 20, 2026

    @omribz156

    I took a small local pass because this looks like a real callbackPath foot-gun.

    I’m not opening a PR because the README says external PRs are not being accepted right now, but the patch shape that passed locally was:

    • in addMcpServer, after deriving callbackHost, warn when sendIdentityOnConnect is true, a callback host exists, and no callbackPath was supplied
    • update the callbackPath JSDoc/docs to say the default /agents/{class}/{name}/callback only works when /agents/* routes through routeAgentRequest; custom routing should provide callbackPath and forward with getAgentByName
    • add a focused worker test that reuses TestHttpMcpDedupAgent and asserts the warning without changing the existing sendIdentityOnConnect: false enforcement tests

    Verification I ran:

    • npm exec -- oxfmt --check on the touched files
    • npm exec -- oxlint --deny-warnings on the touched TS files
    • focused worker Vitest through a temporary local Windows config: 1 file / 10 tests passed
    • git diff --check

    Two local caveats: the normal worker-test command did not discover tests on this Windows checkout because the config builds an absolute backslash include glob, and npm run build -w agents emitted bundles/declarations but failed at the final execFileSync("oxfmt") declaration-format step because the npm shim was not spawnable here.

    This was Codex-assisted; I manually reviewed the final diff and held back from a PR because of the README contribution note.

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

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingmcpIssues related to MCP functionality

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions