Unify Agent Host discovery across local and remote connections

[connor4312]

Summary

• unify editor and standalone Agent Host endpoint discovery through a shared multi-writer registry
• discover and select Agent Hosts across CLI, SSH, and tunnel connections, including editor-host failover
• remember a per-host remote execution preference with an accessible scenario-led modal and immediate reconnect
• support Markdown dialog details so the preference reminder links directly to the change-location command

Validation

• npm run typecheck-client
• npm run valid-layers-check
• npm run precommit
• 118 targeted Agent Host, dialog, SSH, and tunnel tests

[Copilot]

Pull request overview

Unifies Agent Host discovery and remote execution-location preferences across CLI, SSH, tunnels, and editor connections.

Changes:

• Adds a shared multi-writer Agent Host endpoint registry and CLI discovery.
• Adds SSH/tunnel endpoint selection, failover, and persisted location preferences.
• Adds Markdown dialog details and targeted coverage.

Show a summary per file

File group	Description
Dialog handlers and primitives	Supports Markdown details and vertical dialog layouts.
Agent Host registry and CLI discovery	Adds shared endpoint registration, locking, parsing, and discovery.
SSH and tunnel services	Adds endpoint selection, preference handling, and reconnect behavior.
Sessions remote-host contributions	Adds preference commands, options, registration, and documentation.
Agent Host, SSH, tunnel, and dialog tests	Covers discovery, selection, preferences, and dialog behavior.
cli/src/tunnels/user_data_path.rs	Resolves the shared registry’s platform-specific user-data path.

Review details

• Files reviewed: 67/67 changed files
• Comments generated: 5
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (3)

src/vs/platform/agentHost/node/tunnelAgentHostService.ts:462

• Parsing is outside the cleanup try, so a malformed selection acknowledgement leaves the detached WebSocket and relay client open after the pending entry has already been removed. Include parsing in the protected block so parse failures close both resources just like read/send failures.
  src/vs/sessions/contrib/providers/remoteAgentHost/electron-browser/tunnelAgentHostServiceImpl.ts:271
• If preference resolution throws (for example, the dialog service fails), the prepared selection is never cancelled. The main process then retains the open gateway WebSocket and relay client in _pendingSelections indefinitely because no post-inventory timeout closes it. Cancel the prepared selection in a catch/finally path before propagating the error.
  src/vs/platform/agentHost/node/tunnelAgentHostService.ts:416
• A malformed gateway inventory throws here after both the relay client and WebSocket have been opened, but before either is owned by PendingGatewaySelection; neither resource is then closed. Parse inside a cleanup-protected block (or explicitly close ws and dispose relayClient on parse failure).

This issue also appears on line 462 of the same file.

• Files reviewed: 70/70 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (6)

src/vs/platform/agentHost/node/tunnelAgentHostService.ts:462

• A malformed success acknowledgement throws here after the pending selection has been removed and detached, bypassing every cleanup branch. The gateway WebSocket and relay client remain live and are no longer tracked. Parse inside the cleanup try (or explicitly close/dispose on parse failure) before transferring the resources into TunnelConnection.
  src/vs/sessions/contrib/providers/remoteAgentHost/electron-browser/tunnelAgentHostServiceImpl.ts:277
• Once prepareSelection succeeds, an exception from preference lookup or dialog rendering leaves the main-process pending selection open indefinitely: cancelSelection is called only for the normal cancel result. Wrap selection resolution/completion in a cleanup path that cancels the selection unless ownership was successfully transferred.
  cli/src/commands/agent_relay.rs:166
• Returning when either copy reaches EOF breaks valid half-close behavior. For example, if stdin closes after sending a request, this drops the remote connection immediately and can truncate the response still flowing in the other direction. After one copy completes, shut down only its destination writer and continue awaiting the opposite copy (or use a bidirectional-copy helper with equivalent semantics).

	tokio::select! {
		r = &mut a_to_b => { r?; }
		r = &mut b_to_a => { r?; }
	}

cli/src/tunnels/idle_timeout.rs:130

