You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
addMcpServer: default callback URL silently breaks when callers don't set callbackPath + sendIdentityOnConnect: true #1378
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){thrownewError("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:
The provider redirects the browser to that URL.
/agents/* is not in run_worker_first, so the SPA asset handler serves index.html with a 200.
The Durable Object never sees the code.
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:
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`.
Require an explicit opt-in (e.g. `allowDefaultCallbackUrl: true`) when the caller actually wants the legacy behavior.
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:
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.
Activity
chore(wip): reflect PR #1374 landing and split PR 2 into 2a/2b
feat(example): multi-session assistant — sub-agent routing, shared wo…
omribz156 commentedon May 20, 2026
I took a small local pass because this looks like a real
callbackPathfoot-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:
addMcpServer, after derivingcallbackHost, warn whensendIdentityOnConnectis true, a callback host exists, and nocallbackPathwas suppliedcallbackPathJSDoc/docs to say the default/agents/{class}/{name}/callbackonly works when/agents/*routes throughrouteAgentRequest; custom routing should providecallbackPathand forward withgetAgentByNameTestHttpMcpDedupAgentand asserts the warning without changing the existingsendIdentityOnConnect: falseenforcement testsVerification I ran:
npm exec -- oxfmt --checkon the touched filesnpm exec -- oxlint --deny-warningson the touched TS filesgit diff --checkTwo 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 agentsemitted bundles/declarations but failed at the finalexecFileSync("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.