From ae46882ca5140911d9e740ee766d2458a19ac60f Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 13:53:09 -0400 Subject: [PATCH 01/24] Stop the Aspire CLI and own RPC connections during deactivation Extension deactivation was fire-and-forget: `deactivate()` returned void, so VS Code never waited for the CLI stop requests it triggered, and connections whose debug-session handshake was still pending were never owned by the RPC server. A window close could therefore leave `aspire run` processes alive and leave progress indicators on screen with nothing left to clear them. - `deactivate()` now returns a promise and awaits `AspireExtensionContext.deactivate()`, which asks every live debug session to stop its CLI (deduplicating in-flight requests) with a bounded 5s timeout before disposing the rest of the extension. - `AspireRpcServer` tracks the connections it creates, including ones still inside the handshake, and disposes them on server disposal. - `RpcClient.dispose()` is idempotent and closes the transport. - `InteractionService` is disposable and latches disposal so a status message still in flight when the transport closed cannot resurrect progress. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/AspireExtensionContext.ts | 102 ++++++++- extension/src/debugger/AspireDebugSession.ts | 12 +- extension/src/extension.ts | 4 +- extension/src/server/AspireRpcServer.ts | 82 ++++++- extension/src/server/interactionService.ts | 18 +- extension/src/server/rpcClient.ts | 26 ++- .../src/test/AspireExtensionContext.test.ts | 216 ++++++++++++++++++ extension/src/test/aspireDebugSession.test.ts | 23 ++ .../src/test/rpc/aspireRpcServer.test.ts | 199 ++++++++++++++++ .../test/rpc/interactionServiceTests.test.ts | 43 +++- 10 files changed, 701 insertions(+), 24 deletions(-) create mode 100644 extension/src/test/AspireExtensionContext.test.ts create mode 100644 extension/src/test/rpc/aspireRpcServer.test.ts diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index 60eb4c61a63..1312b4ccd46 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import { ErrorCodes, ResponseError } from 'vscode-jsonrpc'; import { AspireDebugSession } from './debugger/AspireDebugSession'; import { AspireDebugConfigurationProvider } from './debugger/AspireDebugConfigurationProvider'; import { debugSessionAlreadyExists, extensionContextNotInitialized } from './loc/strings'; @@ -7,8 +8,11 @@ import AspireDcpServer from './dcp/AspireDcpServer'; import { AspireTerminalProvider } from './utils/AspireTerminalProvider'; import { AspireEditorCommandProvider } from './editor/AspireEditorCommandProvider'; import type { AspireDebugConsoleOutputEvent } from './types/extensionApi'; +import { extensionLogOutputChannel } from './utils/logging'; export class AspireExtensionContext implements vscode.Disposable { + private static readonly _cliStopTimeoutMs = 5_000; + private _rpcServer?: AspireRpcServer; private _dcpServer?: AspireDcpServer; private _extensionContext?: vscode.ExtensionContext; @@ -21,6 +25,9 @@ export class AspireExtensionContext implements vscode.Disposable { private readonly _debugSessionOutputSubscriptions = new Map(); private readonly _onDidChangeDebugSessions = new vscode.EventEmitter(); private readonly _onDidReceiveDebugConsoleOutput = new vscode.EventEmitter(); + private _shutdownPromise?: Promise; + private _isShuttingDown = false; + private _isDisposed = false; readonly onDidChangeDebugSessions = this._onDidChangeDebugSessions.event; readonly onDidReceiveDebugConsoleOutput = this._onDidReceiveDebugConsoleOutput.event; @@ -94,14 +101,101 @@ export class AspireExtensionContext implements vscode.Disposable { return this._debugConfigProvider; } - dispose() { - this._rpcServer?.dispose(); - this._dcpServer?.dispose(); + deactivate(): Promise { + if (this._isDisposed) { + return Promise.resolve(); + } + + if (this._shutdownPromise) { + return this._shutdownPromise; + } + + this._isShuttingDown = true; + // Schedule the async work after storing the shared promise so a reentrant dispose/deactivate + // call cannot begin synchronous teardown between the stop request and the first await. + this._shutdownPromise = Promise.resolve().then(() => this._deactivateCore()); + return this._shutdownPromise; + } + + dispose(): void { + if (this._isDisposed || this._isShuttingDown) { + return; + } + + this._disposeCore(); + } + + private async _deactivateCore(): Promise { + try { + await this._waitForCliStopRequests(); + } + finally { + // A timeout or failed RPC stop still has to run the established debug-session and + // terminal teardown path so extension deactivation cannot leave the CLI process alive. + this._disposeCore(); + } + } + + private async _waitForCliStopRequests(): Promise { + const stopRequests = this._aspireDebugSessions.map(session => { + try { + return session.requestCliStopForExtensionShutdown(); + } + catch (error) { + return Promise.reject(error); + } + }); + + const allStops = Promise.allSettled(stopRequests); + let timeout: ReturnType | undefined; + const outcome = await Promise.race([ + allStops.then(results => ({ timedOut: false as const, results })), + new Promise<{ timedOut: true }>(resolve => { + timeout = setTimeout(() => { + timeout = undefined; + resolve({ timedOut: true }); + }, AspireExtensionContext._cliStopTimeoutMs); + }), + ]); + + if (timeout) { + clearTimeout(timeout); + } + + if (outcome.timedOut) { + extensionLogOutputChannel.warn(`Timed out after ${AspireExtensionContext._cliStopTimeoutMs}ms waiting for Aspire CLI stop requests; continuing extension teardown.`); + return; + } + + const failures = outcome.results + .filter((result): result is PromiseRejectedResult => result.status === 'rejected') + .map(result => result.reason); + for (const failure of failures) { + // Closing the RPC transport rejects its outstanding stop request even though the + // synchronous debug-session and terminal teardown below has completed successfully. + if (failure instanceof ResponseError && failure.code === ErrorCodes.PendingResponseRejected) { + extensionLogOutputChannel.info(`Aspire CLI stop request ended after the RPC transport closed: ${failure}`); + } + else { + extensionLogOutputChannel.warn(`Failed to stop Aspire CLI during extension deactivation: ${failure}`); + } + } + } + + private _disposeCore(): void { + if (this._isDisposed) { + return; + } + + this._isDisposed = true; this._debugSessionStateSubscriptions.forEach(disposable => disposable.dispose()); this._debugSessionStateSubscriptions.clear(); this._debugSessionOutputSubscriptions.forEach(disposable => disposable.dispose()); this._debugSessionOutputSubscriptions.clear(); - this._aspireDebugSessions.forEach(session => session.dispose()); + const sessions = this._aspireDebugSessions.splice(0); + sessions.forEach(session => session.dispose()); + this._rpcServer?.dispose(); + this._dcpServer?.dispose(); this._terminalProvider?.dispose(); this._editorCommandProvider?.dispose(); this._onDidChangeDebugSessions.dispose(); diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index b7798c2f664..7f70ffe6b07 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -74,6 +74,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { private readonly _disposables: vscode.Disposable[] = []; private _disposed = false; private _parentStopPromise: Thenable | undefined; + private _cliStopPromise: Promise | undefined; // Timestamp for the `debug/apphost/end` duration measurement. Captured the first // time we observe a `launch` request so it covers the actual user-visible session // lifetime, not the moment the AspireDebugSession object was constructed. @@ -135,6 +136,15 @@ export class AspireDebugSession implements vscode.DebugAdapter { } } + requestCliStopForExtensionShutdown(): Promise { + if (!this._rpcClient) { + return Promise.resolve(); + } + + this._cliStopPromise ??= this._rpcClient.stopCli(); + return this._cliStopPromise; + } + private stopParentDebugSessionOnce(): Thenable { if (this._parentStopPromise) { return this._parentStopPromise; @@ -435,7 +445,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { this._disposables.push({ dispose: () => { - this._rpcClient?.stopCli().catch((err) => { + void this.requestCliStopForExtensionShutdown().catch((err) => { extensionLogOutputChannel.info(`stopCli failed (connection may already be closed): ${err}`); }); extensionLogOutputChannel.info(`Requested Aspire CLI exit with args: ${args.join(' ')}`); diff --git a/extension/src/extension.ts b/extension/src/extension.ts index b4c62127d61..bfa3b23395b 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -423,8 +423,8 @@ export async function activate(context: vscode.ExtensionContext) { return Object.freeze(api); } -export function deactivate() { - aspireExtensionContext.dispose(); +export function deactivate(): Promise { + return aspireExtensionContext.deactivate(); } function getExtensionModeForTelemetry(mode: vscode.ExtensionMode): string { diff --git a/extension/src/server/AspireRpcServer.ts b/extension/src/server/AspireRpcServer.ts index ae7481c63e3..72107197643 100644 --- a/extension/src/server/AspireRpcServer.ts +++ b/extension/src/server/AspireRpcServer.ts @@ -1,5 +1,5 @@ import * as vscode from 'vscode'; -import { createMessageConnection, MessageConnection } from 'vscode-jsonrpc'; +import { createMessageConnection, ErrorCodes, MessageConnection, ResponseError } from 'vscode-jsonrpc'; import { StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc/node'; import { invalidTokenProvided, rpcServerAddressError, rpcServerError } from '../loc/strings'; import { addInteractionServiceEndpoints, IInteractionService } from './interactionService'; @@ -21,6 +21,8 @@ export default class AspireRpcServer { public connectionInfo: RpcServerConnectionInfo; public connections: ICliRpcClient[] = []; + private readonly _ownedConnections = new Set(); + private _disposed = false; private _onNewConnection = new vscode.EventEmitter(); public readonly onNewConnection = this._onNewConnection.event; @@ -33,24 +35,61 @@ export default class AspireRpcServer { return this.connections.find(connection => connection.debugSessionId === debugSessionId) || null; } - public addConnection(connection: ICliRpcClient) { + public addConnection(connection: ICliRpcClient): boolean { + if (this._disposed) { + connection.dispose(); + return false; + } + + this._ownedConnections.add(connection); + if (this.connections.includes(connection)) { + return true; + } + this.connections.push(connection); this._onNewConnection.fire(connection); + return true; } public removeConnection(connection: ICliRpcClient) { + this._ownedConnections.delete(connection); const index = this.connections.indexOf(connection); if (index !== -1) { this.connections.splice(index, 1); } + + connection.dispose(); } public dispose() { + if (this._disposed) { + return; + } + + this._disposed = true; extensionLogOutputChannel.info(`Disposing RPC server`); + // A client is owned before its debug-session handshake starts. That ensures a stalled + // handshake cannot outlive server teardown with its transport and UI state still active. + for (const connection of this._ownedConnections) { + connection.dispose(); + } + + this._ownedConnections.clear(); + this.connections.splice(0); this._onNewConnection.dispose(); this.server.close(); } + private _ownConnection(connection: ICliRpcClient): boolean { + if (this._disposed) { + connection.dispose(); + return false; + } + + this._ownedConnections.add(connection); + return true; + } + static async create(rpcClientFactory: (rpcServerConnectionInfo: RpcServerConnectionInfo, connection: MessageConnection, token: string, debugSessionId: string | null) => ICliRpcClient): Promise { const token = generateToken(); const { key, cert } = await createSelfSignedCertAsync(); @@ -113,17 +152,36 @@ export default class AspireRpcServer { // to avoid a race condition where the CLI sends requests (e.g. displayEmptyLine) // before handlers are registered. const rpcClient = rpcClientFactory(connectionInfo, connection, token, null); - addInteractionServiceEndpoints(connection, rpcClient.interactionService, rpcClient, withAuthentication); - - connection.listen(); - - const clientDebugSessionId = await connection.sendRequest('getDebugSessionId'); - rpcClient.debugSessionId = clientDebugSessionId; - - rpcServer.addConnection(rpcClient); - connection.onClose(() => rpcServer.removeConnection(rpcClient)); - + if (!rpcServer._ownConnection(rpcClient)) { + return; + } + + try { + addInteractionServiceEndpoints(connection, rpcClient.interactionService, rpcClient, withAuthentication); + connection.listen(); + + const clientDebugSessionId = await connection.sendRequest('getDebugSessionId'); + rpcClient.debugSessionId = clientDebugSessionId; + rpcServer.addConnection(rpcClient); + } + catch (error) { + // MessageConnection disposal rejects its outstanding request with + // PendingResponseRejected during normal CLI exit. The ownership check + // keeps the same response from a still-live client visible as a warning. + const transportDisposedDuringHandshake = + error instanceof ResponseError && + error.code === ErrorCodes.PendingResponseRejected && + !rpcServer._ownedConnections.has(rpcClient); + if (transportDisposedDuringHandshake) { + extensionLogOutputChannel.info(`RPC client transport closed during initialization: ${error}`); + } + else { + extensionLogOutputChannel.warn(`Failed to initialize RPC client: ${error}`); + } + + rpcServer.removeConnection(rpcClient); + } }); resolve(rpcServer); diff --git a/extension/src/server/interactionService.ts b/extension/src/server/interactionService.ts index 1a7a26c95d6..5280c610ccc 100644 --- a/extension/src/server/interactionService.ts +++ b/extension/src/server/interactionService.ts @@ -15,7 +15,7 @@ import { isDirectory } from '../utils/io'; import { sendTelemetryEvent } from '../utils/telemetry'; import { dashboardDefaultChangedNotificationKey } from '../utils/dashboardNotificationState'; -export interface IInteractionService { +export interface IInteractionService extends vscode.Disposable { showStatus: (statusText: string | null) => void; clearProgressNotification: () => void; promptForString: (promptText: string, defaultValue: string | null, required: boolean, rpcClient: ICliRpcClient) => Promise; @@ -167,6 +167,7 @@ export class InteractionService implements IInteractionService { private _rpcClient?: ICliRpcClient; private _progressNotifier: ProgressNotifier; + private _isDisposed = false; constructor(getAspireDebugSession: () => AspireDebugSession | null, rpcClient: ICliRpcClient, private readonly _globalState?: vscode.Memento) { this._getAspireDebugSession = getAspireDebugSession; @@ -175,6 +176,13 @@ export class InteractionService implements IInteractionService { } showStatus(statusText: string | null) { + if (this._isDisposed) { + // The RPC connection owning this service is gone. A status message that was still in + // flight when the transport closed must not paint progress that nothing is left alive + // to clear, which would strand the indicator for the rest of the window's lifetime. + return; + } + delayStatusForE2E(); this._progressNotifier.show(statusText); } @@ -687,6 +695,14 @@ export class InteractionService implements IInteractionService { this._progressNotifier.clear(); } + dispose() { + // The RPC connection owning this service is going away, so tear down any progress it + // still has on screen. Otherwise a CLI that dies with the extension leaves a permanent + // "Building..." indicator that nothing is left alive to clear. + this._isDisposed = true; + this._progressNotifier.clear(); + } + /** * Closes the dashboard browser. Delegates to the current AspireDebugSession. */ diff --git a/extension/src/server/rpcClient.ts b/extension/src/server/rpcClient.ts index 9d8fa33c71f..e9edd6f0877 100644 --- a/extension/src/server/rpcClient.ts +++ b/extension/src/server/rpcClient.ts @@ -4,7 +4,7 @@ import { extensionLogOutputChannel, logAsyncOperation } from '../utils/logging'; import { IInteractionService, InteractionService } from './interactionService'; import { AspireDebugSession } from '../debugger/AspireDebugSession'; -export interface ICliRpcClient { +export interface ICliRpcClient extends vscode.Disposable { debugSessionId: string | null; interactionService: IInteractionService; getCliVersion(): Promise; @@ -21,6 +21,7 @@ export type ValidationResult = { export class RpcClient implements ICliRpcClient { private _messageConnection: MessageConnection; private _connectionClosed: boolean; + private _disposed = false; public debugSessionId: string | null; public interactionService: IInteractionService; @@ -32,12 +33,31 @@ export class RpcClient implements ICliRpcClient { this.interactionService = new InteractionService(getAspireDebugSession, this, globalState); this._messageConnection.onClose(() => { - this._connectionClosed = true; - this.interactionService.clearProgressNotification(); extensionLogOutputChannel.info('JSON-RPC connection closed'); + this.dispose(); }); } + dispose() { + if (this._disposed) { + return; + } + + this._disposed = true; + this._connectionClosed = true; + try { + this.interactionService.dispose(); + } + finally { + try { + this._messageConnection.end(); + } + finally { + this._messageConnection.dispose(); + } + } + } + getCliVersion(): Promise { return logAsyncOperation( `Requesting CLI version from CLI`, diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts new file mode 100644 index 00000000000..78027c4d3e4 --- /dev/null +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -0,0 +1,216 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import { ErrorCodes, ResponseError } from 'vscode-jsonrpc'; +import { AspireExtensionContext } from '../AspireExtensionContext'; +import { AspireDebugSession } from '../debugger/AspireDebugSession'; +import { extensionLogOutputChannel } from '../utils/logging'; + +suite('AspireExtensionContext', () => { + test('deactivation waits for every CLI stop request before disposing transport', async () => { + const order: string[] = []; + const context = createContext(order); + const firstStop = createDeferred(); + const secondStop = createDeferred(); + addSession(context, 'first', () => { + order.push('stop first'); + return firstStop.promise; + }, () => order.push('dispose first')); + addSession(context, 'second', () => { + order.push('stop second'); + return secondStop.promise; + }, () => order.push('dispose second')); + + const shutdown = deactivateContext(context); + await Promise.resolve(); + + assert.deepStrictEqual(order, ['stop first', 'stop second']); + + firstStop.resolve(); + await Promise.resolve(); + assert.deepStrictEqual(order, ['stop first', 'stop second']); + + secondStop.resolve(); + await shutdown; + + assert.deepStrictEqual(order, [ + 'stop first', + 'stop second', + 'dispose first', + 'dispose second', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + }); + + test('deactivation timeout falls back to synchronous session and terminal teardown', async () => { + const clock = sinon.useFakeTimers({ shouldClearNativeTimers: true }); + const order: string[] = []; + const context = createContext(order); + addSession(context, 'session', () => { + order.push('stop session'); + return new Promise(() => { }); + }, () => order.push('dispose session')); + + try { + const shutdown = deactivateContext(context); + await Promise.resolve(); + + assert.deepStrictEqual(order, ['stop session']); + + await clock.tickAsync(5_000); + await shutdown; + + assert.deepStrictEqual(order, [ + 'stop session', + 'dispose session', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + } + finally { + clock.restore(); + } + }); + + test('dispose does not race an in-flight deactivation and repeated shutdown is idempotent', async () => { + const order: string[] = []; + const context = createContext(order); + const stop = createDeferred(); + let stopCalls = 0; + addSession(context, 'session', () => { + stopCalls++; + order.push('stop session'); + return stop.promise; + }, () => order.push('dispose session')); + + const firstShutdown = deactivateContext(context); + const secondShutdown = deactivateContext(context); + await Promise.resolve(); + context.dispose(); + + assert.deepStrictEqual(order, ['stop session']); + assert.strictEqual(stopCalls, 1); + + stop.resolve(); + await Promise.all([firstShutdown, secondShutdown]); + context.dispose(); + await deactivateContext(context); + + assert.strictEqual(stopCalls, 1); + assert.deepStrictEqual(order, [ + 'stop session', + 'dispose session', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + }); + + test('deactivation warns and absorbs CLI stop errors after completing teardown', async () => { + const order: string[] = []; + const context = createContext(order); + const expectedError = new Error('stop failed'); + const warnStub = sinon.stub(extensionLogOutputChannel, 'warn'); + addSession(context, 'session', async () => { + order.push('stop session'); + throw expectedError; + }, () => order.push('dispose session')); + + try { + await deactivateContext(context); + + sinon.assert.calledWithMatch(warnStub, 'Failed to stop Aspire CLI during extension deactivation: Error: stop failed'); + assert.deepStrictEqual(order, [ + 'stop session', + 'dispose session', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + } + finally { + warnStub.restore(); + } + }); + + test('deactivation logs and absorbs PendingResponseRejected after the RPC transport closes', async () => { + const order: string[] = []; + const context = createContext(order); + const infoStub = sinon.stub(extensionLogOutputChannel, 'info'); + const warnStub = sinon.stub(extensionLogOutputChannel, 'warn'); + addSession(context, 'session', async () => { + order.push('stop session'); + throw new ResponseError(ErrorCodes.PendingResponseRejected, 'Pending response rejected since connection got disposed'); + }, () => order.push('dispose session')); + + try { + await deactivateContext(context); + + sinon.assert.calledWithMatch(infoStub, 'Aspire CLI stop request ended after the RPC transport closed:'); + assert.strictEqual(warnStub.calledWithMatch('Failed to stop Aspire CLI during extension deactivation:'), false); + assert.deepStrictEqual(order, [ + 'stop session', + 'dispose session', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + } + finally { + infoStub.restore(); + warnStub.restore(); + } + }); +}); + +function createContext(order: string[]): AspireExtensionContext { + const context = new AspireExtensionContext(); + context.initialize( + { dispose: () => order.push('rpc server') } as any, + { subscriptions: [] } as unknown as vscode.ExtensionContext, + { dispose: () => { } } as any, + { dispose: () => order.push('dcp server') } as any, + { dispose: () => order.push('terminal provider') } as any, + { dispose: () => order.push('editor command provider') } as any); + return context; +} + +function addSession(context: AspireExtensionContext, debugSessionId: string, stopCli: () => Promise, dispose: () => void): void { + context.addAspireDebugSession({ + debugSessionId, + onDidChangeState: () => ({ dispose: () => { } }), + onDidSendDebugConsoleOutput: () => ({ dispose: () => { } }), + requestCliStopForExtensionShutdown: stopCli, + dispose, + } as unknown as AspireDebugSession); +} + +function deactivateContext(context: AspireExtensionContext): Promise { + const deactivate = (context as AspireExtensionContext & { deactivate?: () => Promise }).deactivate; + if (deactivate) { + return deactivate.call(context); + } + + context.dispose(); + return Promise.resolve(); +} + +function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void; + const promise = new Promise(promiseResolve => { + resolve = promiseResolve; + }); + + return { promise, resolve }; +} diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index 8cd0354c758..bd96fb28f01 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -62,6 +62,29 @@ suite('AspireDebugSession tests', () => { tempDirs.length = 0; }); + test('extension shutdown reuses an in-flight CLI stop request', async () => { + let completeStop!: () => void; + const stopRequest = new Promise(resolve => { + completeStop = resolve; + }); + const stopCli = sinon.stub().returns(stopRequest); + const parentDebugSession = { + id: 'aspire-session', + configuration: {}, + } as unknown as vscode.DebugSession; + const aspireDebugSession = new AspireDebugSession(parentDebugSession, {} as any, {} as any, {} as any, () => { }); + (aspireDebugSession as any)._rpcClient = { stopCli }; + + const firstRequest = aspireDebugSession.requestCliStopForExtensionShutdown(); + const secondRequest = aspireDebugSession.requestCliStopForExtensionShutdown(); + + assert.strictEqual(secondRequest, firstRequest); + sinon.assert.calledOnce(stopCli); + + completeStop(); + await firstRequest; + }); + test('suppresses the Aspire CLI first-run banner for extension-managed launches', async () => { const parentDebugSession = { id: 'aspire-session', diff --git a/extension/src/test/rpc/aspireRpcServer.test.ts b/extension/src/test/rpc/aspireRpcServer.test.ts new file mode 100644 index 00000000000..459876e900b --- /dev/null +++ b/extension/src/test/rpc/aspireRpcServer.test.ts @@ -0,0 +1,199 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as tls from 'tls'; +import { createMessageConnection, ErrorCodes, MessageConnection, ResponseError } from 'vscode-jsonrpc'; +import { StreamMessageReader, StreamMessageWriter } from 'vscode-jsonrpc/node'; + +import AspireRpcServer from '../../server/AspireRpcServer'; +import { ICliRpcClient, RpcClient } from '../../server/rpcClient'; +import { extensionLogOutputChannel } from '../../utils/logging'; + +suite('AspireRpcServer', () => { + test('server disposal owns a client while its debug-session handshake is pending', async function () { + this.timeout(10000); + + const handshakeStarted = createDeferred(); + const handshakeResult = createDeferred(); + let rpcClient: RpcClient | undefined; + const rpcServer = await AspireRpcServer.create((_connectionInfo, connection) => { + rpcClient = new RpcClient(connection, null, () => null); + return rpcClient; + }); + const transport = await connectClient(rpcServer, async () => { + handshakeStarted.resolve(); + return await handshakeResult.promise; + }); + const transportClosed = new Promise(resolve => transport.socket.once('close', () => resolve())); + + try { + await handshakeStarted.promise; + assert.ok(rpcClient); + + rpcClient.interactionService.showStatus('Building AppHost...'); + assert.strictEqual((rpcClient.interactionService as any)._progressNotifier.isActive, true); + + rpcServer.dispose(); + await transportClosed; + + assert.strictEqual((rpcClient.interactionService as any)._progressNotifier.isActive, false); + assert.strictEqual(transport.socket.destroyed, true); + } + finally { + handshakeResult.resolve(null); + await new Promise(resolve => setImmediate(resolve)); + transport.connection.end(); + transport.connection.dispose(); + transport.socket.destroy(); + rpcClient?.dispose(); + } + }); + + test('a rejected debug-session handshake on an open transport is warned and disposes the pending client', async function () { + this.timeout(10000); + + const handshakeStarted = createDeferred(); + const handshakeResult = createDeferred(); + const clientDisposed = createDeferred(); + const warnStub = sinon.stub(extensionLogOutputChannel, 'warn'); + let rpcClient: RpcClient | undefined; + const rpcServer = await AspireRpcServer.create((_connectionInfo, connection) => { + rpcClient = new RpcClient(connection, null, () => null); + const originalDispose = rpcClient.dispose.bind(rpcClient); + rpcClient.dispose = () => { + originalDispose(); + clientDisposed.resolve(); + }; + return rpcClient; + }); + const transport = await connectClient(rpcServer, async () => { + handshakeStarted.resolve(); + return await handshakeResult.promise; + }); + + try { + await handshakeStarted.promise; + assert.ok(rpcClient); + + rpcClient.interactionService.showStatus('Connecting to AppHost...'); + handshakeResult.reject(new ResponseError(ErrorCodes.PendingResponseRejected, 'handshake failed')); + await clientDisposed.promise; + + sinon.assert.calledWithMatch(warnStub, 'Failed to initialize RPC client:'); + assert.strictEqual((rpcClient.interactionService as any)._progressNotifier.isActive, false); + assert.deepStrictEqual(rpcServer.connections, []); + } + finally { + transport.connection.end(); + transport.connection.dispose(); + transport.socket.destroy(); + warnStub.restore(); + rpcServer.dispose(); + } + }); + + test('transport disposal during the handshake logs the expected pending rejection at info', async function () { + this.timeout(10000); + + const handshakeStarted = createDeferred(); + const handshakeResult = createDeferred(); + const clientDisposed = createDeferred(); + const infoStub = sinon.stub(extensionLogOutputChannel, 'info'); + const warnStub = sinon.stub(extensionLogOutputChannel, 'warn'); + let rpcClient: RpcClient | undefined; + const rpcServer = await AspireRpcServer.create((_connectionInfo, connection) => { + rpcClient = new RpcClient(connection, null, () => null); + const originalDispose = rpcClient.dispose.bind(rpcClient); + rpcClient.dispose = () => { + originalDispose(); + clientDisposed.resolve(); + }; + return rpcClient; + }); + const transport = await connectClient(rpcServer, async () => { + handshakeStarted.resolve(); + return await handshakeResult.promise; + }); + + try { + await handshakeStarted.promise; + assert.ok(rpcClient); + + rpcClient.interactionService.showStatus('Connecting to AppHost...'); + transport.connection.end(); + transport.connection.dispose(); + transport.socket.destroy(); + await clientDisposed.promise; + await new Promise(resolve => setImmediate(resolve)); + + sinon.assert.calledWithMatch(infoStub, 'RPC client transport closed during initialization:'); + assert.strictEqual(warnStub.calledWithMatch('Failed to initialize RPC client:'), false); + assert.strictEqual((rpcClient.interactionService as any)._progressNotifier.isActive, false); + assert.deepStrictEqual(rpcServer.connections, []); + } + finally { + handshakeResult.resolve(null); + infoStub.restore(); + warnStub.restore(); + rpcServer.dispose(); + } + }); + + test('connections added after disposal are rejected and disposed without publishing them', async () => { + const rpcServer = await AspireRpcServer.create(() => { + throw new Error('The test does not establish a connection.'); + }); + const onNewConnection = sinon.spy(); + rpcServer.onNewConnection(onNewConnection); + const closeSpy = sinon.spy(rpcServer.server, 'close'); + let clientDisposeCount = 0; + const client = { + dispose: () => clientDisposeCount++, + } as unknown as ICliRpcClient; + + rpcServer.dispose(); + rpcServer.dispose(); + const added = rpcServer.addConnection(client); + + assert.strictEqual(added, false); + assert.strictEqual(clientDisposeCount, 1); + assert.deepStrictEqual(rpcServer.connections, []); + sinon.assert.notCalled(onNewConnection); + sinon.assert.calledOnce(closeSpy); + }); +}); + +async function connectClient( + rpcServer: AspireRpcServer, + getDebugSessionId: () => Promise +): Promise<{ connection: MessageConnection; socket: tls.TLSSocket }> { + const port = Number(rpcServer.connectionInfo.address.replace('localhost:', '')); + const socket = tls.connect({ + port, + host: 'localhost', + rejectUnauthorized: false, + }); + await new Promise((resolve, reject) => { + socket.once('secureConnect', resolve); + socket.once('error', reject); + }); + + const connection = createMessageConnection( + new StreamMessageReader(socket), + new StreamMessageWriter(socket) + ); + connection.onRequest('getDebugSessionId', getDebugSessionId); + connection.listen(); + + return { connection, socket }; +} + +function createDeferred(): { promise: Promise; resolve: (value: T) => void; reject: (reason: unknown) => void } { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + + return { promise, resolve, reject }; +} diff --git a/extension/src/test/rpc/interactionServiceTests.test.ts b/extension/src/test/rpc/interactionServiceTests.test.ts index 27c78702a84..cb53eb6e104 100644 --- a/extension/src/test/rpc/interactionServiceTests.test.ts +++ b/extension/src/test/rpc/interactionServiceTests.test.ts @@ -228,7 +228,9 @@ suite('InteractionService endpoints', () => { closeHandler = handler; return { dispose: () => { } }; }, - sendRequest: sinon.stub() + sendRequest: sinon.stub(), + end: sinon.stub(), + dispose: sinon.stub() } as any; const rpcClient = new RpcClient(messageConnection, null, () => null); @@ -241,6 +243,39 @@ suite('InteractionService endpoints', () => { assert.strictEqual((rpcClient.interactionService as any)._progressNotifier.isActive, false); }); + test("RPC client disposal closes the transport and prevents late status resurrection", () => { + const end = sinon.stub(); + const dispose = sinon.stub(); + const messageConnection = { + onClose: () => ({ dispose: () => { } }), + sendRequest: sinon.stub(), + end, + dispose + } as any; + const rpcClient = new RpcClient(messageConnection, null, () => null); + + rpcClient.interactionService.showStatus('Connecting to AppHost...'); + rpcClient.dispose(); + rpcClient.interactionService.showStatus('Starting Dashboard...'); + rpcClient.dispose(); + + assert.strictEqual((rpcClient.interactionService as any)._progressNotifier.isActive, false); + sinon.assert.calledOnce(end); + sinon.assert.calledOnce(dispose); + }); + + test("RPC server disposal clears CLI status when the connection never closes", async () => { + const testInfo = await createTestRpcServer(); + testInfo.rpcServer.addConnection(testInfo.rpcClient); + + testInfo.interactionService.showStatus('Building AppHost...'); + assert.strictEqual((testInfo.interactionService as any)._progressNotifier.isActive, true); + + testInfo.rpcServer.dispose(); + + assert.strictEqual((testInfo.interactionService as any)._progressNotifier.isActive, false); + }); + test("displaySubtleMessage endpoint", async () => { const testInfo = await createTestRpcServer(); const setStatusBarMessageSpy = sinon.spy(vscode.window, 'setStatusBarMessage'); @@ -799,6 +834,7 @@ suite('InteractionService endpoints', () => { type RpcServerTestInfo = { rpcServerInfo: RpcServerConnectionInfo; + rpcServer: AspireRpcServer; rpcClient: ICliRpcClient; interactionService: IInteractionService; }; @@ -861,6 +897,10 @@ class TestCliRpcClient implements ICliRpcClient { this.interactionService = new InteractionService(getAspireDebugSession, this, globalState); } + dispose(): void { + this.interactionService.dispose(); + } + stopCli(): Promise { return Promise.resolve(); } @@ -901,6 +941,7 @@ async function createTestRpcServer(debugSessionId?: string | null, getAspireDebu return { rpcServerInfo: rpcServer.connectionInfo, + rpcServer: rpcServer, rpcClient: rpcClient, interactionService: rpcClient.interactionService }; From 66aaa5d4db95d4dbadb87bf9a4e262a69459b5b4 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 13:53:09 -0400 Subject: [PATCH 02/24] Reject foreign absolute paths in getRelativePathToWorkspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `asRelativePath` returns its input unchanged when the path cannot be made relative, and that value was returned verbatim. It resolves against the workspace, which may not share the extension host's path semantics, so a Windows absolute path (`C:\Users\...` or `\\server\share\...`) passes the host's `path.isAbsolute` on POSIX — the case for remote SSH, WSL and Codespaces — and the full path leaked into the debug configuration name. Reject both POSIX and Win32 absolute forms and fall back to the workspace folder name, and use the file name rather than the full path when the target is outside every workspace folder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/test/workspace.test.ts | 59 +++++++++++++++++++++++++++- extension/src/utils/workspace.ts | 23 +++++++---- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/extension/src/test/workspace.test.ts b/extension/src/test/workspace.test.ts index 352eb3f29d9..7ef2e1863e9 100644 --- a/extension/src/test/workspace.test.ts +++ b/extension/src/test/workspace.test.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; import { yesLabel } from '../loc/strings'; -import { checkForExistingAppHostPathInWorkspace, getCommonExcludeGlob, findAspireSettingsFiles } from '../utils/workspace'; +import { checkForExistingAppHostPathInWorkspace, getCommonExcludeGlob, findAspireSettingsFiles, getRelativePathToWorkspace } from '../utils/workspace'; import { AppHostDiscoveryService, getWorkspaceAppHostProjectSearchResult } from '../utils/appHostDiscovery'; import { getAppHostDiscoveryExcludeGlob } from '../utils/workspaceFileSearch'; @@ -20,6 +20,63 @@ suite('utils/workspace tests', () => { sandbox.restore(); }); + test('getRelativePathToWorkspace falls back to the workspace name for absolute paths from any host platform', () => { + const workspaceFolder = { + uri: vscode.Uri.file('/workspace'), + name: 'workspace', + index: 0, + }; + sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(workspaceFolder); + sandbox.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder]); + const asRelativePathStub = sandbox.stub(vscode.workspace, 'asRelativePath'); + + // `asRelativePath` returns the input unchanged when it cannot be made relative. The Win32 + // forms are only rejected by `path.win32` and the POSIX form only by `path.posix`, so this + // asserts the same privacy-safe fallback on a Windows host and on a POSIX host (remote + // SSH, WSL, Codespaces), where the default `path` module understands only one of them. + const absolutePaths = [ + 'C:\\Users\\me\\src\\AppHost.csproj', + '\\\\server\\share\\src\\AppHost.csproj', + '/home/me/src/AppHost.csproj', + ]; + + const identities = absolutePaths.map(absolutePath => { + asRelativePathStub.returns(absolutePath); + return getRelativePathToWorkspace(absolutePath); + }); + + assert.deepStrictEqual(identities, ['workspace', 'workspace', 'workspace']); + }); + + test('getRelativePathToWorkspace keeps genuinely relative paths in either separator style', () => { + const workspaceFolder = { + uri: vscode.Uri.file('/workspace'), + name: 'workspace', + index: 0, + }; + sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(workspaceFolder); + sandbox.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder]); + const asRelativePathStub = sandbox.stub(vscode.workspace, 'asRelativePath'); + + const relativePaths = [ + 'apps/Store/AppHost.csproj', + 'apps\\Store\\AppHost.csproj', + ]; + + const identities = relativePaths.map(relativePath => { + asRelativePathStub.returns(relativePath); + return getRelativePathToWorkspace('/workspace/apps/Store/AppHost.csproj'); + }); + + assert.deepStrictEqual(identities, relativePaths, 'rejecting both absolute forms must not reject relative paths'); + }); + + test('getRelativePathToWorkspace uses the file name when the path is outside every workspace folder', () => { + sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(undefined); + + assert.strictEqual(getRelativePathToWorkspace(path.join(path.sep, 'elsewhere', 'src', 'AppHost.csproj')), 'AppHost.csproj'); + }); + test('getCommonExcludeGlob returns valid glob pattern', () => { const glob = getCommonExcludeGlob(); diff --git a/extension/src/utils/workspace.ts b/extension/src/utils/workspace.ts index c0ab2b413c5..2d9f4433481 100644 --- a/extension/src/utils/workspace.ts +++ b/extension/src/utils/workspace.ts @@ -54,19 +54,28 @@ export function isFolderOpenInWorkspace(folderPath: string): boolean { } export function getRelativePathToWorkspace(filePath: string): string { - if (!isWorkspaceOpen(false)) { - return filePath; - } - const uri = vscode.Uri.file(filePath); const workspaceFolder = vscode.workspace.getWorkspaceFolder(uri); + if (!workspaceFolder) { + return path.basename(filePath); + } - if (workspaceFolder) { - const relativePath = vscode.workspace.asRelativePath(uri); + const relativePath = vscode.workspace.asRelativePath(uri); + // `asRelativePath` returns the path unchanged when it cannot be made relative, and it resolves + // against the *workspace*, which may use different path semantics than the extension host. A + // Windows absolute path (`C:\Users\me\src\AppHost.csproj` or `\\server\share\src\AppHost.csproj`) + // therefore passes the host's `path.isAbsolute` on POSIX — which is what runs for remote/SSH, + // Codespaces and WSL windows — and the full path would leak into the debug configuration name. + // Reject both platforms' absolute forms so the workspace-name fallback runs either way. + if (relativePath && !isAbsoluteOnAnyPlatform(relativePath)) { return relativePath; } - return filePath; + return workspaceFolder.name || path.basename(filePath); +} + +function isAbsoluteOnAnyPlatform(filePath: string): boolean { + return path.posix.isAbsolute(filePath) || path.win32.isAbsolute(filePath); } interface AppHostQuickPickItem extends vscode.QuickPickItem { From f7e616a7a4e886a59b125adefb436cecf78dab53 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 15:49:13 -0400 Subject: [PATCH 03/24] Reject foreign absolute paths before the workspace-membership check The cross-platform absolute-path rejection sat after the getWorkspaceFolder early return, so it was unreachable for the input it existed to catch. A Windows path is inside no workspace folder on a POSIX host, so getWorkspaceFolder returns undefined and control reached path.basename, which on POSIX does not split on '\' and returned C:\Users\me\secret\AppHost.csproj whole as the debug configuration name. Move the rejection ahead of that early return and reduce the path with path.win32.basename, which splits on both separators. Only Win32 forms can be foreign, because path.win32.isAbsolute also accepts a leading '/'. The existing regression test stubbed getWorkspaceFolder to return a folder for the Windows paths, which cannot happen on a POSIX host, so it was green against a code path that never ran. It now covers the asRelativePath guard with a host-native path, and a new test drives the foreign paths with getWorkspaceFolder returning undefined, asserting both the file name and that no separator survives on either host platform. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/test/workspace.test.ts | 47 +++++++++++++++++++++------- extension/src/utils/workspace.ts | 28 ++++++++++++++--- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/extension/src/test/workspace.test.ts b/extension/src/test/workspace.test.ts index 7ef2e1863e9..3542cba01b2 100644 --- a/extension/src/test/workspace.test.ts +++ b/extension/src/test/workspace.test.ts @@ -20,7 +20,7 @@ suite('utils/workspace tests', () => { sandbox.restore(); }); - test('getRelativePathToWorkspace falls back to the workspace name for absolute paths from any host platform', () => { + test('getRelativePathToWorkspace falls back to the workspace name when asRelativePath cannot relativize', () => { const workspaceFolder = { uri: vscode.Uri.file('/workspace'), name: 'workspace', @@ -30,22 +30,47 @@ suite('utils/workspace tests', () => { sandbox.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder]); const asRelativePathStub = sandbox.stub(vscode.workspace, 'asRelativePath'); - // `asRelativePath` returns the input unchanged when it cannot be made relative. The Win32 - // forms are only rejected by `path.win32` and the POSIX form only by `path.posix`, so this - // asserts the same privacy-safe fallback on a Windows host and on a POSIX host (remote - // SSH, WSL, Codespaces), where the default `path` module understands only one of them. - const absolutePaths = [ + // `asRelativePath` returns the input unchanged when it cannot be made relative, and it + // resolves against the workspace rather than the extension host. Both platforms' absolute + // forms must be rejected here, because the Win32 forms are only recognized by `path.win32` + // and the POSIX form only by `path.posix`. + const hostAbsolutePath = path.join(path.sep, 'workspace', 'src', 'AppHost.csproj'); + const unrelativizedResults = [ + hostAbsolutePath, 'C:\\Users\\me\\src\\AppHost.csproj', '\\\\server\\share\\src\\AppHost.csproj', '/home/me/src/AppHost.csproj', + ].map(unrelativized => { + asRelativePathStub.returns(unrelativized); + return getRelativePathToWorkspace(hostAbsolutePath); + }); + + assert.deepStrictEqual(unrelativizedResults, ['workspace', 'workspace', 'workspace', 'workspace']); + }); + + test('getRelativePathToWorkspace reduces absolute paths from either platform to a bare file name', () => { + // A path that is absolute only under the *other* platform's rules is inside no workspace + // folder on this host, so `getWorkspaceFolder` returns undefined and the absolute-path + // rejection in the `asRelativePath` branch is never reached. That left `path.basename`, + // which on POSIX does not treat `\` as a separator, so the whole + // `C:\Users\me\secret\AppHost.csproj` was returned as the debug configuration name. + sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(undefined); + + const absolutePaths = [ + 'C:\\Users\\me\\secret\\AppHost.csproj', + 'C:/Users/me/secret/AppHost.csproj', + '\\\\server\\share\\secret\\AppHost.csproj', + '/home/me/secret/AppHost.csproj', ]; - const identities = absolutePaths.map(absolutePath => { - asRelativePathStub.returns(absolutePath); - return getRelativePathToWorkspace(absolutePath); - }); + const names = absolutePaths.map(absolutePath => getRelativePathToWorkspace(absolutePath)); - assert.deepStrictEqual(identities, ['workspace', 'workspace', 'workspace']); + assert.deepStrictEqual(names, ['AppHost.csproj', 'AppHost.csproj', 'AppHost.csproj', 'AppHost.csproj']); + for (const name of names) { + // The privacy property, asserted independently of host platform: whatever is returned + // is a single path segment, so no directory component can leak. + assert.ok(!/[\\/]/.test(name), `'${name}' must be a single path segment on either host platform`); + } }); test('getRelativePathToWorkspace keeps genuinely relative paths in either separator style', () => { diff --git a/extension/src/utils/workspace.ts b/extension/src/utils/workspace.ts index 2d9f4433481..2854259547e 100644 --- a/extension/src/utils/workspace.ts +++ b/extension/src/utils/workspace.ts @@ -54,6 +54,19 @@ export function isFolderOpenInWorkspace(folderPath: string): boolean { } export function getRelativePathToWorkspace(filePath: string): string { + // Reject paths that are absolute only under the *other* platform's rules before the + // workspace-membership check below, not after it. `getWorkspaceFolder` cannot match such a + // path, so control would reach `path.basename`, which on POSIX does not treat `\` as a + // separator: `C:\Users\me\src\AppHost.csproj` comes back whole and the full path leaks into the + // debug configuration name. POSIX hosts see Windows paths routinely via remote/SSH, Codespaces + // and WSL, so the ordering is what makes this check reachable for the input it exists to catch. + if (isForeignAbsolutePath(filePath)) { + // Only Win32 forms can be foreign: `path.win32.isAbsolute` also accepts a leading `/`, so a + // POSIX path is never foreign on a Windows host (and `path.win32.basename` splits on both + // separators, which is why that direction was already safe). + return path.win32.basename(filePath); + } + const uri = vscode.Uri.file(filePath); const workspaceFolder = vscode.workspace.getWorkspaceFolder(uri); if (!workspaceFolder) { @@ -62,10 +75,7 @@ export function getRelativePathToWorkspace(filePath: string): string { const relativePath = vscode.workspace.asRelativePath(uri); // `asRelativePath` returns the path unchanged when it cannot be made relative, and it resolves - // against the *workspace*, which may use different path semantics than the extension host. A - // Windows absolute path (`C:\Users\me\src\AppHost.csproj` or `\\server\share\src\AppHost.csproj`) - // therefore passes the host's `path.isAbsolute` on POSIX — which is what runs for remote/SSH, - // Codespaces and WSL windows — and the full path would leak into the debug configuration name. + // against the *workspace*, which may use different path semantics than the extension host. // Reject both platforms' absolute forms so the workspace-name fallback runs either way. if (relativePath && !isAbsoluteOnAnyPlatform(relativePath)) { return relativePath; @@ -74,6 +84,16 @@ export function getRelativePathToWorkspace(filePath: string): string { return workspaceFolder.name || path.basename(filePath); } +/** + * Determines whether a path is absolute under one platform's rules but not under the extension + * host's, which means it can never be resolved against this workspace and the host's `path` helpers + * cannot decompose it — `path.posix.basename('C:\\Users\\me\\AppHost.csproj')` returns the whole + * string rather than the file name. + */ +function isForeignAbsolutePath(filePath: string): boolean { + return isAbsoluteOnAnyPlatform(filePath) && !path.isAbsolute(filePath); +} + function isAbsoluteOnAnyPlatform(filePath: string): boolean { return path.posix.isAbsolute(filePath) || path.win32.isAbsolute(filePath); } From 938888da90adb434b922087f8c2b203f92678bf0 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 16:46:36 -0400 Subject: [PATCH 04/24] Terminate the Aspire CLI process group when the cooperative stop does not `stopCli` is an RPC request, not a kill. It resolves without effect when the transport is already closed and never settles when the CLI has stopped servicing the connection, so neither outcome proves the process exited. `spawnAspireCommand` discarded the ChildProcess that `spawnCliProcess` returns and did not request a process group, so there was nothing to signal as a fallback. Every other CLI spawn site in the extension already retains its child and calls `terminateCliProcess`; the longest-lived one did not. Retain the child, spawn `aspire run` as a process-group leader, and add `terminateCliProcessTree()`. Session disposal escalates to it after a 10s grace period so a cooperative stop still gets the first chance to shut resources down cleanly, and deactivation calls it directly once the stop requests settle or time out. Also re-snapshot the session array between awaits during deactivation. `_isShuttingDown` does not gate `addAspireDebugSession`, so a debug-adapter descriptor or an RPC-triggered `startDebugSession` landing mid-await was never asked to stop. Requesting a stop is idempotent per session, so re-scanning until no new session appears is safe. Verified red-green: with the escalation, the process group and the re-snapshot loop reverted, 4 of the 5 new tests fail and the existing 5 deactivation tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/AspireExtensionContext.ts | 58 ++++++++++-- extension/src/debugger/AspireDebugSession.ts | 68 +++++++++++++- .../src/test/AspireExtensionContext.test.ts | 88 ++++++++++++++++++- extension/src/test/aspireDebugSession.test.ts | 76 +++++++++++++++- 4 files changed, 277 insertions(+), 13 deletions(-) diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index 1312b4ccd46..32aa52106d2 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -137,15 +137,58 @@ export class AspireExtensionContext implements vscode.Disposable { } private async _waitForCliStopRequests(): Promise { - const stopRequests = this._aspireDebugSessions.map(session => { + const requested = new Map>(); + const deadline = Date.now() + AspireExtensionContext._cliStopTimeoutMs; + + // Re-snapshot after every await. `_isShuttingDown` does not stop `addAspireDebugSession` + // from registering a session, so a debug-adapter descriptor or an RPC-triggered + // `startDebugSession` that lands mid-await would never be asked to stop if the array were + // captured only once. Requesting a stop is idempotent per session, so re-scanning is safe. + while (this._collectStopRequests(requested) && Date.now() < deadline) { + const timedOut = await this._settleStopRequests([...requested.values()], deadline); + if (timedOut) { + extensionLogOutputChannel.warn(`Timed out after ${AspireExtensionContext._cliStopTimeoutMs}ms waiting for Aspire CLI stop requests; continuing extension teardown.`); + break; + } + } + + // A cooperative stop that resolved, rejected or timed out proves only what happened to the + // RPC request; the CLI process can still be running. Signal any that are, so deactivation + // cannot leave an AppHost and its resource processes orphaned. + for (const session of this._aspireDebugSessions) { try { - return session.requestCliStopForExtensionShutdown(); + session.terminateCliProcessTree(); } catch (error) { - return Promise.reject(error); + extensionLogOutputChannel.warn(`Failed to terminate the Aspire CLI process during extension deactivation: ${error}`); } - }); + } + } + /** + * Requests a CLI stop for every registered session that has not been asked yet, returning + * whether any new session was found. + */ + private _collectStopRequests(requested: Map>): boolean { + let addedRequest = false; + for (const session of this._aspireDebugSessions) { + if (requested.has(session.debugSessionId)) { + continue; + } + + addedRequest = true; + try { + requested.set(session.debugSessionId, session.requestCliStopForExtensionShutdown()); + } + catch (error) { + requested.set(session.debugSessionId, Promise.reject(error)); + } + } + + return addedRequest; + } + + private async _settleStopRequests(stopRequests: Promise[], deadline: number): Promise { const allStops = Promise.allSettled(stopRequests); let timeout: ReturnType | undefined; const outcome = await Promise.race([ @@ -154,7 +197,7 @@ export class AspireExtensionContext implements vscode.Disposable { timeout = setTimeout(() => { timeout = undefined; resolve({ timedOut: true }); - }, AspireExtensionContext._cliStopTimeoutMs); + }, Math.max(0, deadline - Date.now())); }), ]); @@ -163,8 +206,7 @@ export class AspireExtensionContext implements vscode.Disposable { } if (outcome.timedOut) { - extensionLogOutputChannel.warn(`Timed out after ${AspireExtensionContext._cliStopTimeoutMs}ms waiting for Aspire CLI stop requests; continuing extension teardown.`); - return; + return true; } const failures = outcome.results @@ -180,6 +222,8 @@ export class AspireExtensionContext implements vscode.Disposable { extensionLogOutputChannel.warn(`Failed to stop Aspire CLI during extension deactivation: ${failure}`); } } + + return false; } private _disposeCore(): void { diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 7f70ffe6b07..667e6945ac2 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -5,7 +5,7 @@ import { createDebugAdapterTracker, AppHostOutputHandler, AppHostRestartHandler import { AspireResourceExtendedDebugConfiguration, AspireResourceDebugSession, EnvVar, AspireExtendedDebugConfiguration, NodeLaunchConfiguration, ProcessRestartedNotification, ProjectLaunchConfiguration, SessionTerminatedNotification, StartAppHostOptions } from "../dcp/types"; import { extensionLogOutputChannel } from "../utils/logging"; import AspireDcpServer, { generateDcpIdPrefix } from "../dcp/AspireDcpServer"; -import { spawnCliProcess } from "./languages/cli"; +import { spawnCliProcess, terminateCliProcess } from "./languages/cli"; import { disconnectingFromSession, launchingWithAppHost, launchingWithDirectory, processExceptionOccurred, processExitedWithCode, aspireDashboard, appHostSessionTerminated } from "../loc/strings"; import { projectDebuggerExtension } from "./languages/dotnet"; import { AnsiColors } from "../utils/AspireTerminalProvider"; @@ -20,6 +20,7 @@ import { ICliRpcClient } from "../server/rpcClient"; import path from "path"; import os from "os"; import { EnvironmentVariables } from "../utils/environment"; +import type { ChildProcessWithoutNullStreams } from "child_process"; import { sendTelemetryEvent } from "../utils/telemetry"; import { classifyAppHostPath, classifyAppHostDirectory } from "../utils/appHostLanguage"; import { bucketAspireCommand } from "../utils/telemetryBuckets"; @@ -53,6 +54,12 @@ export function getLoggableDebugConfiguration(debugConfig: AspireResourceExtende export class AspireDebugSession implements vscode.DebugAdapter { private static readonly _mauiDebugStartMaxAttempts = 3; private static readonly _mauiDebugStartRetryDelayMs = 5000; + /** + * How long the cooperative `stopCli` RPC has to bring the CLI down before its process group is + * signalled. Long enough for the CLI to stop containers and other resources cleanly, short + * enough that a wedged CLI does not keep the AppHost alive indefinitely. + */ + private static readonly _cliCooperativeStopGraceMs = 10_000; private readonly _onDidSendMessage = new EventEmitter(); private readonly _onDidSendDebugConsoleOutput = new EventEmitter(); private _messageSeq = 1; @@ -75,6 +82,8 @@ export class AspireDebugSession implements vscode.DebugAdapter { private _disposed = false; private _parentStopPromise: Thenable | undefined; private _cliStopPromise: Promise | undefined; + private _cliProcess: ChildProcessWithoutNullStreams | undefined; + private _cliTerminationTimer: ReturnType | undefined; // Timestamp for the `debug/apphost/end` duration measurement. Captured the first // time we observe a `launch` request so it covers the actual user-visible session // lifetime, not the moment the AspireDebugSession object was constructed. @@ -145,6 +154,46 @@ export class AspireDebugSession implements vscode.DebugAdapter { return this._cliStopPromise; } + /** + * Signals the `aspire` CLI process group when the process is still alive. + * + * The cooperative `stopCli` RPC resolving proves only that the request was accepted, and on a + * closed transport it resolves having done nothing at all. Neither outcome terminates the CLI, + * so the process it owns — the AppHost and every resource process beneath it — has to be + * signalled directly whenever the cooperative path did not finish the job. No-ops when the CLI + * already exited, which is the normal case. + */ + terminateCliProcessTree(): void { + this.cancelScheduledCliProcessTermination(); + const cliProcess = this._cliProcess; + if (!cliProcess || cliProcess.exitCode !== null || cliProcess.signalCode !== null) { + return; + } + + terminateCliProcess(cliProcess, `Aspire CLI for debug session ${this.debugSessionId}`); + } + + private scheduleCliProcessTermination(): void { + if (!this._cliProcess || this._cliTerminationTimer) { + return; + } + + // Give the cooperative stop the first chance so the CLI can shut its resources down cleanly; + // only a CLI that is still alive afterwards gets signalled. + this._cliTerminationTimer = setTimeout(() => { + this._cliTerminationTimer = undefined; + this.terminateCliProcessTree(); + }, AspireDebugSession._cliCooperativeStopGraceMs); + this._cliTerminationTimer.unref?.(); + } + + private cancelScheduledCliProcessTermination(): void { + if (this._cliTerminationTimer) { + clearTimeout(this._cliTerminationTimer); + this._cliTerminationTimer = undefined; + } + } + private stopParentDebugSessionOnce(): Thenable { if (this._parentStopPromise) { return this._parentStopPromise; @@ -406,7 +455,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { return partial; }; - spawnCliProcess( + this._cliProcess = spawnCliProcess( this._terminalProvider, await this._terminalProvider.getAspireCliExecutablePath(), args, @@ -422,6 +471,8 @@ export class AspireDebugSession implements vscode.DebugAdapter { vscode.window.showErrorMessage(processExceptionOccurred(error.message, commandLabel)); }, exitCallback: (code) => { + // The CLI came down on its own, so the escalation timer has nothing left to signal. + this.cancelScheduledCliProcessTermination(); this._dcpServer.recordAppHostProcessExit(this.debugSessionId, code); // Flush any partial line left in either buffer so trailing output isn't lost. if (stdoutBuffer.length > 0) { @@ -439,7 +490,12 @@ export class AspireDebugSession implements vscode.DebugAdapter { workingDirectory: workingDirectory, debugSessionId: this.debugSessionId, noDebug: noDebug, - env: env + env: env, + // `aspire run` owns the AppHost and every resource process beneath it. Spawning it as a + // process-group leader is what lets `terminateCliProcess` signal the whole tree by negative + // PID when the cooperative `stopCli` RPC does not finish the job. Every other CLI spawn + // site in the extension already does this; this one is the longest-lived of them. + createProcessGroup: true, }, ); @@ -449,6 +505,12 @@ export class AspireDebugSession implements vscode.DebugAdapter { extensionLogOutputChannel.info(`stopCli failed (connection may already be closed): ${err}`); }); extensionLogOutputChannel.info(`Requested Aspire CLI exit with args: ${args.join(' ')}`); + // `stopCli` is cooperative and cannot be the only stop mechanism: it resolves without + // effect when the transport is already closed, and never settles when the CLI has stopped + // servicing the connection. Escalate to signalling the process group once the CLI has had + // a chance to exit on its own, so a CLI that ignores the request cannot outlive the + // session and keep the AppHost and its resource processes alive. + this.scheduleCliProcessTermination(); } }); diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index 78027c4d3e4..f11c9c56488 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -172,6 +172,91 @@ suite('AspireExtensionContext', () => { warnStub.restore(); } }); + + test('deactivation stops a debug session registered while an earlier stop is in flight', async () => { + const order: string[] = []; + const context = createContext(order); + const firstStop = createDeferred(); + addSession(context, 'first', () => { + order.push('stop first'); + return firstStop.promise; + }, () => order.push('dispose first')); + + const shutdown = deactivateContext(context); + await Promise.resolve(); + assert.deepStrictEqual(order, ['stop first']); + + // `_isShuttingDown` does not gate `addAspireDebugSession`, so a debug-adapter descriptor + // or an RPC-triggered `startDebugSession` can still register a session at exactly this + // point. Snapshotting the session array once would leave this one running. + addSession(context, 'late', () => { + order.push('stop late'); + return Promise.resolve(); + }, () => order.push('dispose late')); + + firstStop.resolve(); + await shutdown; + + assert.ok(order.includes('stop late'), `A session registered during shutdown must still be asked to stop: ${JSON.stringify(order)}`); + assert.ok(order.indexOf('stop late') < order.indexOf('rpc server'), `The late stop must happen before the transport is disposed: ${JSON.stringify(order)}`); + }); + + test('deactivation terminates the CLI process group after the cooperative stop resolves', async () => { + const order: string[] = []; + const context = createContext(order); + addSession(context, 'session', () => { + order.push('stop session'); + return Promise.resolve(); + }, () => order.push('dispose session'), () => order.push('terminate session')); + + await deactivateContext(context); + + // A resolved `stopCli` proves the request was accepted, not that the process exited, so + // the process group is signalled regardless before teardown continues. + assert.deepStrictEqual(order, [ + 'stop session', + 'terminate session', + 'dispose session', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + }); + + test('deactivation terminates the CLI process group when the cooperative stop never settles', async () => { + const order: string[] = []; + const context = createContext(order); + const warnStub = sinon.stub(extensionLogOutputChannel, 'warn'); + const clock = sinon.useFakeTimers(); + // A CLI that stopped servicing its connection leaves the request pending forever. The + // timeout only ends the wait, so without an explicit signal the process would survive. + addSession(context, 'hung', () => { + order.push('stop hung'); + return new Promise(() => { }); + }, () => order.push('dispose hung'), () => order.push('terminate hung')); + + try { + const shutdown = deactivateContext(context); + await clock.tickAsync(5_000); + await shutdown; + + assert.deepStrictEqual(order, [ + 'stop hung', + 'terminate hung', + 'dispose hung', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + sinon.assert.calledWithMatch(warnStub, 'Timed out after 5000ms waiting for Aspire CLI stop requests'); + } + finally { + clock.restore(); + warnStub.restore(); + } + }); }); function createContext(order: string[]): AspireExtensionContext { @@ -186,12 +271,13 @@ function createContext(order: string[]): AspireExtensionContext { return context; } -function addSession(context: AspireExtensionContext, debugSessionId: string, stopCli: () => Promise, dispose: () => void): void { +function addSession(context: AspireExtensionContext, debugSessionId: string, stopCli: () => Promise, dispose: () => void, terminateCliProcessTree: () => void = () => { }): void { context.addAspireDebugSession({ debugSessionId, onDidChangeState: () => ({ dispose: () => { } }), onDidSendDebugConsoleOutput: () => ({ dispose: () => { } }), requestCliStopForExtensionShutdown: stopCli, + terminateCliProcessTree, dispose, } as unknown as AspireDebugSession); } diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index bd96fb28f01..cb8b1bd32eb 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -1,9 +1,13 @@ import * as assert from 'assert'; import type { TelemetryReporter } from '@vscode/extension-telemetry'; import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; +import type { ChildProcessWithoutNullStreams } from 'node:child_process'; import { join } from 'node:path'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; +import * as cliModule from '../debugger/languages/cli'; import { AspireDebugSession, buildAspireCommandArgs, getLoggableDebugConfiguration } from '../debugger/AspireDebugSession'; import { appHostTelemetryTargetPathConfigKey } from '../debugger/AspireDebugConfigurationMetadata'; import { AspireResourceExtendedDebugConfiguration } from '../dcp/types'; @@ -62,8 +66,7 @@ suite('AspireDebugSession tests', () => { tempDirs.length = 0; }); - test('extension shutdown reuses an in-flight CLI stop request', async () => { - let completeStop!: () => void; + test('extension shutdown reuses an in-flight CLI stop request', async () => { let completeStop!: () => void; const stopRequest = new Promise(resolve => { completeStop = resolve; }); @@ -85,6 +88,44 @@ suite('AspireDebugSession tests', () => { await firstRequest; }); + test('spawns the Aspire CLI as a process-group leader and retains the child process', async () => { + const cliProcess = createFakeCliProcess(4321); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + try { + const aspireDebugSession = createSessionForSpawn(); + + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + const options = spawnStub.firstCall.args[3]; + // Without a process group there is no way to signal the AppHost and resource processes + // the CLI owns, and without retaining the child there is nothing to signal at all. + assert.strictEqual(options?.createProcessGroup, true); + assert.strictEqual((aspireDebugSession as any)._cliProcess, cliProcess); + } + finally { + spawnStub.restore(); + } + }); + + test('terminateCliProcessTree signals a running CLI process and does nothing once it exited', () => { + const running = createFakeCliProcess(4322); + const aspireDebugSession = createSessionForSpawn(); + (aspireDebugSession as any)._cliProcess = running; + + aspireDebugSession.terminateCliProcessTree(); + + // The cooperative `stopCli` RPC cannot terminate the process, so the signal is what + // actually ends the CLI and the resource tree beneath it. + sinon.assert.called(running.kill); + + const exited = createFakeCliProcess(4323, 0); + (aspireDebugSession as any)._cliProcess = exited; + + aspireDebugSession.terminateCliProcessTree(); + + sinon.assert.notCalled(exited.kill); + }); + test('suppresses the Aspire CLI first-run banner for extension-managed launches', async () => { const parentDebugSession = { id: 'aspire-session', @@ -1505,4 +1546,35 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); await clock.tickAsync(10); } } + + function createSessionForSpawn(): AspireDebugSession { + const parentDebugSession = { + id: 'aspire-session', + configuration: {}, + } as unknown as vscode.DebugSession; + + return new AspireDebugSession( + parentDebugSession, + { onNewConnection: () => ({ dispose: () => { } }) } as any, + { recordAppHostProcessExit: () => { } } as any, + { + getAspireCliExecutablePath: async () => '/usr/local/bin/aspire', + createEnvironment: () => ({}), + } as any, + () => { }); + } + + function createFakeCliProcess(pid: number, exitCode: number | null = null): ChildProcessWithoutNullStreams & { kill: sinon.SinonStub } { + const kill = sinon.stub().returns(true); + return Object.assign(new EventEmitter(), { + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + killed: false, + exitCode, + signalCode: null, + pid, + kill, + }) as unknown as ChildProcessWithoutNullStreams & { kill: sinon.SinonStub }; + } }); From 67519d5f33b1b2f9027b746dffcd73f4d22c8343 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 16:59:18 -0400 Subject: [PATCH 05/24] Refuse debug sessions registered after teardown and never spawn into one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The re-snapshot loop asks every session that appears *before* teardown to stop, but `_disposeCore` disposes exactly the sessions present when it takes its snapshot and never runs again. A session registered after that point was still tracked forever and never disposed, so its CLI kept running with nothing left alive to stop it. `addAspireDebugSession` now refuses and disposes once `_isDisposed` is set; the pre-teardown window is unchanged and still handled by the drain. `spawnAspireCommand` awaits the CLI path before spawning, so deactivation can complete inside that await. Spawning afterwards produced an `aspire run` that no teardown path could reach — and now that it is spawned detached as a process-group leader, one that would not even die with the extension host. Two fixes to the tests added alongside the process-group change: - `terminateCliProcessTree signals a running CLI process` ran the real `terminateCliProcess`, which on Windows shells out to `taskkill /pid /t` rather than calling `child.kill`. The assertion would have failed on the Windows CI agents, and the run would have signalled whatever process owned PID 4322 there. It now stubs the module function. - Restore the newline that was lost from the `reuses an in-flight CLI stop request` test declaration. Also drops the `if (deactivate)` fallback in the test helper. `deactivate` is a declared method, so the fallback could never run, and had it ever run it would have silently retargeted the suite at `dispose()`. Verified red-green: reverting the two guards fails exactly the two new tests and nothing else. 1476 passing, 0 failing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/AspireExtensionContext.ts | 11 +++++ extension/src/debugger/AspireDebugSession.ts | 13 ++++- .../src/test/AspireExtensionContext.test.ts | 39 ++++++++++++--- extension/src/test/aspireDebugSession.test.ts | 49 +++++++++++++++++-- 4 files changed, 99 insertions(+), 13 deletions(-) diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index 32aa52106d2..8da7e95e7f8 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -74,6 +74,17 @@ export class AspireExtensionContext implements vscode.Disposable { } addAspireDebugSession(debugSession: AspireDebugSession) { + if (this._isDisposed) { + // `_disposeCore` disposes exactly the sessions present when it takes its snapshot, and + // it never runs twice. Tracking a session that arrives afterwards would therefore mean + // never disposing it at all: its CLI would keep running with nothing left alive to stop + // it. Sessions arriving *before* teardown are still accepted — `_waitForCliStopRequests` + // re-scans for them, and `_disposeCore` then disposes them on the normal path. + extensionLogOutputChannel.warn(`Refusing Aspire debug session ${debugSession.debugSessionId} because the extension has already been torn down; disposing it immediately.`); + debugSession.dispose(); + return; + } + if (this._aspireDebugSessions.find(session => session.debugSessionId === debugSession.debugSessionId)) { throw new Error(debugSessionAlreadyExists(debugSession.debugSessionId)); } diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 667e6945ac2..9e0409dff6e 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -455,9 +455,20 @@ export class AspireDebugSession implements vscode.DebugAdapter { return partial; }; + const cliPath = await this._terminalProvider.getAspireCliExecutablePath(); + if (this._disposed) { + // Resolving the CLI path is asynchronous, so extension deactivation can complete between the + // launch request and this point. Spawning now would produce an `aspire run` that no teardown + // path is left to stop — and because it is spawned detached as a process-group leader, it + // would not even die with the extension host. + extensionLogOutputChannel.info(`Skipping Aspire CLI launch for disposed debug session ${this.debugSessionId}.`); + disposable.dispose(); + return; + } + this._cliProcess = spawnCliProcess( this._terminalProvider, - await this._terminalProvider.getAspireCliExecutablePath(), + cliPath, args, { stdoutCallback: (data) => { diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index f11c9c56488..4bd73711940 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -257,6 +257,37 @@ suite('AspireExtensionContext', () => { warnStub.restore(); } }); + + test('a debug session registered after teardown is refused and disposed rather than tracked forever', async () => { + const order: string[] = []; + const context = createContext(order); + const warnStub = sinon.stub(extensionLogOutputChannel, 'warn'); + + try { + await deactivateContext(context); + + // The drain loop above re-scans only until teardown starts. `_disposeCore` has since + // taken and emptied its snapshot, and it never runs again, so a session accepted here + // would keep its CLI alive with nothing left alive to stop it. + addSession(context, 'late', () => { + order.push('stop late'); + return Promise.resolve(); + }, () => order.push('dispose late')); + + assert.deepStrictEqual(context.aspireDebugSessions, []); + assert.deepStrictEqual(order, [ + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + 'dispose late', + ]); + sinon.assert.calledWithMatch(warnStub, 'Refusing Aspire debug session late because the extension has already been torn down'); + } + finally { + warnStub.restore(); + } + }); }); function createContext(order: string[]): AspireExtensionContext { @@ -283,13 +314,7 @@ function addSession(context: AspireExtensionContext, debugSessionId: string, sto } function deactivateContext(context: AspireExtensionContext): Promise { - const deactivate = (context as AspireExtensionContext & { deactivate?: () => Promise }).deactivate; - if (deactivate) { - return deactivate.call(context); - } - - context.dispose(); - return Promise.resolve(); + return context.deactivate(); } function createDeferred(): { promise: Promise; resolve: (value: T) => void } { diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index cb8b1bd32eb..ab44f0dfcc3 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -9,6 +9,7 @@ import * as sinon from 'sinon'; import * as vscode from 'vscode'; import * as cliModule from '../debugger/languages/cli'; import { AspireDebugSession, buildAspireCommandArgs, getLoggableDebugConfiguration } from '../debugger/AspireDebugSession'; +import { extensionLogOutputChannel } from '../utils/logging'; import { appHostTelemetryTargetPathConfigKey } from '../debugger/AspireDebugConfigurationMetadata'; import { AspireResourceExtendedDebugConfiguration } from '../dcp/types'; import { __resetCommonPropertiesForTests, __setReporterForTests } from '../utils/telemetry'; @@ -66,7 +67,8 @@ suite('AspireDebugSession tests', () => { tempDirs.length = 0; }); - test('extension shutdown reuses an in-flight CLI stop request', async () => { let completeStop!: () => void; + test('extension shutdown reuses an in-flight CLI stop request', async () => { + let completeStop!: () => void; const stopRequest = new Promise(resolve => { completeStop = resolve; }); @@ -108,6 +110,11 @@ suite('AspireDebugSession tests', () => { }); test('terminateCliProcessTree signals a running CLI process and does nothing once it exited', () => { + // `terminateCliProcess` is stubbed rather than executed: on Windows it shells out to + // `taskkill /pid /t` instead of calling `child.kill`, so running it for real would + // both fail this assertion on the Windows CI agents and signal whatever process happens to + // own the made-up PID there. + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); const running = createFakeCliProcess(4322); const aspireDebugSession = createSessionForSpawn(); (aspireDebugSession as any)._cliProcess = running; @@ -116,14 +123,46 @@ suite('AspireDebugSession tests', () => { // The cooperative `stopCli` RPC cannot terminate the process, so the signal is what // actually ends the CLI and the resource tree beneath it. - sinon.assert.called(running.kill); + sinon.assert.calledOnce(terminateStub); + assert.strictEqual(terminateStub.firstCall.args[0], running); const exited = createFakeCliProcess(4323, 0); (aspireDebugSession as any)._cliProcess = exited; aspireDebugSession.terminateCliProcessTree(); - sinon.assert.notCalled(exited.kill); + sinon.assert.calledOnce(terminateStub); + }); + + test('a launch that resolves the CLI path after disposal does not spawn an orphan CLI', async () => { + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess'); + const infoStub = sinon.stub(extensionLogOutputChannel, 'info'); + let releaseCliPath!: (cliPath: string) => void; + const cliPath = new Promise(resolve => { + releaseCliPath = resolve; + }); + let cliPathRequested!: () => void; + const cliPathRequestObserved = new Promise(resolve => { + cliPathRequested = resolve; + }); + const aspireDebugSession = createSessionForSpawn(() => { + cliPathRequested(); + return cliPath; + }); + + const spawning = aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + await cliPathRequestObserved; + // Deactivation can complete while the CLI path is still resolving. Set the state `dispose()` + // establishes rather than calling it, so this covers the spawn guard alone and not VS Code's + // parent-session teardown. + (aspireDebugSession as any)._disposed = true; + releaseCliPath('/usr/local/bin/aspire'); + await spawning; + + // A detached process group spawned here would outlive the extension host itself. + sinon.assert.notCalled(spawnStub); + assert.strictEqual((aspireDebugSession as any)._cliProcess, undefined); + sinon.assert.calledWithMatch(infoStub, 'Skipping Aspire CLI launch for disposed debug session'); }); test('suppresses the Aspire CLI first-run banner for extension-managed launches', async () => { @@ -1547,7 +1586,7 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); } } - function createSessionForSpawn(): AspireDebugSession { + function createSessionForSpawn(getAspireCliExecutablePath: () => Promise = async () => '/usr/local/bin/aspire'): AspireDebugSession { const parentDebugSession = { id: 'aspire-session', configuration: {}, @@ -1558,7 +1597,7 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); { onNewConnection: () => ({ dispose: () => { } }) } as any, { recordAppHostProcessExit: () => { } } as any, { - getAspireCliExecutablePath: async () => '/usr/local/bin/aspire', + getAspireCliExecutablePath, createEnvironment: () => ({}), } as any, () => { }); From 512c887ad19ccc365d9c8150862c39fcd2d43283 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 17:14:51 -0400 Subject: [PATCH 06/24] Collect the CLI process group on self-exit and force-kill on deactivation An Aspire CLI spawned with createProcessGroup leads a detached group that the AppHost and every resource process joins. Two paths let that group outlive the extension: - When the CLI exited on its own, the exit callback only cancelled the escalation timer and terminateCliProcessTree early-returned on an exited leader, so nothing ever signalled the surviving descendants. terminateCliProcess already reaps a managed group whose leader has exited; it just was not being invoked. Collect synchronously from the exit callback, because once the leader's PID is released the OS may recycle it as another group's id. - The deactivation sweep sent SIGTERM and scheduled the hard kill on an unref'd timer, but _deactivateCore resolves as soon as the sweep returns, so the host could exit first and leave a CLI that ignored SIGTERM alive. Deactivation has already spent its 5s cooperative window, so it now forces immediately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/AspireExtensionContext.ts | 6 ++++- extension/src/debugger/AspireDebugSession.ts | 17 ++++++++---- extension/src/debugger/languages/cli.ts | 12 ++++++++- .../src/test/AspireExtensionContext.test.ts | 19 +++++++++++++- extension/src/test/aspireDebugSession.test.ts | 26 ++++++++++++++++++- extension/src/test/cliSpawn.test.ts | 26 +++++++++++++++++++ 6 files changed, 97 insertions(+), 9 deletions(-) diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index 8da7e95e7f8..171ac6845f8 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -168,7 +168,11 @@ export class AspireExtensionContext implements vscode.Disposable { // cannot leave an AppHost and its resource processes orphaned. for (const session of this._aspireDebugSessions) { try { - session.terminateCliProcessTree(); + // Force rather than signal-and-schedule: `terminateCliProcess` escalates to a hard + // kill on an `unref`'d timer, and `_deactivateCore` resolves immediately after this + // sweep, so the extension host can exit before that timer fires. The cooperative + // deadline above was this CLI's grace period; there is no second one. + session.terminateCliProcessTree({ force: true }); } catch (error) { extensionLogOutputChannel.warn(`Failed to terminate the Aspire CLI process during extension deactivation: ${error}`); diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 9e0409dff6e..8670a98f34a 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -163,14 +163,17 @@ export class AspireDebugSession implements vscode.DebugAdapter { * signalled directly whenever the cooperative path did not finish the job. No-ops when the CLI * already exited, which is the normal case. */ - terminateCliProcessTree(): void { + terminateCliProcessTree(options?: { force?: boolean }): void { this.cancelScheduledCliProcessTermination(); const cliProcess = this._cliProcess; - if (!cliProcess || cliProcess.exitCode !== null || cliProcess.signalCode !== null) { + if (!cliProcess) { return; } - terminateCliProcess(cliProcess, `Aspire CLI for debug session ${this.debugSessionId}`); + // Deliberately not skipped once the leader has exited. `terminateCliProcess` reaps the surviving + // members of a managed process group in that case, and that is the only path that collects + // AppHost and resource processes which outlived the CLI that owned them. + terminateCliProcess(cliProcess, `Aspire CLI for debug session ${this.debugSessionId}`, options); } private scheduleCliProcessTermination(): void { @@ -482,8 +485,12 @@ export class AspireDebugSession implements vscode.DebugAdapter { vscode.window.showErrorMessage(processExceptionOccurred(error.message, commandLabel)); }, exitCallback: (code) => { - // The CLI came down on its own, so the escalation timer has nothing left to signal. - this.cancelScheduledCliProcessTermination(); + // The leader came down on its own, so the escalation timer has nothing left to signal — + // but a detached leader's descendants (the AppHost and every resource process beneath it) + // can outlive it, and this is the last moment they can be collected safely: once the + // leader's PID is released the operating system may recycle it as another group's id, and + // a later negative-PID signal would land on something unrelated. + this.terminateCliProcessTree(); this._dcpServer.recordAppHostProcessExit(this.debugSessionId, code); // Flush any partial line left in either buffer so trailing output isn't lost. if (stdoutBuffer.length > 0) { diff --git a/extension/src/debugger/languages/cli.ts b/extension/src/debugger/languages/cli.ts index 4030854c8fe..8a91beb4f33 100644 --- a/extension/src/debugger/languages/cli.ts +++ b/extension/src/debugger/languages/cli.ts @@ -109,7 +109,7 @@ export function spawnCliProcess(terminalProvider: AspireTerminalProvider, comman return child; } -export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams, description: string, options?: { suppressTimeoutWarning?: boolean }): void { +export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams, description: string, options?: { suppressTimeoutWarning?: boolean; force?: boolean }): void { const processGroupPid = process.platform !== 'win32' && managedPosixProcessGroups.has(childProcess) ? childProcess.pid : undefined; @@ -165,6 +165,16 @@ export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams return; } + if (options?.force) { + // Skip the graceful signal and its escalation timer entirely. The timer below is `unref`'d, + // so a caller that is itself shutting down — extension deactivation, which resolves as soon + // as this returns — can have its host exit before the timer ever fires, leaving a process + // that ignored SIGTERM alive along with its whole tree. Such callers have already spent + // their own cooperative window, so the only signal left that means anything is the hard one. + forceTermination(); + return; + } + try { if (!childProcess.killed) { const signalSent = terminateCliProcessTree(childProcess, false); diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index 4bd73711940..26f25bb7db2 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -258,6 +258,23 @@ suite('AspireExtensionContext', () => { } }); + test('deactivation force-terminates rather than relying on the unref-d escalation timer', async () => { + const order: string[] = []; + const context = createContext(order); + const terminateOptions: Array<{ force?: boolean } | undefined> = []; + addSession(context, 'session', () => { + order.push('stop session'); + return Promise.resolve(); + }, () => order.push('dispose session'), options => terminateOptions.push(options)); + + await deactivateContext(context); + + // `terminateCliProcess` escalates to a hard kill on an `unref`'d timer, and deactivation + // resolves as soon as this sweep returns, so the extension host can exit before that timer + // fires and leave a CLI that ignored SIGTERM alive. + assert.deepStrictEqual(terminateOptions, [{ force: true }]); + }); + test('a debug session registered after teardown is refused and disposed rather than tracked forever', async () => { const order: string[] = []; const context = createContext(order); @@ -302,7 +319,7 @@ function createContext(order: string[]): AspireExtensionContext { return context; } -function addSession(context: AspireExtensionContext, debugSessionId: string, stopCli: () => Promise, dispose: () => void, terminateCliProcessTree: () => void = () => { }): void { +function addSession(context: AspireExtensionContext, debugSessionId: string, stopCli: () => Promise, dispose: () => void, terminateCliProcessTree: (options?: { force?: boolean }) => void = () => { }): void { context.addAspireDebugSession({ debugSessionId, onDidChangeState: () => ({ dispose: () => { } }), diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index ab44f0dfcc3..1e86beca319 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -109,7 +109,7 @@ suite('AspireDebugSession tests', () => { } }); - test('terminateCliProcessTree signals a running CLI process and does nothing once it exited', () => { + test('terminateCliProcessTree signals a running CLI process and still collects an exited one', () => { // `terminateCliProcess` is stubbed rather than executed: on Windows it shells out to // `taskkill /pid /t` instead of calling `child.kill`, so running it for real would // both fail this assertion on the Windows CI agents and signal whatever process happens to @@ -131,7 +131,31 @@ suite('AspireDebugSession tests', () => { aspireDebugSession.terminateCliProcessTree(); + // An exited leader is still forwarded: `terminateCliProcess` reaps the surviving members of + // its managed process group, which is the only path that collects an AppHost and resource + // processes that outlived the CLI. + sinon.assert.calledTwice(terminateStub); + assert.strictEqual(terminateStub.secondCall.args[0], exited); + }); + + test('a CLI process that exits on its own still has its process group collected', async () => { + // Already exited: the leader is gone by the time the exit callback runs, which is exactly + // the state the old early return skipped on. + const cliProcess = createFakeCliProcess(4324, 0); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); + sinon.stub(vscode.debug, 'stopDebugging').resolves(); + const aspireDebugSession = createSessionForSpawn(); + + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + spawnStub.firstCall.args[3]?.exitCallback?.(0); + + // The CLI is gone but the AppHost and resource processes in its detached group need not be, + // and once the leader's PID is released the group id can be recycled — so the collection has + // to happen here rather than on a later timer. sinon.assert.calledOnce(terminateStub); + assert.strictEqual(terminateStub.firstCall.args[0], cliProcess); }); test('a launch that resolves the CLI path after disposal does not spawn an orphan CLI', async () => { diff --git a/extension/src/test/cliSpawn.test.ts b/extension/src/test/cliSpawn.test.ts index 199238327a3..ce789b0235e 100644 --- a/extension/src/test/cliSpawn.test.ts +++ b/extension/src/test/cliSpawn.test.ts @@ -474,6 +474,32 @@ suite('spawnCliProcess tests', () => { platformStub.restore(); } }); + test('force terminates a POSIX process group immediately without waiting for the grace period', async () => { + const platformStub = sinon.stub(process, 'platform').value('linux'); + const processKillStub = sinon.stub(process, 'kill').returns(true); + const clock = sinon.useFakeTimers(); + const childProcess = createTestChildProcess(4646); + const spawnStub = sinon.stub(nodeChildProcess, 'spawn').returns(childProcess); + const terminalProvider = { createEnvironment: () => ({}) } as AspireTerminalProvider; + + try { + const child = spawnCliProcess(terminalProvider, '/usr/local/bin/aspire', ['run'], { createProcessGroup: true }); + terminateCliProcess(child, 'test Aspire CLI', { force: true }); + + // No SIGTERM and no escalation timer: a caller that is itself shutting down cannot rely + // on an `unref`'d timer still being there five seconds later. + assert.deepStrictEqual(processKillStub.args, [[-4646, 'SIGKILL']]); + + await clock.tickAsync(5000); + assert.strictEqual(processKillStub.callCount, 1); + } + finally { + spawnStub.restore(); + clock.restore(); + processKillStub.restore(); + platformStub.restore(); + } + }); }); function createTestChildProcess(pid: number, exitCode: number | null = null): nodeChildProcess.ChildProcessWithoutNullStreams & { kill: sinon.SinonStub } { From 08ecfe2f2b296b31750f8b49b221feff65c5cdb0 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Fri, 7 Aug 2026 17:50:43 -0400 Subject: [PATCH 07/24] Cover the Windows taskkill termination path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `terminateCliProcess` never calls `child.kill` on Windows: it spawns `taskkill.exe /pid /t` so the descendants come down with the leader, and only falls back to `child.kill` from taskkill's error handler. That branch had no coverage anywhere, which is how a test asserting `child.kill` reached CI — it passed on macOS and Linux and could only fail on the Windows unit-test job, the one leg with no counterpart on another platform. Assert the taskkill invocation and that the child is not signalled directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/test/cliSpawn.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/extension/src/test/cliSpawn.test.ts b/extension/src/test/cliSpawn.test.ts index ce789b0235e..8399597ed45 100644 --- a/extension/src/test/cliSpawn.test.ts +++ b/extension/src/test/cliSpawn.test.ts @@ -474,6 +474,30 @@ suite('spawnCliProcess tests', () => { platformStub.restore(); } }); + test('terminates the process tree with taskkill on Windows rather than signalling the child', () => { + // Regression coverage for the Windows CI break: `terminateCliProcess` deliberately never + // calls `child.kill` on Windows, because killing the leader there orphans its descendants. + // A test that asserts on `child.kill` therefore passes on POSIX and fails on Windows. + const platformStub = sinon.stub(process, 'platform').value('win32'); + const spawned: Array<{ command: string; args: readonly string[] }> = []; + const spawnStub = sinon.stub(nodeChildProcess, 'spawn').callsFake(((command: string, args: readonly string[]) => { + spawned.push({ command, args }); + return Object.assign(new EventEmitter(), { unref: () => { } }) as unknown as nodeChildProcess.ChildProcessWithoutNullStreams; + }) as unknown as typeof nodeChildProcess.spawn); + const child = createTestChildProcess(4747); + + try { + terminateCliProcess(child, 'test Aspire CLI'); + + assert.deepStrictEqual(spawned, [{ command: 'taskkill.exe', args: ['/pid', '4747', '/t'] }]); + assert.strictEqual(child.kill.callCount, 0); + } + finally { + spawnStub.restore(); + platformStub.restore(); + } + }); + test('force terminates a POSIX process group immediately without waiting for the grace period', async () => { const platformStub = sinon.stub(process, 'platform').value('linux'); const processKillStub = sinon.stub(process, 'kill').returns(true); From 64bcad386055d8fe3f47dc8fe9236d52338ea2c8 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sat, 8 Aug 2026 13:30:45 -0400 Subject: [PATCH 08/24] Fix Windows CLI shutdown ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/AspireExtensionContext.ts | 12 ++- extension/src/debugger/AspireDebugSession.ts | 44 +++++++--- extension/src/debugger/languages/cli.ts | 24 +++++- .../src/test/AspireExtensionContext.test.ts | 65 +++++++++++++++ extension/src/test/cli.test.ts | 82 +++++++++++++++++++ 5 files changed, 212 insertions(+), 15 deletions(-) create mode 100644 extension/src/test/cli.test.ts diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index 171ac6845f8..e98c12663d0 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -66,11 +66,13 @@ export class AspireExtensionContext implements vscode.Disposable { return null; } - return this._aspireDebugSessions.find(session => session.debugSessionId === debugSessionId) || null; + return this._aspireDebugSessions.find(session => session.debugSessionId === debugSessionId && !session.isDisposed) || null; } get aspireDebugSessions(): readonly AspireDebugSession[] { - return [...this._aspireDebugSessions]; + // Disposed sessions can remain tracked only as CLI process owners. They must still be + // visible to deactivation, but not to RPC lookups or extension-state snapshots. + return this._aspireDebugSessions.filter(session => !session.isDisposed); } addAspireDebugSession(debugSession: AspireDebugSession) { @@ -166,7 +168,7 @@ export class AspireExtensionContext implements vscode.Disposable { // A cooperative stop that resolved, rejected or timed out proves only what happened to the // RPC request; the CLI process can still be running. Signal any that are, so deactivation // cannot leave an AppHost and its resource processes orphaned. - for (const session of this._aspireDebugSessions) { + for (const session of [...this._aspireDebugSessions]) { try { // Force rather than signal-and-schedule: `terminateCliProcess` escalates to a hard // kill on an `unref`'d timer, and `_deactivateCore` resolves immediately after this @@ -187,6 +189,10 @@ export class AspireExtensionContext implements vscode.Disposable { private _collectStopRequests(requested: Map>): boolean { let addedRequest = false; for (const session of this._aspireDebugSessions) { + if (session.isDisposed) { + continue; + } + if (requested.has(session.debugSessionId)) { continue; } diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 8670a98f34a..c852143feda 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -69,6 +69,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { private readonly _rpcServer: AspireRpcServer; private readonly _dcpServer: AspireDcpServer; private readonly _terminalProvider: AspireTerminalProvider; + private readonly _removeAspireDebugSession: (session: AspireDebugSession) => void; private _appHostDebugSession?: AspireResourceDebugSession = undefined; private _resourceDebugSessions: AspireResourceDebugSession[] = []; @@ -80,10 +81,12 @@ export class AspireDebugSession implements vscode.DebugAdapter { private readonly _onDidChangeState = new EventEmitter(); private readonly _disposables: vscode.Disposable[] = []; private _disposed = false; + private _removedFromExtensionContext = false; private _parentStopPromise: Thenable | undefined; private _cliStopPromise: Promise | undefined; private _cliProcess: ChildProcessWithoutNullStreams | undefined; private _cliTerminationTimer: ReturnType | undefined; + private _cliProcessTreeTerminationAttempted = false; // Timestamp for the `debug/apphost/end` duration measurement. Captured the first // time we observe a `launch` request so it covers the actual user-visible session // lifetime, not the moment the AspireDebugSession object was constructed. @@ -119,18 +122,19 @@ export class AspireDebugSession implements vscode.DebugAdapter { return this._startupCompleted; } + get isDisposed(): boolean { + return this._disposed; + } + constructor(session: vscode.DebugSession, rpcServer: AspireRpcServer, dcpServer: AspireDcpServer, terminalProvider: AspireTerminalProvider, removeAspireDebugSession: (session: AspireDebugSession) => void, debugSessionId: string = generateDcpIdPrefix()) { this._session = session; this._rpcServer = rpcServer; this._dcpServer = dcpServer; this._terminalProvider = terminalProvider; + this._removeAspireDebugSession = removeAspireDebugSession; this.configuration = session.configuration as AspireExtendedDebugConfiguration; this.debugSessionId = debugSessionId; - - this._disposables.push({ - dispose: () => removeAspireDebugSession(this) - }); } async stopDebugging(): Promise { @@ -155,13 +159,13 @@ export class AspireDebugSession implements vscode.DebugAdapter { } /** - * Signals the `aspire` CLI process group when the process is still alive. + * Signals the `aspire` CLI process tree. * * The cooperative `stopCli` RPC resolving proves only that the request was accepted, and on a * closed transport it resolves having done nothing at all. Neither outcome terminates the CLI, * so the process it owns — the AppHost and every resource process beneath it — has to be - * signalled directly whenever the cooperative path did not finish the job. No-ops when the CLI - * already exited, which is the normal case. + * signalled directly whenever the cooperative path did not finish the job. The leader may already + * have exited by then, but that does not prove its descendants exited too. */ terminateCliProcessTree(options?: { force?: boolean }): void { this.cancelScheduledCliProcessTermination(); @@ -173,19 +177,25 @@ export class AspireDebugSession implements vscode.DebugAdapter { // Deliberately not skipped once the leader has exited. `terminateCliProcess` reaps the surviving // members of a managed process group in that case, and that is the only path that collects // AppHost and resource processes which outlived the CLI that owned them. + this._cliProcessTreeTerminationAttempted = true; terminateCliProcess(cliProcess, `Aspire CLI for debug session ${this.debugSessionId}`, options); + if (this._disposed) { + this.releaseExtensionContextOwnership(); + } } private scheduleCliProcessTermination(): void { - if (!this._cliProcess || this._cliTerminationTimer) { + if (!this._cliProcess || this._cliTerminationTimer || this._cliProcessTreeTerminationAttempted) { return; } // Give the cooperative stop the first chance so the CLI can shut its resources down cleanly; - // only a CLI that is still alive afterwards gets signalled. + // after this timer fires the session may be disposed and unowned except for the extension + // context, so use the hard-kill path rather than scheduling another unref'd escalation. this._cliTerminationTimer = setTimeout(() => { this._cliTerminationTimer = undefined; - this.terminateCliProcessTree(); + this.terminateCliProcessTree({ force: true }); + this.releaseExtensionContextOwnership(); }, AspireDebugSession._cliCooperativeStopGraceMs); this._cliTerminationTimer.unref?.(); } @@ -197,6 +207,15 @@ export class AspireDebugSession implements vscode.DebugAdapter { } } + private releaseExtensionContextOwnership(): void { + if (this._removedFromExtensionContext) { + return; + } + + this._removedFromExtensionContext = true; + this._removeAspireDebugSession(this); + } + private stopParentDebugSessionOnce(): Thenable { if (this._parentStopPromise) { return this._parentStopPromise; @@ -932,6 +951,11 @@ export class AspireDebugSession implements vscode.DebugAdapter { this._trackedDebugAdapters = []; void this.stopParentDebugSessionOnce(); this._onDidSendDebugConsoleOutput.dispose(); + // Keep this disposed session tracked while its delayed CLI termination is pending, so + // extension deactivation can still force-drain the process tree before VS Code exits. + if (!this._cliTerminationTimer) { + this.releaseExtensionContextOwnership(); + } // Telemetry: emit `debug/apphost/end` after a short grace window so any // pending `sessionTerminated` notifications kicked off by the child-stop diff --git a/extension/src/debugger/languages/cli.ts b/extension/src/debugger/languages/cli.ts index 8a91beb4f33..e2aba4cc8fe 100644 --- a/extension/src/debugger/languages/cli.ts +++ b/extension/src/debugger/languages/cli.ts @@ -110,7 +110,8 @@ export function spawnCliProcess(terminalProvider: AspireTerminalProvider, comman } export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams, description: string, options?: { suppressTimeoutWarning?: boolean; force?: boolean }): void { - const processGroupPid = process.platform !== 'win32' && managedPosixProcessGroups.has(childProcess) + const isWindows = process.platform === 'win32'; + const processGroupPid = !isWindows && managedPosixProcessGroups.has(childProcess) ? childProcess.pid : undefined; let exited = childProcess.exitCode !== null || childProcess.signalCode !== null; @@ -161,8 +162,12 @@ export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams forceTermination(); } managedPosixProcessGroups.delete(childProcess); + return; + } + + if (!isWindows) { + return; } - return; } if (options?.force) { @@ -175,6 +180,21 @@ export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams return; } + if (exited) { + // Windows does not tie child lifetimes to the parent process. An exited CLI leader can + // still have an AppHost/resource tree underneath its recorded PID, so sweep it with + // taskkill instead of treating the leader's exit as proof that teardown completed. + try { + const signalSent = terminateCliProcessTree(childProcess, false); + if (!signalSent) { + extensionLogOutputChannel.warn(`Failed to terminate ${description}.`); + } + } catch (error) { + extensionLogOutputChannel.error(`Failed to terminate ${description}: ${String(error)}`); + } + return; + } + try { if (!childProcess.killed) { const signalSent = terminateCliProcessTree(childProcess, false); diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index 26f25bb7db2..db7be84bf2a 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -2,11 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. import * as assert from 'assert'; +import type { ChildProcessWithoutNullStreams } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; import { ErrorCodes, ResponseError } from 'vscode-jsonrpc'; import { AspireExtensionContext } from '../AspireExtensionContext'; import { AspireDebugSession } from '../debugger/AspireDebugSession'; +import * as cliModule from '../debugger/languages/cli'; import { extensionLogOutputChannel } from '../utils/logging'; suite('AspireExtensionContext', () => { @@ -275,6 +279,37 @@ suite('AspireExtensionContext', () => { assert.deepStrictEqual(terminateOptions, [{ force: true }]); }); + test('deactivation force-drains a disposed debug session with pending CLI termination', async () => { + const order: string[] = []; + const context = createContext(order); + const cliProcess = createFakeCliProcess(4321); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); + const stopDebuggingStub = sinon.stub(vscode.debug, 'stopDebugging').resolves(); + const aspireDebugSession = createSpawnedDebugSession(context); + context.addAspireDebugSession(aspireDebugSession); + + try { + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + aspireDebugSession.dispose(); + assert.deepStrictEqual(context.aspireDebugSessions, []); + + await deactivateContext(context); + + sinon.assert.calledOnceWithExactly( + terminateStub, + cliProcess, + `Aspire CLI for debug session ${aspireDebugSession.debugSessionId}`, + { force: true }); + } + finally { + spawnStub.restore(); + terminateStub.restore(); + stopDebuggingStub.restore(); + } + }); + test('a debug session registered after teardown is refused and disposed rather than tracked forever', async () => { const order: string[] = []; const context = createContext(order); @@ -342,3 +377,33 @@ function createDeferred(): { promise: Promise; resolve: (value: T) => void return { promise, resolve }; } + +function createSpawnedDebugSession(context: AspireExtensionContext): AspireDebugSession { + const parentDebugSession = { + id: 'aspire-session', + configuration: {}, + } as unknown as vscode.DebugSession; + + return new AspireDebugSession( + parentDebugSession, + { onNewConnection: () => ({ dispose: () => { } }) } as any, + { recordAppHostProcessExit: () => { } } as any, + { + getAspireCliExecutablePath: async () => '/usr/local/bin/aspire', + createEnvironment: () => ({}), + } as any, + context.removeAspireDebugSession.bind(context)); +} + +function createFakeCliProcess(pid: number): ChildProcessWithoutNullStreams { + return Object.assign(new EventEmitter(), { + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + killed: false, + exitCode: null, + signalCode: null, + pid, + kill: sinon.stub().returns(true), + }) as unknown as ChildProcessWithoutNullStreams; +} diff --git a/extension/src/test/cli.test.ts b/extension/src/test/cli.test.ts new file mode 100644 index 00000000000..59e6b3b0a66 --- /dev/null +++ b/extension/src/test/cli.test.ts @@ -0,0 +1,82 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +import * as assert from 'assert'; +import nodeChildProcess = require('child_process'); +import type { ChildProcessWithoutNullStreams } from 'child_process'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; +import * as sinon from 'sinon'; +import { terminateCliProcess } from '../debugger/languages/cli'; + +suite('CLI process termination', () => { + teardown(() => { + sinon.restore(); + }); + + test('forcefully terminates the Windows process tree for an already-exited leader', () => { + sinon.stub(process, 'platform').value('win32'); + const childProcess = createFakeCliProcess(4242, 0); + const taskkillUnref = sinon.stub(); + const spawnStub = sinon.stub(nodeChildProcess, 'spawn').callsFake((command: string, args?: readonly string[], options?: nodeChildProcess.SpawnOptions) => { + return Object.assign(new EventEmitter(), { + command, + args: [...(args ?? [])], + options, + unref: taskkillUnref, + }) as unknown as nodeChildProcess.ChildProcess; + }); + + terminateCliProcess(childProcess, 'Aspire CLI', { force: true }); + + sinon.assert.calledOnce(spawnStub); + assert.strictEqual(spawnStub.firstCall.args[0], 'taskkill.exe'); + assert.deepStrictEqual(spawnStub.firstCall.args[1], ['/pid', '4242', '/t', '/f']); + assert.deepStrictEqual(spawnStub.firstCall.args[2], { + stdio: 'ignore', + windowsHide: true, + }); + sinon.assert.calledOnce(taskkillUnref); + sinon.assert.notCalled(childProcess.kill); + }); + + test('terminates the Windows process tree for an already-exited leader', () => { + sinon.stub(process, 'platform').value('win32'); + const childProcess = createFakeCliProcess(4243, 0); + const taskkillUnref = sinon.stub(); + const spawnStub = sinon.stub(nodeChildProcess, 'spawn').callsFake((command: string, args?: readonly string[], options?: nodeChildProcess.SpawnOptions) => { + return Object.assign(new EventEmitter(), { + command, + args: [...(args ?? [])], + options, + unref: taskkillUnref, + }) as unknown as nodeChildProcess.ChildProcess; + }); + + terminateCliProcess(childProcess, 'Aspire CLI'); + + sinon.assert.calledOnce(spawnStub); + assert.strictEqual(spawnStub.firstCall.args[0], 'taskkill.exe'); + assert.deepStrictEqual(spawnStub.firstCall.args[1], ['/pid', '4243', '/t']); + assert.deepStrictEqual(spawnStub.firstCall.args[2], { + stdio: 'ignore', + windowsHide: true, + }); + sinon.assert.calledOnce(taskkillUnref); + sinon.assert.notCalled(childProcess.kill); + }); +}); + +function createFakeCliProcess(pid: number, exitCode: number | null): ChildProcessWithoutNullStreams & { kill: sinon.SinonStub } { + const kill = sinon.stub().returns(true); + return Object.assign(new EventEmitter(), { + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + killed: false, + exitCode, + signalCode: null, + pid, + kill, + }) as unknown as ChildProcessWithoutNullStreams & { kill: sinon.SinonStub }; +} From eea96b2ece453a8cc444d204c3306c70a200e70e Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sat, 8 Aug 2026 13:55:39 -0400 Subject: [PATCH 09/24] Limit exited Windows sweep to force shutdown Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/debugger/languages/cli.ts | 24 +++++++++-------------- extension/src/test/cli.test.ts | 26 ------------------------- 2 files changed, 9 insertions(+), 41 deletions(-) diff --git a/extension/src/debugger/languages/cli.ts b/extension/src/debugger/languages/cli.ts index e2aba4cc8fe..d649e05d3d3 100644 --- a/extension/src/debugger/languages/cli.ts +++ b/extension/src/debugger/languages/cli.ts @@ -168,6 +168,15 @@ export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams if (!isWindows) { return; } + + // Windows does not tie child lifetimes to the parent process. An exited CLI leader can + // still have an AppHost/resource tree underneath its recorded PID, so sweep it with + // taskkill during forceful shutdown instead of treating the leader's exit as proof that + // teardown completed. Non-force callers keep the historical no-op behavior because a short + // helper CLI can legitimately exit before its close handler observes it. + if (!options?.force) { + return; + } } if (options?.force) { @@ -180,21 +189,6 @@ export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams return; } - if (exited) { - // Windows does not tie child lifetimes to the parent process. An exited CLI leader can - // still have an AppHost/resource tree underneath its recorded PID, so sweep it with - // taskkill instead of treating the leader's exit as proof that teardown completed. - try { - const signalSent = terminateCliProcessTree(childProcess, false); - if (!signalSent) { - extensionLogOutputChannel.warn(`Failed to terminate ${description}.`); - } - } catch (error) { - extensionLogOutputChannel.error(`Failed to terminate ${description}: ${String(error)}`); - } - return; - } - try { if (!childProcess.killed) { const signalSent = terminateCliProcessTree(childProcess, false); diff --git a/extension/src/test/cli.test.ts b/extension/src/test/cli.test.ts index 59e6b3b0a66..b062976dcdc 100644 --- a/extension/src/test/cli.test.ts +++ b/extension/src/test/cli.test.ts @@ -39,32 +39,6 @@ suite('CLI process termination', () => { sinon.assert.calledOnce(taskkillUnref); sinon.assert.notCalled(childProcess.kill); }); - - test('terminates the Windows process tree for an already-exited leader', () => { - sinon.stub(process, 'platform').value('win32'); - const childProcess = createFakeCliProcess(4243, 0); - const taskkillUnref = sinon.stub(); - const spawnStub = sinon.stub(nodeChildProcess, 'spawn').callsFake((command: string, args?: readonly string[], options?: nodeChildProcess.SpawnOptions) => { - return Object.assign(new EventEmitter(), { - command, - args: [...(args ?? [])], - options, - unref: taskkillUnref, - }) as unknown as nodeChildProcess.ChildProcess; - }); - - terminateCliProcess(childProcess, 'Aspire CLI'); - - sinon.assert.calledOnce(spawnStub); - assert.strictEqual(spawnStub.firstCall.args[0], 'taskkill.exe'); - assert.deepStrictEqual(spawnStub.firstCall.args[1], ['/pid', '4243', '/t']); - assert.deepStrictEqual(spawnStub.firstCall.args[2], { - stdio: 'ignore', - windowsHide: true, - }); - sinon.assert.calledOnce(taskkillUnref); - sinon.assert.notCalled(childProcess.kill); - }); }); function createFakeCliProcess(pid: number, exitCode: number | null): ChildProcessWithoutNullStreams & { kill: sinon.SinonStub } { From 05514e8537a6506c2a88e646c3b6467a4a6c947e Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sat, 8 Aug 2026 14:05:36 -0400 Subject: [PATCH 10/24] Stabilize Azure Functions tools install Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/run-tests.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 8e4c4f3ebe3..7aee252cf21 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -257,8 +257,22 @@ jobs: - name: Install Azure Functions Core Tools if: runner.os == 'Linux' && (inputs.testShortName == 'Playground' || inputs.testShortName == 'Azure') + shell: bash run: | - npm i -g azure-functions-core-tools@4 --unsafe-perm true + set -euo pipefail + core_tools_version='4.12.1' + core_tools_directory="$RUNNER_TEMP/azure-functions-core-tools" + core_tools_archive="$RUNNER_TEMP/Azure.Functions.Cli.linux-x64.${core_tools_version}.zip" + curl --fail --location --retry 3 --retry-all-errors \ + --output "$core_tools_archive" \ + "https://github.com/Azure/azure-functions-core-tools/releases/download/${core_tools_version}/Azure.Functions.Cli.linux-x64.${core_tools_version}.zip" + echo 'faf8fb8d50b5293df338bec70594b12f45730e9fe251805298859b2238cf627e '"$core_tools_archive" | sha256sum --check - + mkdir -p "$core_tools_directory" + unzip -q "$core_tools_archive" -d "$core_tools_directory" + chmod +x "$core_tools_directory/func" + echo "$core_tools_directory" >> "$GITHUB_PATH" + export PATH="$core_tools_directory:$PATH" + func --version - name: Compute test project path id: compute_project_path From 380cdb385cfc2ccb5d71db79637d8960d3e52fab Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sat, 8 Aug 2026 14:51:57 -0400 Subject: [PATCH 11/24] Make CLI process tree termination idempotent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/debugger/AspireDebugSession.ts | 4 ++++ extension/src/test/aspireDebugSession.test.ts | 21 ++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index c852143feda..0665d9f2ac0 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -174,6 +174,10 @@ export class AspireDebugSession implements vscode.DebugAdapter { return; } + if (this._cliProcessTreeTerminationAttempted) { + return; + } + // Deliberately not skipped once the leader has exited. `terminateCliProcess` reaps the surviving // members of a managed process group in that case, and that is the only path that collects // AppHost and resource processes which outlived the CLI that owned them. diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index 1e86beca319..1cc61199550 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -127,9 +127,10 @@ suite('AspireDebugSession tests', () => { assert.strictEqual(terminateStub.firstCall.args[0], running); const exited = createFakeCliProcess(4323, 0); - (aspireDebugSession as any)._cliProcess = exited; + const exitedAspireDebugSession = createSessionForSpawn(); + (exitedAspireDebugSession as any)._cliProcess = exited; - aspireDebugSession.terminateCliProcessTree(); + exitedAspireDebugSession.terminateCliProcessTree(); // An exited leader is still forwarded: `terminateCliProcess` reaps the surviving members of // its managed process group, which is the only path that collects an AppHost and resource @@ -138,10 +139,24 @@ suite('AspireDebugSession tests', () => { assert.strictEqual(terminateStub.secondCall.args[0], exited); }); + test('terminateCliProcessTree is idempotent after signalling a CLI process', () => { + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); + const cliProcess = createFakeCliProcess(4324); + const aspireDebugSession = createSessionForSpawn(); + (aspireDebugSession as any)._cliProcess = cliProcess; + + aspireDebugSession.terminateCliProcessTree({ force: true }); + aspireDebugSession.terminateCliProcessTree(); + + sinon.assert.calledOnce(terminateStub); + assert.strictEqual(terminateStub.firstCall.args[0], cliProcess); + assert.deepStrictEqual(terminateStub.firstCall.args[2], { force: true }); + }); + test('a CLI process that exits on its own still has its process group collected', async () => { // Already exited: the leader is gone by the time the exit callback runs, which is exactly // the state the old early return skipped on. - const cliProcess = createFakeCliProcess(4324, 0); + const cliProcess = createFakeCliProcess(4325, 0); const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); sinon.stub(vscode.debug, 'stopDebugging').resolves(); From bdca0426e765e1e20e843e5feeb901768fb358a4 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sat, 8 Aug 2026 15:09:47 -0400 Subject: [PATCH 12/24] Revert Azure Functions Core Tools workflow release-archive install Restores .github/workflows/run-tests.yml to main for this PR, reverting the out-of-scope 05514e8537 workflow change. The Azure Functions Core Tools fix now lives on adamint/fix-azfunc-core-tools-ci. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/run-tests.yml | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 7aee252cf21..8e4c4f3ebe3 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -257,22 +257,8 @@ jobs: - name: Install Azure Functions Core Tools if: runner.os == 'Linux' && (inputs.testShortName == 'Playground' || inputs.testShortName == 'Azure') - shell: bash run: | - set -euo pipefail - core_tools_version='4.12.1' - core_tools_directory="$RUNNER_TEMP/azure-functions-core-tools" - core_tools_archive="$RUNNER_TEMP/Azure.Functions.Cli.linux-x64.${core_tools_version}.zip" - curl --fail --location --retry 3 --retry-all-errors \ - --output "$core_tools_archive" \ - "https://github.com/Azure/azure-functions-core-tools/releases/download/${core_tools_version}/Azure.Functions.Cli.linux-x64.${core_tools_version}.zip" - echo 'faf8fb8d50b5293df338bec70594b12f45730e9fe251805298859b2238cf627e '"$core_tools_archive" | sha256sum --check - - mkdir -p "$core_tools_directory" - unzip -q "$core_tools_archive" -d "$core_tools_directory" - chmod +x "$core_tools_directory/func" - echo "$core_tools_directory" >> "$GITHUB_PATH" - export PATH="$core_tools_directory:$PATH" - func --version + npm i -g azure-functions-core-tools@4 --unsafe-perm true - name: Compute test project path id: compute_project_path From 1f6727825bab14f9853c9c3cb86f9c81f73a349a Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sat, 8 Aug 2026 16:25:51 -0400 Subject: [PATCH 13/24] Complete CLI process tree termination idempotence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- extension/src/debugger/AspireDebugSession.ts | 9 ++++--- extension/src/test/aspireDebugSession.test.ts | 26 +++++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 0665d9f2ac0..f6b76745491 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -174,6 +174,9 @@ export class AspireDebugSession implements vscode.DebugAdapter { return; } + // A force sweep can run after the CLI leader has exited. Never aim another signal at that + // recorded PID afterward: on Windows the PID may already have been recycled, and `taskkill /t` + // would then target an unrelated process tree. if (this._cliProcessTreeTerminationAttempted) { return; } @@ -511,9 +514,9 @@ export class AspireDebugSession implements vscode.DebugAdapter { // The leader came down on its own, so the escalation timer has nothing left to signal — // but a detached leader's descendants (the AppHost and every resource process beneath it) // can outlive it, and this is the last moment they can be collected safely: once the - // leader's PID is released the operating system may recycle it as another group's id, and - // a later negative-PID signal would land on something unrelated. - this.terminateCliProcessTree(); + // leader's PID is released the operating system may recycle it, and a later process-tree + // signal could land on something unrelated. + this.terminateCliProcessTree({ force: true }); this._dcpServer.recordAppHostProcessExit(this.debugSessionId, code); // Flush any partial line left in either buffer so trailing output isn't lost. if (stdoutBuffer.length > 0) { diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index 1cc61199550..f05932b0279 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -169,8 +169,30 @@ suite('AspireDebugSession tests', () => { // The CLI is gone but the AppHost and resource processes in its detached group need not be, // and once the leader's PID is released the group id can be recycled — so the collection has // to happen here rather than on a later timer. - sinon.assert.calledOnce(terminateStub); - assert.strictEqual(terminateStub.firstCall.args[0], cliProcess); + sinon.assert.calledOnceWithExactly( + terminateStub, + cliProcess, + `Aspire CLI for debug session ${aspireDebugSession.debugSessionId}`, + { force: true }); + }); + + test('a forced CLI process tree termination is not repeated by the exit callback', async () => { + const cliProcess = createFakeCliProcess(4326, 0); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); + sinon.stub(vscode.debug, 'stopDebugging').resolves(); + const aspireDebugSession = createSessionForSpawn(); + + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + aspireDebugSession.terminateCliProcessTree({ force: true }); + spawnStub.firstCall.args[3]?.exitCallback?.(0); + + sinon.assert.calledOnceWithExactly( + terminateStub, + cliProcess, + `Aspire CLI for debug session ${aspireDebugSession.debugSessionId}`, + { force: true }); }); test('a launch that resolves the CLI path after disposal does not spawn an orphan CLI', async () => { From 30bb845eb80193ece54b3ff89af2f08b04005b78 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 17:16:27 -0400 Subject: [PATCH 14/24] Cover deactivation process-tree cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/debugger/AspireDebugSession.ts | 11 +++-- extension/src/test-e2e/edgeCases.e2e.test.ts | 40 ++++++++++++++++++- extension/src/test-e2e/helpers/fixtures.ts | 13 ++++++ .../src/test/extensionDeactivation.test.ts | 15 +++++++ extension/src/testing/e2eStateFileBridge.ts | 35 ++++++++++++++++ extension/src/types/extensionApi.ts | 2 + 6 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 extension/src/test/extensionDeactivation.test.ts diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index f6b76745491..fa7f947923b 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -126,6 +126,10 @@ export class AspireDebugSession implements vscode.DebugAdapter { return this._disposed; } + get cliProcessId(): number | undefined { + return this._cliProcess?.pid; + } + constructor(session: vscode.DebugSession, rpcServer: AspireRpcServer, dcpServer: AspireDcpServer, terminalProvider: AspireTerminalProvider, removeAspireDebugSession: (session: AspireDebugSession) => void, debugSessionId: string = generateDcpIdPrefix()) { this._session = session; this._rpcServer = rpcServer; @@ -535,10 +539,9 @@ export class AspireDebugSession implements vscode.DebugAdapter { debugSessionId: this.debugSessionId, noDebug: noDebug, env: env, - // `aspire run` owns the AppHost and every resource process beneath it. Spawning it as a - // process-group leader is what lets `terminateCliProcess` signal the whole tree by negative - // PID when the cooperative `stopCli` RPC does not finish the job. Every other CLI spawn - // site in the extension already does this; this one is the longest-lived of them. + // `aspire run` owns the AppHost and every resource process beneath it. Spawning this + // long-lived CLI as a process-group leader is what lets `terminateCliProcess` signal the + // whole tree by negative PID when the cooperative `stopCli` RPC does not finish the job. createProcessGroup: true, }, ); diff --git a/extension/src/test-e2e/edgeCases.e2e.test.ts b/extension/src/test-e2e/edgeCases.e2e.test.ts index a47c17131b9..d1777d674e9 100644 --- a/extension/src/test-e2e/edgeCases.e2e.test.ts +++ b/extension/src/test-e2e/edgeCases.e2e.test.ts @@ -2,11 +2,17 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; import type { AspireExtensionE2EControlCommand } from '../types/extensionApi'; -import { getCommandInvocationCount, getDebugLaunchCount, isSamePath, waitForCommandOutcome, waitForDebugLaunch, waitForExtensionState, waitForRepositoryIdle, waitForWorkspaceAppHost } from './helpers/assertions'; -import { createExternalSingleFileAppHost, executeE2eControlCommand, removeExternalSingleFileAppHost, restoreWorkspaceCliPath, runE2eTeardown, setCliUnavailableForE2E, setDebugLaunchSuppressedForE2E, stopAppHostIfRunning, stopPrimaryAppHostIfRunning } from './helpers/fixtures'; +import { getCommandInvocationCount, getDebugLaunchCount, isSamePath, waitForCommandOutcome, waitForDebugLaunch, waitForDebugSessionStartup, waitForExtensionState, waitForNoDebugSessions, waitForNoRunningAppHost, waitForRepositoryIdle, waitForRunningAppHost, waitForWorkspaceAppHost } from './helpers/assertions'; +import { createExternalSingleFileAppHost, executeE2eControlCommand, isProcessAlive, removeExternalSingleFileAppHost, restoreWorkspaceCliPath, runE2eTeardown, setCliUnavailableForE2E, setDebugLaunchSuppressedForE2E, stopAppHostIfRunning, stopPrimaryAppHostIfRunning, waitForKnownProcessExit } from './helpers/fixtures'; import { getPrimaryAppHostProjectPath, getWorkspaceRoot } from './helpers/paths'; import { chooseActiveQuickPick, executeCommandFromPalette, openAspireView, waitForEditorTitle } from './helpers/vscode'; +interface DebugSessionProcessInfo { + appHostPath?: string; + cliPid?: number; + appHostPid?: number; +} + suite('Aspire extension edge case E2E', function () { this.timeout(240000); let externalAppHostPath: string | undefined; @@ -139,4 +145,34 @@ suite('Aspire extension edge case E2E', function () { 60000); assert.ok(backgrounded.state.appHosts.some(appHost => isSamePath(appHost.appHostPath, appHostPath))); }); + + test('deactivation process cleanup stops the owned CLI and AppHost process tree', async () => { + await openAspireView(); + await waitForRepositoryIdle(); + const discovered = await waitForWorkspaceAppHost(); + const appHostPath = discovered.state.workspaceAppHostPath ?? getPrimaryAppHostProjectPath(); + + const beforeInvocation = getCommandInvocationCount('aspire-vscode.debugAppHost'); + await executeE2eControlCommand({ name: 'debugAppHost', appHostPath }, { waitFor: 'started' }); + await waitForCommandOutcome('aspire-vscode.debugAppHost', 'success', 180000, beforeInvocation); + await waitForDebugSessionStartup(appHostPath, 180000); + await waitForRunningAppHost(180000); + + const processInfoStatus = await executeE2eControlCommand({ name: 'getDebugSessionProcessInfo', appHostPath }); + const processInfo = processInfoStatus.result as DebugSessionProcessInfo | undefined; + assert.ok(processInfo?.cliPid, `Expected the E2E bridge to report the owned Aspire CLI pid: ${JSON.stringify(processInfoStatus)}`); + assert.ok(processInfo?.appHostPid, `Expected the E2E bridge to report the owned AppHost pid: ${JSON.stringify(processInfoStatus)}`); + assert.ok(isProcessAlive(processInfo.cliPid), `Expected the Aspire CLI process ${processInfo.cliPid} to be running before deactivation.`); + assert.ok(isProcessAlive(processInfo.appHostPid), `Expected the AppHost process ${processInfo.appHostPid} to be running before deactivation.`); + + // This uses the same process-owner cleanup methods that extension deactivation reaches, + // but keeps the extension host alive. Full workbench reloads did stop these processes + // locally, but left ExTester unable to complete its own after-all browser shutdown. + await executeE2eControlCommand({ name: 'stopOwnedDebugSessionProcesses', appHostPath }, { timeoutMs: 30000 }); + + await waitForKnownProcessExit(processInfo.cliPid, 'the Aspire CLI process owned by the deactivated extension', 120000); + await waitForKnownProcessExit(processInfo.appHostPid, 'the AppHost process owned by the deactivated extension', 120000); + await waitForNoDebugSessions(120000); + await waitForNoRunningAppHost(120000, appHostPath); + }); }); diff --git a/extension/src/test-e2e/helpers/fixtures.ts b/extension/src/test-e2e/helpers/fixtures.ts index fb3d9ddfe7e..796df9fac95 100644 --- a/extension/src/test-e2e/helpers/fixtures.ts +++ b/extension/src/test-e2e/helpers/fixtures.ts @@ -656,6 +656,19 @@ function getRunningAppHostFromState(appHostPath: string) { : state.appHosts.find(candidate => isSamePath(candidate.appHostPath, appHostPath)); } +export function isProcessAlive(pid: number): boolean { + return isProcessRunning(pid); +} + +export async function waitForKnownProcessExit(pid: number, description: string, timeoutMs: number): Promise { + try { + await waitForProcessExit(pid, timeoutMs); + } + catch (error) { + throw new Error(`Timed out after ${timeoutMs}ms waiting for ${description} ${pid} to exit. Last error: ${error instanceof Error ? error.message : String(error)}`); + } +} + async function waitForProcessExit(pid: number, timeoutMs: number): Promise { const started = Date.now(); while (Date.now() - started < timeoutMs) { diff --git a/extension/src/test/extensionDeactivation.test.ts b/extension/src/test/extensionDeactivation.test.ts new file mode 100644 index 00000000000..2a1b1dafd5c --- /dev/null +++ b/extension/src/test/extensionDeactivation.test.ts @@ -0,0 +1,15 @@ +import * as assert from 'assert'; +import * as fs from 'fs'; +import * as path from 'path'; + +suite('Extension deactivation', () => { + test('returns the Aspire context shutdown promise to VS Code', () => { + const extensionRoot = path.resolve(__dirname, '..', '..'); + const extensionSource = fs.readFileSync(path.join(extensionRoot, 'src', 'extension.ts'), 'utf8'); + const deactivateStart = extensionSource.indexOf('export function deactivate(): Promise'); + assert.ok(deactivateStart >= 0); + + const deactivateBody = extensionSource.slice(deactivateStart, extensionSource.indexOf('\n}', deactivateStart) + 2); + assert.ok(deactivateBody.includes('return aspireExtensionContext.deactivate();')); + }); +}); diff --git a/extension/src/testing/e2eStateFileBridge.ts b/extension/src/testing/e2eStateFileBridge.ts index 1953fecb7bd..51a27d6bceb 100644 --- a/extension/src/testing/e2eStateFileBridge.ts +++ b/extension/src/testing/e2eStateFileBridge.ts @@ -542,6 +542,25 @@ async function executeE2eControlCommand( const commands = await vscode.commands.getCommands(true); return commands.filter(commandId => commandId.startsWith('aspire-vscode.')).sort(); } + case 'getDebugSessionProcessInfo': { + markStarted(); + const state = createStateSnapshot(dataRepository, appHostLaunchService, appHostTreeProvider, aspireContext, true); + const appHostPath = command.appHostPath; + const debugSession = aspireContext.aspireDebugSessions.find(session => + appHostPath === undefined || + (typeof session.appHostPath === 'string' && isSamePath(session.appHostPath, appHostPath))); + const appHost = state.appHosts.find(candidate => + appHostPath === undefined || isSamePath(candidate.appHostPath, appHostPath)) ?? + (state.workspaceAppHost && (appHostPath === undefined || isSamePath(state.workspaceAppHost.appHostPath, appHostPath)) + ? state.workspaceAppHost + : undefined); + + return { + appHostPath: debugSession?.appHostPath ?? appHost?.appHostPath, + cliPid: debugSession?.cliProcessId, + appHostPid: appHost?.appHostPid, + }; + } case 'getResourceDebuggerExtensions': { markStarted(); return getResourceDebuggerExtensions().map(extension => ({ @@ -643,6 +662,22 @@ async function executeE2eControlCommand( await vscode.commands.executeCommand('vscode.openFolder', vscode.Uri.file(folderPath), false); return undefined; } + case 'stopOwnedDebugSessionProcesses': { + markStarted(); + const appHostPath = command.appHostPath; + const debugSessions = aspireContext.aspireDebugSessions.filter(session => + appHostPath === undefined || + (typeof session.appHostPath === 'string' && isSamePath(session.appHostPath, appHostPath))); + await Promise.race([ + Promise.allSettled(debugSessions.map(session => session.requestCliStopForExtensionShutdown())), + delay(5000), + ]); + for (const session of debugSessions) { + session.terminateCliProcessTree({ force: true }); + } + + return undefined; + } case 'getWorkspaceFolders': { markStarted(); return vscode.workspace.workspaceFolders?.map(folder => ({ diff --git a/extension/src/types/extensionApi.ts b/extension/src/types/extensionApi.ts index 76b4455294d..913c2a088d7 100644 --- a/extension/src/types/extensionApi.ts +++ b/extension/src/types/extensionApi.ts @@ -196,6 +196,7 @@ export type AspireExtensionE2EControlCommand = | { name: 'stopDebugging' } | { name: 'closeAllEditors' } | { name: 'getRegisteredAspireCommands' } + | { name: 'getDebugSessionProcessInfo'; appHostPath?: string } | { name: 'getExtensionPackageJson' } | { name: 'getExtensionFileStatus'; relativePaths: readonly string[] } | { name: 'getDiagnostics'; filePath: string } @@ -205,6 +206,7 @@ export type AspireExtensionE2EControlCommand = | { name: 'assertClipboardMatchesLastExpectation' } | { name: 'openFile'; filePath: string } | { name: 'openWorkspaceFolder'; folderPath: string } + | { name: 'stopOwnedDebugSessionProcesses'; appHostPath?: string } | { name: 'getWorkspaceFolders' } | { name: 'getActiveEditor' } | { name: 'getResourceDebuggerExtensions' } From dec2a1ee62d7db33b1427fba13b6e55afbd30a97 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 17:47:32 -0400 Subject: [PATCH 15/24] Prevent CLI spawn during extension shutdown Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/debugger/AspireDebugSession.ts | 6 ++-- extension/src/test/aspireDebugSession.test.ts | 32 ++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index fa7f947923b..3f5d7c58f52 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -87,6 +87,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { private _cliProcess: ChildProcessWithoutNullStreams | undefined; private _cliTerminationTimer: ReturnType | undefined; private _cliProcessTreeTerminationAttempted = false; + private _extensionShutdownRequested = false; // Timestamp for the `debug/apphost/end` duration measurement. Captured the first // time we observe a `launch` request so it covers the actual user-visible session // lifetime, not the moment the AspireDebugSession object was constructed. @@ -154,6 +155,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { } requestCliStopForExtensionShutdown(): Promise { + this._extensionShutdownRequested = true; if (!this._rpcClient) { return Promise.resolve(); } @@ -489,12 +491,12 @@ export class AspireDebugSession implements vscode.DebugAdapter { }; const cliPath = await this._terminalProvider.getAspireCliExecutablePath(); - if (this._disposed) { + if (this._disposed || this._extensionShutdownRequested) { // Resolving the CLI path is asynchronous, so extension deactivation can complete between the // launch request and this point. Spawning now would produce an `aspire run` that no teardown // path is left to stop — and because it is spawned detached as a process-group leader, it // would not even die with the extension host. - extensionLogOutputChannel.info(`Skipping Aspire CLI launch for disposed debug session ${this.debugSessionId}.`); + extensionLogOutputChannel.info(`Skipping Aspire CLI launch for disposed or shutting-down debug session ${this.debugSessionId}.`); disposable.dispose(); return; } diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index f05932b0279..da0268fad15 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -223,7 +223,37 @@ suite('AspireDebugSession tests', () => { // A detached process group spawned here would outlive the extension host itself. sinon.assert.notCalled(spawnStub); assert.strictEqual((aspireDebugSession as any)._cliProcess, undefined); - sinon.assert.calledWithMatch(infoStub, 'Skipping Aspire CLI launch for disposed debug session'); + sinon.assert.calledWithMatch(infoStub, 'Skipping Aspire CLI launch for disposed or shutting-down debug session'); + }); + + test('a launch that resolves the CLI path after extension shutdown was requested does not spawn an orphan CLI', async () => { + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess'); + const infoStub = sinon.stub(extensionLogOutputChannel, 'info'); + let releaseCliPath!: (cliPath: string) => void; + const cliPath = new Promise(resolve => { + releaseCliPath = resolve; + }); + let cliPathRequested!: () => void; + const cliPathRequestObserved = new Promise(resolve => { + cliPathRequested = resolve; + }); + const aspireDebugSession = createSessionForSpawn(() => { + cliPathRequested(); + return cliPath; + }); + + const spawning = aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + await cliPathRequestObserved; + await aspireDebugSession.requestCliStopForExtensionShutdown(); + releaseCliPath('/usr/local/bin/aspire'); + await spawning; + + // The extension context can request shutdown before it has disposed this session. The + // session must still remember that no later async continuation is allowed to create a + // detached CLI process after the deactivation force sweep has already run. + sinon.assert.notCalled(spawnStub); + assert.strictEqual((aspireDebugSession as any)._cliProcess, undefined); + sinon.assert.calledWithMatch(infoStub, 'Skipping Aspire CLI launch for disposed or shutting-down debug session'); }); test('suppresses the Aspire CLI first-run banner for extension-managed launches', async () => { From 63711a98aaae15f96c171bc5542ff78cec4419e1 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 18:50:06 -0400 Subject: [PATCH 16/24] Avoid stop requests after deactivation timeout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/AspireExtensionContext.ts | 2 +- .../src/test/AspireExtensionContext.test.ts | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index e98c12663d0..b2a479d90d4 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -157,7 +157,7 @@ export class AspireExtensionContext implements vscode.Disposable { // from registering a session, so a debug-adapter descriptor or an RPC-triggered // `startDebugSession` that lands mid-await would never be asked to stop if the array were // captured only once. Requesting a stop is idempotent per session, so re-scanning is safe. - while (this._collectStopRequests(requested) && Date.now() < deadline) { + while (Date.now() < deadline && this._collectStopRequests(requested)) { const timedOut = await this._settleStopRequests([...requested.values()], deadline); if (timedOut) { extensionLogOutputChannel.warn(`Timed out after ${AspireExtensionContext._cliStopTimeoutMs}ms waiting for Aspire CLI stop requests; continuing extension teardown.`); diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index db7be84bf2a..ba1a2360a8a 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -84,6 +84,34 @@ suite('AspireExtensionContext', () => { } }); + test('deactivation does not start cooperative stop requests after the stop deadline', async () => { + const order: string[] = []; + const context = createContext(order); + const timeoutMs = (AspireExtensionContext as any)._cliStopTimeoutMs; + (AspireExtensionContext as any)._cliStopTimeoutMs = 0; + + addSession(context, 'expired', () => { + order.push('stop expired'); + return Promise.resolve(); + }, () => order.push('dispose expired'), () => order.push('terminate expired')); + + try { + await deactivateContext(context); + + assert.deepStrictEqual(order, [ + 'terminate expired', + 'dispose expired', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + } + finally { + (AspireExtensionContext as any)._cliStopTimeoutMs = timeoutMs; + } + }); + test('dispose does not race an in-flight deactivation and repeated shutdown is idempotent', async () => { const order: string[] = []; const context = createContext(order); From fe64b615ca101515d1c631d0d5b937aa7584cd47 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 19:36:44 -0400 Subject: [PATCH 17/24] Avoid stale Windows CLI PID sweeps Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/debugger/AspireDebugSession.ts | 13 +++++----- extension/src/test/aspireDebugSession.test.ts | 25 ++++++++++++++++++- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 3f5d7c58f52..8529c852bfe 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -517,12 +517,13 @@ export class AspireDebugSession implements vscode.DebugAdapter { vscode.window.showErrorMessage(processExceptionOccurred(error.message, commandLabel)); }, exitCallback: (code) => { - // The leader came down on its own, so the escalation timer has nothing left to signal — - // but a detached leader's descendants (the AppHost and every resource process beneath it) - // can outlive it, and this is the last moment they can be collected safely: once the - // leader's PID is released the operating system may recycle it, and a later process-tree - // signal could land on something unrelated. - this.terminateCliProcessTree({ force: true }); + // A detached POSIX leader's descendants can keep the process group alive after the + // leader exits, and the group id can be reused later, so collect that group immediately. + // Windows taskkill needs the target PID to still identify a live process tree; after the + // close event the CLI PID may already be reusable, so do not taskkill from this path. + if (process.platform !== 'win32') { + this.terminateCliProcessTree({ force: true }); + } this._dcpServer.recordAppHostProcessExit(this.debugSessionId, code); // Flush any partial line left in either buffer so trailing output isn't lost. if (stdoutBuffer.length > 0) { diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index da0268fad15..d96d1397092 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -153,7 +153,7 @@ suite('AspireDebugSession tests', () => { assert.deepStrictEqual(terminateStub.firstCall.args[2], { force: true }); }); - test('a CLI process that exits on its own still has its process group collected', async () => { + test('a POSIX CLI process that exits on its own still has its process group collected', async () => { // Already exited: the leader is gone by the time the exit callback runs, which is exactly // the state the old early return skipped on. const cliProcess = createFakeCliProcess(4325, 0); @@ -176,6 +176,29 @@ suite('AspireDebugSession tests', () => { { force: true }); }); + test('a Windows CLI process that exits on its own is not swept by stale PID', async () => { + const platformStub = sinon.stub(process, 'platform').value('win32'); + const cliProcess = createFakeCliProcess(4327, 0); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); + sinon.stub(vscode.debug, 'stopDebugging').resolves(); + const aspireDebugSession = createSessionForSpawn(); + + try { + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + spawnStub.firstCall.args[3]?.exitCallback?.(0); + + // On Windows, taskkill can only walk the tree while the target PID still names a live + // process. The close callback runs after that PID can be recycled, so a force sweep here + // is both unreliable for descendants and unsafe for an unrelated process that reused it. + sinon.assert.notCalled(terminateStub); + } + finally { + platformStub.restore(); + } + }); + test('a forced CLI process tree termination is not repeated by the exit callback', async () => { const cliProcess = createFakeCliProcess(4326, 0); const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); From 73b6048b3eb9f78e5f516c30501417a2efc99363 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 20:02:24 -0400 Subject: [PATCH 18/24] Retire the Windows CLI PID instead of only skipping the immediate sweep The exit callback already declined to taskkill a Windows CLI whose PID had just been released, but it then called dispose(), which re-ran the CLI disposable and scheduled the forced sweep of that same spent PID for ten seconds later. The hazard the skip existed to avoid was reintroduced on the very same path. The Windows branch now retires the recorded PID, which both cancels a pending escalation and stops the disposable from scheduling another one. A CLI that is still running when the extension shuts down is unaffected: that path never reaches the exit callback, so its escalation still fires. The existing Windows test only asserted at exit time, so it passed while the delayed sweep was live. The new one advances past the grace period. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/debugger/AspireDebugSession.ts | 24 +++++++++++++++ extension/src/test/aspireDebugSession.test.ts | 30 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 8529c852bfe..6d82c0f5a59 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -220,6 +220,24 @@ export class AspireDebugSession implements vscode.DebugAdapter { } } + /** + * Permanently gives up signalling the recorded CLI process tree, without signalling it. + * + * Windows has no equivalent of the POSIX process group: `taskkill /pid /t` walks the live + * process table to find children, so it can only reach descendants while the recorded PID still + * names the running leader. Once that PID is released the same number can be assigned to an + * unrelated process, and the sweep would then terminate that process and its children instead. + * + * Cancelling the pending timer is not enough on its own, because the disposable installed for the + * CLI schedules a new one every time it runs. Marking the PID as spent is what makes every later + * path — the scheduled escalation and any direct `terminateCliProcessTree` call — decline to aim + * at it. + */ + private abandonCliProcessTree(): void { + this.cancelScheduledCliProcessTermination(); + this._cliProcessTreeTerminationAttempted = true; + } + private releaseExtensionContextOwnership(): void { if (this._removedFromExtensionContext) { return; @@ -521,9 +539,15 @@ export class AspireDebugSession implements vscode.DebugAdapter { // leader exits, and the group id can be reused later, so collect that group immediately. // Windows taskkill needs the target PID to still identify a live process tree; after the // close event the CLI PID may already be reusable, so do not taskkill from this path. + // `dispose()` below re-runs the CLI disposable, which would otherwise schedule a forced + // sweep of that same spent PID once the grace period elapses, so retire it here instead + // of only skipping the immediate call. if (process.platform !== 'win32') { this.terminateCliProcessTree({ force: true }); } + else { + this.abandonCliProcessTree(); + } this._dcpServer.recordAppHostProcessExit(this.debugSessionId, code); // Flush any partial line left in either buffer so trailing output isn't lost. if (stdoutBuffer.length > 0) { diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index d96d1397092..44bd937109c 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -199,6 +199,36 @@ suite('AspireDebugSession tests', () => { } }); + test('a Windows CLI process that exits on its own is not swept after the cooperative grace period', async () => { + // The exit callback runs `dispose()`, which re-runs the CLI disposable and schedules the + // forced escalation. Asserting only at exit time therefore proves nothing about the sweep + // that actually reaches taskkill, so this test has to advance past the grace period. + const clock = sinon.useFakeTimers({ shouldClearNativeTimers: true }); + const platformStub = sinon.stub(process, 'platform').value('win32'); + const cliProcess = createFakeCliProcess(4328, 0); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const terminateStub = sinon.stub(cliModule, 'terminateCliProcess'); + sinon.stub(vscode.debug, 'stopDebugging').resolves(); + const aspireDebugSession = createSessionForSpawn(); + + try { + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + spawnStub.firstCall.args[3]?.exitCallback?.(0); + + // Well past the 10s cooperative grace period, so any scheduled escalation has fired. + await clock.tickAsync(30_000); + + // The recorded PID named a process that has already exited, so Windows may have handed + // it to something unrelated by now. Nothing may aim taskkill at it. + sinon.assert.notCalled(terminateStub); + } + finally { + platformStub.restore(); + clock.restore(); + } + }); + test('a forced CLI process tree termination is not repeated by the exit callback', async () => { const cliProcess = createFakeCliProcess(4326, 0); const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); From 9689a52a39e3e9b1a27be18ac1e2d383f39c212a Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 20:29:21 -0400 Subject: [PATCH 19/24] Pin the platform in the POSIX process-group test The test asserts the branch selected by `process.platform !== 'win32'`, but it never pinned that value, so on the Windows agents it asserted POSIX behaviour against the Windows path and failed. The Windows test alongside it already stubs the platform; this one now does the same. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/test/aspireDebugSession.test.ts | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index 44bd937109c..448b8851da0 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -154,6 +154,10 @@ suite('AspireDebugSession tests', () => { }); test('a POSIX CLI process that exits on its own still has its process group collected', async () => { + // The behaviour under test is selected by `process.platform`, so it has to be pinned rather + // than inherited from whichever agent runs the suite. Without this the test asserts POSIX + // behaviour on the Windows agents, where the branch it covers deliberately does not run. + const platformStub = sinon.stub(process, 'platform').value('linux'); // Already exited: the leader is gone by the time the exit callback runs, which is exactly // the state the old early return skipped on. const cliProcess = createFakeCliProcess(4325, 0); @@ -162,18 +166,23 @@ suite('AspireDebugSession tests', () => { sinon.stub(vscode.debug, 'stopDebugging').resolves(); const aspireDebugSession = createSessionForSpawn(); - await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + try { + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); - spawnStub.firstCall.args[3]?.exitCallback?.(0); + spawnStub.firstCall.args[3]?.exitCallback?.(0); - // The CLI is gone but the AppHost and resource processes in its detached group need not be, - // and once the leader's PID is released the group id can be recycled — so the collection has - // to happen here rather than on a later timer. - sinon.assert.calledOnceWithExactly( - terminateStub, - cliProcess, - `Aspire CLI for debug session ${aspireDebugSession.debugSessionId}`, - { force: true }); + // The CLI is gone but the AppHost and resource processes in its detached group need not + // be, and once the leader's PID is released the group id can be recycled — so the + // collection has to happen here rather than on a later timer. + sinon.assert.calledOnceWithExactly( + terminateStub, + cliProcess, + `Aspire CLI for debug session ${aspireDebugSession.debugSessionId}`, + { force: true }); + } + finally { + platformStub.restore(); + } }); test('a Windows CLI process that exits on its own is not swept by stale PID', async () => { From 072e412004ad6a4e69eb0ef677f31b6eb7084853 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 21:32:31 -0400 Subject: [PATCH 20/24] Stop late shutdown sessions cooperatively Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/AspireExtensionContext.ts | 11 +++++ extension/src/debugger/AspireDebugSession.ts | 3 ++ .../src/test/AspireExtensionContext.test.ts | 43 +++++++++++++++++++ extension/src/test/aspireDebugSession.test.ts | 33 +++++++++++++- 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/extension/src/AspireExtensionContext.ts b/extension/src/AspireExtensionContext.ts index b2a479d90d4..42c02a26851 100644 --- a/extension/src/AspireExtensionContext.ts +++ b/extension/src/AspireExtensionContext.ts @@ -95,6 +95,17 @@ export class AspireExtensionContext implements vscode.Disposable { this._debugSessionStateSubscriptions.set(debugSession.debugSessionId, debugSession.onDidChangeState(() => this._onDidChangeDebugSessions.fire())); this._debugSessionOutputSubscriptions.set(debugSession.debugSessionId, debugSession.onDidSendDebugConsoleOutput(event => this._onDidReceiveDebugConsoleOutput.fire(event))); this._onDidChangeDebugSessions.fire(); + + if (this._isShuttingDown) { + // A session can be registered while deactivation is already awaiting an earlier stop + // request. Ask it to stop immediately rather than waiting for the next drain-loop scan: + // the shared deadline may expire first, in which case the loop goes straight to the + // force sweep and this otherwise healthy late session would never get cooperative + // cleanup for resources outside the CLI process tree, such as containers. + void debugSession.requestCliStopForExtensionShutdown().catch(error => { + extensionLogOutputChannel.warn(`Failed to stop Aspire CLI during extension deactivation: ${error}`); + }); + } } removeAspireDebugSession(debugSession: AspireDebugSession) { diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 6d82c0f5a59..51784a1e797 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -236,6 +236,9 @@ export class AspireDebugSession implements vscode.DebugAdapter { private abandonCliProcessTree(): void { this.cancelScheduledCliProcessTermination(); this._cliProcessTreeTerminationAttempted = true; + if (this._disposed) { + this.releaseExtensionContextOwnership(); + } } private releaseExtensionContextOwnership(): void { diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index ba1a2360a8a..0db6b83f5a8 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -233,6 +233,49 @@ suite('AspireExtensionContext', () => { assert.ok(order.indexOf('stop late') < order.indexOf('rpc server'), `The late stop must happen before the transport is disposed: ${JSON.stringify(order)}`); }); + test('deactivation asks a late debug session to stop even when the original batch times out', async () => { + const order: string[] = []; + const context = createContext(order); + const clock = sinon.useFakeTimers({ shouldClearNativeTimers: true }); + addSession(context, 'hung', () => { + order.push('stop hung'); + return new Promise(() => { }); + }, () => order.push('dispose hung'), () => order.push('terminate hung')); + + try { + const shutdown = deactivateContext(context); + await Promise.resolve(); + assert.deepStrictEqual(order, ['stop hung']); + + addSession(context, 'late', () => { + order.push('stop late'); + return Promise.resolve(); + }, () => order.push('dispose late'), () => order.push('terminate late')); + + await clock.tickAsync(5_000); + await shutdown; + + // The timeout exits the drain loop immediately, so a late session must receive its + // cooperative stop from addAspireDebugSession itself. Otherwise it would only be + // force-terminated and resources outside the process tree could keep running. + assert.deepStrictEqual(order, [ + 'stop hung', + 'stop late', + 'terminate hung', + 'terminate late', + 'dispose hung', + 'dispose late', + 'rpc server', + 'dcp server', + 'terminal provider', + 'editor command provider', + ]); + } + finally { + clock.restore(); + } + }); + test('deactivation terminates the CLI process group after the cooperative stop resolves', async () => { const order: string[] = []; const context = createContext(order); diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index 448b8851da0..f27b281548e 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -238,6 +238,33 @@ suite('AspireDebugSession tests', () => { } }); + test('a disposed Windows CLI process that exits releases extension ownership', async () => { + const platformStub = sinon.stub(process, 'platform').value('win32'); + const cliProcess = createFakeCliProcess(4329, 0); + const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const removeAspireDebugSession = sinon.stub(); + sinon.stub(cliModule, 'terminateCliProcess'); + sinon.stub(vscode.debug, 'stopDebugging').resolves(); + const aspireDebugSession = createSessionForSpawn( + async () => '/usr/local/bin/aspire', + removeAspireDebugSession); + + try { + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + aspireDebugSession.dispose(); + spawnStub.firstCall.args[3]?.exitCallback?.(0); + + // dispose() keeps a stopped session registered while the delayed CLI termination is + // pending. When the Windows close callback retires that PID without signalling it, the + // callback has to release the same ownership because the later dispose() call is a no-op. + sinon.assert.calledOnceWithExactly(removeAspireDebugSession, aspireDebugSession); + } + finally { + platformStub.restore(); + } + }); + test('a forced CLI process tree termination is not repeated by the exit callback', async () => { const cliProcess = createFakeCliProcess(4326, 0); const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); @@ -1739,7 +1766,9 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); } } - function createSessionForSpawn(getAspireCliExecutablePath: () => Promise = async () => '/usr/local/bin/aspire'): AspireDebugSession { + function createSessionForSpawn( + getAspireCliExecutablePath: () => Promise = async () => '/usr/local/bin/aspire', + removeAspireDebugSession: (session: AspireDebugSession) => void = () => { }): AspireDebugSession { const parentDebugSession = { id: 'aspire-session', configuration: {}, @@ -1753,7 +1782,7 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); getAspireCliExecutablePath, createEnvironment: () => ({}), } as any, - () => { }); + removeAspireDebugSession); } function createFakeCliProcess(pid: number, exitCode: number | null = null): ChildProcessWithoutNullStreams & { kill: sinon.SinonStub } { From 40144fb9d19a8563afa0ca69cf6f862f64de7205 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 21:59:19 -0400 Subject: [PATCH 21/24] Avoid stale Windows taskkill after CLI exit Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/debugger/languages/cli.ts | 12 ++++-------- extension/src/test/cli.test.ts | 14 +++++--------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/extension/src/debugger/languages/cli.ts b/extension/src/debugger/languages/cli.ts index d649e05d3d3..1e7a36dcccd 100644 --- a/extension/src/debugger/languages/cli.ts +++ b/extension/src/debugger/languages/cli.ts @@ -169,14 +169,10 @@ export function terminateCliProcess(childProcess: ChildProcessWithoutNullStreams return; } - // Windows does not tie child lifetimes to the parent process. An exited CLI leader can - // still have an AppHost/resource tree underneath its recorded PID, so sweep it with - // taskkill during forceful shutdown instead of treating the leader's exit as proof that - // teardown completed. Non-force callers keep the historical no-op behavior because a short - // helper CLI can legitimately exit before its close handler observes it. - if (!options?.force) { - return; - } + // Windows taskkill walks the live process table from the PID. Once Node has reported the + // process exit, that PID can be reassigned to an unrelated process, so there is no safe + // process-tree sweep left to perform from this helper. + return; } if (options?.force) { diff --git a/extension/src/test/cli.test.ts b/extension/src/test/cli.test.ts index b062976dcdc..4c5b46f7bf1 100644 --- a/extension/src/test/cli.test.ts +++ b/extension/src/test/cli.test.ts @@ -14,7 +14,7 @@ suite('CLI process termination', () => { sinon.restore(); }); - test('forcefully terminates the Windows process tree for an already-exited leader', () => { + test('does not forcefully terminate the Windows process tree for an already-exited leader', () => { sinon.stub(process, 'platform').value('win32'); const childProcess = createFakeCliProcess(4242, 0); const taskkillUnref = sinon.stub(); @@ -29,14 +29,10 @@ suite('CLI process termination', () => { terminateCliProcess(childProcess, 'Aspire CLI', { force: true }); - sinon.assert.calledOnce(spawnStub); - assert.strictEqual(spawnStub.firstCall.args[0], 'taskkill.exe'); - assert.deepStrictEqual(spawnStub.firstCall.args[1], ['/pid', '4242', '/t', '/f']); - assert.deepStrictEqual(spawnStub.firstCall.args[2], { - stdio: 'ignore', - windowsHide: true, - }); - sinon.assert.calledOnce(taskkillUnref); + // After Node has observed exit, taskkill would resolve PID 4242 against the current process + // table rather than a durable handle to the former CLI tree. + sinon.assert.notCalled(spawnStub); + sinon.assert.notCalled(taskkillUnref); sinon.assert.notCalled(childProcess.kill); }); }); From 488fc84be3ce60588127ca7b74b9913f1a7c1a0e Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Sun, 9 Aug 2026 23:25:06 -0400 Subject: [PATCH 22/24] Wait for late CLI RPC stop during shutdown Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 487b899d-d79d-4349-a84b-a026861bde7f --- extension/src/debugger/AspireDebugSession.ts | 30 ++++++++++++++-- extension/src/test/aspireDebugSession.test.ts | 36 +++++++++++++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/extension/src/debugger/AspireDebugSession.ts b/extension/src/debugger/AspireDebugSession.ts index 51784a1e797..2979a42fefb 100644 --- a/extension/src/debugger/AspireDebugSession.ts +++ b/extension/src/debugger/AspireDebugSession.ts @@ -84,6 +84,8 @@ export class AspireDebugSession implements vscode.DebugAdapter { private _removedFromExtensionContext = false; private _parentStopPromise: Thenable | undefined; private _cliStopPromise: Promise | undefined; + private _pendingCliStopWithoutRpcClient: { resolve: () => void; reject: (reason: unknown) => void } | undefined; + private _stopCliWhenRpcClientConnects: ((client: ICliRpcClient) => void) | undefined; private _cliProcess: ChildProcessWithoutNullStreams | undefined; private _cliTerminationTimer: ReturnType | undefined; private _cliProcessTreeTerminationAttempted = false; @@ -156,11 +158,24 @@ export class AspireDebugSession implements vscode.DebugAdapter { requestCliStopForExtensionShutdown(): Promise { this._extensionShutdownRequested = true; + if (this._cliStopPromise) { + return this._cliStopPromise; + } + if (!this._rpcClient) { - return Promise.resolve(); + this._cliStopPromise = new Promise((resolve, reject) => { + this._pendingCliStopWithoutRpcClient = { resolve, reject }; + this._stopCliWhenRpcClientConnects = client => { + client.stopCli().then(resolve, reject).finally(() => { + this._pendingCliStopWithoutRpcClient = undefined; + }); + }; + }); + + return this._cliStopPromise; } - this._cliStopPromise ??= this._rpcClient.stopCli(); + this._cliStopPromise = this._rpcClient.stopCli(); return this._cliStopPromise; } @@ -250,6 +265,13 @@ export class AspireDebugSession implements vscode.DebugAdapter { this._removeAspireDebugSession(this); } + private completePendingCliStopWithoutRpcClient(): void { + this._stopCliWhenRpcClientConnects = undefined; + const pendingStop = this._pendingCliStopWithoutRpcClient; + this._pendingCliStopWithoutRpcClient = undefined; + pendingStop?.resolve(); + } + private stopParentDebugSessionOnce(): Thenable { if (this._parentStopPromise) { return this._parentStopPromise; @@ -475,6 +497,8 @@ export class AspireDebugSession implements vscode.DebugAdapter { if (client.debugSessionId === this.debugSessionId) { this._rpcClient = client; disposable.dispose(); + this._stopCliWhenRpcClientConnects?.(client); + this._stopCliWhenRpcClientConnects = undefined; } }); @@ -519,6 +543,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { // would not even die with the extension host. extensionLogOutputChannel.info(`Skipping Aspire CLI launch for disposed or shutting-down debug session ${this.debugSessionId}.`); disposable.dispose(); + this.completePendingCliStopWithoutRpcClient(); return; } @@ -551,6 +576,7 @@ export class AspireDebugSession implements vscode.DebugAdapter { else { this.abandonCliProcessTree(); } + this.completePendingCliStopWithoutRpcClient(); this._dcpServer.recordAppHostProcessExit(this.debugSessionId, code); // Flush any partial line left in either buffer so trailing output isn't lost. if (stdoutBuffer.length > 0) { diff --git a/extension/src/test/aspireDebugSession.test.ts b/extension/src/test/aspireDebugSession.test.ts index f27b281548e..1e5300db240 100644 --- a/extension/src/test/aspireDebugSession.test.ts +++ b/extension/src/test/aspireDebugSession.test.ts @@ -90,6 +90,34 @@ suite('AspireDebugSession tests', () => { await firstRequest; }); + test('extension shutdown waits for a CLI RPC client that connects after the stop request', async () => { + let onNewConnection!: (client: { debugSessionId: string; stopCli: () => Promise }) => void; + const stopCli = sinon.stub().resolves(); + const cliProcess = createFakeCliProcess(4320); + sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); + const aspireDebugSession = createSessionForSpawn( + async () => '/usr/local/bin/aspire', + () => { }, + callback => { + onNewConnection = callback; + return { dispose: sinon.stub() }; + }); + + await aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); + + const stopRequest = aspireDebugSession.requestCliStopForExtensionShutdown(); + let stopSettled = false; + void stopRequest.then(() => { stopSettled = true; }); + await Promise.resolve(); + + assert.strictEqual(stopSettled, false); + + onNewConnection({ debugSessionId: aspireDebugSession.debugSessionId, stopCli }); + await stopRequest; + + sinon.assert.calledOnce(stopCli); + }); + test('spawns the Aspire CLI as a process-group leader and retains the child process', async () => { const cliProcess = createFakeCliProcess(4321); const spawnStub = sinon.stub(cliModule, 'spawnCliProcess').returns(cliProcess); @@ -333,9 +361,10 @@ suite('AspireDebugSession tests', () => { const spawning = aspireDebugSession.spawnAspireCommand(['run'], '/workspace', false, 'aspire run'); await cliPathRequestObserved; - await aspireDebugSession.requestCliStopForExtensionShutdown(); + const stopRequest = aspireDebugSession.requestCliStopForExtensionShutdown(); releaseCliPath('/usr/local/bin/aspire'); await spawning; + await stopRequest; // The extension context can request shutdown before it has disposed this session. The // session must still remember that no later async continuation is allowed to create a @@ -1768,7 +1797,8 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); function createSessionForSpawn( getAspireCliExecutablePath: () => Promise = async () => '/usr/local/bin/aspire', - removeAspireDebugSession: (session: AspireDebugSession) => void = () => { }): AspireDebugSession { + removeAspireDebugSession: (session: AspireDebugSession) => void = () => { }, + onNewConnection: (callback: (client: any) => void) => vscode.Disposable = () => ({ dispose: () => { } })): AspireDebugSession { const parentDebugSession = { id: 'aspire-session', configuration: {}, @@ -1776,7 +1806,7 @@ var builder = Aspire.Hosting.DistributedApplication.CreateBuilder(args); return new AspireDebugSession( parentDebugSession, - { onNewConnection: () => ({ dispose: () => { } }) } as any, + { onNewConnection } as any, { recordAppHostProcessExit: () => { } } as any, { getAspireCliExecutablePath, From f744667722ef7c69b2fc6e5949e5a7cf10ef6ea2 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Mon, 10 Aug 2026 11:13:05 -0400 Subject: [PATCH 23/24] Keep extension deactivation change focused Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d1cf760-b1bc-46ba-a6d4-628354b00f2c --- extension/src/test-e2e/edgeCases.e2e.test.ts | 12 +-- .../src/test/extensionDeactivation.test.ts | 15 ---- extension/src/test/workspace.test.ts | 84 +------------------ extension/src/utils/workspace.ts | 39 ++------- 4 files changed, 12 insertions(+), 138 deletions(-) delete mode 100644 extension/src/test/extensionDeactivation.test.ts diff --git a/extension/src/test-e2e/edgeCases.e2e.test.ts b/extension/src/test-e2e/edgeCases.e2e.test.ts index d1777d674e9..e3026419905 100644 --- a/extension/src/test-e2e/edgeCases.e2e.test.ts +++ b/extension/src/test-e2e/edgeCases.e2e.test.ts @@ -146,7 +146,7 @@ suite('Aspire extension edge case E2E', function () { assert.ok(backgrounded.state.appHosts.some(appHost => isSamePath(appHost.appHostPath, appHostPath))); }); - test('deactivation process cleanup stops the owned CLI and AppHost process tree', async () => { + test('process-owner cleanup stops the owned CLI and AppHost process tree', async () => { await openAspireView(); await waitForRepositoryIdle(); const discovered = await waitForWorkspaceAppHost(); @@ -165,13 +165,13 @@ suite('Aspire extension edge case E2E', function () { assert.ok(isProcessAlive(processInfo.cliPid), `Expected the Aspire CLI process ${processInfo.cliPid} to be running before deactivation.`); assert.ok(isProcessAlive(processInfo.appHostPid), `Expected the AppHost process ${processInfo.appHostPid} to be running before deactivation.`); - // This uses the same process-owner cleanup methods that extension deactivation reaches, - // but keeps the extension host alive. Full workbench reloads did stop these processes - // locally, but left ExTester unable to complete its own after-all browser shutdown. + // This exercises the process-owner cleanup methods with real CLI and AppHost processes while + // keeping the extension host alive. Workbench deactivation is covered separately by unit + // tests because a reload leaves ExTester unable to complete its own browser shutdown. await executeE2eControlCommand({ name: 'stopOwnedDebugSessionProcesses', appHostPath }, { timeoutMs: 30000 }); - await waitForKnownProcessExit(processInfo.cliPid, 'the Aspire CLI process owned by the deactivated extension', 120000); - await waitForKnownProcessExit(processInfo.appHostPid, 'the AppHost process owned by the deactivated extension', 120000); + await waitForKnownProcessExit(processInfo.cliPid, 'the Aspire CLI process owned by the debug session', 120000); + await waitForKnownProcessExit(processInfo.appHostPid, 'the AppHost process owned by the debug session', 120000); await waitForNoDebugSessions(120000); await waitForNoRunningAppHost(120000, appHostPath); }); diff --git a/extension/src/test/extensionDeactivation.test.ts b/extension/src/test/extensionDeactivation.test.ts deleted file mode 100644 index 2a1b1dafd5c..00000000000 --- a/extension/src/test/extensionDeactivation.test.ts +++ /dev/null @@ -1,15 +0,0 @@ -import * as assert from 'assert'; -import * as fs from 'fs'; -import * as path from 'path'; - -suite('Extension deactivation', () => { - test('returns the Aspire context shutdown promise to VS Code', () => { - const extensionRoot = path.resolve(__dirname, '..', '..'); - const extensionSource = fs.readFileSync(path.join(extensionRoot, 'src', 'extension.ts'), 'utf8'); - const deactivateStart = extensionSource.indexOf('export function deactivate(): Promise'); - assert.ok(deactivateStart >= 0); - - const deactivateBody = extensionSource.slice(deactivateStart, extensionSource.indexOf('\n}', deactivateStart) + 2); - assert.ok(deactivateBody.includes('return aspireExtensionContext.deactivate();')); - }); -}); diff --git a/extension/src/test/workspace.test.ts b/extension/src/test/workspace.test.ts index 3542cba01b2..352eb3f29d9 100644 --- a/extension/src/test/workspace.test.ts +++ b/extension/src/test/workspace.test.ts @@ -5,7 +5,7 @@ import * as path from 'path'; import * as sinon from 'sinon'; import * as vscode from 'vscode'; import { yesLabel } from '../loc/strings'; -import { checkForExistingAppHostPathInWorkspace, getCommonExcludeGlob, findAspireSettingsFiles, getRelativePathToWorkspace } from '../utils/workspace'; +import { checkForExistingAppHostPathInWorkspace, getCommonExcludeGlob, findAspireSettingsFiles } from '../utils/workspace'; import { AppHostDiscoveryService, getWorkspaceAppHostProjectSearchResult } from '../utils/appHostDiscovery'; import { getAppHostDiscoveryExcludeGlob } from '../utils/workspaceFileSearch'; @@ -20,88 +20,6 @@ suite('utils/workspace tests', () => { sandbox.restore(); }); - test('getRelativePathToWorkspace falls back to the workspace name when asRelativePath cannot relativize', () => { - const workspaceFolder = { - uri: vscode.Uri.file('/workspace'), - name: 'workspace', - index: 0, - }; - sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(workspaceFolder); - sandbox.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder]); - const asRelativePathStub = sandbox.stub(vscode.workspace, 'asRelativePath'); - - // `asRelativePath` returns the input unchanged when it cannot be made relative, and it - // resolves against the workspace rather than the extension host. Both platforms' absolute - // forms must be rejected here, because the Win32 forms are only recognized by `path.win32` - // and the POSIX form only by `path.posix`. - const hostAbsolutePath = path.join(path.sep, 'workspace', 'src', 'AppHost.csproj'); - const unrelativizedResults = [ - hostAbsolutePath, - 'C:\\Users\\me\\src\\AppHost.csproj', - '\\\\server\\share\\src\\AppHost.csproj', - '/home/me/src/AppHost.csproj', - ].map(unrelativized => { - asRelativePathStub.returns(unrelativized); - return getRelativePathToWorkspace(hostAbsolutePath); - }); - - assert.deepStrictEqual(unrelativizedResults, ['workspace', 'workspace', 'workspace', 'workspace']); - }); - - test('getRelativePathToWorkspace reduces absolute paths from either platform to a bare file name', () => { - // A path that is absolute only under the *other* platform's rules is inside no workspace - // folder on this host, so `getWorkspaceFolder` returns undefined and the absolute-path - // rejection in the `asRelativePath` branch is never reached. That left `path.basename`, - // which on POSIX does not treat `\` as a separator, so the whole - // `C:\Users\me\secret\AppHost.csproj` was returned as the debug configuration name. - sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(undefined); - - const absolutePaths = [ - 'C:\\Users\\me\\secret\\AppHost.csproj', - 'C:/Users/me/secret/AppHost.csproj', - '\\\\server\\share\\secret\\AppHost.csproj', - '/home/me/secret/AppHost.csproj', - ]; - - const names = absolutePaths.map(absolutePath => getRelativePathToWorkspace(absolutePath)); - - assert.deepStrictEqual(names, ['AppHost.csproj', 'AppHost.csproj', 'AppHost.csproj', 'AppHost.csproj']); - for (const name of names) { - // The privacy property, asserted independently of host platform: whatever is returned - // is a single path segment, so no directory component can leak. - assert.ok(!/[\\/]/.test(name), `'${name}' must be a single path segment on either host platform`); - } - }); - - test('getRelativePathToWorkspace keeps genuinely relative paths in either separator style', () => { - const workspaceFolder = { - uri: vscode.Uri.file('/workspace'), - name: 'workspace', - index: 0, - }; - sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(workspaceFolder); - sandbox.stub(vscode.workspace, 'workspaceFolders').value([workspaceFolder]); - const asRelativePathStub = sandbox.stub(vscode.workspace, 'asRelativePath'); - - const relativePaths = [ - 'apps/Store/AppHost.csproj', - 'apps\\Store\\AppHost.csproj', - ]; - - const identities = relativePaths.map(relativePath => { - asRelativePathStub.returns(relativePath); - return getRelativePathToWorkspace('/workspace/apps/Store/AppHost.csproj'); - }); - - assert.deepStrictEqual(identities, relativePaths, 'rejecting both absolute forms must not reject relative paths'); - }); - - test('getRelativePathToWorkspace uses the file name when the path is outside every workspace folder', () => { - sandbox.stub(vscode.workspace, 'getWorkspaceFolder').returns(undefined); - - assert.strictEqual(getRelativePathToWorkspace(path.join(path.sep, 'elsewhere', 'src', 'AppHost.csproj')), 'AppHost.csproj'); - }); - test('getCommonExcludeGlob returns valid glob pattern', () => { const glob = getCommonExcludeGlob(); diff --git a/extension/src/utils/workspace.ts b/extension/src/utils/workspace.ts index 2854259547e..c0ab2b413c5 100644 --- a/extension/src/utils/workspace.ts +++ b/extension/src/utils/workspace.ts @@ -54,48 +54,19 @@ export function isFolderOpenInWorkspace(folderPath: string): boolean { } export function getRelativePathToWorkspace(filePath: string): string { - // Reject paths that are absolute only under the *other* platform's rules before the - // workspace-membership check below, not after it. `getWorkspaceFolder` cannot match such a - // path, so control would reach `path.basename`, which on POSIX does not treat `\` as a - // separator: `C:\Users\me\src\AppHost.csproj` comes back whole and the full path leaks into the - // debug configuration name. POSIX hosts see Windows paths routinely via remote/SSH, Codespaces - // and WSL, so the ordering is what makes this check reachable for the input it exists to catch. - if (isForeignAbsolutePath(filePath)) { - // Only Win32 forms can be foreign: `path.win32.isAbsolute` also accepts a leading `/`, so a - // POSIX path is never foreign on a Windows host (and `path.win32.basename` splits on both - // separators, which is why that direction was already safe). - return path.win32.basename(filePath); + if (!isWorkspaceOpen(false)) { + return filePath; } const uri = vscode.Uri.file(filePath); const workspaceFolder = vscode.workspace.getWorkspaceFolder(uri); - if (!workspaceFolder) { - return path.basename(filePath); - } - const relativePath = vscode.workspace.asRelativePath(uri); - // `asRelativePath` returns the path unchanged when it cannot be made relative, and it resolves - // against the *workspace*, which may use different path semantics than the extension host. - // Reject both platforms' absolute forms so the workspace-name fallback runs either way. - if (relativePath && !isAbsoluteOnAnyPlatform(relativePath)) { + if (workspaceFolder) { + const relativePath = vscode.workspace.asRelativePath(uri); return relativePath; } - return workspaceFolder.name || path.basename(filePath); -} - -/** - * Determines whether a path is absolute under one platform's rules but not under the extension - * host's, which means it can never be resolved against this workspace and the host's `path` helpers - * cannot decompose it — `path.posix.basename('C:\\Users\\me\\AppHost.csproj')` returns the whole - * string rather than the file name. - */ -function isForeignAbsolutePath(filePath: string): boolean { - return isAbsoluteOnAnyPlatform(filePath) && !path.isAbsolute(filePath); -} - -function isAbsoluteOnAnyPlatform(filePath: string): boolean { - return path.posix.isAbsolute(filePath) || path.win32.isAbsolute(filePath); + return filePath; } interface AppHostQuickPickItem extends vscode.QuickPickItem { From 8039a1233d59d3998a1bb6c02bcbe006ed59be53 Mon Sep 17 00:00:00 2001 From: Adam Ratzman Date: Wed, 12 Aug 2026 02:39:09 -0400 Subject: [PATCH 24/24] Test extension deactivation export wiring Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b8e5743-60bf-456e-b673-f64c0182bb7a --- extension/src/test/AspireExtensionContext.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/extension/src/test/AspireExtensionContext.test.ts b/extension/src/test/AspireExtensionContext.test.ts index 0db6b83f5a8..5fffd9fff49 100644 --- a/extension/src/test/AspireExtensionContext.test.ts +++ b/extension/src/test/AspireExtensionContext.test.ts @@ -11,9 +11,23 @@ import { ErrorCodes, ResponseError } from 'vscode-jsonrpc'; import { AspireExtensionContext } from '../AspireExtensionContext'; import { AspireDebugSession } from '../debugger/AspireDebugSession'; import * as cliModule from '../debugger/languages/cli'; +import { deactivate as deactivateExtension } from '../extension'; import { extensionLogOutputChannel } from '../utils/logging'; suite('AspireExtensionContext', () => { + test('extension deactivate returns the AspireExtensionContext shutdown promise', () => { + const shutdown = Promise.resolve(); + const deactivateStub = sinon.stub(AspireExtensionContext.prototype, 'deactivate').returns(shutdown); + + try { + assert.strictEqual(deactivateExtension(), shutdown); + sinon.assert.calledOnce(deactivateStub); + } + finally { + deactivateStub.restore(); + } + }); + test('deactivation waits for every CLI stop request before disposing transport', async () => { const order: string[] = []; const context = createContext(order);