Repository navigation
pathing - #94
pathing#94
Conversation
|
CodeAnt AI is reviewing your PR. |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe broker discovery now checks prioritized candidate connection paths (current ChangesBroker connection migration to dual-layout support
🎯 3 (Moderate) | ⏱️ ~20 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:M
![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 updates the broker manager to support a new connection path (.agentworkforce/relay/connection.json) alongside the legacy path (.agent-relay/connection.json), including updates to connection resolution, cleanup paths, and existing broker reuse logic. Feedback on these changes highlights a potential false-positive status in getBrokerConnectionFileInfo when both connection files exist and one is stale, and suggests a minor optimization in resolveBrokerConnectionPath to avoid redundant array allocations.
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.
| } | ||
|
|
||
| const connectionPath = join(cwd, '.agent-relay', 'connection.json') | ||
| const connectionPath = resolveBrokerConnectionPath(cwd) |
There was a problem hiding this comment.
Potential false-positive 'different' status in getBrokerConnectionFileInfo
When both .agentworkforce/relay/connection.json and .agent-relay/connection.json exist (e.g., during a migration or transition period), resolveBrokerConnectionPath(cwd) will always return the first one that exists (the .agentworkforce path).
However, if that file is stale/invalid, connectExistingBroker will fall back and successfully connect using the legacy .agent-relay path. In this scenario, getBrokerConnectionFileInfo will read the stale .agentworkforce file, compare it against the active session's URL/PID, and incorrectly report the status as 'different' instead of 'matches'.
Recommendation
Consider refactoring getBrokerConnectionFileInfo to iterate over all candidates in brokerConnectionPathCandidates(cwd) and check if any of them match the active session's baseUrl and brokerPid. If a match is found, return that file's info with 'matches' status.
| function resolveBrokerConnectionPath(cwd: string): string { | ||
| return brokerConnectionPathCandidates(cwd).find((candidate) => existsSync(candidate)) ?? | ||
| brokerConnectionPathCandidates(cwd)[0] | ||
| } |
There was a problem hiding this comment.
Avoid calling brokerConnectionPathCandidates(cwd) twice to prevent redundant array allocations and path joins. Storing the candidates in a local variable is cleaner and more efficient.
function resolveBrokerConnectionPath(cwd: string): string {
const candidates = brokerConnectionPathCandidates(cwd)
return candidates.find((candidate) => existsSync(candidate)) ?? candidates[0]
}![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| for (const connectionPath of connectionPaths) { | ||
| try { | ||
| const client = AgentRelayClient.connect({ cwd, connectionPath }) | ||
| await client.getSession() | ||
| console.log(`[broker] Reusing existing broker for project ${projectId}: ${connectionPath}`) | ||
| return client | ||
| } catch (err) { | ||
| console.warn(`[broker] Existing broker connection is not reusable for project ${projectId}:`, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggestion: When probing multiple connection files, a client instance is created before health-checking with getSession(), but the failure path only logs and continues. If connect() allocates sockets/listeners, failed attempts are never torn down, so repeated starts can leak broker connections/resources. Explicitly disconnect/release the temporary client inside the catch path before trying the next candidate. [resource leak]
Severity Level: Major ⚠️
- ❌ Main process can leak AgentRelayClient sockets on failed reuse.
- ⚠️ Repeated broker:start calls may exhaust broker connection resources.
- ⚠️ Long-lived sessions risk instability from accumulated leaked clients.Steps of Reproduction ✅
1. Open the Electron app and trigger a broker start for a project, which sends the IPC
call `'broker:start'` handled in `src/main/ipc-handlers.ts` (lines 170–199 from tool
output) where `brokerManager.start(projectId, cwd, name, win, channels)` is invoked.
2. Inside `BrokerManager.start` in `src/main/broker.ts` (lines 52–79 from tool output),
the `startBroker` closure is created; its first step is `const existingClient = await
this.connectExistingBroker(normalizedProjectId, cwd)`, which calls
`connectExistingBroker`.
3. Ensure the project `cwd` contains at least one stale or unusable local broker
connection file (for example, `.agentworkforce/relay/connection.json` left over from a
previous run) so that `brokerConnectionPathCandidates(cwd).filter((candidate) =>
existsSync(candidate))` in `connectExistingBroker` (file `src/main/broker.ts`, around
lines 69–75) returns one or more paths that exist but point to a dead or incompatible
broker.
4. When `connectExistingBroker` runs, for each such `connectionPath` it executes the loop
shown in `src/main/broker.ts` lines 1293–1302: it calls `const client =
AgentRelayClient.connect({ cwd, connectionPath })` and then `await client.getSession()`.
If the broker behind that file is unreachable or mismatched, `client.getSession()` throws,
execution enters the `catch (err)` block, and only a warning is logged
(`console.warn('[broker] Existing broker connection is not reusable…')`) without calling
`client.disconnect()` or any shutdown.
5. Because the failing `client` is never stored in `this.sessions` and never explicitly
disconnected in the catch block, any sockets/listeners allocated by
`AgentRelayClient.connect` remain live in the Electron main process. The loop then
continues to the next candidate (if any), or returns `null`, causing `startBroker` to
spawn a new broker via `AgentRelayClient.spawn(opts)` while the failed client instance is
effectively leaked.
6. Repeating the `'broker:start'` IPC for the same project while the stale connection file
remains (for example, reopen the project or let `reviveSession` call `this.start` again
from `src/main/broker.ts` lines 1312–1335) causes `connectExistingBroker` to allocate and
abandon additional `AgentRelayClient` instances, leading to a cumulative leak of broker
client resources over time.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:** 1293:1302
**Comment:**
*Resource Leak: When probing multiple connection files, a client instance is created before health-checking with `getSession()`, but the failure path only logs and continues. If `connect()` allocates sockets/listeners, failed attempts are never torn down, so repeated starts can leak broker connections/resources. Explicitly disconnect/release the temporary client inside the `catch` path before trying the next candidate.
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. |
|
✅ pr-reviewer applied fixes — committed and pushed Fixed a real PR issue: current-format broker connection files using Changes made:
Validated locally:
No separate bot review artifacts were present under |
|
Fixed the validated remaining issue: current Validated locally:
I also checked the bot comments: the stale connection-info, duplicate candidate allocation, and failed-probe disconnect findings were already addressed in the current checkout. |
|
✅ pr-reviewer applied fixes — committed and pushed Fixed the validated remaining issue: current Validated locally:
I also checked the bot comments: the stale connection-info, duplicate candidate allocation, and failed-probe disconnect findings were already addressed in the current checkout. |
|
Fixed the remaining validated issue in the current checkout: current I also extended the broker regression in broker.test.ts so a reused current connection reports Validated:
I checked the bot review comments against the current checkout. The stale connection-info selection, duplicate candidate allocation, and failed-probe disconnect findings were already addressed; the schema/API-key gap was the remaining demonstrated issue. |
1 participant
CodeAnt-AI Description
Reuse existing broker connections from the current workspace path
What Changed
.agentworkforce/relaylocation, while still supporting the older.agent-relaypath.Impact
✅ Fewer duplicate broker startups✅ Smoother reconnects after workspace restarts✅ Fewer failures with older broker files🔄 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.