Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames the send_to_agent tool to send_message and introduces a more robust message delivery mechanism using a structured <loop-bridge> XML-like envelope that includes message IDs. It also replaces the polling-based message delivery for Claude with a file-system watcher (fs.watch) and a fallback sweep timer to improve responsiveness and efficiency. Feedback focuses on improving the robustness of the new delivery logic, specifically suggesting a debounce for the file watcher to prevent redundant processing during rapid file writes, more specific regex for tag stripping to avoid accidental matches, and ensuring the fallback timer is cleared before rescheduling to prevent overlapping timers.
| const scheduleClaudeSweep = (): void => { | ||
| if (!(source === "claude" && channelReady) || closed || fallbackSweep) { | ||
| return; | ||
| } | ||
| fallbackSweep = setTimeout(() => { | ||
| fallbackSweep = undefined; | ||
| if (closed) { | ||
| return; | ||
| } | ||
| await new Promise((resolve) => { | ||
| setTimeout(resolve, CHANNEL_POLL_DELAY_MS); | ||
| }); | ||
| } | ||
| triggerClaudeFlush(); | ||
| scheduleClaudeSweep(); | ||
| }, CLAUDE_CHANNEL_FALLBACK_SWEEP_MS); | ||
| fallbackSweep.unref?.(); | ||
| }; |
There was a problem hiding this comment.
The fallback sweep logic could potentially lead to multiple overlapping timers if scheduleClaudeSweep is called concurrently before fallbackSweep is assigned. While the current usage in notifications/initialized seems safe, adding a check to ensure any existing timer is cleared before scheduling a new one would be more robust.
| bridgeWatcher = watch(runDir, (_eventType, filename) => { | ||
| if (!isBridgeWatchEvent(runDir, filename)) { | ||
| return; | ||
| } | ||
| await handleBridgeRequest(runDir, source, request); | ||
| await queueClaudeFlush(); | ||
| }; | ||
| requestQueue = requestQueue.then(handleRequest, handleRequest); | ||
| }, | ||
| () => { | ||
| closed = true; | ||
| triggerClaudeFlush(); | ||
| }); |
There was a problem hiding this comment.
The fs.watch callback is triggered on every file system event in the runDir. Since triggerClaudeFlush initiates a promise chain that reads the entire bridge inbox, frequent events (e.g., multiple rapid writes to the bridge file) could lead to redundant processing. Consider adding a small debounce to triggerClaudeFlush to improve efficiency during bursts of activity.
src/loop/bridge-message-format.ts
Outdated
| @@ -1,18 +1,34 @@ | |||
| import type { Agent } from "./types"; | |||
|
|
|||
| const BRIDGE_TAG_RE = /<\/?loop-bridge(?:\s+[^>]*)?>/gi; | |||
There was a problem hiding this comment.
The BRIDGE_TAG_RE regex is quite permissive with [^\u003e]*. While it works for the current structured envelope, it might accidentally match and strip other tag-like strings in the message body if they happen to start with loop-bridge. A more specific regex matching only the expected attributes (source and message_id) would be safer.
1 participant
Summary
This change cleans up paired bridge messaging in interactive tmux mode.
It renames the user-facing bridge tool from
send_to_agenttosend_message, updates the paired and tmux guidance so the prompts teach that contract consistently, replaces Claude-side bridge polling withfs.watchplus a bounded fallback sweep, and wraps Codex-bound peer messages in a structured<loop-bridge ...>text envelope with stable metadata.Why
The bridge contract was still teaching transport-flavored behavior instead of the user action we actually want agents to take. Claude delivery also depended on a fixed polling loop, and Codex inbound messages were less structurally distinct than Claude channel messages.
Impact
Paired prompts, bridge config, and tests now consistently use
send_message. The oldsend_to_agentname is rejected with a clear rename hint. Claude-side peer delivery now flushes on file events instead of relying on a 500 ms poll loop, and Codex receives bridge messages with explicit peer-message structure.Root Cause
The bridge contract had drifted toward transport-specific naming and polling-based delivery. While adding the new Codex wrapper, normalization also needed an extra trim step so wrapped and unwrapped forms collapse to the same value and bounce detection still works.
Validation
bun run fixbun run checkbun test