diff --git a/src/vs/platform/agentHost/test/node/e2e/README.md b/src/vs/platform/agentHost/test/node/e2e/README.md index e82eccd184b635..c544af5b40780c 100644 --- a/src/vs/platform/agentHost/test/node/e2e/README.md +++ b/src/vs/platform/agentHost/test/node/e2e/README.md @@ -226,6 +226,8 @@ Each test needs an agent host server (a forked subprocess) fronted by a `CapiRep The lease also owns a fresh suite data directory. Every server it starts uses that directory as its home and VS Code user-data directory and prevents provider-specific config overrides from escaping it, so both shared and provider-specific scenarios are isolated from developer-machine configuration. +On Windows, test-server cleanup records descendants before requesting graceful shutdown and terminates any survivors after the server exits, before temporary directories are removed. Recording descendants and waiting for graceful exit share the existing shutdown deadline. + - **Per-test** (always while recording) — fork a fresh server + proxy for every test and kill it in teardown. Full isolation: nothing carries over between tests. The cost is that every test re-pays the server fork **and** the provider SDK/CLI cold start (`_ensureClient` spawns and caches the CLI subprocess per server). - **Shared** (the default in replay, for every provider) — reuse a server + proxy across tests, swapping the per-test fixture and reconnecting a fresh client. The lease recycles after 25 model-backed tests or 40 total tests, whichever comes first. The model cap bounds provider-process load; the total cap bounds host-owned terminals, watchers, subscriptions, and other resource accumulation in host-only suites. diff --git a/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.test.ts b/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.test.ts new file mode 100644 index 00000000000000..b63cc2e3bc7e52 --- /dev/null +++ b/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.test.ts @@ -0,0 +1,122 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { spawn } from 'child_process'; +import { once } from 'events'; +import { mkdtemp, rm } from 'fs/promises'; +import { tmpdir } from 'os'; +import { DeferredPromise, Promises, raceTimeout } from '../../../../base/common/async.js'; +import { getErrorCode } from '../../../../base/common/errors.js'; +import { join } from '../../../../base/common/path.js'; +import { isWindows } from '../../../../base/common/platform.js'; +import { killTree } from '../../../../base/node/processes.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { killServer, stopServer } from './serverIntegrationTestHelpers.js'; + +suite('Agent Host test server cleanup', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('a stalled descendant snapshot still sends EOF and reaches forced shutdown', async function () { + this.timeout(15_000); + const server = spawn(process.execPath, ['-e', ` + process.stdin.resume(); + process.stdout.write('ready'); + setTimeout(() => process.exit(99), 30000); + `], { + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }); + const snapshot = new DeferredPromise(); + let stopped: Promise | undefined; + try { + assert.ok(await raceTimeout(once(server.stdout, 'data'), 5_000), 'Server did not start'); + stopped = stopServer({ process: server, port: 0 }, () => snapshot.p, 0).then( + () => undefined, + (error: Error) => error, + ); + const error = await raceTimeout(stopped, 5_000); + assert.deepStrictEqual({ + message: error?.message, + cause: error?.cause instanceof Error ? error.cause.message : undefined, + eof: server.stdin.writableEnded, + exited: server.exitCode !== null || server.signalCode !== null, + }, { + message: 'Failed to capture Agent Host test server descendants', + cause: 'Timed out capturing Agent Host test server descendants', + eof: true, + exited: true, + }); + } finally { + snapshot.complete([]); + await stopped; + await killServer({ process: server, port: 0 }); + } + }); + + (isWindows ? test : test.skip)('stops owned descendants after the server exits gracefully', async function () { + this.timeout(30_000); + const directory = await mkdtemp(join(tmpdir(), 'vscode-test-server-cleanup-')); + const descendantCode = ` + require('fs').writeFileSync('owned.txt', String(process.pid)); + process.send(process.pid); + process.disconnect(); + setTimeout(() => process.exit(99), 30000); + `; + const server = spawn(process.execPath, ['-e', ` + const { spawn } = require('child_process'); + const holder = spawn(process.execPath, ['-e', ${JSON.stringify(descendantCode)}], { + detached: true, + windowsHide: true, + stdio: ['ignore', 'ignore', 'ignore', 'ipc'], + env: process.env, + }); + holder.once('message', pid => { + holder.unref(); + process.send(pid); + }); + process.stdin.resume(); + process.stdin.once('end', () => process.exit(0)); + setTimeout(() => process.exit(99), 30000); + `], { + cwd: directory, + env: { ...process.env, ELECTRON_RUN_AS_NODE: '1' }, + stdio: ['pipe', 'pipe', 'pipe', 'ipc'], + windowsHide: true, + }); + let stderr = ''; + server.stderr?.on('data', chunk => stderr += chunk.toString()); + let descendantPid: number | undefined; + try { + const ready = await raceTimeout(once(server, 'message'), 5_000); + assert.ok(ready, `Descendant did not start: ${stderr}`); + const message: unknown = ready[0]; + assert.ok(typeof message === 'number'); + descendantPid = message; + await stopServer({ process: server, port: 0 }); + + assert.strictEqual(server.exitCode, 0); + assert.throws(() => process.kill(message, 0), { code: 'ESRCH' }); + await rm(directory, { recursive: true }); + } finally { + await Promises.settled([server.pid, descendantPid].map(async pid => { + if (pid === undefined) { + return; + } + try { + process.kill(pid, 0); + } catch (error) { + if (getErrorCode(error) === 'ESRCH') { + return; + } + throw error; + } + await killTree(pid, true); + })); + await rm(directory, { recursive: true, force: true }); + } + }); +}); diff --git a/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.ts b/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.ts index 44a2fc6bb5283f..6d87b27fbff51a 100644 --- a/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.ts +++ b/src/vs/platform/agentHost/test/node/serverIntegrationTestHelpers.ts @@ -5,11 +5,13 @@ import { ChildProcess, fork } from 'child_process'; import { cp, lstat, mkdir, readFile, readdir, realpath, rename, rm, stat, writeFile } from 'fs/promises'; -import { raceTimeout } from '../../../../base/common/async.js'; +import { Promises, raceTimeout } from '../../../../base/common/async.js'; +import { getErrorCode } from '../../../../base/common/errors.js'; import { Schemas } from '../../../../base/common/network.js'; import { createRequire } from 'module'; import { mkdirSync } from 'fs'; import { userInfo } from 'os'; +import { promisify } from 'util'; import { fileURLToPath } from 'url'; import { WebSocket } from 'ws'; import { CapiReplayProxy, type CapiReplayMode, type ICapiReplayResponse } from './e2e/harness/capiReplayProxy.js'; @@ -662,13 +664,23 @@ export interface IServerHandle { const SERVER_SHUTDOWN_TIMEOUT_MS = isCI || isWindows || AGENT_HOST_E2E_COVERAGE ? 30_000 : 5_000; +async function getServerDescendants(pid: number): Promise { + if (!isWindows) { + return []; + } + // Once the parent exits, taskkill /T can no longer discover its descendants. + const { getProcessList } = await import('@vscode/windows-process-tree'); + return (await promisify(getProcessList)(pid)).filter(process => process.pid !== pid).map(process => process.pid); +} + /** Gracefully stop an Agent Host test server, killing it if shutdown stalls. */ -export async function stopServer(server: IServerHandle | undefined): Promise { +export async function stopServer(server: IServerHandle | undefined, getDescendants = getServerDescendants, timeoutMs = SERVER_SHUTDOWN_TIMEOUT_MS): Promise { const serverProcess = server?.process; if (!serverProcess || serverProcess.exitCode !== null || serverProcess.signalCode !== null) { return; } + const deadline = Date.now() + timeoutMs; const serverExit = new Promise(resolve => { const onExit = () => resolve(); serverProcess.once('exit', onExit); @@ -677,8 +689,21 @@ export async function stopServer(server: IServerHandle | undefined): Promise true), SERVER_SHUTDOWN_TIMEOUT_MS)) { + if (!await raceTimeout(serverExit.then(() => true), Math.max(0, deadline - Date.now()))) { try { if (serverProcess.exitCode === null && serverProcess.signalCode === null) { const pid = serverProcess.pid; @@ -694,6 +719,25 @@ export async function stopServer(server: IServerHandle | undefined): Promise { + try { + await killTree(pid, true); + } catch (error) { + try { + process.kill(pid, 0); + } catch (probeError) { + if (getErrorCode(probeError) === 'ESRCH') { + return; // The descendant already exited during graceful shutdown. + } + throw probeError; + } + throw error; + } + })); } /** Forcefully kill an Agent Host test server and its child processes without graceful shutdown. */