Port Registrar: visualStudio agent migration + C# discovery client#2374
Open
TalZaccai wants to merge 7 commits into
Open
Port Registrar: visualStudio agent migration + C# discovery client#2374TalZaccai wants to merge 7 commits into
TalZaccai wants to merge 7 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the Visual Studio agent bridge from a fixed, hardcoded port to OS-assigned ephemeral ports registered via PortRegistrar, and updates the VSIX host to discover the bridge port via the agent-server discovery channel on each reconnect.
Changes:
- Reworks the TS WebSocket bridge lifecycle to support ephemeral ports, origin gating, shared bridge reuse, and per-session port registration/release.
- Adds a .NET Framework discovery client (
BridgeDiscovery) and updatesAgentBridgeClientto resolve the bridge port dynamically on every reconnect. - Updates Visual Studio agent docs and adds
debuglogging dependency.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ts/pnpm-lock.yaml | Adds debug deps for visualStudio agent but also contains broad lockfile churn. |
| ts/packages/agents/visualStudio/src/visualStudioActionHandler.ts | New async bridge startup with origin gate, shared lifecycle/refcount, and PortRegistrar registration. |
| ts/packages/agents/visualStudio/src/originAllowlist.ts | New origin allowlist helper for the Visual Studio bridge. |
| ts/packages/agents/visualStudio/README.md | Documents discovery flow and new env knobs; updates architecture diagram/troubleshooting. |
| ts/packages/agents/visualStudio/package.json | Adds debug + @types/debug. |
| ts/packages/agents/visualStudio/host/csharp/VisualStudioTypeAgent.csproj | Includes the new discovery source file in the VSIX build. |
| ts/packages/agents/visualStudio/host/csharp/Bridge/BridgeDiscovery.cs | Implements discovery-channel lookup of (visualStudio, default) port with fallback behavior. |
| ts/packages/agents/visualStudio/host/csharp/Bridge/AgentBridgeClient.cs | Removes hardcoded URI; performs discovery on each reconnect attempt. |
Files not reviewed (1)
- ts/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
ts/packages/agents/visualStudio/src/visualStudioActionHandler.ts:191
send()has no timeout or cancellation path if the plugin never replies, so actions can hang forever even if the socket stays open. Adding a per-request timeout (and cleaning up the pending entry on timeout) would prevent stuck sessions and unboundedpendinggrowth.
async send(actionName: string, parameters: unknown): Promise<unknown> {
if (!this.client) {
throw new Error(`No host plugin connected on port ${this.port}`);
}
const id = `${Date.now()}-${Math.random().toString(36).slice(2)}`;
return new Promise((resolve, reject) => {
this.pending.set(id, (res) =>
res.success
? resolve(res.result)
: reject(new Error(res.error)),
);
this.client!.send(
JSON.stringify({
id,
actionName,
parameters,
} satisfies BridgeRequest),
);
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TalZaccai
added a commit
that referenced
this pull request
May 20, 2026
- Bind the WebSocket bridge server to 127.0.0.1 instead of 0.0.0.0 so ephemeral ports are not reachable from the LAN even though the C# ClientWebSocket has no Origin header (review comments #1, #3). - Track in-flight send() requests with a per-request timer and clean the pending map on socket close, error, and stop. Adds a configurable timeout (VISUALSTUDIO_BRIDGE_SEND_TIMEOUT_MS, default 30s) so disconnected awaits no longer hang forever and the map cannot grow unbounded across reconnects (review comment #2 + the suppressed-low-confidence note about send() lacking a timeout). - Regenerate ts/pnpm-lock.yaml from main so the diff is limited to the new debug / @types/debug deps and pnpm's dedupe of pre-existing @types/node entries (review comment #6). Comments #4 and #5 (BridgeDiscovery.cs hardcoded fallback / XML doc mismatch) were already addressed in adf1426. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TS bridge:
- Rewrite VisualStudioBridge as async start(port=0) factory
- Add verifyClient Origin gate (loopback + absent Origin only)
- Shared bridge with refcount, start/close serialization
- Per-session registerPort('default', port) + idempotent release
- VISUALSTUDIO_BRIDGE_PORT escape hatch for debugging
C# host:
- New BridgeDiscovery.cs: speaks agent-rpc discovery wire protocol
over ClientWebSocket; resolves the bridge port at each reconnect
- AgentBridgeClient: drop hardcoded ws://localhost:5680, call
ResolveBridgePortAsync per attempt; log port on error
- Env vars: AGENT_SERVER_PORT, TYPEAGENT_VS_USE_DISCOVERY,
TYPEAGENT_VS_FALLBACK_PORT (all documented)
Docs: README updated with new architecture diagram, port discovery
section, env-var table, troubleshooting.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrated TS clients (browser, code, coda) all return undefined on discovery failure and rely on the reconnect loop. The C# bridge client was the odd one out, silently dialing 5680 when discovery was unreachable - which in the post-migration world connects to nothing useful (the agent no longer binds 5680 by default). Changes: - BridgeDiscovery.ResolveBridgePortAsync now returns int? and throws on transport failure, matching the TS pattern. - AgentBridgeClient retries on the existing 3s reconnect cadence when discovery returns null or throws. - New TYPEAGENT_VS_BRIDGE_PORT env var pins an explicit port and bypasses discovery (mirrors CODE_WEBSOCKET_HOST). Drops the TYPEAGENT_VS_USE_DISCOVERY + TYPEAGENT_VS_FALLBACK_PORT pair whose only purpose was selecting that hardcoded fallback. - README env-var table + troubleshooting updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Bind the WebSocket bridge server to 127.0.0.1 instead of 0.0.0.0 so ephemeral ports are not reachable from the LAN even though the C# ClientWebSocket has no Origin header (review comments #1, #3). - Track in-flight send() requests with a per-request timer and clean the pending map on socket close, error, and stop. Adds a configurable timeout (VISUALSTUDIO_BRIDGE_SEND_TIMEOUT_MS, default 30s) so disconnected awaits no longer hang forever and the map cannot grow unbounded across reconnects (review comment #2 + the suppressed-low-confidence note about send() lacking a timeout). - Regenerate ts/pnpm-lock.yaml from main so the diff is limited to the new debug / @types/debug deps and pnpm's dedupe of pre-existing @types/node entries (review comment #6). Comments #4 and #5 (BridgeDiscovery.cs hardcoded fallback / XML doc mismatch) were already addressed in adf1426. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that PR #2358 has landed the shared websocket-utils/originAllowlist helper, collapse the visualStudio agent's per-file copy into a one-line call to createAgentOriginAllowlist() (no extension schemes — the only legitimate client is the C# ClientWebSocket, which doesn't send an Origin header). Behavior unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ba4ae20 to
092c87a
Compare
Wires the bridge's connect/disconnect signal into SessionContext.notifyClientCountChanged so the count column in @System ports shows the real number of host-plugin connections (0 or 1, since the VS extension only opens one socket) instead of "N/A". Without this hook the bridge runs but no agent ever calls the SDK method, leaving the registrar's client count undefined. Implementation mirrors the code agent's pattern for a shared, process-singleton WS server: - VisualStudioBridge exposes onClientCountChanged and getConnectedCount(); emits on connect and on the first of close/error (guarded so close+error don't double-emit). - The lifecycle code maintains sharedActiveSessions and uses the primary-session pattern (first session in insertion order publishes the global count, the rest publish 0) so the per-session entries the @System ports summing logic sees don't multiply the count across sessions registered to the SAME physical bridge. - Primary handoff on disable transfers the count to the new primary so the column doesn't go to 0 while clients are still connected. No new tests — visualStudio has no test suite today; the pattern is exercised by the existing code-agent tests in codeUpdateContext.spec.ts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…counted Replace single-client his.client with Map<clientId, WebSocket> so two or more VS extensions connecting concurrently are all tracked instead of silently displacing each other. @System ports now shows the true client count for the visualStudio/default port, matching the behavior of code-agent and browser-agent. Per-request clientId tracking means a disconnect on one VS only fails the pending requests that targeted it; other VS instances stay live. send() routes to the most-recently-connected OPEN client (legacy last-wins behavior; smarter per-solution routing is future work). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…agent-migration # Conflicts: # ts/pnpm-lock.yaml
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
The
visualStudiobridge bound a hardcoded port (5680) and the VSIX dialed the matching hardcoded URI. This blocks the design goal of OS-assigned ephemeral ports brokered throughPortRegistrar.What changes
TS bridge (
visualStudioActionHandler.ts) — rewritten to mirror thecodeandbrowserhandlers:VisualStudioBridge.start(port = 0)factory; resolves onlistening, rejects on firsterror.verifyClientOrigin gate via a neworiginAllowlist.ts(loopback + absent/null; the C#ClientWebSocketsends no Origin header).EADDRINUSE.registerPort("default", bridge.port), released in bothupdateAgentContext(false)andcloseAgentContext.VISUALSTUDIO_BRIDGE_PORTescape hatch, mirroring the other agents.@system portsintegration — bridge tracks all connected VS instances viaMap<clientId, WebSocket>(instead of silently displacing on second connect); per-requestclientIdso a disconnect only fails that client's pending RPCs. EmitsonClientCountChangedto the session context, surfaced vianotifyClientCountChangedwith a primary-session fanout so shared sessions don't double-count.send()routes to the most-recently-connected OPEN client (preserves legacy last-wins behavior).C# bridge — VSIX is .NET Framework 4.7.2, can't reuse the TS discovery helper:
Bridge/BridgeDiscovery.cs(~190 LOC, single public method) speaks the agent-rpc discovery protocol overClientWebSocket. Never throws — returns the discovered port or a fallback.AgentBridgeClient.csdrops the hardcoded URI; resolves the port on every reconnect, so a running VS adapts to an agent-server restart without an extension reload.AGENT_SERVER_PORT(default8999),TYPEAGENT_VS_USE_DISCOVERY(default on),TYPEAGENT_VS_FALLBACK_PORT(default5680).Docs — README diagram + "Port discovery" / troubleshooting sections updated.
Validation
pnpm --filter visualstudio-agent build— green.Notes
"default"(not"ws-bridge"from the design doc) to matchcode/browserand keep@system portsconsistent.originAllowlist.tsis deliberately duplicated rather than shared — different scheme prefixes per agent, ~30 LOC.Reading order
src/originAllowlist.ts— origin policy.src/visualStudioActionHandler.ts— TS bridge rewrite.host/csharp/Bridge/BridgeDiscovery.cs— new discovery client.host/csharp/Bridge/AgentBridgeClient.cs— hardcoded URI removed.README.md— new env vars, discovery flow.