-
Notifications
You must be signed in to change notification settings - Fork 14
Fix Codex bridge tool guidance #57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,17 +1,21 @@ | ||||||||||||||||
| import { BRIDGE_SERVER, CLAUDE_CHANNEL_USER } from "./bridge-constants"; | ||||||||||||||||
| import type { Agent } from "./types"; | ||||||||||||||||
|
|
||||||||||||||||
| type BridgeTool = "bridge_status" | "receive_messages" | "send_message"; | ||||||||||||||||
|
|
||||||||||||||||
| const bridgeTargetLiteral = (agent: Agent): string => `target: "${agent}"`; | ||||||||||||||||
| const codexBridgeToolName = (tool: BridgeTool): string => | ||||||||||||||||
| `mcp__${BRIDGE_SERVER.replaceAll("-", "_")}__${tool}`; | ||||||||||||||||
|
|
||||||||||||||||
| export const bridgeToolName = (agent: Agent, tool: BridgeTool): string => | ||||||||||||||||
| agent === "claude" ? tool : codexBridgeToolName(tool); | ||||||||||||||||
|
Comment on lines
+10
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The quotedBridgeTool helper is currently duplicated in paired-loop.ts and tmux.ts. It should be defined once here and exported to improve maintainability. Ensure that the resulting strings are used within newline-delimited JSON for agent communication.
Suggested change
References
|
||||||||||||||||
|
|
||||||||||||||||
| export const bridgeStatusStuckGuidance = | ||||||||||||||||
| 'Use "bridge_status" only when direct delivery appears stuck.'; | ||||||||||||||||
|
|
||||||||||||||||
| export const receiveMessagesStuckGuidance = | ||||||||||||||||
| 'Use "bridge_status" or "receive_messages" only if delivery looks stuck.'; | ||||||||||||||||
|
|
||||||||||||||||
| export const sendToClaudeGuidance = (): string => | ||||||||||||||||
| `Use "send_message" with ${bridgeTargetLiteral("claude")} for Claude-facing messages, not a human-facing message.`; | ||||||||||||||||
|
|
||||||||||||||||
| export const sendProactiveCodexGuidance = (): string => | ||||||||||||||||
| `Use "send_message" with ${bridgeTargetLiteral("codex")} for Codex-facing messages, including replies to inbound Codex channel messages; do not send Codex-facing responses as a human-facing message.`; | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { | |||||
| acknowledgeBridgeDelivery, | ||||||
| readNextPendingBridgeMessage, | ||||||
| } from "./bridge-dispatch"; | ||||||
| import { bridgeToolName } from "./bridge-guidance"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import the shared quotedBridgeTool helper from bridge-guidance.ts to reduce local duplication.
Suggested change
References
|
||||||
| import { formatCodexBridgeMessage } from "./bridge-message-format"; | ||||||
| import { getLastClaudeSessionId } from "./claude-sdk-server"; | ||||||
| import { getLastCodexThreadId } from "./codex-app-server"; | ||||||
|
|
@@ -47,6 +48,7 @@ import type { | |||||
| import { hasSignal } from "./utils"; | ||||||
|
|
||||||
| const MAX_BRIDGE_HOPS = 12; | ||||||
| type BridgeTool = "bridge_status" | "receive_messages" | "send_message"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
|
|
||||||
| interface PairedState { | ||||||
| manifest: RunManifest; | ||||||
|
|
@@ -100,30 +102,34 @@ const bridgeGuidance = (agent: Agent): string => { | |||||
| const target = agent === "claude" ? "codex" : "claude"; | ||||||
| return [ | ||||||
| "Paired mode:", | ||||||
| `You are in a persistent Claude/Codex pair. Use the MCP tool "send_message" 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" only if delivery looks stuck.', | ||||||
| 'Use "receive_messages" only if "bridge_status" shows pending messages addressed to you and direct delivery looks stuck.', | ||||||
| `You are in a persistent Claude/Codex pair. Use the MCP tool ${quotedBridgeTool(agent, "send_message")} 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 ${quotedBridgeTool(agent, "bridge_status")} only if delivery looks stuck.`, | ||||||
| `Use ${quotedBridgeTool(agent, "receive_messages")} only if ${quotedBridgeTool(agent, "bridge_status")} shows pending messages addressed to you and direct delivery looks stuck.`, | ||||||
| ].join("\n"); | ||||||
| }; | ||||||
|
|
||||||
| const bridgeToolGuidance = [ | ||||||
| 'You can use the MCP tools "send_message", "bridge_status", and "receive_messages" for direct Claude/Codex coordination.', | ||||||
| 'Only use "bridge_status" or "receive_messages" when delivery looks stuck.', | ||||||
| "Do not ask the human to relay messages between agents.", | ||||||
| ].join("\n"); | ||||||
| const quotedBridgeTool = (agent: Agent, tool: BridgeTool): string => | ||||||
| `"${bridgeToolName(agent, tool)}"`; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
|
|
||||||
| const bridgeToolGuidance = (agent: Agent): string => | ||||||
| [ | ||||||
| `You can use the MCP tools ${quotedBridgeTool(agent, "send_message")}, ${quotedBridgeTool(agent, "bridge_status")}, and ${quotedBridgeTool(agent, "receive_messages")} for direct Claude/Codex coordination.`, | ||||||
| `Only use ${quotedBridgeTool(agent, "bridge_status")} or ${quotedBridgeTool(agent, "receive_messages")} when delivery looks stuck.`, | ||||||
| "Do not ask the human to relay messages between agents.", | ||||||
| ].join("\n"); | ||||||
|
|
||||||
| const reviewDeliveryGuidance = (reviewer: Agent, opts: Options): string => { | ||||||
| if (reviewer === opts.agent) { | ||||||
| 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_message" using ${bridgeTargetLiteral(opts.agent)} before returning your final review signal.`; | ||||||
| return `If review is needed, send the actionable notes to ${capitalize(opts.agent)} with ${quotedBridgeTool(reviewer, "send_message")} using ${bridgeTargetLiteral(opts.agent)} before returning your final review signal.`; | ||||||
| }; | ||||||
|
|
||||||
| const reviewToolGuidance = (reviewer: Agent, opts: Options): string => | ||||||
| reviewer === opts.agent | ||||||
| ? "Use the review body itself for follow-up notes. No bridge message is needed for a self-review." | ||||||
| : bridgeToolGuidance; | ||||||
| : bridgeToolGuidance(reviewer); | ||||||
|
|
||||||
| const formatSelfReviewNotes = ( | ||||||
| failures: ReviewFailure[], | ||||||
|
|
@@ -158,22 +164,26 @@ const forwardBridgePrompt = ({ | |||||
| }: { | ||||||
| message: string; | ||||||
| source: Agent; | ||||||
| }): string => | ||||||
| (source === "claude" | ||||||
| ? [ | ||||||
| formatCodexBridgeMessage(source, message), | ||||||
| "Treat this as direct agent-to-agent coordination. Do not reply to the human.", | ||||||
| 'Send a message to the other agent with "send_message" only when you have something useful for them to act on.', | ||||||
| "Do not acknowledge receipt without new information.", | ||||||
| ] | ||||||
| : [ | ||||||
| `Message from ${capitalize(source)} via the loop bridge:`, | ||||||
| message.trim(), | ||||||
| "Treat this as direct agent-to-agent coordination. Do not reply to the human.", | ||||||
| 'Send a message to the other agent with "send_message" only when you have something useful for them to act on.', | ||||||
| "Do not acknowledge receipt without new information.", | ||||||
| ] | ||||||
| }): string => { | ||||||
| const agent = source === "claude" ? "codex" : "claude"; | ||||||
| const replyGuidance = `Send a message to the other agent with ${quotedBridgeTool(agent, "send_message")} only when you have something useful for them to act on.`; | ||||||
| return ( | ||||||
| source === "claude" | ||||||
| ? [ | ||||||
| formatCodexBridgeMessage(source, message), | ||||||
| "Treat this as direct agent-to-agent coordination. Do not reply to the human.", | ||||||
| replyGuidance, | ||||||
| "Do not acknowledge receipt without new information.", | ||||||
| ] | ||||||
| : [ | ||||||
| `Message from ${capitalize(source)} via the loop bridge:`, | ||||||
| message.trim(), | ||||||
| "Treat this as direct agent-to-agent coordination. Do not reply to the human.", | ||||||
| replyGuidance, | ||||||
| "Do not acknowledge receipt without new information.", | ||||||
| ] | ||||||
| ).join("\n\n"); | ||||||
| }; | ||||||
|
|
||||||
| const updateIds = (state: PairedState): void => { | ||||||
| const next = touchRunManifest( | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,9 +13,9 @@ import { | |||||||
| resolveClaudeChannelServerName, | ||||||||
| } from "./bridge-config"; | ||||||||
| import { | ||||||||
| bridgeToolName, | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import the shared quotedBridgeTool helper from bridge-guidance.ts instead of defining it locally.
Suggested change
References
|
||||||||
| receiveMessagesStuckGuidance, | ||||||||
| sendProactiveCodexGuidance, | ||||||||
| sendToClaudeGuidance, | ||||||||
| } from "./bridge-guidance"; | ||||||||
| import { getCodexAppServerUrl, getLastCodexThreadId } from "./codex-app-server"; | ||||||||
| import { | ||||||||
|
|
@@ -155,6 +155,11 @@ const appendProofPrompt = (parts: string[], proof: string): void => { | |||||||
| parts.push(`Proof requirements:\n${trimmed}`); | ||||||||
| }; | ||||||||
|
|
||||||||
| const quotedBridgeTool = ( | ||||||||
| agent: Agent, | ||||||||
| tool: "bridge_status" | "receive_messages" | "send_message" | ||||||||
| ): string => `"${bridgeToolName(agent, tool)}"`; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||
|
|
||||||||
| const pairedBridgeGuidance = ( | ||||||||
| agent: Agent, | ||||||||
| _runId: string, | ||||||||
|
|
@@ -168,7 +173,10 @@ const pairedBridgeGuidance = ( | |||||||
| ].join("\n"); | ||||||||
| } | ||||||||
|
|
||||||||
| return [sendToClaudeGuidance(), receiveMessagesStuckGuidance].join("\n"); | ||||||||
| return [ | ||||||||
| `Use the MCP tool ${quotedBridgeTool(agent, "send_message")} with target: "claude" for Claude-facing messages, not a human-facing message.`, | ||||||||
| `Use ${quotedBridgeTool(agent, "bridge_status")} or ${quotedBridgeTool(agent, "receive_messages")} only if delivery looks stuck.`, | ||||||||
| ].join("\n"); | ||||||||
| }; | ||||||||
|
|
||||||||
| const pairedWorkflowGuidance = (opts: Options, agent: Agent): string => { | ||||||||
|
|
@@ -202,7 +210,7 @@ const buildPrimaryPrompt = ( | |||||||
| const parts = [ | ||||||||
| `Agent-to-agent pair programming: you are the primary ${capitalize(opts.agent)} agent for this run.`, | ||||||||
| `Task:\n${task.trim()}`, | ||||||||
| `Your peer is ${peer}. Do the initial pass yourself, then use "send_message" when you want review or targeted help from ${peer}.`, | ||||||||
| `Your peer is ${peer}. Do the initial pass yourself, then use ${quotedBridgeTool(opts.agent, "send_message")} when you want review or targeted help from ${peer}.`, | ||||||||
| ]; | ||||||||
| appendProofPrompt(parts, opts.proof); | ||||||||
| parts.push(SPAWN_TEAM_WITH_WORKTREE_ISOLATION); | ||||||||
|
|
@@ -245,7 +253,7 @@ const buildInteractivePrimaryPrompt = ( | |||||||
| const parts = [ | ||||||||
| `Agent-to-agent pair programming: you are the primary ${capitalize(opts.agent)} agent for this run.`, | ||||||||
| "No task has been assigned yet.", | ||||||||
| `Your peer is ${peer}. Use "send_message" for review or help once the human gives you a task.`, | ||||||||
| `Your peer is ${peer}. Use ${quotedBridgeTool(opts.agent, "send_message")} for review or help once the human gives you a task.`, | ||||||||
| ]; | ||||||||
| appendProofPrompt(parts, opts.proof); | ||||||||
| parts.push( | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { bridgeToolName } from "../../src/loop/bridge-guidance"; | ||
|
|
||
| test("bridgeToolName namespaces Codex bridge tools only", () => { | ||
| expect(bridgeToolName("codex", "send_message")).toBe( | ||
| "mcp__loop_bridge__send_message" | ||
| ); | ||
| expect(bridgeToolName("codex", "bridge_status")).toBe( | ||
| "mcp__loop_bridge__bridge_status" | ||
| ); | ||
| expect(bridgeToolName("claude", "send_message")).toBe("send_message"); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BridgeTool type is used in multiple files (paired-loop.ts, tmux.ts). Exporting it from here centralizes the definition and avoids duplication.
References