Port Registrar: migrate code agent + Coda extension to dynamic ports#2334
Merged
Conversation
f7c346f to
02563e3
Compare
PR 2 of the port-registrar series. Drops the hardcoded 8082 port for the `code` agent's WebSocket server and rewires the in-repo Coda VS Code extension to discover the live port through the agent-server's discovery channel before connecting. * `CodeAgentWebSocketServer` is now an async `start(port = 0)` factory that resolves only after the `listening` event fires, with `close()` returning a Promise. Lets us read the OS-assigned port reliably and serialize rapid disable/enable cycles under a fixed `CODE_WEBSOCKET_PORT` override. * `updateCodeContext` registers the server's actual port per session via `sessionContext.registerPort`. The shared module-scoped server is kept (refcounted), but each enabling session gets its own registration handle stored on its `agentContext`; `releaseAllForSession` is the crash-safety backstop. The registrar already supports multiple registrations per `(agent, role)` so this drops the previously considered ownership-transfer state machine. * `readiness.ts` treats port as `number | undefined` and omits the port suffix from setup-card and NOT-RUNNING messages when no override is set. `resolveCodePort` is now `resolveCodePortOverride` to make the env-var semantics explicit. * Coda's `createWebSocket` is async and uses `discoverPort()` (from the `@typeagent/agent-server-client/discovery` subpath introduced in PR 1) with a legacy 8082 fallback only when the discovery channel itself is unreachable (not when lookup returns `not-registered`). Reconnect loop now guards against the multi-interval leak in the prior `reconnectWebSocket` implementation and re-discovers the port on every retry (port can change across agent-server restarts). * Tests: 5 new integration tests for the shared-server lifecycle (single + multi-session enable / disable / rollback on bind failure). Existing 13 readiness tests updated for the new `number | undefined` port type. All 18 code-agent tests pass; full workspace build green; lint clean. Branched off and depends on PR 1 ([port-registrar foundation], talzacc/port-registrar-foundation tip `a8fd7cf7`). Does not modify SecretAgents. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
02563e3 to
e371098
Compare
…query separator, refresh stale docs This is a research project; there are no in-the-wild pre-discovery clients to support, so the back-compat fallback in coda's webSocket.ts is dead weight. Replace it with: when discovery is unreachable or the code agent isn't yet registered, return undefined and let the existing 5s reconnect loop in wsConnect.ts retry. Also fix a pre-existing bug in createWebSocket: `clientId=` was appended to the query string without a `&` separator, producing malformed URLs like `...&role=clientclientId=xyz`. Caught by code review. Stale doc references to port 8082 in commandExecutor/README.md, commandExecutor/VSCODE_CAPABILITIES.md, and shell/demo/DEMO_SETUP.md updated to describe the discovery-based flow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Extract getKnownCodePort() helper with a comment explaining the two-phase lifecycle (live bound port after start; static env-var prediction before start). The previous inline `getSharedCodePort() ?? resolveCodePortOverride(...)` read like a fallback when it's actually 'live if known, else predicted'. - Tighten getCodeBindPort doc: explicitly call out that EADDRINUSE on the requested port is a hard failure (no silent fallback to OS-assigned), since the user explicitly asked for that port. The previous 'falling back to OS-assigned port' wording in the malformed-input branch could mislead readers into thinking bind failures also fall back, which they don't. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the Code agent and Coda VS Code extension from a hard-coded WebSocket port to dynamic port registration/discovery through the agent-server registrar introduced by PR #2332.
Changes:
- Converts the Code agent WebSocket server to async dynamic binding and per-session port registration.
- Updates readiness/setup messaging and tests for optional/dynamic ports.
- Updates Coda to discover the Code agent port and improves reconnect timer handling.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ts/packages/agents/code/src/codeActionHandler.ts |
Adds shared dynamic server lifecycle, registration, refcounting, and readiness port lookup. |
ts/packages/agents/code/src/codeAgentWebSocketServer.ts |
Converts WebSocket server construction/close to async bind/teardown with actual bound port exposure. |
ts/packages/agents/code/src/readiness.ts |
Makes readiness/setup port messaging optional and renames override parsing. |
ts/packages/agents/code/test/codeReadiness.spec.ts |
Updates readiness tests for optional dynamic ports. |
ts/packages/agents/code/test/codeUpdateContext.spec.ts |
Adds shared-server lifecycle and per-session registration tests. |
ts/packages/coda/src/webSocket.ts |
Adds discovery-based endpoint resolution before connecting to the Code agent WebSocket. |
ts/packages/coda/src/wsConnect.ts |
Adds a single reconnect interval guard and retries discovery on reconnect. |
ts/packages/coda/package.json |
Adds workspace dependencies needed for discovery. |
ts/pnpm-lock.yaml |
Records the new Coda workspace dependencies. |
ts/packages/shell/demo/DEMO_SETUP.md |
Updates demo setup/troubleshooting docs for dynamic port discovery. |
ts/packages/commandExecutor/README.md |
Updates architecture docs to remove the hard-coded Coda port. |
ts/packages/commandExecutor/VSCODE_CAPABILITIES.md |
Updates VS Code capability docs to describe discovery-based connection. |
Files not reviewed (1)
- ts/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…est cleanup createWebSocket previously used an `async` Promise executor (`new Promise(async (resolve) => ...`). If `resolveCodeEndpoint()` rejects or `new WebSocket(endpoint)` throws synchronously (e.g. an invalid `CODE_WEBSOCKET_HOST` override), the rejection lands on the executor's implicit promise rather than the outer one — leaving createWebSocket pending forever and stalling the reconnect loop. Refactor to do the awaits at the top level and try/catch around `new WebSocket()`. codeUpdateContext.spec.ts: replace the placeholder afterEach with a real teardown that tracks every StubContext newSession() hands out and force-disables any still-enabled session. Asserts the shared server is down after cleanup so a leak surfaces loudly instead of contaminating later tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
robgruen
approved these changes
May 14, 2026
…nored Per @robgruen review feedback on resolveCodePortOverride: surface a debug log when the env-var override is in effect (so a user wondering why the port isn't OS-assigned can confirm), and another when an invalid value is ignored (so a typo doesn't silently fall back to OS-assigned without any diagnostic). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Why
PR #2332 stood up the registrar, the discovery channel, the client-side
discoverPort()helper, and the SDK. But no shipping agent uses any of it yet — every agent still hard-codes its port. This PR is the first concrete migration and proves the model end-to-end: an in-process agent bindingport = 0, registering via the SDK, and an external client discovering the live port over the discovery channel before connecting.What this PR changes
codeagent: dynamic port + per-session registrationCodeAgentWebSocketServeris now an asyncstart(port = 0)factory rather than a constructor. It resolves only after thelisteningevent fires, which lets us read the OS-assigned port reliably, andclose()returns a Promise so callers can serialize adisable → re-enablecycle (matters when an operator pins a fixed port viaCODE_WEBSOCKET_PORT).updateCodeContextregisters the server's actual bound port viasessionContext.registerPort("default", server.port)on every enable, and stores the registration handle on the per-sessionagentContextsodisablecan release it. The shared module-scoped server (refcounted across sessions) is preserved — only the registrations are per-session. This is intentional: the registrar already supports multiple registrations for the same(agent, role)andlookupreturns the most recent, so we don't need an ownership-transfer state machine. If the dispatcher tears a session down hard,releaseAllForSessionis the crash-safety backstop.Race handling worth noting:
sharedStartingPromiselock keeps two concurrent enables from racing each other into a double-bind.sharedClosingPromiselock makes a rapid disable→enable wait for the prior close to land before re-binding (only matters under a fixed-port override; withport = 0the OS would just hand us a different port).agentContext.enabledso the agent's view of state stays consistent, and refcount is left untouched (it only increments after a successful register).readiness.ts: port becomes optionalThe readiness probe and setup card now treat port as
number | undefined. The setup card and NOT-RUNNING messaging omit the:portsuffix entirely when no override is set, since "the port is whatever the OS gave us, ask the registrar" is the new normal.resolveCodePortwas renamed toresolveCodePortOverrideto make its env-var semantics explicit (CODE_WEBSOCKET_PORTis now strictly an escape hatch).Coda VS Code extension: discovery-then-connect
coda/src/webSocket.tsnow does:CODE_WEBSOCKET_HOSTis set, use it as a hard override (unchanged escape hatch).discoverPort("code")(from@typeagent/agent-server-client/discovery) against the agentServer's known port.unreachable(no agentServer), fall back tows://localhost:8082for back-compat with old in-process agentServers that haven't been updated yet.not-registered, surface as a connect failure (the agent simply isn't loaded right now).The reconnect loop (
wsConnect.ts) now guards against the multi-interval leak in the prior implementation — every disconnect previously spawned a freshsetInterval, so a flapping connection would stack timers — and re-runs discovery on every retry, since the code agent's port can change across agentServer restarts now that it's OS-assigned.createWebSockethad to become async because the discovery handshake is async; one call site inwsConnect.tswas updated accordingly.Validation
pnpm run buildfromts/— green.pnpm run lintfromts/— clean.pnpm --filter code-agent test— 18/18 passing (13 readiness tests updated for the new optional-port type, 5 new integration tests for the shared-server lifecycle exercising real WebSocket binds onport = 0).Reading order for reviewers
This PR is one squashed commit, but reads naturally in two layers:
agents/code/src/{codeAgentWebSocketServer,codeActionHandler,readiness}.ts+ tests — agent-side migration. The interesting bits are the async start/close, theensureSharedServerrace locks, and the per-session registration handle onagentContext.coda/src/{webSocket,wsConnect}.ts— extension-side consumer. Demonstrates the discovery-then-connect pattern (usingdiscoverPort()from PR 1) and the reconnect-loop guard.Follow-up PRs
browseragent service worker (uses the samediscoverPort)visualStudiohost webviewonboarding-scaffolder+ tightenoriginAllowlistonagentServer