Upgrade to agent-relay v8 + terminal latency (predictive echo)#81
Merged
Conversation
Migrate the broker integration from @agent-relay/sdk (v7) to @agent-relay/harness-driver (v8.1.2), and add predictive local echo to the terminal: - Bump agent-relay packages + bundled broker binary to 8.1.2. - Predictive (mosh-style) echo: optimistically render printable keystrokes and reconcile against authoritative PTY output. Adaptive on measured latency, so it stays invisible on fast local links. - Feed the broker's PtyInputStream input→ack SRTT into the engine's adaptive engage decision so prediction engages promptly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR migrates the Pear app from ChangesDependency Migration & Predictive Echo Integration
🎯 4 (Complex) | ⏱️ ~75 minutes
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
codeant-ai
Bot
added
the
size:XXL
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2524b868cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| __dirname, '..', '..', 'node_modules', '@agent-relay', `broker-${suffix}`, 'bin', | ||
| process.platform === 'win32' ? 'agent-relay-broker.exe' : 'agent-relay-broker' | ||
| ) | ||
| if (canExecute(optionalPackageBinary)) return unpackIfPackaged(optionalPackageBinary) |
There was a problem hiding this comment.
When this runs in a packaged app, line 330 rewrites the v8 optional-package broker path to app.asar.unpacked, but electron-builder.yml still only unpacks node_modules/@agent-relay/sdk/bin/**. Since the new node_modules/@agent-relay/broker-${suffix}/bin/... files are not placed under app.asar.unpacked, packaged local broker startup will be handed a missing/non-executable path after the v8 upgrade. Please add the @agent-relay/broker-*/bin/** packages to asarUnpack (or otherwise keep this path executable).
Useful? React with 👍 / 👎.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| const bundledMcpScript = resolveBundledAgentRelayMcpScript() | ||
| const nodeCommand = bundledMcpScript ? resolveNodeCommandForMcp() : undefined | ||
| if (bundledMcpScript && nodeCommand) { | ||
| return `${nodeCommand} ${bundledMcpScript}` |
There was a problem hiding this comment.
Suggestion: The MCP command is built as a single unquoted string, so paths containing spaces (common on Windows/macOS app install locations) will be split incorrectly and fail to execute. Quote/escape command components or pass executable and args separately to avoid path-splitting failures. [possible bug]
Severity Level: Major ⚠️
- ⚠️ MCP servers fail to start when Node path has spaces.
- ⚠️ Agents relying on MCP tools silently lose functionality.Steps of Reproduction ✅
1. When starting a local broker, the renderer calls `pear.broker.start()`
(`src/preload/index.ts:61-62`), which is handled by `'broker:start'` IPC in
`src/main/ipc-handlers.ts:173-183`, calling `brokerManager.start(...)` in
`src/main/broker.ts:981-1115`.
2. In `BrokerManager.start`, before spawning the broker, `const agentRelayMcpCommand =
resolveAgentRelayMcpCommand()` is evaluated at `src/main/broker.ts:1055`, and if non-empty
it is passed through to the child via `env: { AGENT_RELAY_MCP_COMMAND:
agentRelayMcpCommand }` in the spawn options at `src/main/broker.ts:1062-1068`.
3. `resolveAgentRelayMcpCommand()` (`src/main/broker.ts:165-183`) resolves a Node
executable and the bundled MCP script: `bundledMcpScript` is found by
`resolveBundledAgentRelayMcpScript()` (lines 149-163) and `nodeCommand` is resolved by
`resolveNodeCommandForMcp()` (lines 140-147), which typically returns a path like
`C:\Program Files\nodejs\node.exe` on Windows. When both are present, the command is
constructed as a single unquoted string `\`${nodeCommand} ${bundledMcpScript}\`` at line
172.
4. The broker process (spawned via `AgentRelayClient.spawn(opts)` at
`src/main/broker.ts:1075`) reads `AGENT_RELAY_MCP_COMMAND` as a raw command string and
executes it using its own command-line parsing. Because `nodeCommand` can contain spaces
(e.g. under `Program Files` on Windows or any custom install path with spaces), composing
`C:\Program Files\nodejs\node.exe C:\path\to\agent-relay-mcp.js` without quoting or
splitting into `{file, args}` causes downstream parsing to split on spaces and treat
`C:\Program` as the executable. This mis-parsing breaks MCP startup (Node executable or
script cannot be found), so attempts by the broker to launch MCP servers for tools fail
whenever the Node or MCP script path includes spaces.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/main/broker.ts
**Line:** 172:172
**Comment:**
*Possible Bug: The MCP command is built as a single unquoted string, so paths containing spaces (common on Windows/macOS app install locations) will be split incorrectly and fail to execute. Quote/escape command components or pass executable and args separately to avoid path-splitting failures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+322
to
+330
| const unpackIfPackaged = (binary: string): string => | ||
| app.isPackaged ? binary.replace('app.asar', 'app.asar.unpacked') : binary | ||
|
|
||
| // v8 ships the broker as a per-platform optional package (@agent-relay/broker-*). | ||
| const optionalPackageBinary = join( | ||
| __dirname, '..', '..', 'node_modules', '@agent-relay', `broker-${suffix}`, 'bin', | ||
| process.platform === 'win32' ? 'agent-relay-broker.exe' : 'agent-relay-broker' | ||
| ) | ||
| if (canExecute(optionalPackageBinary)) return unpackIfPackaged(optionalPackageBinary) |
There was a problem hiding this comment.
Suggestion: The optional v8 broker binary check is done on the packed app.asar path, then only rewritten to app.asar.unpacked after the check. In packaged builds this can incorrectly fail the canExecute test and skip a valid unpacked optional binary, causing fallback to a path that may not exist. Apply the asar-unpack rewrite before the executability check. [logic error]
Severity Level: Critical 🚨
- ❌ Packaged Pear app fails to spawn local broker binary.
- ❌ Terminal attach operations fail; no PTY-backed agent sessions.
- ⚠️ Local Relay-powered features become unavailable for all projects.Steps of Reproduction ✅
1. In the renderer, opening a project starts the local broker via `pear.broker.start()` in
`src/preload/index.ts:61-62`, which invokes IPC channel `'broker:start'`.
2. The main process handles `'broker:start'` in `src/main/ipc-handlers.ts:173-183`,
calling `brokerManager.start(projectId, cwd, name, win, channels)` implemented in
`src/main/broker.ts:981-1115`.
3. Inside `BrokerManager.start`, when no existing broker is found, it prepares spawn
options with `binaryPath: resolveBundledBrokerBinary()` at `src/main/broker.ts:1062-1068`,
so every local broker start calls `resolveBundledBrokerBinary()`.
4. In `resolveBundledBrokerBinary()` (`src/main/broker.ts:310-338`), the optional v8
broker path is computed as `optionalPackageBinary` at lines 321-329. In a packaged build
(`app.isPackaged === true`), the real binary is unpacked under an `app.asar.unpacked`
path, but executability is checked against the *packed* path: `if
(canExecute(optionalPackageBinary))` at line 330, only rewriting to the unpacked path via
`unpackIfPackaged` on return. This causes `canExecute` to fail even when the unpacked
binary exists, so the code wrongly skips the optional broker and falls through to the
legacy SDK path `brokerBinary` (lines 334-338). If the SDK no longer ships a per-platform
broker binary at that fallback path (the intended v8 layout),
`AgentRelayClient.spawn(opts)` at `src/main/broker.ts:1075` will attempt to spawn a
non-existent binary and the broker fails to start in production builds.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/main/broker.ts
**Line:** 322:330
**Comment:**
*Logic Error: The optional v8 broker binary check is done on the packed `app.asar` path, then only rewritten to `app.asar.unpacked` after the check. In packaged builds this can incorrectly fail the `canExecute` test and skip a valid unpacked optional binary, causing fallback to a path that may not exist. Apply the asar-unpack rewrite before the executability check.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| // Prime the engine's confirmed-screen model with the same bytes (this | ||
| // does not re-write to the terminal) so its cursor matches the real | ||
| // screen before any prediction is made. | ||
| await predictiveEchoRef.current?.seed(result.snapshot.screen) |
There was a problem hiding this comment.
Suggestion: Awaiting predictive-echo seeding before subscribing to buffered PTY chunks increases the attach race window and can skip output that arrives between snapshot write and buffer subscription. Seed without delaying replay-state handoff (or subscribe first) so no server output is dropped during attach. [race condition]
Severity Level: Major ⚠️
- ⚠️ Terminal intermittently drops PTY output during attach handshake.
- ⚠️ Users may miss initial prompts or command responses.Steps of Reproduction ✅
1. A user opens an agent terminal in the UI, which uses `useTerminal()` from
`src/renderer/src/hooks/use-terminal.ts:141-148`. That hook wires the terminal to the
broker via IPC (`pear.broker.attachTerminal`, `pear.broker.onPtyChunk`) defined in
`src/preload/index.ts:80-101`.
2. Inside `useTerminal`, the main effect (lines 173-470) initializes the xterm instance in
`init()` (lines 305-413). After creating the `Terminal`, predictive-echo engine, and
sizing it, the code calls `attachAndSeedTerminal(term, initialSize)` at
`src/renderer/src/hooks/use-terminal.ts:374` to attach to the remote PTY.
3. `attachAndSeedTerminal` (lines 262-303) calls `pear.broker.attachTerminal(...)` at
lines 269-275, which returns a snapshot containing `result.snapshot.screen` for the
authoritative PTY screen. If a snapshot is present, it writes that screen to the live
terminal at 279-280, then *awaits*
`predictiveEchoRef.current?.seed(result.snapshot.screen)` at line 284 to prime the
predictive-echo engine. Only after this asynchronous `seed` completes does it set
`writtenChunksRef.current = useAgentStore.getState().getAgentBuffer(projectId,
agentName!).length` at line 285 and later call `subscribeToBuffer(targetTerm)` at line
302.
4. Meanwhile, PTY output from the broker is streamed into the renderer via
`'broker:pty-chunk'` IPC in `src/main/broker.ts:1482-1495`, with the renderer appending
chunks to the PTY buffer through `appendPtyChunk` in
`src/renderer/src/stores/pty-buffer-store.ts:17-33`. `getAgentBuffer` in the agent store
(`src/renderer/src/stores/agent-store.ts:1011-1012`) simply calls `getPtyChunks`, so any
chunks that arrive between the snapshot being taken and the async `seed()` completion are
already in the buffer but have not yet been written to the terminal. Because
`writtenChunksRef.current` is then advanced to the *full* buffer length before the first
`writeFromBuffer(getPtyChunks(...))` in `subscribeToBuffer` (lines 237-255), those
in-between chunks are sliced away and never written, causing a race where PTY output that
arrives during attach is permanently dropped from the terminal view.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/renderer/src/hooks/use-terminal.ts
**Line:** 284:284
**Comment:**
*Race Condition: Awaiting predictive-echo seeding before subscribing to buffered PTY chunks increases the attach race window and can skip output that arrives between snapshot write and buffer subscription. Seed without delaying replay-state handoff (or subscribe first) so no server output is dropped during attach.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
What
Two related changes that ship together (predictive echo requires the v8 runtime):
1. agent-relay v7 → v8 migration
@agent-relay/sdk(v7) to@agent-relay/harness-driver(v8.1.2);HarnessDriverClientreplacesAgentRelayClient.@agent-relay/broker-${platform}-${arch}) and falls back to the SDK-bundled binary, with asar-unpacking applied to both paths for packaged builds.agent-relay,@agent-relay/sdk,@agent-relay/cloud,@agent-relay/harness-driver, and the bundled broker binary to 8.1.2.2. Terminal latency improvements
v8.1.1/8.1.2 (relay #1045) brought the broker-side wins for free — ~16ms leading-edge PTY output flush and FIFO input pipelining. On top of that:
src/renderer/src/lib/predictive-echo.ts+use-terminal.ts): optimistically renders printable keystrokes (underlined) and reconciles against authoritative PTY output, rolling back mispredictions. Backed by a dedicated headless xterm as the confirmed-screen model so predictions never corrupt reconciliation. Adaptive on measured latency — invisible/dormant on fast local links.PtyInputStreaminput→ack SRTT is exposed viapear.broker.inputSrtt(...)and polled into the engine's adaptive engage decision, so prediction engages from the first acked keystroke and re-engages promptly when latency spikes (rather than waiting to measure a full echo-confirmation cycle).The predictive-echo engine is consumed via the
@agent-relay/harness-driver/predictive-echosubpath, added upstream in relay #1048 (shipped in 8.1.2) so the renderer doesn't pull in the node-only broker transport.Testing
npm run build✓npm test(node) — 22 passnpx vitest run— 28 pass (4 files)Notes
main(which had a large refactor since the work began); the broker binary-resolution conflict and the@relayfile/sdkimport (root, not/cli, for the 0.7.23 transitive version on main) were reconciled.tsc --noEmitwarnings incloud-agent.ts(Project.cloudAgent) and a few renderer files are unchanged tech debt onmain(the build uses esbuild/transpile-only) — not introduced here.🤖 Generated with Claude Code
CodeAnt-AI Description
Upgrade Agent Relay and add predictive terminal echo
What Changed
Impact
✅ Faster typing response in remote terminals✅ Fewer delays before agent output appears✅ More reliable persona launch on Agent Relay v8🔄 Retrigger CodeAnt AI Review
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.