send_message server tool does not block a chat from messaging itself (Codex + Claude)
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
## What's broken
The `send_message` server tool lets an agent send a message to another session or chat. It's supposed to **refuse** when an agent tries to message the chat it is already running in — otherwise a chat can kick off new work on itself, again and again.
That safety check works for Copilot. For **Codex and Claude it silently does nothing**, so the message goes through and a new turn starts in the same chat.
The E2E test for this is currently skipped for both providers via the `supportsSelfSendRejection` gate in [`serverToolsSuite.ts:79`](../blob/main/src/vs/platform/agentHost/test/node/e2e/suites/serverToolsSuite.ts#L79), and it's written up in `KNOWN_ISSUES.md` twice — once for Claude (line 390) and once for Codex (line 427).
## Why it happens
The check is a plain string comparison at [`sessionServerTools.ts:743`](../blob/main/src/vs/platform/agentHost/node/shared/sessionServerTools.ts#L743):
```ts
if (currentChannel && chat.toString() === URI.parse(currentChannel).toString()) {
```
The left side is **always** a chat URI (`ahp-chat://default/`), built by `buildDefaultChatUri`. The right side is whatever the provider handed us as "the channel I'm running on" — and the three providers hand us **different kinds of thing**.
```mermaid
flowchart TD
A["send_message(session: 'x')"] --> B["target chat
= ahp-chat://default/base64(x)"]
B --> C{"is the target chat the same string
as the channel I'm running on?"}
C -->|"Copilot passes
ahp-chat://default/base64(x)
→ same string"| D["Rejected ✅"]
C -->|"Codex / Claude pass
codex:/x or claude:/x
→ different string"| E["Sent. The chat messages itself ❌"]
```
Where each provider gets it from:
| Provider | Call site | What it passes |
|---|---|---|
| Copilot | `copilot/copilotAgentSession.ts:1692` → `_chatChannelUri` | `ahp-chat://default/` — a **chat** URI |
| Codex | `codex/codexAgent.ts:1781` → `session.sessionUri` | `codex:/` — a **session** URI |
| Claude | `claude/claudeServerToolMcpServer.ts:62`, fed `_storageUri` from `claude/claudeAgentSession.ts:768` (`_storageUri` is defined at `claudeAgentSession.ts:133-135` and collapses the default chat down to the session URI) | `claude:/` — a **session** URI |
## The proof: `delete_session` gets this right
`delete_session` has the same kind of "don't target yourself" check and it works on **all three** providers. The only difference is that it cleans up the channel first, and `send_message` doesn't.
```mermaid
flowchart LR
CH["channel from provider
(chat URI or session URI —
depends on the provider)"]
CH --> D["delete_session
dispatch at line 1104"]
CH --> S["send_message
dispatch at line 1097"]
D --> DN["currentSessionUri()
lines 175-178
turns a chat URI into a session URI"]
DN --> DG["guard at line 974
compares like with like
works everywhere ✅"]
S --> SG["no cleanup at all
guard at line 743
only works for Copilot ❌"]
```
Side note on why nobody caught this: the unit test at `test/node/sessionServerTools.test.ts:441` hardcodes `buildDefaultChatUri('copilot:/s1')` as the channel, so it only ever exercises the Copilot shape.
## Which way should we normalize?
Careful here — the obvious move (reuse `currentSessionUri()`) is the **wrong** one. That squashes a chat down to its session, which would make `send_message` refuse to message **any** chat in the current session, not just the one it's running in.
```mermaid
flowchart TD
CH["channel: claude:/x"]
CH --> W["❌ currentSessionUri()
→ claude:/x (a session)"]
CH --> R["✅ currentChatUri() (new)
→ ahp-chat://default/base64(x)"]
W --> WB["Too strict: also blocks messaging a
sibling chat in the same session"]
R --> RB["Just right: blocks only the exact
chat that invoked the tool"]
```
Four things say the rule is meant to be **per chat**, not per session:
1. The code comment: "Refuses to target `currentChannel` (**the chat channel the tool runs on**)" — `sessionServerTools.ts:737`
2. The error message users see: "refusing to send a message to **the current chat**" — line 744
3. The test name and what it matches on: `send_message refuses to target the invoking chat`, `/current chat/i` — `serverToolsSuite.ts:743,750`
4. It would break a real workflow: `create_chat` defaults to the **current** session (lines 674-676) and hands back a link whose whole point is to be passed to `send_message` (see the tool description on line 154). Squashing to the session would reject an agent messaging the sibling chat it just made.
## Where to fix it: shared code, not the providers
Two options were considered:
- **(A) Normalize inside the guard.** One shared change, fixes all three providers at once and any future provider too.
- **(B) Make Codex and Claude pass a chat URI like Copilot does.** Riskier, touches live session wiring in two agents, and doesn't protect the next guard someone writes. For Claude specifically, `_storageUri` is *deliberately* the session URI for the default chat because it keys per-chat storage — changing what reaches `buildServerToolMcpServer` means introducing a second URI concept at that seam.
**Recommend (A).** It's smaller and safer.
Worth noting Codex is single-chat, so mapping its session URI to "the default chat" is unambiguous — `chats.createChat` throws `'Codex agent does not support multiple chats'` (`codexAgent.ts:2804-2806`). Claude and Copilot already pass their real peer-chat URI when they're in a peer chat, so for them the normalization is a no-op.
Proposed helper, mirroring the existing `currentSessionUri` and using only helpers already imported in that file:
```ts
/** Resolves the chat channel URI for the channel a tool call runs on. */
export function currentChatUri(toolCallChannel: ProtocolURI): URI {
const canonical = URI.parse(toolCallChannel).toString();
return URI.parse(parseChatUri(toolCallChannel) ? canonical : buildDefaultChatUri(canonical));
}
```
Then compare against `currentChatUri(currentChannel)` at line 743. Running both sides through `URI.parse(...).toString()` first keeps the strings byte-identical before base64 encoding, matching what `getSendMessageArgs` already does with the session URI from `listSessions`.
## Anything else with the same bug?
No — I checked every `.toString() ===` in the file and every use of the dispatch channel:
- The `delete_session` guard (line 974) and `serializeCurrentSession` (line 938) are handed already-normalized input from lines 1104 / 1076. Fine.
- The `create_session` / `create_chat` recursion guards (lines 593, 685) call `currentSessionUri` themselves. Fine.
- `create_session` / `create_chat` do pass the raw channel to `getCreationDefaults` (lines 600, 687 → `agentService.ts:860`), but that lands on `AgentHostStateManager.getSessionState`, which explicitly accepts either form (`agentHostStateManager.ts:328-342`). Fine.
`send_message` is the only one.
## What this unblocks
- **`supportsSelfSendRejection`** (`serverToolsSuite.ts:79`) can become unconditional — one test, for **both** Codex and Claude. The rest of that test's assertions follow for free: the throw happens before `accessor.startPrompt`, so no extra turn is ever started, and both providers already report the failure correctly (Codex wraps the throw via `_toolFailure` at `codexAgent.ts:1783-1785`; Claude's `isError: true` from `claudeServerToolMcpServer.ts:66` maps to `success: !isError` at `claudeMapSessionEvents.ts:386`).
- **Not** unblocked: `supportsCrossSessionSend` (`serverToolsSuite.ts:75`). Codex's `send_message starts a turn in another session` fails earlier with `Authorization header is badly formatted` — different root cause, leave that gate alone.
- `KNOWN_ISSUES.md`: delete the Claude section (lines 390-401) and the Codex section (lines 427-438), and narrow the two repro commands at line 408 (drop `|send_message refuses`, and "all three Claude tests" becomes two) and line 445 (`send_message` becomes `send_message starts a turn`).
One practical catch: E2E replay is strict — an unrecorded request is a hard failure — and `e2e/captures/` only has `copilotcli-server-tool-send-message-refuses-to-target-the-invoking-chat.yaml`. Ungating means recording Claude and Codex fixtures against real CAPI (`AGENT_HOST_REPLAY_RECORD=1`, needs a token).
## Plan
1. Add `currentChatUri()` next to `currentSessionUri()` (`sessionServerTools.ts:178`) and use it in the guard at line 743.
2. Document on `IAgentServerToolHost.executeTool` (`shared/agentServerToolHost.ts:137`) that the channel can be **either** a chat URI or a bare session URI, and that guards must normalize with `currentSessionUri` / `currentChatUri`. This is the bit that stops the same bug coming back.
3. Extend the unit test at `test/node/sessionServerTools.test.ts:435` to run the guard with all three channel shapes — chat URI (Copilot), bare session URI (Codex/Claude default chat), and a peer-chat URI — asserting the last one still **allows** messaging a sibling chat in the same session. This catches it without needing recorded fixtures.
4. Record Claude + Codex captures, flip `supportsSelfSendRejection` to `true`, update `KNOWN_ISSUES.md`.
Steps 1-3 are fully verifiable offline. Step 4 needs a token.
All line numbers are against `main` @ `28a37ffe0f3`.
Contributor guide
Assessment
This issue has not been assessed yet.