Upgrade Agent Relay dependencies to v10.6.3#398
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughRelay v10 dependencies and packaging filters are updated. PTY offsets and generations now flow through broker, IPC, and renderer buffering for snapshot-aware replay. The local fleet sidecar polls broker sessions and starts nodes through ChangesPTY offset propagation and replay
Relay v10 fleet sidecar migration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Broker
participant PreloadIPC
participant PtyBufferStore
participant TerminalRuntimeRegistry
Broker->>PreloadIPC: send PTY chunks with offset and generation
PreloadIPC->>PtyBufferStore: append chunk metadata
TerminalRuntimeRegistry->>PtyBufferStore: select chunks after snapshot offset
PtyBufferStore-->>TerminalRuntimeRegistry: post-snapshot chunks
TerminalRuntimeRegistry->>TerminalRuntimeRegistry: write before live subscription
sequenceDiagram
participant BrokerSession
participant resolvePearFleetConnection
participant startPearFleetSidecar
participant startServeNode
BrokerSession->>resolvePearFleetConnection: provide node credentials
resolvePearFleetConnection-->>startPearFleetSidecar: resolved connection and node name
startPearFleetSidecar->>startServeNode: start serving node
startServeNode-->>startPearFleetSidecar: registered FleetNodeInfo
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
khaliqgant
added
the
no-agent-relay-review
![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 upgrades various @agent-relay dependencies to version 10.6.3 and refactors the local fleet node connection logic to use the standard @agent-relay/fleet node server instead of a custom WebSocket loop. It also introduces cumulative raw-PTY byte offsets to terminal snapshots and PTY chunks to prevent duplicate output rendering when attaching to terminals. The review feedback suggests two improvements for robustness: using optional chaining when checking the resolved broker session to prevent potential TypeErrors during polling, and strictly validating that the snapshot offset is a number before filtering PTY chunks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try { | ||
| lastSession = await readBrokerSession() | ||
| lastError = undefined | ||
| if (lastSession.node_id && lastSession.node_token) { |
There was a problem hiding this comment.
If readBrokerSession() resolves to null or undefined (for example, if the session is not yet initialized), accessing lastSession.node_id will throw a TypeError. While this error is caught by the try-catch block, it is cleaner and more robust to use optional chaining to avoid throwing exceptions during normal polling.
| if (lastSession.node_id && lastSession.node_token) { | |
| if (lastSession?.node_id && lastSession?.node_token) { |
There was a problem hiding this comment.
Fixed in d713f34. Broker session reads now accept null/undefined during normal polling and use optional chaining before credential access.
| // snapshot instead of treating the entire roundtrip window as covered | ||
| // (which could drop output emitted after snapshot capture). | ||
| flushPtyChunksNow(key) | ||
| if (result.snapshot.offset !== undefined) { |
There was a problem hiding this comment.
To ensure type safety and prevent passing non-numeric values (like null if present in the JSON response) to getPtyChunksAfterOffset, it is safer to explicitly check if result.snapshot.offset is a number.
| if (result.snapshot.offset !== undefined) { | |
| if (typeof result.snapshot.offset === 'number') { |
There was a problem hiding this comment.
Fixed in d713f34. Snapshot offset correlation now requires a numeric offset; main also rejects non-finite snapshot offsets.
![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: 0ac19bf07e
ℹ️ 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".
| } else { | ||
| pending.set(key, [chunk]) | ||
| pending.set(key, [entry]) | ||
| } |
There was a problem hiding this comment.
When the same broker:pty-chunk delivery reaches the renderer twice—for example through IPC replay or duplicate renderer listeners—this blindly queues both entries even though the new offset metadata can identify an exact replay. Both copies are then written to xterm, which can duplicate control sequences and recreate stacked-screen corruption. AGENTS.md explicitly requires the renderer buffer to guard PTY delivery and to dedupe by identity plus content, so suppress the same offset-and-chunk within the current broker generation while retaining identical content at different offsets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d713f34. The renderer final guard now suppresses only when event-stream generation, PTY offset, and content hash all match. Tests retain identical content at different offsets/generations and identity-less repeats.
| const bufferOffsets = offsets.get(key) ?? [] | ||
| return buffer.filter((_, index) => { | ||
| const offset = bufferOffsets[index] | ||
| return offset === undefined || offset > snapshotOffset |
There was a problem hiding this comment.
If the broker daemon restarts while this key's lazy buffer still contains old output, its cumulative PTY offset resets but getPtyChunksAfterOffset compares the new snapshot against every retained offset without a generation check. Old-generation chunks with numerically higher offsets are therefore replayed over the fresh snapshot when the terminal is first attached, corrupting the screen. This is the counter-reset case warned about in AGENTS.md; clear or generation-tag offsets across reconnects and deliver uncertain chunks rather than treating the raw number as globally comparable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d713f34. PTY IPC and attach snapshots now carry the main event-stream generation. Attach filtering drops known older-generation chunks, compares offsets only within the same generation, and delivers future/unknown generations.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/pear-fleet-node.ts`:
- Around line 221-224: Update the broker-session polling loop around
readBrokerSession() so each read is raced or cancelled against both the
remaining deadline and signal, allowing hung reads to terminate promptly. Apply
the same change to the corresponding polling path around the additionally
referenced section, preserve existing retry behavior for completed reads, and
add coverage using a never-resolving reader to verify timeout and stop()
completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e344254-b739-483d-9e2b-0b45f99a5f7f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
electron-builder.mcp-resources.ymlpackage.jsonsrc/main/broker.test.tssrc/main/broker.tssrc/main/pear-fleet-node.test.tssrc/main/pear-fleet-node.tssrc/preload/index.tssrc/renderer/src/hooks/use-broker-events.tssrc/renderer/src/lib/ipc-mock.tssrc/renderer/src/lib/terminal-runtime-registry.dom.test.tssrc/renderer/src/lib/terminal-runtime-registry.tssrc/renderer/src/stores/pty-buffer-store.test.tssrc/renderer/src/stores/pty-buffer-store.tssrc/shared/types/ipc.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66b7aef6b5
ℹ️ 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".
| // of the generation comparison, the numeric offset is not globally safe; | ||
| // deliver the uncertain chunk instead of risking dropped terminal bytes. | ||
| if (snapshotGeneration === undefined || generation === undefined) return true | ||
| if (generation < snapshotGeneration) return false |
There was a problem hiding this comment.
When an event-stream refresh races with terminal attachment, this drops every buffered chunk tagged with the previous listener generation even if its offset is greater than the snapshot offset. The checked refresh path in src/main/broker.ts:2388-2395 increments eventStreamGeneration for ordinary focus/visibility rebinds without restarting the PTY, while attachTerminal stamps the snapshot with the generation current after its asynchronous snapshot request; therefore output emitted after snapshot capture but before a concurrent rebind can be tagged with the older generation and be lost here, corrupting the terminal stream. Listener generations should not be treated as PTY/daemon generations, or older-generation chunks should only be discarded when a real PTY restart is established.
Useful? React with 👍 / 👎.
…402) Clicking any issue in the Issues control center (Attention Inbox) threw "Integration remote directory is outside this project integration scope" from IntegrationsManager.listRemoteDirectory. The Issues tab lists /linear/issues (issues-store.ts) and reads/writes the issue records beneath it for status writeback and comments. But Linear integrations mount at the team level — /linear/teams or /linear/teams/AR (integrations.catalog.ts defaultMountPaths, unchanged since #9) — and the adapter materializes /linear/issues as a SIBLING of that mount, not inside it. So canListRemoteDirectoryForMountPaths rejected every /linear/issues listing. This was latent since the Attention Inbox shipped; #226 added an identical carve-out for /linear/states but its commit message wrongly assumed "integrations configure mounts like /linear/issues", so it never noticed /linear/issues had the same problem. Not a regression from #398/#399/#384 (none touch scope logic) and not the .integrations symlink pattern (this guard is a remote relayfile-path check, never touches disk). Add isLinearIssuesListablePath mirroring the states carve-out, gated on a visible Linear integration, wired into listRemoteDirectory (guard + entry filter) and readRemoteFile. Unlike states, the carve-out is also wired into writeRemoteFile: issue status changes and comments are the Issues control center's whole purpose. Rename the private gate linearStatesListingEnabledForProject -> linearListingEnabledForProject since it now backs both Linear reference subtrees. Tests: integrations.test.ts exercises list+read+write against the operator's real team-scoped mount (/linear/teams/AR); predicate unit tests cover the subtree boundary and states/issues disjointness. Full integrations suite (70) + predicate suite (10) green; typecheck:node + eslint clean. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
startServeNodeengine handshakeCompatibility audit
nodeManifestand theregister_node/register_handlers/deregister_node/handler_resultclient flow. Pear now waits for the broker-minted node id and token, then attaches its provider to that same node.erasePredictionRegionearly-return strand vector: prediction state can still be cleared before the cursor-row mismatch return leaves optimistic glyphs unerased. This PR does not claim that vector is fixed.reset()dropsonServerOutputwork still waiting in its async tail. The runtime now seals the router, releases capture-held chunks, awaits its accepted ordering chain, then resets/disposes the engine and terminal. Direct↔engine route changes, remounts, resizes, and rollbacks reuse the same engine and do not dispose it.Isolated live fleet gate
pear-relay10-persist-gate, state directory, and port63136; noagent-relay local up/downcommand was usednode_f1b6ed06639b0d71a9536bf79b30f59e; broker query reported online/live,handlersLive: true, andrelay-broker/10.6.3inv_203841067181731840completed through that node; the isolated broker exposed the spawned PTY worker and its snapshot showed the deterministic task executingValidation
npm run buildnpm run verify:mcp-resources-driftnpm run typechecknpm run test:fidelity: 8 passednpm run test:redraw: 1 passed,drained=true,finalRenderedSeq=1920,consoleErrorCount=0git diff --checkThe live
pearbroker on port 3889 was not touched.