diff --git a/src/loop/bridge.ts b/src/loop/bridge.ts index b0bc3d5..93caac7 100644 --- a/src/loop/bridge.ts +++ b/src/loop/bridge.ts @@ -96,6 +96,14 @@ const isRecord = (value: unknown): value is Record => const asString = (value: unknown): string | undefined => typeof value === "string" && value.trim() ? value : undefined; +const normalizeLowerString = (value: unknown): string | undefined => { + if (typeof value !== "string") { + return undefined; + } + const normalized = value.trim().toLowerCase(); + return normalized || undefined; +}; + const normalizeAgent = (value: unknown): Agent | undefined => { if (value === "claude" || value === "codex") { return value; @@ -435,7 +443,7 @@ const claudeChannelInstructions = (): string => `Messages from the Codex agent arrive as .`, 'When you are replying to an inbound channel message, use the "reply" tool and pass back the same chat_id.', "Never answer the human when the inbound message came from Codex. Send the response back through the bridge tools instead.", - 'Use the "send_to_agent" tool for proactive messages to Codex that are not direct replies to a channel message.', + 'Use the "send_to_agent" tool with target: "codex" for proactive messages that are not direct replies to a channel message.', 'Use "bridge_status" only when direct delivery appears stuck.', ].join("\n"); @@ -711,13 +719,22 @@ const handleSendToAgentTool = async ( source: Agent, args: Record ): Promise => { - const target = normalizeAgent(args.target); + const normalizedTarget = normalizeLowerString(args.target); const message = asString(args.message); + if (!normalizedTarget) { + writeError( + id, + MCP_INVALID_PARAMS, + "send_to_agent requires a non-empty target" + ); + return; + } + const target = normalizeAgent(normalizedTarget); if (!target) { writeError( id, MCP_INVALID_PARAMS, - "send_to_agent requires target=claude|codex" + `Unknown target "${normalizedTarget}" - expected "claude" or "codex"` ); return; } diff --git a/src/loop/paired-loop.ts b/src/loop/paired-loop.ts index 54405e0..9474174 100644 --- a/src/loop/paired-loop.ts +++ b/src/loop/paired-loop.ts @@ -80,6 +80,8 @@ export const pairedLoopInternals = { const capitalize = (value: string): string => value.slice(0, 1).toUpperCase() + value.slice(1); +const bridgeTargetLiteral = (agent: Agent): string => `target: "${agent}"`; + const resolvePairedReviewers = ( review: ReviewMode | undefined, agent: Agent @@ -91,9 +93,10 @@ const pairedResumeHint = (runId: string): void => { const bridgeGuidance = (agent: Agent): string => { const peer = agent === "claude" ? "Codex" : "Claude"; + const target = agent === "claude" ? "codex" : "claude"; return [ "Paired mode:", - `You are in a persistent Claude/Codex pair. Use the MCP tool "send_to_agent" when you want ${peer} to act, review, or answer.`, + `You are in a persistent Claude/Codex pair. Use the MCP tool "send_to_agent" with ${bridgeTargetLiteral(target)} when you want ${peer} to act, review, or answer.`, 'Do not ask the human to relay messages between agents or answer the human on the other agent\'s behalf. Use "bridge_status" if you need the current bridge state.', 'If "bridge_status" shows pending messages addressed to you, call "receive_messages" to read them.', ].join("\n"); @@ -109,7 +112,7 @@ const reviewDeliveryGuidance = (reviewer: Agent, opts: Options): string => { return "If review is needed, keep the actionable notes in your review body before the final review signal."; } - return `If review is needed, send the actionable notes to ${capitalize(opts.agent)} with "send_to_agent" before returning your final review signal.`; + return `If review is needed, send the actionable notes to ${capitalize(opts.agent)} with "send_to_agent" using ${bridgeTargetLiteral(opts.agent)} before returning your final review signal.`; }; const reviewToolGuidance = (reviewer: Agent, opts: Options): string => diff --git a/src/loop/tmux.ts b/src/loop/tmux.ts index b4dcc45..e088722 100644 --- a/src/loop/tmux.ts +++ b/src/loop/tmux.ts @@ -135,6 +135,8 @@ const capitalize = (value: string): string => const peerAgent = (agent: Agent): Agent => agent === "claude" ? "codex" : "claude"; +const bridgeTargetLiteral = (agent: Agent): string => `target: "${agent}"`; + const appendProofPrompt = (parts: string[], proof: string): void => { const trimmed = proof.trim(); if (!trimmed) { @@ -150,13 +152,13 @@ const pairedBridgeGuidance = (agent: Agent, runId: string): string => { return [ `Your bridge MCP server is "${serverName}". All bridge tool calls must use the mcp__${serverName}__ prefix.`, 'Reply to inbound Codex channel messages with the MCP tool "reply" and the same chat_id.', - 'Use "send_to_agent" only for new proactive messages to Codex; do not send Codex-facing responses as a human-facing message.', + `Use "send_to_agent" with ${bridgeTargetLiteral("codex")} only for new proactive messages to Codex; do not send Codex-facing responses as a human-facing message.`, 'Use "bridge_status" or "receive_messages" only if delivery looks stuck.', ].join("\n"); } return [ - 'Message Claude with "send_to_agent", not a human-facing message.', + `Use "send_to_agent" with ${bridgeTargetLiteral("claude")} for Claude-facing messages, not a human-facing message.`, 'Use "bridge_status" or "receive_messages" only if delivery looks stuck.', ].join("\n"); }; diff --git a/tests/loop/bridge.test.ts b/tests/loop/bridge.test.ts index 42f7d8c..bd28bd3 100644 --- a/tests/loop/bridge.test.ts +++ b/tests/loop/bridge.test.ts @@ -253,6 +253,119 @@ test("bridge MCP send_to_agent queues a direct message through the CLI path", as rmSync(root, { recursive: true, force: true }); }); +test("bridge MCP send_to_agent normalizes target case and whitespace", async () => { + const bridge = await loadBridge(); + const root = makeTempDir(); + const runDir = join(root, "run"); + mkdirSync(runDir, { recursive: true }); + + const result = await runBridgeProcess( + runDir, + "codex", + [ + encodeFrame({ + id: 1, + jsonrpc: "2.0", + method: "tools/call", + params: { + arguments: { + message: "ship it", + target: " CLAUDE ", + }, + name: "send_to_agent", + }, + }), + "\n", + ].join("") + ); + + expect(result.code).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain("queued"); + expect(bridge.readPendingBridgeMessages(runDir)).toEqual([ + expect.objectContaining({ + message: "ship it", + source: "codex", + target: "claude", + }), + ]); + rmSync(root, { recursive: true, force: true }); +}); + +test("bridge MCP send_to_agent rejects an empty target after trimming", async () => { + const root = makeTempDir(); + const runDir = join(root, "run"); + mkdirSync(runDir, { recursive: true }); + + const result = await runBridgeProcess( + runDir, + "codex", + [ + encodeFrame({ + id: 1, + jsonrpc: "2.0", + method: "tools/call", + params: { + arguments: { + message: "ship it", + target: " ", + }, + name: "send_to_agent", + }, + }), + "\n", + ].join("") + ); + + expect(result.code).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + error: { + code: -32_602, + message: "send_to_agent requires a non-empty target", + }, + id: 1, + jsonrpc: "2.0", + }); + rmSync(root, { recursive: true, force: true }); +}); + +test("bridge MCP send_to_agent rejects an unknown normalized target", async () => { + const root = makeTempDir(); + const runDir = join(root, "run"); + mkdirSync(runDir, { recursive: true }); + + const result = await runBridgeProcess( + runDir, + "codex", + [ + encodeFrame({ + id: 1, + jsonrpc: "2.0", + method: "tools/call", + params: { + arguments: { + message: "ship it", + target: " FOO ", + }, + name: "send_to_agent", + }, + }), + "\n", + ].join("") + ); + + expect(result.code).toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + error: { + code: -32_602, + message: 'Unknown target "foo" - expected "claude" or "codex"', + }, + id: 1, + jsonrpc: "2.0", + }); + rmSync(root, { recursive: true, force: true }); +}); + test("bridge MCP handles standard empty-list and ping requests through the CLI path", async () => { const root = makeTempDir(); const runDir = join(root, "run"); diff --git a/tests/loop/paired-loop.test.ts b/tests/loop/paired-loop.test.ts index 8e0a7c4..3fc6d64 100644 --- a/tests/loop/paired-loop.test.ts +++ b/tests/loop/paired-loop.test.ts @@ -865,7 +865,7 @@ test("runPairedLoop preserves claudex reviewers in paired mode", async () => { "concrete file paths, commands, and code locations that must change" ); expect(reviewPrompts[1]?.prompt).toContain( - 'send the actionable notes to Claude with "send_to_agent"' + 'send the actionable notes to Claude with "send_to_agent" using target: "claude"' ); }); }); diff --git a/tests/loop/tmux.test.ts b/tests/loop/tmux.test.ts index ac61a5b..afa42ed 100644 --- a/tests/loop/tmux.test.ts +++ b/tests/loop/tmux.test.ts @@ -770,7 +770,7 @@ test("tmux prompts keep the paired review workflow explicit", () => { ); expect(primaryPrompt).toContain("Wait briefly if it arrives"); expect(primaryPrompt).toContain( - 'Message Claude with "send_to_agent", not a human-facing message' + 'Use "send_to_agent" with target: "claude" for Claude-facing messages' ); expect(primaryPrompt).toContain("worktree isolation"); expect(peerPrompt).toContain("You are the reviewer/support agent."); @@ -778,7 +778,7 @@ test("tmux prompts keep the paired review workflow explicit", () => { expect(peerPrompt).toContain("Wait for Codex to send you a targeted request"); expect(peerPrompt).toContain('"reply"'); expect(peerPrompt).toContain( - 'Use "send_to_agent" only for new proactive messages to Codex; do not send Codex-facing responses as a human-facing message.' + 'Use "send_to_agent" with target: "codex" only for new proactive messages to Codex; do not send Codex-facing responses as a human-facing message.' ); expect(primaryPrompt).not.toContain("mcp__loop-bridge-1__ prefix"); expect(peerPrompt).toContain("mcp__loop-bridge-1__ prefix"); @@ -797,7 +797,7 @@ test("interactive tmux prompts tell both agents to wait for the human", () => { expect(primaryPrompt).toContain("No task has been assigned yet."); expect(primaryPrompt).toContain("Wait for the first human task"); expect(primaryPrompt).toContain( - 'Message Claude with "send_to_agent", not a human-facing message' + 'Use "send_to_agent" with target: "claude" for Claude-facing messages' ); expect(primaryPrompt).toContain("worktree isolation"); expect(peerPrompt).toContain("No task has been assigned yet."); @@ -805,7 +805,7 @@ test("interactive tmux prompts tell both agents to wait for the human", () => { expect(peerPrompt).toContain("human clearly assigns you separate work"); expect(peerPrompt).toContain('"reply"'); expect(peerPrompt).toContain( - 'Use "send_to_agent" only for new proactive messages to Codex; do not send Codex-facing responses as a human-facing message.' + 'Use "send_to_agent" with target: "codex" only for new proactive messages to Codex; do not send Codex-facing responses as a human-facing message.' ); expect(peerPrompt).toContain( "If you are answering Codex, use the bridge tools instead of a human-facing reply."