diff --git a/cli/src/download_cache.rs b/cli/src/download_cache.rs index 87ca1924a798..a35cde8377bd 100644 --- a/cli/src/download_cache.rs +++ b/cli/src/download_cache.rs @@ -4,20 +4,27 @@ *--------------------------------------------------------------------------------------------*/ use std::{ - fs::create_dir_all, + fs::{create_dir, create_dir_all, OpenOptions}, path::{Path, PathBuf}, }; use futures::Future; -use tokio::fs::remove_dir_all; +use uuid::Uuid; use crate::{ state::PersistedState, - util::errors::{wrap, AnyError, WrappedError}, + util::{ + errors::{wrap, AnyError, WrappedError}, + file_lock::{FileLock, Lock}, + }, }; const KEEP_LRU: usize = 5; const STAGING_SUFFIX: &str = ".staging"; +const LOCKS_DIRECTORY: &str = ".locks"; +const LOCK_WAIT_INITIAL_DELAY: std::time::Duration = std::time::Duration::from_millis(200); +const LOCK_WAIT_MAX_DELAY: std::time::Duration = std::time::Duration::from_secs(2); +const LOCK_WAIT_HEARTBEAT_INTERVAL: std::time::Duration = std::time::Duration::from_secs(5); const RENAME_ATTEMPTS: u32 = 20; const RENAME_DELAY: std::time::Duration = std::time::Duration::from_millis(200); const PERSISTED_STATE_FILE_NAME: &str = "lru.json"; @@ -28,6 +35,16 @@ pub struct DownloadCache { state: PersistedState>, } +struct StagingDirectory(PathBuf); + +impl Drop for StagingDirectory { + fn drop(&mut self) { + // Drop cannot await, so use blocking cleanup to also remove staging directories + // when the creating future is cancelled. + let _ = std::fs::remove_dir_all(&self.0); + } +} + impl DownloadCache { pub fn new(path: PathBuf) -> DownloadCache { DownloadCache { @@ -90,20 +107,78 @@ impl DownloadCache { return Ok(target_dir); } - let temp_dir = self.path.join(format!("{name}{STAGING_SUFFIX}")); - let _ = remove_dir_all(&temp_dir).await; // cleanup any existing + create_dir_all(&self.path).map_err(|e| wrap(e, "error creating server directory"))?; - create_dir_all(&temp_dir).map_err(|e| wrap(e, "error creating server directory"))?; - do_create(temp_dir.clone()).await?; + let lock_path = self.path.join(LOCKS_DIRECTORY).join(name); + if let Some(lock_parent) = lock_path.parent() { + create_dir_all(lock_parent).map_err(|e| wrap(e, "error creating server download lock"))?; + } + + let mut lock_wait_started = None; + let mut lock_wait_delay = LOCK_WAIT_INITIAL_DELAY; + let mut next_lock_wait_heartbeat = LOCK_WAIT_HEARTBEAT_INTERVAL; + let _lock = loop { + let lock_file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(&lock_path) + .map_err(|e| wrap(e, "error creating server download lock"))?; + + match FileLock::acquire(lock_file) + .map_err(|e| wrap(e, "error acquiring server download lock"))? + { + Lock::Acquired(lock) => break lock, + Lock::AlreadyLocked(_) if target_dir.exists() => { + let _ = self.touch(name.to_string()); + return Ok(target_dir); + } + Lock::AlreadyLocked(_) => { + let first_wait = lock_wait_started.is_none(); + let wait_started = lock_wait_started.get_or_insert_with(std::time::Instant::now); + let elapsed = wait_started.elapsed(); + if first_wait { + log::info!( + "Another instance is already downloading the server; waiting for it to finish" + ); + } else if elapsed >= next_lock_wait_heartbeat { + log::info!( + "Another instance is still downloading the server; waited {} seconds", + elapsed.as_secs() + ); + next_lock_wait_heartbeat = elapsed + LOCK_WAIT_HEARTBEAT_INTERVAL; + } + + tokio::time::sleep(lock_wait_delay).await; + lock_wait_delay = + std::cmp::min(lock_wait_delay.saturating_mul(2), LOCK_WAIT_MAX_DELAY); + } + } + }; + + if target_dir.exists() { + let _ = self.touch(name.to_string()); + return Ok(target_dir); + } + + let temp_dir = self + .path + .join(format!("{name}{STAGING_SUFFIX}-{}", Uuid::new_v4())); + create_dir(&temp_dir).map_err(|e| wrap(e, "error creating server directory"))?; + let temp_dir = StagingDirectory(temp_dir); + do_create(temp_dir.0.clone()).await?; let _ = self.touch(name.to_string()); // retry the rename, it seems on WoA sometimes it takes a second for the // directory to be 'unlocked' after doing file/process operations in it. for attempt_no in 0..=RENAME_ATTEMPTS { - match std::fs::rename(&temp_dir, &target_dir) { + match std::fs::rename(&temp_dir.0, &target_dir) { Ok(_) => { break; } + Err(_) if target_dir.exists() => { + return Ok(target_dir); + } Err(e) if attempt_no == RENAME_ATTEMPTS => { return Err(wrap(e, "error renaming downloaded server").into()) } @@ -138,3 +213,96 @@ impl DownloadCache { Ok(()) } } + +#[cfg(test)] +mod tests { + use std::sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }; + + use super::*; + + fn staging_directories(cache: &DownloadCache, name: &str) -> Vec { + std::fs::read_dir(cache.path()) + .unwrap() + .filter_map(Result::ok) + .map(|entry| entry.path()) + .filter(|path| { + path.file_name() + .unwrap() + .to_string_lossy() + .starts_with(&format!("{name}{STAGING_SUFFIX}")) + }) + .collect() + } + + #[tokio::test] + async fn test_concurrent_create_runs_creator_once() { + let dir = tempfile::tempdir().unwrap(); + let cache = DownloadCache::new(dir.path().join("cache")); + let create_count = Arc::new(AtomicUsize::new(0)); + + let first_count = create_count.clone(); + let first = cache.create("server", move |path| { + first_count.fetch_add(1, Ordering::SeqCst); + async move { + std::fs::write(path.join("created"), "").unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + Ok(()) + } + }); + let second_count = create_count.clone(); + let second = cache.create("server", move |_| { + second_count.fetch_add(1, Ordering::SeqCst); + async { Ok(()) } + }); + + let (first, second) = tokio::join!(first, second); + assert_eq!(first.unwrap(), second.unwrap()); + assert_eq!(create_count.load(Ordering::SeqCst), 1); + } + + #[tokio::test] + async fn test_failed_create_removes_staging_directory() { + let dir = tempfile::tempdir().unwrap(); + let cache = DownloadCache::new(dir.path().join("cache")); + + let result = cache + .create("server", |_| async { + Err::<(), AnyError>( + wrap( + std::io::Error::new(std::io::ErrorKind::Other, "expected failure"), + "test failure", + ) + .into(), + ) + }) + .await; + + assert!(result.is_err()); + assert!(staging_directories(&cache, "server").is_empty()); + } + + #[tokio::test] + async fn test_lost_rename_race_returns_existing_target() { + let dir = tempfile::tempdir().unwrap(); + let cache = DownloadCache::new(dir.path().join("cache")); + let target_dir = cache.path().join("server"); + + let result = cache + .create("server", move |path| { + let target_dir = target_dir.clone(); + async move { + std::fs::write(path.join("created"), "").unwrap(); + std::fs::create_dir(&target_dir).unwrap(); + std::fs::write(target_dir.join("winner"), "").unwrap(); + Ok(()) + } + }) + .await; + + assert_eq!(result.unwrap(), cache.path().join("server")); + assert!(staging_directories(&cache, "server").is_empty()); + } +} diff --git a/src/vs/platform/agentHost/browser/remoteAgentHostServiceImpl.ts b/src/vs/platform/agentHost/browser/remoteAgentHostServiceImpl.ts index a2683f5c7c05..4742f1a04ddc 100644 --- a/src/vs/platform/agentHost/browser/remoteAgentHostServiceImpl.ts +++ b/src/vs/platform/agentHost/browser/remoteAgentHostServiceImpl.ts @@ -297,6 +297,20 @@ export class RemoteAgentHostService extends Disposable implements IRemoteAgentHo return; } const normalized = normalizeRemoteAgentHostAddress(address); + // A dial already in flight is itself a fresh attempt, so neither a + // retry nor a user request gains anything by tearing it down and + // starting a second one — that is what produced concurrent remote + // bootstraps. Join it instead. A user-initiated request still restores + // the retry budget, so pressing reconnect while a slow bootstrap runs + // is not silently useless if that bootstrap ultimately fails. + if (this._pendingConnects.has(normalized)) { + if (userInitiated) { + this._failedReconnects.delete(normalized); + this._cancelReconnect(normalized); + this._reconnectAttempts.delete(normalized); + } + return; + } this._failedReconnects.delete(normalized); const configuredEntry = this._configuredEntries.get().find( @@ -318,7 +332,10 @@ export class RemoteAgentHostService extends Disposable implements IRemoteAgentHo // Cancel any pending reconnect this._cancelReconnect(normalized); - this._reconnectAttempts.delete(normalized); + if (userInitiated) { + // An automatic retry must not resurrect its own exhausted attempt budget. + this._reconnectAttempts.delete(normalized); + } // Tear down existing connection if present const entry = this._entries.get(normalized); diff --git a/src/vs/platform/agentHost/common/wslRemoteAgentHost.ts b/src/vs/platform/agentHost/common/wslRemoteAgentHost.ts index ddf1700a4756..27a057bf483f 100644 --- a/src/vs/platform/agentHost/common/wslRemoteAgentHost.ts +++ b/src/vs/platform/agentHost/common/wslRemoteAgentHost.ts @@ -97,8 +97,8 @@ export interface IWSLRemoteAgentHostService { listRunningDistros(): Promise; connect(config: IWSLAgentHostConfig): Promise; disconnect(distro: string): Promise; - /** Reconnect a user-selected cached distro. */ - reconnect(distro: string, name: string): Promise; + /** Reconnect a cached distro, optionally as an automatic recovery attempt. */ + reconnect(distro: string, name: string, userInitiated?: boolean): Promise; /** * Distros the user has connected to, persisted across windows. Drives the * remote agent host service's startup auto-connect. WSL connections diff --git a/src/vs/platform/agentHost/electron-browser/wslRemoteAgentHostServiceImpl.ts b/src/vs/platform/agentHost/electron-browser/wslRemoteAgentHostServiceImpl.ts index 2e512d0b5e7f..45a48caf665f 100644 --- a/src/vs/platform/agentHost/electron-browser/wslRemoteAgentHostServiceImpl.ts +++ b/src/vs/platform/agentHost/electron-browser/wslRemoteAgentHostServiceImpl.ts @@ -173,10 +173,10 @@ class WSLConnectionFactory extends Disposable implements IRemoteAgentHostConnect return entry; } - stageEntry(distro: string, name: string): IRemoteAgentHostEntry { + stageEntry(distro: string, name: string, userInitiated = true): IRemoteAgentHostEntry { const entry = this._createEntry(distro, name); this._stagedConfigurations.set(getEntryAddress(entry), { - config: { distro, name, remoteAgentHostCommand: this._getRemoteAgentHostCommand(), userInitiated: true }, + config: { distro, name, remoteAgentHostCommand: this._getRemoteAgentHostCommand(), userInitiated }, isInitialConnection: false, }); this._storeEntry(entry); @@ -448,15 +448,15 @@ export class WSLRemoteAgentHostService extends Disposable implements IWSLRemoteA await this._mainService.disconnect(distro); } - async reconnect(distro: string, name: string): Promise { + async reconnect(distro: string, name: string, userInitiated = true): Promise { if (!this._configurationService.getValue(RemoteAgentHostsEnabledSettingId)) { throw new Error('Remote agent host connections are not enabled.'); } - const entry = this._connectionFactory.stageEntry(distro, name); + const entry = this._connectionFactory.stageEntry(distro, name, userInitiated); const address = getEntryAddress(entry); this._logService.info(`[WSLRemoteAgentHost] Reconnecting to distro ${distro}`); - this._remoteAgentHostService.reconnect(address, true); + this._remoteAgentHostService.reconnect(address, userInitiated); await this._remoteAgentHostService.waitForConnection(address); return this._getConnectionHandle(address); } diff --git a/src/vs/platform/agentHost/node/devContainerAgentHostService.ts b/src/vs/platform/agentHost/node/devContainerAgentHostService.ts index 2d1775583627..6a0bd0d8b170 100644 --- a/src/vs/platform/agentHost/node/devContainerAgentHostService.ts +++ b/src/vs/platform/agentHost/node/devContainerAgentHostService.ts @@ -30,6 +30,7 @@ import { buildAgentHostSpawnCommand, buildAgentRelayCommand, filterLiveAgentHostEndpoints, + getNewAgentHostRegistrationTimeoutMs, getRemoteCLIDataDir, ISshExec, resolveRemotePlatform, @@ -144,7 +145,7 @@ export class DevContainerAgentHostMainService extends Disposable implements IDev const serverDataFolderName = this._productService.serverDataFolderName ?? '.vscode-server-oss'; const quality = this._productService.quality || 'insider'; - const cliBin = await ensureRemoteAgentHostCliInstalled(exec, platform, { + const cliInstallation = await ensureRemoteAgentHostCliInstalled(exec, platform, { serverDataFolderName, quality, commit: this._productService.commit, @@ -152,6 +153,7 @@ export class DevContainerAgentHostMainService extends Disposable implements IDev logService: this._logService, logPrefix: LOG_PREFIX, }); + const { cliBin } = cliInstallation; const cliDataDir = getRemoteCLIDataDir(serverDataFolderName); const initial = await runAgentEndpoints(exec, cliBin, cliDataDir); const live = await filterLiveAgentHostEndpoints(exec, initial.endpoints); @@ -168,13 +170,18 @@ export class DevContainerAgentHostMainService extends Disposable implements IDev void exec(spawnCommand, { ignoreExitCode: true }).catch(error => { this._logService.warn(`${LOG_PREFIX} Agent Host spawn command failed`, error); }); + this._logService.info(`${LOG_PREFIX} Waiting for the new agent host to register...`); endpoint = await waitForNewStandaloneEndpoint( exec, cliBin, cliDataDir, initial.userDataPath, live, - { token: tokenSource.token }, + { + timeoutMs: getNewAgentHostRegistrationTimeoutMs(cliInstallation.installed), + token: tokenSource.token, + progress: elapsedMs => this._logService.info(`${LOG_PREFIX} Waiting for the new agent host to register... (${Math.floor(elapsedMs / 1000)} seconds elapsed)`), + }, ); } diff --git a/src/vs/platform/agentHost/node/remoteAgentHostCliInstaller.ts b/src/vs/platform/agentHost/node/remoteAgentHostCliInstaller.ts index e0fa1413b225..ccf678096ece 100644 --- a/src/vs/platform/agentHost/node/remoteAgentHostCliInstaller.ts +++ b/src/vs/platform/agentHost/node/remoteAgentHostCliInstaller.ts @@ -24,6 +24,12 @@ export interface IRemoteAgentHostCliInstallOptions { readonly logPrefix?: string; } +/** The resolved CLI path and whether this invocation installed it. */ +export interface IRemoteAgentHostCliInstallResult { + readonly cliBin: string; + readonly installed: boolean; +} + /** * Ensure that a VS Code CLI suitable for launching an Agent Host is installed * on a remote execution target. @@ -32,7 +38,7 @@ export async function ensureRemoteAgentHostCliInstalled( exec: ISshExec, platform: { readonly os: string; readonly arch: string }, options: IRemoteAgentHostCliInstallOptions, -): Promise { +): Promise { return options.commit ? ensurePinnedCliInstalled(exec, platform, options, options.commit) : ensureLooseCliInstalled(exec, platform, options); @@ -43,7 +49,7 @@ async function ensurePinnedCliInstalled( platform: { readonly os: string; readonly arch: string }, options: IRemoteAgentHostCliInstallOptions, commit: string, -): Promise { +): Promise { const cliBin = getRemoteCLIBin(options.serverDataFolderName, options.quality, commit); const installRoot = getRemoteCLIInstallRoot(options.serverDataFolderName); const logPrefix = options.logPrefix ?? '[RemoteAgentHostCliInstaller]'; @@ -56,7 +62,7 @@ async function ensurePinnedCliInstalled( } else { options.logService.warn(`${logPrefix} Skipping CLI retention cleanup: touch exited ${touchCode}`); } - return cliBin; + return { cliBin, installed: false }; } options.reportInstalling(); @@ -78,14 +84,14 @@ async function ensurePinnedCliInstalled( } options.logService.info(`${logPrefix} Installed remote CLI at ${cliBin}`); await exec(buildCleanupOldCLIsCommand(options.serverDataFolderName, options.quality), { ignoreExitCode: true }); - return cliBin; + return { cliBin, installed: true }; } catch (error) { const message = error instanceof Error ? error.message : String(error); options.logService.warn(`${logPrefix} Could not install matching CLI for commit ${commit}: ${message}. Looking for a fallback CLI...`); const fallback = await findFallbackCli(exec, options); if (fallback) { options.logService.warn(`${logPrefix} Using fallback CLI at ${fallback} (does not match desktop commit ${commit}).`); - return fallback; + return { cliBin: fallback, installed: false }; } throw error; } @@ -95,7 +101,7 @@ async function ensureLooseCliInstalled( exec: ISshExec, platform: { readonly os: string; readonly arch: string }, options: IRemoteAgentHostCliInstallOptions, -): Promise { +): Promise { const cliBin = getRemoteCLIBin(options.serverDataFolderName, options.quality); const installRoot = getRemoteCLIInstallRoot(options.serverDataFolderName); const logPrefix = options.logPrefix ?? '[RemoteAgentHostCliInstaller]'; @@ -110,7 +116,7 @@ async function ensureLooseCliInstalled( options.logService.warn(`${logPrefix} Could not refresh the dev-build remote CLI at ${cliBin}; reusing the existing executable: update exited ${updateExitCode}`); } options.logService.info(`${logPrefix} Reusing remote CLI at ${cliBin} (dev build, latest-version refresh attempted)`); - return cliBin; + return { cliBin, installed: false }; } options.reportInstalling(); @@ -121,7 +127,7 @@ async function ensureLooseCliInstalled( `chmod +x ${cliBin}`, ].join(' && ')); options.logService.info(`${logPrefix} Installed remote CLI at ${cliBin}`); - return cliBin; + return { cliBin, installed: true }; } async function findFallbackCli(exec: ISshExec, options: IRemoteAgentHostCliInstallOptions): Promise { diff --git a/src/vs/platform/agentHost/node/sshRemoteAgentHostHelpers.ts b/src/vs/platform/agentHost/node/sshRemoteAgentHostHelpers.ts index 9adcaf51b2d7..25a5e7d5334c 100644 --- a/src/vs/platform/agentHost/node/sshRemoteAgentHostHelpers.ts +++ b/src/vs/platform/agentHost/node/sshRemoteAgentHostHelpers.ts @@ -5,6 +5,7 @@ import { timeout } from '../../../base/common/async.js'; import { CancellationToken } from '../../../base/common/cancellation.js'; +import { CancellationError } from '../../../base/common/errors.js'; import { vArray, vObj, vString, vUnknown } from '../../../base/common/validation.js'; import { TelemetryConfiguration } from '../../telemetry/common/telemetry.js'; import { getAgentHostEndpointIdentityKey, IAgentHostEndpointMetadata, parseAgentHostEndpointRegistry } from '../common/agentHostEndpointRegistry.js'; @@ -487,17 +488,32 @@ export function findNewAgentHostEndpoint(before: readonly IAgentHostEndpointMeta } export interface IWaitForNewEndpointOptions { - /** Maximum number of `agent endpoints` polls before giving up. Defaults to 20. */ - readonly attempts?: number; - /** Delay between polls, in milliseconds. Defaults to 500. */ + /** + * Overall deadline for endpoint registration in milliseconds. When omitted, + * the deadline is twenty initial polling intervals (10 seconds by default). + */ + readonly timeoutMs?: number; + /** Initial delay between polls in milliseconds. Defaults to 500. */ readonly intervalMs?: number; readonly token?: CancellationToken; + /** Called periodically while endpoint registration is still pending. */ + readonly progress?: (elapsedMs: number) => void; +} + +const DEFAULT_ENDPOINT_REGISTRATION_POLL_COUNT = 20; +const MAX_ENDPOINT_REGISTRATION_POLL_INTERVAL_MS = 5_000; +const ENDPOINT_REGISTRATION_PROGRESS_INTERVAL_MS = 10_000; +const COLD_AGENT_HOST_REGISTRATION_TIMEOUT_MS = 300_000; + +/** Gets the endpoint-registration deadline for a newly installed CLI. */ +export function getNewAgentHostRegistrationTimeoutMs(installedCLI: boolean): number | undefined { + return installedCLI ? COLD_AGENT_HOST_REGISTRATION_TIMEOUT_MS : undefined; } /** * Poll `code agent endpoints` until a newly spawned standalone entry shows - * up (see {@link findNewAgentHostEndpoint}), or throw once the attempt - * budget is exhausted. The spawn command itself is fire-and-forget (its + * up (see {@link findNewAgentHostEndpoint}), or throw once the deadline + * expires. The spawn command itself is fire-and-forget (its * process is not tied to the SSH exec channel that launched it — see * {@link buildAgentHostSpawnCommand}), so this is the only way to learn * the freshly assigned TCP address/token/instanceId. @@ -510,21 +526,42 @@ export async function waitForNewStandaloneEndpoint( before: readonly IAgentHostEndpointMetadata[], options?: IWaitForNewEndpointOptions, ): Promise { - const attempts = options?.attempts ?? 20; - const intervalMs = options?.intervalMs ?? 500; - for (let attempt = 0; attempt < attempts; attempt++) { + const initialIntervalMs = options?.intervalMs ?? 500; + const timeoutMs = options?.timeoutMs ?? DEFAULT_ENDPOINT_REGISTRATION_POLL_COUNT * initialIntervalMs; + const startTime = Date.now(); + const deadline = startTime + timeoutMs; + let polls = 0; + let nextProgressReport = ENDPOINT_REGISTRATION_PROGRESS_INTERVAL_MS; + + while (true) { + if (options?.token?.isCancellationRequested) { + throw new CancellationError(); + } const { endpoints } = await runAgentEndpoints(exec, cliBin, cliDataDir, userDataPath); const found = findNewAgentHostEndpoint(before, endpoints); if (found) { return found; } - if (attempt < attempts - 1) { - if (options?.token) { - await timeout(intervalMs, options.token); - } else { - await timeout(intervalMs); - } + + polls++; + const elapsedMs = Date.now() - startTime; + if (elapsedMs >= nextProgressReport) { + options?.progress?.(elapsedMs); + nextProgressReport = (Math.floor(elapsedMs / ENDPOINT_REGISTRATION_PROGRESS_INTERVAL_MS) + 1) * ENDPOINT_REGISTRATION_PROGRESS_INTERVAL_MS; + } + if (Date.now() >= deadline) { + throw new Error(`Timed out waiting for the newly spawned agent host to register itself after ${Date.now() - startTime}ms (deadline ${timeoutMs}ms)`); + } + + const intervalMs = Math.min( + initialIntervalMs * 2 ** Math.floor((polls - 1) / 10), + MAX_ENDPOINT_REGISTRATION_POLL_INTERVAL_MS, + deadline - Date.now(), + ); + if (options?.token) { + await timeout(intervalMs, options.token); + } else { + await timeout(intervalMs); } } - throw new Error(`Timed out waiting for the newly spawned agent host to register itself (checked ${attempts} times, ~${Math.round(attempts * intervalMs / 1000)}s)`); } diff --git a/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts b/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts index e27480563d87..6aaa253288ad 100644 --- a/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts +++ b/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts @@ -61,6 +61,7 @@ import { buildAgentRelayCommand, extractAgentHostWebSocketURL, filterLiveAgentHostEndpoints, + getNewAgentHostRegistrationTimeoutMs, getRemoteCLIDataDir, redactToken, resolveRemotePlatform, @@ -69,7 +70,7 @@ import { validateAgentHostTelemetryLevel, waitForNewStandaloneEndpoint, } from './sshRemoteAgentHostHelpers.js'; -import { ensureRemoteAgentHostCliInstalled } from './remoteAgentHostCliInstaller.js'; +import { ensureRemoteAgentHostCliInstalled, type IRemoteAgentHostCliInstallResult } from './remoteAgentHostCliInstaller.js'; import { parseSSHConfigHostEntries, parseSSHGOutput, stripSSHComment } from '../common/sshConfigParsing.js'; import { removeAnsiEscapeCodes } from '../../../base/common/strings.js'; @@ -939,7 +940,8 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem } this._logService.info(`${LOG_PREFIX} Remote platform: ${platform.os}-${platform.arch}`); reportProgress(localize('sshProgressInstallingCLI', "Checking remote CLI installation...")); - cliBin = await this._ensureCLIInstalled(sshClient, platform, reportProgress); + const cliInstallation = await this._ensureCLIInstalled(sshClient, platform, reportProgress); + cliBin = cliInstallation.cliBin; cliDataDir = getRemoteCLIDataDir(this._serverDataFolderName); // 3. Discover every live endpoint on the remote via the shared registry. @@ -963,7 +965,10 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem this._logService.warn(`${LOG_PREFIX} Spawn command for dedicated agent host reported an error: ${err instanceof Error ? err.message : String(err)}`); }); reportProgress(localize('sshProgressAwaitingAgent', "Waiting for the new agent host to register...")); - return waitForNewStandaloneEndpoint(exec, cliBin, cliDataDir, userDataPath, live); + return waitForNewStandaloneEndpoint(exec, cliBin, cliDataDir, userDataPath, live, { + timeoutMs: getNewAgentHostRegistrationTimeoutMs(cliInstallation.installed), + progress: elapsedMs => reportProgress(localize('sshProgressStillAwaitingAgent', "Waiting for the new agent host to register... ({0} seconds elapsed)", Math.floor(elapsedMs / 1000))), + }); }; // Deterministic dedicated (standalone) selection: reuse a live @@ -2079,9 +2084,9 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem * at `~//`. Existing CLIs self-update * against the latest release before reuse. * - * Returns the resolved CLI binary path to run. + * Returns the resolved CLI binary path and its install outcome. */ - private async _ensureCLIInstalled(client: SSHClient, platform: { os: string; arch: string }, reportProgress: (message: string) => void): Promise { + private async _ensureCLIInstalled(client: SSHClient, platform: { os: string; arch: string }, reportProgress: (message: string) => void): Promise { return ensureRemoteAgentHostCliInstalled(bindSshExec(client), platform, { serverDataFolderName: this._serverDataFolderName, quality: this._quality, diff --git a/src/vs/platform/agentHost/node/wslRemoteAgentHostHelpers.ts b/src/vs/platform/agentHost/node/wslRemoteAgentHostHelpers.ts index 575c63377cc4..e862b86244d9 100644 --- a/src/vs/platform/agentHost/node/wslRemoteAgentHostHelpers.ts +++ b/src/vs/platform/agentHost/node/wslRemoteAgentHostHelpers.ts @@ -283,7 +283,8 @@ export function composeAgentHostBootstrapScript(args: IComposeAgentHostBootstrap const cliBin = getRemoteCLIBin(args.serverDataFolderName, args.quality, args.commit); const cliDataDir = getRemoteCLIDataDir(args.serverDataFolderName); const url = buildCLIDownloadUrl(args.os, args.arch, args.quality, args.commit); - const launch = `exec ${buildAgentHostBaseCommand(cliBin, cliDataDir, telemetryLevel)}`; + const agentHostCommand = buildAgentHostBaseCommand(cliBin, cliDataDir, telemetryLevel); + const launch = buildWslAgentHostLaunch(agentHostCommand); if (args.commit) { // Pinned-install path. Mirrors SSH's _ensureCLIInstalledPinned: stage @@ -318,6 +319,16 @@ export function composeAgentHostBootstrapScript(args: IComposeAgentHostBootstrap ].join(' && '); } +/** + * Build the WSL launch command with the CLI's disconnected-host reaper. + */ +function buildWslAgentHostLaunch(command: string, idleTimeoutSec = 300): string { + if (!Number.isSafeInteger(idleTimeoutSec) || idleTimeoutSec <= 0) { + throw new Error(`Unsafe idle timeout value for shell interpolation: ${JSON.stringify(idleTimeoutSec)}`); + } + return `exec ${command} --idle-timeout ${idleTimeoutSec}`; +} + /** * Validate that a string is safe to interpolate as a `wsl.exe -d ` * argument. WSL distro names are user-creatable so they could in principle diff --git a/src/vs/platform/agentHost/node/wslRemoteAgentHostService.ts b/src/vs/platform/agentHost/node/wslRemoteAgentHostService.ts index d9837cffbe32..fd43925e4cf1 100644 --- a/src/vs/platform/agentHost/node/wslRemoteAgentHostService.ts +++ b/src/vs/platform/agentHost/node/wslRemoteAgentHostService.ts @@ -37,8 +37,11 @@ import { const LOG_PREFIX = '[WSLRemoteAgentHost]'; -/** Max time to wait for `code agent host` inside the distro to print its `ws://` URL. */ -const AGENT_HOST_READY_TIMEOUT_MS = 60_000; +/** Max time `code agent host` may be silent before printing its `ws://` URL. */ +const AGENT_HOST_OUTPUT_IDLE_TIMEOUT_MS = 60_000; + +/** Absolute upper bound for bootstrap, including CLI and server downloads. */ +const AGENT_HOST_READY_OVERALL_TIMEOUT_MS = 10 * 60_000; /** Max time to wait for the host-side WebSocket to complete its handshake. */ const WEBSOCKET_OPEN_TIMEOUT_MS = 30_000; @@ -76,6 +79,7 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem private readonly _connections = new Map(); private readonly _distroToConnectionId = new Map(); + private readonly _pendingConnects = new Map>(); private _nativeRequire: NodeJS.Require | undefined; @@ -160,7 +164,7 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem } } - async connect(config: IWSLAgentHostConfig): Promise { + connect(config: IWSLAgentHostConfig): Promise { const distro = validateDistroName(config.distro); // Idempotent: a second `connect` for an already-live distro returns @@ -171,16 +175,34 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem if (existingId) { const existing = this._connections.get(existingId); if (existing) { - return { + return Promise.resolve({ connectionId: existing.connectionId, address: existing.address, distro: existing.distro, name: existing.name, connectionToken: existing.connectionToken, - }; + }); } } + const existingPendingConnect = this._pendingConnects.get(distro); + if (existingPendingConnect) { + return existingPendingConnect; + } + + // Reserve synchronously, before _connectUnguarded reaches its first + // await, so simultaneous callers cannot start concurrent downloads. + const pendingConnect = this._connectUnguarded(config, distro); + this._pendingConnects.set(distro, pendingConnect); + void pendingConnect.finally(() => { + if (this._pendingConnects.get(distro) === pendingConnect) { + this._pendingConnects.delete(distro); + } + }).catch(() => { /* The caller observes the original rejection. */ }); + return pendingConnect; + } + + private async _connectUnguarded(config: IWSLAgentHostConfig, distro: string): Promise { const connectionKey = `wsl:${distro}`; const reportProgress = (message: string) => { this._onDidReportConnectProgress.fire({ connectionKey, message }); @@ -209,10 +231,7 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem // agent host's stdout/stderr, which is already valid UTF-8 from a // Linux process. Keeping the bytes untouched also avoids surprising // the URL/PID regex. - const child = cp.spawn(getWslExePath(), ['-d', distro, '-e', 'bash', '-lc', script], { - windowsHide: true, - stdio: ['ignore', 'pipe', 'pipe'], - }); + const child = this._spawnAgentHost(distro, script); let url: string | undefined; let urlResolve: ((value: { url: string; token: string | undefined }) => void) | undefined; @@ -232,6 +251,34 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem } }; + let outputIdleTimeoutHandle: ReturnType | undefined; + let overallTimeoutHandle: ReturnType | undefined; + const clearReadyTimeouts = () => { + if (outputIdleTimeoutHandle !== undefined) { + clearTimeout(outputIdleTimeoutHandle); + outputIdleTimeoutHandle = undefined; + } + if (overallTimeoutHandle !== undefined) { + clearTimeout(overallTimeoutHandle); + overallTimeoutHandle = undefined; + } + }; + const rejectForTimeout = (message: string) => { + clearReadyTimeouts(); + urlReject?.(new Error(`${LOG_PREFIX} ${message}\nOutput: ${outputLines.join('\n')}`)); + }; + const armOutputIdleTimeout = () => { + if (url) { + return; + } + if (outputIdleTimeoutHandle !== undefined) { + clearTimeout(outputIdleTimeoutHandle); + } + outputIdleTimeoutHandle = setTimeout(() => { + rejectForTimeout(`Timed out waiting for agent host in '${distro}' to print its WebSocket URL: no output for ${AGENT_HOST_OUTPUT_IDLE_TIMEOUT_MS}ms.`); + }, AGENT_HOST_OUTPUT_IDLE_TIMEOUT_MS); + }; + const onStreamData = (data: Buffer) => { // `decodeWslOutput` handles both UTF-8 (the agent host's own // stdout when running with `WSL_UTF8` unset, which is what we @@ -244,6 +291,7 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem if (!line) { continue; } + armOutputIdleTimeout(); appendLine(line); this._logService.trace(`${LOG_PREFIX} [${distro}] ${redactToken(line)}`); if (!url) { @@ -259,32 +307,37 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem child.stdout?.on('data', onStreamData); child.stderr?.on('data', onStreamData); - const childExited = new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((res) => { - child.once('exit', (code, signal) => res({ code, signal })); - }); - - // Race the URL parse against the child dying and the global timeout. + // Race the URL parse against the child dying, output going idle, and + // an overall ceiling. Bootstrap downloads regularly report progress, + // so only a period of silence indicates that it has become stuck. // `outputLines` is already redacted in `appendLine` — no extra wrap needed. - const readyTimeoutHandle = setTimeout(() => { - urlReject?.(new Error(`${LOG_PREFIX} Timed out waiting for agent host in '${distro}' to print its WebSocket URL after ${AGENT_HOST_READY_TIMEOUT_MS}ms.\nOutput: ${outputLines.join('\n')}`)); - }, AGENT_HOST_READY_TIMEOUT_MS); + armOutputIdleTimeout(); + overallTimeoutHandle = setTimeout(() => { + rejectForTimeout(`Timed out waiting for agent host in '${distro}' to print its WebSocket URL: exceeded the overall ${AGENT_HOST_READY_OVERALL_TIMEOUT_MS}ms bootstrap ceiling.`); + }, AGENT_HOST_READY_OVERALL_TIMEOUT_MS); - const earlyExitGuard = childExited.then(({ code, signal }) => { + child.once('exit', (code, signal) => { if (!url) { + clearReadyTimeouts(); urlReject?.(new Error(`${LOG_PREFIX} Agent host in '${distro}' exited (code=${code}, signal=${signal}) before printing its WebSocket URL.\nOutput: ${outputLines.join('\n')}`)); } }); + child.once('error', err => { + if (!url) { + clearReadyTimeouts(); + urlReject?.(new Error(`${LOG_PREFIX} Failed to start agent host in '${distro}': ${err.message}\nOutput: ${outputLines.join('\n')}`)); + } + }); let resolvedUrl: { url: string; token: string | undefined }; try { resolvedUrl = await urlPromise; } catch (err) { - clearTimeout(readyTimeoutHandle); + clearReadyTimeouts(); this._killChild(child); - await earlyExitGuard.catch(() => { /* already surfaced */ }); throw err; } - clearTimeout(readyTimeoutHandle); + clearReadyTimeouts(); reportProgress(localize('wslProgressConnecting', "Connecting to agent host in {0}...", distro)); let ws: WebSocket; @@ -354,6 +407,9 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem if (existingId) { this._closeConnection(existingId); } + // A pending connection is already a fresh bootstrap. Joining it avoids + // starting a competing downloader; callers that reconnect after it + // fails receive that failure and a subsequent reconnect starts anew. return this.connect({ distro, name, remoteAgentHostCommand, userInitiated }); } @@ -392,12 +448,15 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem if (child.exitCode !== null || child.signalCode !== null) { return; } + // A detached distro-side host relies on the bootstrap's --idle-timeout to exit. try { child.kill(); } catch { /* ignore */ } // Escalate to SIGKILL if the process is still alive after 2s. The // `unref` cast avoids the dom/node `setTimeout` typing collision in - // strict mode — we only care that escalation never blocks process exit. + // strict mode — we only care that escalation never blocks process exit, + // so it is optional: outside Node (the unit-test renderer) there is no + // `unref` and keeping the timer referenced is harmless. const escalate = setTimeout(() => { if (child.exitCode === null && child.signalCode === null) { try { @@ -405,11 +464,18 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem } catch { /* ignore */ } } }, 2_000) as unknown as NodeJS.Timeout; - escalate.unref(); + escalate.unref?.(); child.once('exit', () => clearTimeout(escalate)); } - private async _resolvePlatform(distro: string): Promise<{ os: string; arch: string }> { + protected _spawnAgentHost(distro: string, script: string): cp.ChildProcess { + return cp.spawn(getWslExePath(), ['-d', distro, '-e', 'bash', '-lc', script], { + windowsHide: true, + stdio: ['ignore', 'pipe', 'pipe'], + }); + } + + protected async _resolvePlatform(distro: string): Promise<{ os: string; arch: string }> { const result = await runWslCommand(['-e', 'uname', '-s', '-m'], { distro, timeout: 10_000 }); if (result.exitCode !== 0) { throw new Error(`${LOG_PREFIX} Failed to detect platform in '${distro}' (exit ${result.exitCode}): ${result.stderr.trim() || result.stdout.trim()}`); @@ -425,7 +491,7 @@ export class WSLRemoteAgentHostMainService extends Disposable implements IWSLRem return resolved; } - private async _openWebSocket(url: string): Promise { + protected async _openWebSocket(url: string): Promise { const nativeRequire = await this._getNativeRequire(); const WS = nativeRequire('ws') as typeof WebSocket; const deadline = Date.now() + WEBSOCKET_OPEN_TIMEOUT_MS; diff --git a/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostService.test.ts b/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostService.test.ts index 723a3e11092a..5d87f7057654 100644 --- a/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostService.test.ts +++ b/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostService.test.ts @@ -25,6 +25,14 @@ import type { StorageValue } from '../../../../base/parts/storage/common/storage import type { Implementation } from '../../common/state/protocol/common/commands.js'; import { agentsWindowAgentHostClientInfo, editorWindowAgentHostClientInfo } from '../../common/agentHostClientInfo.js'; import { PROTOCOL_VERSION } from '../../common/state/protocol/version/registry.js'; +import { computeReconnectDelay } from '../../common/reconnectPolicy.js'; + +interface IRemoteAgentHostServiceTestAccess { + readonly _reconnectAttempts: Map; + readonly _reconnectTimeouts: ReadonlyMap>; + _scheduleReconnect(address: string, connectionToken?: string): void; + _cancelReconnect(address: string): void; +} // ---- Mock transport --------------------------------------------------------- @@ -723,6 +731,76 @@ suite('RemoteAgentHostService', () => { await wait; } + test('preserves automatic reconnect attempts while resetting them for a user reconnect', async () => { + const factory = createFactory(); + const entry = cloudSandboxEntry('Cloud Sandbox', 'cloud:reconnect-budget'); + const automaticClient = new MockProtocolClient('cloud:reconnect-budget'); + const address = getEntryAddress(entry); + const internals = service as unknown as IRemoteAgentHostServiceTestAccess; + const reconnectPolicy = getEntryTypeConfig(RemoteAgentHostEntryType.CloudSandbox).reconnect; + internals._reconnectAttempts.set(address, 3); + + factory.stage(entry, automaticClient); + service.reconnect(address, false); + // An automatic retry never spends the budget it depends on, whether + // it starts the dial or joins one already in flight. + service.reconnect(address, false); + assert.deepStrictEqual({ + automaticAttempts: internals._reconnectAttempts.get(address), + automaticCreates: factory.createdConnectionCount, + }, { + automaticAttempts: 3, + automaticCreates: 1, + }); + + service.reconnect(address, true); + + // The user request joins the in-flight dial rather than starting a + // second one, but still restores the budget so a later failure is + // retried instead of being reported as exhausted. + assert.deepStrictEqual({ + automaticAttempts: internals._reconnectAttempts.get(address), + pendingReconnectCreates: factory.createdConnectionCount, + }, { + automaticAttempts: undefined, + pendingReconnectCreates: 1, + }); + + const automaticWait = service.waitForConnection(address); + await waitForFactoryConnection(factory, 1); + automaticClient.connectDeferred.complete(); + await automaticWait; + + const automaticDelays: number[] = []; + for (let attempt = 1; attempt <= reconnectPolicy.maxAttempts; attempt++) { + internals._scheduleReconnect(address); + automaticDelays.push(computeReconnectDelay(reconnectPolicy, attempt)); + internals._cancelReconnect(address); + } + internals._scheduleReconnect(address); + assert.deepStrictEqual({ + delaysForSuccessiveAutomaticFailures: automaticDelays, + attemptsAtLimit: internals._reconnectAttempts.get(address), + hasRetryAtLimit: internals._reconnectTimeouts.has(address), + }, { + delaysForSuccessiveAutomaticFailures: [1000, 2000, 4000, 8000, 16000, 30000, 30000, 30000, 30000, 30000], + attemptsAtLimit: reconnectPolicy.maxAttempts, + hasRetryAtLimit: false, + }); + + const userClient = new MockProtocolClient('cloud:reconnect-budget'); + internals._reconnectAttempts.set(address, 3); + factory.stage(entry, userClient); + service.reconnect(address, true); + + assert.strictEqual(internals._reconnectAttempts.get(address), undefined); + + const userWait = service.waitForConnection(address); + await waitForFactoryConnection(factory, 2); + userClient.connectDeferred.complete(); + await userWait; + }); + test('keeps an incompatible factory connection addressable for server upgrade', async () => { const factory = createFactory(); const entry = cloudSandboxEntry('Cloud Sandbox', 'cloud:incompatible'); diff --git a/src/vs/platform/agentHost/test/electron-browser/wslRemoteAgentHostService.test.ts b/src/vs/platform/agentHost/test/electron-browser/wslRemoteAgentHostService.test.ts new file mode 100644 index 000000000000..9c9fcbf67f05 --- /dev/null +++ b/src/vs/platform/agentHost/test/electron-browser/wslRemoteAgentHostService.test.ts @@ -0,0 +1,95 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { Event } from '../../../../base/common/event.js'; +import { DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js'; +import type { IChannel } from '../../../../base/parts/ipc/common/ipc.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { IConfigurationService } from '../../../configuration/common/configuration.js'; +import { TestInstantiationService } from '../../../instantiation/test/common/instantiationServiceMock.js'; +import { ISharedProcessService } from '../../../ipc/electron-browser/services.js'; +import { ILogService, NullLogService } from '../../../log/common/log.js'; +import { InMemoryStorageService, IStorageService } from '../../../storage/common/storage.js'; +import { IRemoteAgentHostService, type IRemoteAgentHostConnectionFactory } from '../../common/remoteAgentHostService.js'; +import { IWSLRelayClientFactory, WSLRemoteAgentHostService } from '../../electron-browser/wslRemoteAgentHostServiceImpl.js'; + +class MockWSLMainService { + readonly onDidCloseConnection = Event.None; + readonly onDidReportConnectProgress = Event.None; +} + +class MockRemoteAgentHostService { + readonly reconnectCalls: Array<{ readonly address: string; readonly userInitiated: boolean }> = []; + + registerConnectionFactory(_factory: IRemoteAgentHostConnectionFactory) { + return toDisposable(() => undefined); + } + + reconnect(address: string, userInitiated = true): void { + this.reconnectCalls.push({ address, userInitiated }); + } + + async waitForConnection(_address: string): Promise { + throw new Error('Connection was not established in this forwarding test.'); + } +} + +function asChannel(target: object): IChannel { + return { + call: async (method: string, args?: unknown): Promise => { + const fn = (target as Record)[method]; + if (typeof fn !== 'function') { + throw new Error(`MockChannel: no method ${method}`); + } + return (fn as (...a: unknown[]) => Promise).apply(target, (args as unknown[]) ?? []); + }, + listen: (event: string): Event => { + const value = (target as Record)[event]; + if (typeof value !== 'function') { + throw new Error(`MockChannel: no event ${event}`); + } + return value as Event; + }, + }; +} + +suite('WSLRemoteAgentHostService (renderer)', () => { + const disposables = new DisposableStore(); + let remoteAgentHostService: MockRemoteAgentHostService; + let service: WSLRemoteAgentHostService; + + setup(() => { + const mainService = new MockWSLMainService(); + remoteAgentHostService = new MockRemoteAgentHostService(); + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(ILogService, new NullLogService()); + instantiationService.stub(IConfigurationService, { + getValue: () => true, + } as Partial); + instantiationService.stub(ISharedProcessService, { + getChannel: () => asChannel(mainService), + } as Partial); + instantiationService.stub(IStorageService, disposables.add(new InMemoryStorageService())); + instantiationService.stub(IRemoteAgentHostService, remoteAgentHostService as Partial); + instantiationService.stub(IWSLRelayClientFactory, { + createClient: () => { throw new Error('Unexpected relay client creation.'); }, + } as Partial); + service = disposables.add(instantiationService.createInstance(WSLRemoteAgentHostService)); + }); + + teardown(() => disposables.clear()); + ensureNoDisposablesAreLeakedInTestSuite(); + + test('forwards whether reconnect was user-initiated', async () => { + await assert.rejects(() => service.reconnect('Ubuntu', 'Ubuntu'), /not established/); + await assert.rejects(() => service.reconnect('Ubuntu', 'Ubuntu', false), /not established/); + + assert.deepStrictEqual(remoteAgentHostService.reconnectCalls, [ + { address: 'wsl:Ubuntu', userInitiated: true }, + { address: 'wsl:Ubuntu', userInitiated: false }, + ]); + }); +}); diff --git a/src/vs/platform/agentHost/test/node/sshRemoteAgentHostHelpers.test.ts b/src/vs/platform/agentHost/test/node/sshRemoteAgentHostHelpers.test.ts index 1a0151f77162..bbc9e340b4e8 100644 --- a/src/vs/platform/agentHost/test/node/sshRemoteAgentHostHelpers.test.ts +++ b/src/vs/platform/agentHost/test/node/sshRemoteAgentHostHelpers.test.ts @@ -4,7 +4,9 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { NullLogService } from '../../../log/common/log.js'; import { TelemetryConfiguration } from '../../../telemetry/common/telemetry.js'; import { AGENT_HOST_ENDPOINT_REGISTRY_SCHEMA_VERSION, type IAgentHostEndpointMetadata } from '../../common/agentHostEndpointRegistry.js'; import { @@ -17,6 +19,7 @@ import { buildFindFallbackCLICommand, filterLiveAgentHostEndpoints, findNewAgentHostEndpoint, + getNewAgentHostRegistrationTimeoutMs, getRemoteCLIArchiveName, getRemoteCLIBin, getRemoteCLIDataDir, @@ -33,6 +36,7 @@ import { waitForNewStandaloneEndpoint, type ISshExec, } from '../../node/sshRemoteAgentHostHelpers.js'; +import { ensureRemoteAgentHostCliInstalled } from '../../node/remoteAgentHostCliInstaller.js'; suite('SSH Remote Agent Host Helpers', () => { @@ -668,6 +672,72 @@ suite('SSH Remote Agent Host Helpers', () => { }); }); + suite('ensureRemoteAgentHostCliInstalled', () => { + test('reports whether a CLI was reused or installed', async () => { + const cliBin = getRemoteCLIBin('.vscode-server', 'insider'); + const options = { + serverDataFolderName: '.vscode-server', + quality: 'insider', + commit: undefined, + reportInstalling: () => { }, + logService: new NullLogService(), + }; + const commit = '1234567890abcdef1234567890abcdef12345678'; + const pinnedOptions = { ...options, commit }; + const pinnedCliBin = getRemoteCLIBin('.vscode-server', 'insider', commit); + const reused = await ensureRemoteAgentHostCliInstalled( + async () => ({ stdout: '1.0.0\n__vscode_cli_update_exit_code__:0\n', stderr: '', code: 0 }), + { os: 'linux', arch: 'x64' }, + options, + ); + let calls = 0; + const installed = await ensureRemoteAgentHostCliInstalled( + async () => { + calls++; + return { stdout: '', stderr: '', code: calls === 1 ? 1 : 0 }; + }, + { os: 'linux', arch: 'x64' }, + options, + ); + const reusedPinned = await ensureRemoteAgentHostCliInstalled( + async () => ({ stdout: '', stderr: '', code: 0 }), + { os: 'linux', arch: 'x64' }, + pinnedOptions, + ); + calls = 0; + const installedPinned = await ensureRemoteAgentHostCliInstalled( + async () => { + calls++; + return { stdout: '', stderr: '', code: calls === 1 ? 1 : 0 }; + }, + { os: 'linux', arch: 'x64' }, + pinnedOptions, + ); + + assert.deepStrictEqual( + { + reused, + installed, + reusedPinned, + installedPinned, + registrationTimeouts: { + reused: getNewAgentHostRegistrationTimeoutMs(reused.installed), + installed: getNewAgentHostRegistrationTimeoutMs(installed.installed), + reusedPinned: getNewAgentHostRegistrationTimeoutMs(reusedPinned.installed), + installedPinned: getNewAgentHostRegistrationTimeoutMs(installedPinned.installed), + }, + }, + { + reused: { cliBin, installed: false }, + installed: { cliBin, installed: true }, + reusedPinned: { cliBin: pinnedCliBin, installed: false }, + installedPinned: { cliBin: pinnedCliBin, installed: true }, + registrationTimeouts: { reused: undefined, installed: 300_000, reusedPinned: undefined, installedPinned: 300_000 }, + }, + ); + }); + }); + suite('waitForNewStandaloneEndpoint', () => { test('resolves as soon as the new endpoint appears', async () => { const before = [makeEndpoint({ type: 'standalone', pid: 1, instanceId: 'old' })]; @@ -683,13 +753,48 @@ suite('SSH Remote Agent Host Helpers', () => { assert.ok(poll >= 2); }); - test('throws once the attempt budget is exhausted', async () => { + test('uses the default short deadline when no timeout is supplied', async () => { const before = [makeEndpoint({ type: 'standalone', pid: 1, instanceId: 'old' })]; const exec: ISshExec = async () => ({ stdout: JSON.stringify({ userDataPath: '/x', endpoints: before }), stderr: '', code: 0 }); await assert.rejects( - () => waitForNewStandaloneEndpoint(exec, '~/.vscode-server/code', '~/.vscode-server/cli', '/x', before, { attempts: 2, intervalMs: 1 }), - /Timed out waiting/, + () => waitForNewStandaloneEndpoint(exec, '~/.vscode-server/code', '~/.vscode-server/cli', '/x', before, { intervalMs: 1 }), + /deadline 20ms/, ); }); + + test('keeps polling past the default deadline when given a longer deadline', async () => { + const before = [makeEndpoint({ type: 'standalone', pid: 1, instanceId: 'old' })]; + const spawned = makeEndpoint({ type: 'standalone', pid: 2, instanceId: 'new' }); + let polls = 0; + const exec: ISshExec = async () => { + polls++; + const endpoints = polls <= 20 ? before : [...before, spawned]; + return { stdout: JSON.stringify({ userDataPath: '/x', endpoints }), stderr: '', code: 0 }; + }; + + const result = await waitForNewStandaloneEndpoint(exec, '~/.vscode-server/code', '~/.vscode-server/cli', '/x', before, { intervalMs: 1, timeoutMs: getNewAgentHostRegistrationTimeoutMs(true) }); + assert.deepStrictEqual({ result, polls }, { result: spawned, polls: 21 }); + }); + + test('cancels promptly while waiting for registration', async () => { + const before = [makeEndpoint({ type: 'standalone', pid: 1, instanceId: 'old' })]; + const cancellationSource = new CancellationTokenSource(); + let polls = 0; + const exec: ISshExec = async () => { + polls++; + cancellationSource.cancel(); + return { stdout: JSON.stringify({ userDataPath: '/x', endpoints: before }), stderr: '', code: 0 }; + }; + + try { + await assert.rejects( + () => waitForNewStandaloneEndpoint(exec, '~/.vscode-server/code', '~/.vscode-server/cli', '/x', before, { timeoutMs: 60_000, token: cancellationSource.token }), + /Canceled/, + ); + assert.deepStrictEqual(polls, 1); + } finally { + cancellationSource.dispose(); + } + }); }); }); diff --git a/src/vs/platform/agentHost/test/node/wslRemoteAgentHostHelpers.test.ts b/src/vs/platform/agentHost/test/node/wslRemoteAgentHostHelpers.test.ts index 600eee006a8a..d05ec0926d99 100644 --- a/src/vs/platform/agentHost/test/node/wslRemoteAgentHostHelpers.test.ts +++ b/src/vs/platform/agentHost/test/node/wslRemoteAgentHostHelpers.test.ts @@ -114,7 +114,7 @@ suite('WSL Remote Agent Host Helpers', () => { telemetryLevel: TelemetryConfiguration.OFF, }); - assert.ok(script.endsWith(`exec ~/.vscode-server/code-${commit} --cli-data-dir ~/.vscode-server/cli --telemetry-level off agent host --port 0`)); + assert.ok(script.endsWith(`exec ~/.vscode-server/code-${commit} --cli-data-dir ~/.vscode-server/cli --telemetry-level off agent host --port 0 --idle-timeout 300`)); }); test('exports telemetry disablement for a custom command', () => { diff --git a/src/vs/platform/agentHost/test/node/wslRemoteAgentHostService.test.ts b/src/vs/platform/agentHost/test/node/wslRemoteAgentHostService.test.ts new file mode 100644 index 000000000000..95ea58912ec3 --- /dev/null +++ b/src/vs/platform/agentHost/test/node/wslRemoteAgentHostService.test.ts @@ -0,0 +1,170 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import * as cp from 'child_process'; +import { EventEmitter } from 'events'; +import { DeferredPromise, timeout } from '../../../../base/common/async.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { runWithFakedTimers } from '../../../../base/test/common/timeTravelScheduler.js'; +import { NullLogService } from '../../../log/common/log.js'; +import type { IProductService } from '../../../product/common/productService.js'; +import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js'; +import type { IWSLConnectResult } from '../../common/wslRemoteAgentHost.js'; +import { WSLRemoteAgentHostMainService } from '../../node/wslRemoteAgentHostService.js'; +import type WebSocket from 'ws'; + +class MockWSLChild extends EventEmitter { + readonly stdout = new EventEmitter(); + readonly stderr = new EventEmitter(); + + exitCode: number | null = null; + signalCode: NodeJS.Signals | null = null; + killCalls = 0; + + kill(_signal?: NodeJS.Signals): boolean { + this.killCalls++; + if (this.exitCode === null && this.signalCode === null) { + this.signalCode = 'SIGTERM'; + queueMicrotask(() => this.emit('exit', null, 'SIGTERM')); + } + return true; + } + + emitStdout(text: string): void { + this.stdout.emit('data', Buffer.from(text)); + } +} + +class MockWebSocket { + on(_event: string, _listener: (...args: never[]) => void): this { + return this; + } + + close(): void { + } +} + +/** + * In-process WSL service double that controls platform detection, process + * output, and WebSocket creation without spawning WSL or loading `ws`. + */ +class TestableWSLRemoteAgentHostMainService extends WSLRemoteAgentHostMainService { + readonly children: MockWSLChild[] = []; + + private readonly _platform = new DeferredPromise<{ os: string; arch: string }>(); + + resolvePlatform(): void { + this._platform.complete({ os: 'linux', arch: 'x64' }); + } + + protected override _spawnAgentHost(_distro: string, _script: string): cp.ChildProcess { + const child = new MockWSLChild(); + this.children.push(child); + return child as unknown as cp.ChildProcess; + } + + protected override _resolvePlatform(_distro: string): Promise<{ os: string; arch: string }> { + return this._platform.p; + } + + protected override async _openWebSocket(_url: string): Promise { + return new MockWebSocket() as never; + } +} + +function createService(): TestableWSLRemoteAgentHostMainService { + const productService: Pick = { + _serviceBrand: undefined, + quality: 'insider', + serverDataFolderName: '.vscode-server', + commit: 'a'.repeat(40), + }; + return new TestableWSLRemoteAgentHostMainService( + new NullLogService(), + productService as IProductService, + NullTelemetryService, + ); +} + +suite('WSL Remote Agent Host Service', () => { + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + test('deduplicates simultaneous connects to one distro', async () => { + const service = disposables.add(createService()); + const first = service.connect({ distro: 'Ubuntu', name: 'Ubuntu' }); + const second = service.connect({ distro: 'Ubuntu', name: 'Ubuntu' }); + + assert.strictEqual(first, second); + + service.resolvePlatform(); + await Promise.resolve(); + service.children[0].emitStdout('ws://127.0.0.1:3000?tkn=token\n'); + const [firstResult, secondResult] = await Promise.all([first, second]); + + assert.deepStrictEqual( + { spawnCount: service.children.length, sameResult: firstResult === secondResult, results: [firstResult, secondResult] }, + { + spawnCount: 1, + sameResult: true, + results: [ + { + connectionId: firstResult.connectionId, + address: 'wsl:Ubuntu', + distro: 'Ubuntu', + name: 'Ubuntu', + connectionToken: 'token', + }, + { + connectionId: firstResult.connectionId, + address: 'wsl:Ubuntu', + distro: 'Ubuntu', + name: 'Ubuntu', + connectionToken: 'token', + }, + ], + }, + ); + }); + + test('keeps a chatty bootstrap alive past the output-idle timeout', async () => { + return runWithFakedTimers({ useFakeTimers: true, maxTaskCount: 10_000 }, async () => { + const service = disposables.add(createService()); + const connect = service.connect({ distro: 'Ubuntu', name: 'Ubuntu' }); + service.resolvePlatform(); + await Promise.resolve(); + + const child = service.children[0]; + await timeout(59_000); + child.emitStdout('Downloading server 50%\n'); + await timeout(59_000); + child.emitStdout('ws://127.0.0.1:3000?tkn=token\n'); + + const result = await connect; + assert.deepStrictEqual( + { distro: result.distro, address: result.address, connectionToken: result.connectionToken }, + { distro: 'Ubuntu', address: 'wsl:Ubuntu', connectionToken: 'token' }, + ); + }); + }); + + test('fails a silent bootstrap after the output-idle timeout', async () => { + return runWithFakedTimers({ useFakeTimers: true, maxTaskCount: 10_000 }, async () => { + const service = disposables.add(createService()); + const rejected = service.connect({ distro: 'Ubuntu', name: 'Ubuntu' }).then( + result => result, + error => error instanceof Error ? error : new Error(String(error)), + ); + service.resolvePlatform(); + await Promise.resolve(); + + await timeout(60_001); + const result = await rejected; + + assert.ok(result instanceof Error); + assert.match(result.message, /no output for 60000ms/); + }); + }); +}); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts index 0d5110d609ba..977f5f21bb92 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts @@ -6,7 +6,7 @@ import { Event } from '../../../../../base/common/event.js'; import { Disposable, DisposableMap, DisposableStore, MutableDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; import { disposableTimeout, IntervalTimer } from '../../../../../base/common/async.js'; -import { isCancellationError } from '../../../../../base/common/errors.js'; +import { isCancellationError, onUnexpectedError } from '../../../../../base/common/errors.js'; import { StopWatch } from '../../../../../base/common/stopwatch.js'; import { URI } from '../../../../../base/common/uri.js'; import * as nls from '../../../../../nls.js'; @@ -228,6 +228,20 @@ class ConnectionState extends Disposable { } } +/** + * Entry types whose sessions provider is created by + * {@link RemoteAgentHostContribution}. Every other kind has a dedicated + * contribution that owns its provider — tunnels, WSL, cloud sandbox and dev + * containers each register their own. Since connection factories publish all + * kinds into `configuredEntries`, this contribution would otherwise try to + * register a second provider for an address another contribution already + * owns, which throws and aborts the rest of the reconcile. + */ +const SHARED_SESSIONS_PROVIDER_ENTRY_TYPES: ReadonlySet = new Set([ + RemoteAgentHostEntryType.WebSocket, + RemoteAgentHostEntryType.SSH, +]); + /** * Discovers available agents from each connected remote agent host and * dynamically registers each one as a chat session type with its own @@ -316,7 +330,18 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc } private _reconcile(): void { - this._reconcileProviders(); + // Provider registration and connection wiring are independent + // responsibilities. Keep them isolated: a provider that fails to + // register must not stop connections from being wired, because + // `_reconcileConnections` is what registers the filesystem authority + // and subscribes to root state (agent and model discovery). Losing + // that silently leaves a host that looks connected but can neither + // read files nor report any models. + try { + this._reconcileProviders(); + } catch (err) { + onUnexpectedError(err); + } this._reconcileConnections(); // Ensure every live connection is wired to its provider. This covers @@ -352,7 +377,9 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc private _reconcileProviders(): void { const enabled = this._configurationService.getValue(RemoteAgentHostsEnabledSettingId); - const entries = enabled ? this._remoteAgentHostService.configuredEntries : []; + const entries = enabled + ? this._remoteAgentHostService.configuredEntries.filter(entry => SHARED_SESSIONS_PROVIDER_ENTRY_TYPES.has(entry.connection.type)) + : []; const desiredAddresses = new Set(entries.map(e => getEntryAddress(e))); // Remove providers no longer configured diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/wslAgentHost.contribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/wslAgentHost.contribution.ts index 1eaaf6fdd649..adff80f49395 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/wslAgentHost.contribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/wslAgentHost.contribution.ts @@ -129,18 +129,18 @@ export class WSLAgentHostContribution extends ManagedReconnectAgentHostContribut } } this._reconnectStates.get(distro)?.resetForResume(); - await this._attemptWSLReconnect(distro, name, address); + await this._attemptWSLReconnect(distro, name, address, true); } - private async _attemptWSLReconnect(distro: string, name: string, address: string): Promise { + private async _attemptWSLReconnect(distro: string, name: string, address: string, userInitiated: boolean): Promise { await this._attemptManagedReconnect({ kind: 'WSL', key: distro, address, - userInitiated: true, + userInitiated, reconnectPolicy: getEntryTypeConfig(RemoteAgentHostEntryType.WSL).reconnect, shouldPause: shouldPauseWSLReconnectAfterFailure, - doConnect: () => this._wslService.reconnect(distro, name).then(() => undefined), + doConnect: () => this._wslService.reconnect(distro, name, userInitiated).then(() => undefined), }); } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHost.contribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHost.contribution.test.ts index 3d41b7dafb3a..a8ee6c6dc43c 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHost.contribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHost.contribution.test.ts @@ -11,7 +11,7 @@ import { runWithFakedTimers } from '../../../../../../base/test/common/timeTrave import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { type IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; -import { IRemoteAgentHostSSHConnection, RemoteAgentHostEntryType } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; +import { IRemoteAgentHostEntry, IRemoteAgentHostSSHConnection, RemoteAgentHostEntryType, getEntryAddress } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { SSHHostKeyDeniedError } from '../../../../../../platform/agentHost/common/sshRemoteAgentHost.js'; import { AuthRequiredReason, NotificationType, type INotification } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import { type ProtectedResourceMetadata } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; @@ -441,3 +441,46 @@ suite('sshConnectionKey', () => { }); }); }); + +interface IReconcileProvidersHarness { + _configurationService: { getValue(key: string): boolean }; + _remoteAgentHostService: { readonly configuredEntries: readonly IRemoteAgentHostEntry[] }; + _providerStores: Map & { deleteAndDispose(address: string): void }; + _providerInstances: Map; + _createProvider(entry: IRemoteAgentHostEntry): void; + _reconcileProviders(): void; +} + +suite('RemoteAgentHostContribution provider ownership', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('only creates providers for the entry types it owns', () => { + const entries: IRemoteAgentHostEntry[] = [ + { name: 'Tunnel', connection: { type: RemoteAgentHostEntryType.Tunnel, tunnelId: 'my-tunnel', clusterId: 'usw2' } }, + { name: 'WSL', connection: { type: RemoteAgentHostEntryType.WSL, address: 'wsl:Ubuntu-24.04', distro: 'Ubuntu-24.04' } }, + { name: 'Sandbox', connection: { type: RemoteAgentHostEntryType.CloudSandbox, address: 'cloudsandbox:abc', environmentId: 'abc' } }, + { name: 'Dev Container', connection: { type: RemoteAgentHostEntryType.DevContainer, address: 'devcontainer:abc', hostPath: '/repo' } }, + { name: 'Socket', connection: { type: RemoteAgentHostEntryType.WebSocket, address: 'ws://host:8080' } }, + { name: 'Remote', connection: { type: RemoteAgentHostEntryType.SSH, address: 'localhost:4321', sshConfigHost: 'myserver', hostName: 'myserver' } }, + ]; + + const created: string[] = []; + const contribution = Object.create(RemoteAgentHostContribution.prototype) as IReconcileProvidersHarness; + contribution._configurationService = { getValue: () => true }; + contribution._remoteAgentHostService = { configuredEntries: entries }; + const providerStores = new Map(); + contribution._providerStores = Object.assign(providerStores, { + deleteAndDispose: (address: string) => { providerStores.delete(address); }, + }); + contribution._providerInstances = new Map(); + // Tunnels, WSL, cloud sandbox and dev containers each register their own + // sessions provider. Creating a second one here throws out of the + // reconcile and skips the connection wiring that registers the + // filesystem authority and discovers models. + contribution._createProvider = entry => { created.push(getEntryAddress(entry)); }; + + contribution._reconcileProviders(); + + assert.deepStrictEqual(created, ['ws://host:8080', 'localhost:4321']); + }); +});