• tokio::select! is unbiased by default. If the idle timer and an already-queued Connected event become ready on the same poll, it may choose the timer and shut down despite an accepted active client, contradicting the stated lifecycle guarantee. Use a biased select with the activity branch before the timer, and add a simultaneous-readiness regression test.

			tokio::select! {
				_ = sleeper.sleep(idle_timeout) => return,
				event = events.recv() => match event {

cli/src/tunnels/agent_host.rs:1716

• This accepts an ambiguous payload containing both a non-empty instanceId and newDedicated: true, silently choosing the existing process. The wire contract says these modes are mutually exclusive, and selecting an existing editor when the caller requested a dedicated host is unsafe. Reject requests unless exactly one valid selection mode is present.

		match (self.instance_id, self.new_dedicated) {
			(Some(id), _) if !id.is_empty() => Ok(GatewaySelection::Existing { instance_id: id }),
			(_, Some(true)) => Ok(GatewaySelection::NewDedicated),
			_ => Err(
				"Selection must include either a non-empty `instanceId` or `newDedicated: true`",
			),

src/vs/platform/agentHost/node/tunnelAgentHostService.ts:416

• If the gateway returns malformed inventory, this parser throws after both the WebSocket and relayClient have been created but before they are inserted into _pendingSelections; neither resource is then disposed. Include parsing in the preceding cleanup scope (or dispose both resources in a parse-error catch) before rethrowing.

This issue also appears on line 462 of the same file.

• Files reviewed: 70/70 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (6)

src/vs/platform/agentHost/node/tunnelAgentHostService.ts:462

• A malformed gateway acknowledgement throws here outside the preceding cleanup block. Since the pending selection was already removed with deleteAndLeak() and detached, no owner remains to close the WebSocket or dispose the relay client. Parse inside the guarded ownership block so every failure releases both resources.
  src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts:408
• For config-less SSH entries, user is optional, but the connection path defaults the SSH username to hostName. Passing undefined here produces a persisted key such as undefined@host:22, while the SSH service reads the preference under host@host:22; the saved location is therefore never applied. Use the same username fallback as the connection setup.
  src/vs/platform/agentHost/node/tunnelAgentHostService.ts:416
• If the gateway returns malformed inventory JSON, this parser throws after the cleanup catch has already completed. The newly opened WebSocket and relay client are then neither stored nor disposed, leaking both resources. Include parsing in the guarded block (or explicitly close/dispose on parser failure).

This issue also appears on line 462 of the same file.
src/vs/sessions/contrib/providers/remoteAgentHost/electron-browser/tunnelAgentHostServiceImpl.ts:283

• If preference lookup or the dialog path throws, the prepared selection is never completed or cancelled. The main-process pending-selection map then retains its gateway WebSocket and relay client indefinitely. Wrap selection resolution/completion in a failure path that calls cancelSelection(session.selectionId) before rethrowing.
  cli/src/tunnels/idle_timeout.rs:138
• At the timeout boundary, both the sleep and a queued Connected event can be ready; unbiased select! may choose the timer and shut down while a client has just connected. Give the event branch priority (for example with biased; and the event branch first) so a ready connection always cancels the idle shutdown.

			tokio::select! {
				_ = sleeper.sleep(idle_timeout) => return,
				event = events.recv() => match event {
					Some(ActivityEvent::Connected) => active_clients += 1,
					Some(ActivityEvent::Disconnected) => {
						// Spurious/duplicate disconnect while already at
						// zero; ignore rather than underflow.
					}
					None => std::future::pending::<()>().await,
				},

src/vs/sessions/contrib/providers/remoteAgentHost/electron-browser/remoteAgentHostLocationPreferenceCommand.ts:73

• This command computes undefined@host:port for config-less SSH entries without an explicit user, but the actual SSH connection defaults the username to hostName and reads a different preference key. As a result, changing the location through this command has no effect for those entries. Apply the same fallback here.

• Files reviewed: 70/70 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@benibenj

Matched files:

• src/vs/sessions/browser/parts/dialogs/mobileAwareDialogHandler.ts
• src/vs/workbench/browser/parts/dialogs/dialogHandler.ts