Register the agent host protocol channel before starting the local endpoint - #328862
Conversation
There was a problem hiding this comment.
Pull request overview
Adds bounded retry handling for transient agent-host IPC channel registration timeouts.
Changes:
- Adds cancellable exponential-backoff connection retries.
- Adds tests for success, failure, exhaustion, disposal, and listener counts.
Show a summary per file
| File | Description |
|---|---|
agentHostIpcChannelTransport.ts |
Implements retry and cancellation logic. |
agentHostIpcChannelTransport.test.ts |
Tests retry behavior and teardown. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
connor4312
left a comment
There was a problem hiding this comment.
I don't think this is the right approach. We should instead figure out why the channel is taking so long to get registered, and solve that, which happens here:
@connor4312 , Fair feedback, below is the root cause and proposed fix. Let me know your feedback Root cause + proposed fixThe renderer connects to the agent host over the MessagePort That window was being blown by Proposed Fix: register the MessagePort Timing (time-to-registration)Before — just over the 1s budget: After — well inside the budget: ResultThe agent host option now appears reliably in the picker (chat editor + Agents window) across cold boots — the channel registers in ~170ms instead of ~1.1s. Notes
|
… ready (before the endpoint)
There was a problem hiding this comment.
Review details
Suppressed comments (5)
src/vs/platform/agentHost/node/agentHostMain.ts:392
- This multi-line inline comment exceeds the repository's one-line limit for comments inside methods. Keep only the non-obvious ordering constraint.
// Register the renderer's protocol channel BEFORE starting the external
// endpoint: the renderer connects over this channel, and the IPC
// ChannelServer drops calls to a not-yet-registered channel after its
// unknown-channel timeout (~1s), so the endpoint's socket startup must
// not sit on this path.
src/vs/platform/agentHost/node/agentHostMain.ts:397
- This multi-line inline comment exceeds the repository's one-line limit for comments inside methods. Condense it to the ordering decision.
// The external local endpoint (out-of-process local clients such as the
// CLI) is not on the renderer's path; start it after registration and
// give it its own handler.
src/vs/platform/agentHost/node/agentHostMain.ts:408
- This multi-line inline comment exceeds the repository's one-line limit for comments inside methods. The connection-gap constraint can be stated in one line.
// Wire the endpoint's handler (subscribing to its connections) BEFORE
// publishing the metadata that advertises it, so a client can't connect
// in the gap and be missed.
src/vs/platform/agentHost/node/agentHostMain.ts:425
- If metadata publication fails, only the WebSocket server is disposed; the
ProtocolServerHandleradded above remains subscribed to state, MCP, and OTLP events for the process lifetime and keeps filling its replay buffer even though it can no longer serve clients. Dispose the handler with the server by grouping both in an endpoint-local store and adopting that store only after publication succeeds.
localEndpoint.server.dispose();
src/vs/platform/agentHost/node/agentHostMain.ts:325
- This multi-line inline comment exceeds the repository's one-line limit for comments inside methods. Condense it so the identifiers carry the remaining explanation.
This issue also appears in the following locations of the same file:
- line 388
- line 395
- line 406
// Shared config for the local data-plane protocol handlers (renderer
// MessagePort + the external endpoint, which each get their own handler).
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Balanced
…channel-timeout # Conflicts: # src/vs/platform/agentHost/node/agentHostMain.ts
Problem
On a slow agent host boot, the "agent host Copilot" option was frequently missing from the session-target picker (both the chat editor and the Agents window).
The renderer reaches the agent host over the MessagePort
agentHostProtocolIPC channel. The IPCChannelServerdrops calls to a not-yet-registered channel after its unknown-channel timeout (~1s), timed from when it processes the renderer'sconnect.That window was being consumed by
startLocalAgentHostEndpoint— the external local WebSocket/named-pipe endpoint — sitting on the path beforeregisterChannel. Its ~850ms+ startup pushed registration just over the 1s budget, so the renderer's bufferedconnecttimed out, the connection closed, root state never hydrated, and the option was missing.Fix
Register the MessagePort
Protocolchannel as soon as providers are ready — before starting the external endpoint — and start the endpoint afterward with its ownProtocolServerHandler(the same pattern the dynamic/env-var WebSocket servers already use). The endpoint no longer sits on the renderer's connect path.Timing (time-to-registration)
Before — just over the 1s budget:
After — well inside the budget:
Result
The agent host option now appears reliably in both the chat editor and the Agents window across cold boots — the channel registers in ~170ms instead of ~1.1s.
Notes
CompositeProtocolServer— each handler manages its own clients/broadcasts/replay-buffer/orphan-detection via its own_clientsmap, and the dynamic/env-var WS handlers already coexist this way.agentHostMain.ts). No protocol/wire changes; no change to the WebSocket/remote paths.