Reuse existing local broker on startup#12
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesConcurrent Broker Startup Deduplication with Error Reporting
🎯 2 (Simple) | ⏱️ ~12 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (inFlight) { | ||
| await inFlight | ||
| const started = this.sessions.get(normalizedProjectId) | ||
| if (!started) { | ||
| throw new Error(`Broker start completed without a session for project ${normalizedProjectId}`) | ||
| } | ||
| started.window = win | ||
| started.cwd = cwd | ||
| started.name = name | ||
| await this.syncChannels(normalizedProjectId, nextChannels) | ||
| this.sendStatus(normalizedProjectId, 'connected') | ||
| return |
There was a problem hiding this comment.
🟡 Concurrent start caller's window never receives error status when in-flight start fails
When a second start() call piggybacks on an in-flight start via await inFlight at line 539, and the in-flight promise rejects, the rejection propagates out of start() without calling sendStatusToWindow(win, ...) for the second caller's BrowserWindow. The primary caller's catch block at src/main/broker.ts:623-626 sends the error status to its own window, but the second caller's window is never notified. This leaves the second caller's renderer in a stale "starting" state since it never receives the broker:status error event.
| if (inFlight) { | |
| await inFlight | |
| const started = this.sessions.get(normalizedProjectId) | |
| if (!started) { | |
| throw new Error(`Broker start completed without a session for project ${normalizedProjectId}`) | |
| } | |
| started.window = win | |
| started.cwd = cwd | |
| started.name = name | |
| await this.syncChannels(normalizedProjectId, nextChannels) | |
| this.sendStatus(normalizedProjectId, 'connected') | |
| return | |
| if (inFlight) { | |
| try { | |
| await inFlight | |
| } catch (err) { | |
| this.sendStatusToWindow(win, normalizedProjectId, 'error', String(err)) | |
| throw err | |
| } | |
| const started = this.sessions.get(normalizedProjectId) | |
| if (!started) { | |
| throw new Error(`Broker start completed without a session for project ${normalizedProjectId}`) | |
| } | |
| started.window = win | |
| started.cwd = cwd | |
| started.name = name | |
| await this.syncChannels(normalizedProjectId, nextChannels) | |
| this.sendStatus(normalizedProjectId, 'connected') | |
| return |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 4c83710. The in-flight startup branch now wraps the awaited shared promise and follow-up session sync in a try/catch, sends broker:status error to the piggybacking caller window, then rethrows.
khaliqgant
force-pushed
the
codex/reuse-existing-broker
branch
from
61a73db to
4c83710
Compare
Fix #9: track last-sent rows/cols and skip duplicate resizePty IPC. The ResizeObserver fires on every dragged pixel; the cell grid only changes at discrete steps. Fix #12: refuse a second concurrent mount of the same runtime into a different container. Currently chunkAgents doesn't trigger this, but silently reparenting would tear xterm out from under the original owner. Fix #15: post-font-settle metrics may differ from the pre-settle ones the predictor was built with. Call predictiveEcho.onResize after the refit so column wraps line up with the real grid. Also adds refreshOnShow() and setInputSrttGetter() to the runtime interface in preparation for use-terminal wiring.
1 participant
Summary
.agent-relay/connection.jsonbroker before spawning a new local broker.BrokerManager.start()calls for the same project so startup does not race into duplicate broker spawns.Verification
git diff --checkpassed.npm run buildblocked on currentorigin/main: Rollup cannot resolve@agentworkforce/deployfromsrc/main/proactive-agent.bundle.ts.npm run typecheckis not defined on currentorigin/main; fallbacknpx tsc -b --noEmitis blocked by existing unrelated cloud/proactive/store/diff/graph/type errors.Runtime note
Before opening the PR, this change was verified in the active local Pear worktree by launching with an existing broker present. Startup logged
Reusing existing broker .../.agent-relay/connection.jsonand stopped emitting the repeatedanother broker instance is already runningfailure.