From fc58dfbe7f41a3d90cd6e4d7bc38509d1cc8c5a3 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Tue, 28 Apr 2026 14:38:44 -0700 Subject: [PATCH 1/8] Enhance Python environment configuration tools with optional interpreter path support for direct setup Co-authored-by: Copilot --- package.json | 12 ++++- src/client/chat/configurePythonEnvTool.ts | 47 ++++++++++++++++-- src/client/chat/createVirtualEnvTool.ts | 22 +++++---- src/client/chat/selectEnvTool.ts | 58 ++++++++++++++++++++--- 4 files changed, 118 insertions(+), 21 deletions(-) diff --git a/package.json b/package.json index 30a9fb237c20..e3441b066420 100644 --- a/package.json +++ b/package.json @@ -1593,7 +1593,7 @@ { "name": "configure_python_environment", "displayName": "Configure Python Environment", - "modelDescription": "This tool configures a Python environment in the given workspace. ALWAYS Use this tool to set up the user's chosen environment and ALWAYS call this tool before using any other Python related tools or running any Python command in the terminal. IMPORTANT: This tool is only for Python environments (venv, virtualenv, conda, pipenv, poetry, pyenv, pixi, or any other Python environment manager). Do not use this tool for npm packages, system packages, Ruby gems, or any other non-Python dependencies.", + "modelDescription": "This tool configures a Python environment in the given workspace. ALWAYS Use this tool to set up the user's chosen environment and ALWAYS call this tool before using any other Python related tools or running any Python command in the terminal. If you already know which Python interpreter to use (e.g. from a previous tool call or user message), pass it as 'pythonPath' to skip interactive prompts and configure the environment automatically. IMPORTANT: This tool is only for Python environments (venv, virtualenv, conda, pipenv, poetry, pyenv, pixi, or any other Python environment manager). Do not use this tool for npm packages, system packages, Ruby gems, or any other non-Python dependencies.", "userDescription": "%python.languageModelTools.configure_python_environment.userDescription%", "toolReferenceName": "configurePythonEnvironment", "tags": [ @@ -1609,6 +1609,10 @@ "resourcePath": { "type": "string", "description": "The path to the Python file or workspace for which a Python Environment needs to be configured." + }, + "pythonPath": { + "type": "string", + "description": "Optional absolute path to a Python interpreter to use. When provided, the environment is configured automatically without any interactive prompts. Use this to avoid blocking the session on user input." } }, "required": [] @@ -1642,7 +1646,7 @@ { "name": "selectEnvironment", "displayName": "Select a Python Environment", - "modelDescription": "This tool will prompt the user to select an existing Python Environment", + "modelDescription": "This tool will prompt the user to select an existing Python Environment. If pythonPath is provided, it sets that interpreter directly without showing any UI.", "tags": [], "canBeReferencedInPrompt": false, "inputSchema": { @@ -1651,6 +1655,10 @@ "resourcePath": { "type": "string", "description": "The path to the Python file or workspace for which a Python Environment needs to be configured." + }, + "pythonPath": { + "type": "string", + "description": "Optional absolute path to a Python interpreter to use. When provided, the interpreter is set directly without showing any UI picker." } }, "required": [] diff --git a/src/client/chat/configurePythonEnvTool.ts b/src/client/chat/configurePythonEnvTool.ts index 914a92f81c52..7371e728db80 100644 --- a/src/client/chat/configurePythonEnvTool.ts +++ b/src/client/chat/configurePythonEnvTool.ts @@ -29,9 +29,20 @@ import { IRecommendedEnvironmentService } from '../interpreter/configuration/typ import { CreateVirtualEnvTool } from './createVirtualEnvTool'; import { ISelectPythonEnvToolArguments, SelectPythonEnvTool } from './selectEnvTool'; import { BaseTool } from './baseTool'; +import { traceVerbose } from '../logging'; -export class ConfigurePythonEnvTool extends BaseTool - implements LanguageModelTool { +export interface IConfigurePythonEnvToolArguments extends IResourceReference { + /** + * Optional path to a Python interpreter. When provided, the tool sets this + * interpreter directly without any user interaction (no Quick Pick, no + * create-venv prompt). This is the recommended way for Copilot to call + * the tool in autopilot / bypass-approvals mode. + */ + pythonPath?: string; +} + +export class ConfigurePythonEnvTool extends BaseTool + implements LanguageModelTool { private readonly terminalExecutionService: TerminalCodeExecutionProvider; private readonly terminalHelper: ITerminalHelper; private readonly recommendedEnvService: IRecommendedEnvironmentService; @@ -53,7 +64,7 @@ export class ConfigurePythonEnvTool extends BaseTool } async invokeImpl( - options: LanguageModelToolInvocationOptions, + options: LanguageModelToolInvocationOptions, resource: Uri | undefined, token: CancellationToken, ): Promise { @@ -63,6 +74,11 @@ export class ConfigurePythonEnvTool extends BaseTool return notebookResponse; } + // Fast path: if the caller provided a pythonPath, set it directly without any UI. + if (options.input.pythonPath) { + return this.setEnvironmentDirectly(options.input.pythonPath, resource, token); + } + const workspaceSpecificEnv = await raceCancellationError( this.hasAlreadyGotAWorkspaceSpecificEnvironment(resource), token, @@ -107,8 +123,31 @@ export class ConfigurePythonEnvTool extends BaseTool } } + /** + * Sets the given interpreter path directly without user interaction, then + * resolves and returns the environment details. + */ + private async setEnvironmentDirectly( + pythonPath: string, + resource: Uri | undefined, + token: CancellationToken, + ): Promise { + traceVerbose(`${ConfigurePythonEnvTool.toolName}: setting environment directly from pythonPath: ${pythonPath}`); + await raceCancellationError(this.api.updateActiveEnvironmentPath(pythonPath, resource), token); + const envPath = this.api.getActiveEnvironmentPath(resource); + const environment = await raceCancellationError(this.api.resolveEnvironment(envPath), token); + return getEnvDetailsForResponse( + environment, + this.api, + this.terminalExecutionService, + this.terminalHelper, + resource, + token, + ); + } + async prepareInvocationImpl( - _options: LanguageModelToolInvocationPrepareOptions, + _options: LanguageModelToolInvocationPrepareOptions, _resource: Uri | undefined, _token: CancellationToken, ): Promise { diff --git a/src/client/chat/createVirtualEnvTool.ts b/src/client/chat/createVirtualEnvTool.ts index 56760d2b4bef..9e285e439850 100644 --- a/src/client/chat/createVirtualEnvTool.ts +++ b/src/client/chat/createVirtualEnvTool.ts @@ -45,7 +45,7 @@ import { hideEnvCreation } from '../pythonEnvironments/creation/provider/hideEnv import { BaseTool } from './baseTool'; interface ICreateVirtualEnvToolParams extends IResourceReference { - packageList?: string[]; // Added only becausewe have ability to create a virtual env with list of packages same tool within the in Python Env extension. + packageList?: string[]; // Added only because we have ability to create a virtual env with list of packages same tool within the in Python Env extension. } export class CreateVirtualEnvTool extends BaseTool @@ -92,18 +92,24 @@ export class CreateVirtualEnvTool extends BaseTool let createdEnvPath: string | undefined = undefined; if (useEnvExtension()) { - const result: PythonEnvironment | undefined = await commands.executeCommand('python-envs.createAny', { - quickCreate: true, - additionalPackages: options.input.packageList || [], - uri: workspaceFolder.uri, - selectEnvironment: true, - }); + const result: PythonEnvironment | undefined = await raceCancellationError( + Promise.resolve( + commands.executeCommand('python-envs.createAny', { + quickCreate: true, + additionalPackages: options.input.packageList || [], + uri: workspaceFolder.uri, + selectEnvironment: true, + }), + ), + token, + ); createdEnvPath = result?.environmentPath.fsPath; } else { const created = await raceCancellationError( createVirtualEnvironment({ interpreter: preferredGlobalPythonEnv.id, workspaceFolder, + installPackages: false, }), token, ); @@ -120,7 +126,7 @@ export class CreateVirtualEnvTool extends BaseTool const stopWatch = new StopWatch(); let env: ResolvedEnvironment | undefined; - while (stopWatch.elapsedTime < 5_000 || !env) { + while (stopWatch.elapsedTime < 5_000 && !env) { env = await this.api.resolveEnvironment(createdEnvPath); if (env) { break; diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index 9eeebdfc1b56..356c5bc73035 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -25,6 +25,7 @@ import { getEnvDetailsForResponse, getToolResponseIfNotebook, IResourceReference, + raceCancellationError, } from './utils'; import { ITerminalHelper } from '../common/terminal/types'; import { raceTimeout } from '../common/utils/async'; @@ -40,6 +41,13 @@ import { BaseTool } from './baseTool'; export interface ISelectPythonEnvToolArguments extends IResourceReference { reason?: 'cancelled'; + /** + * Optional path to a Python interpreter. When provided, the tool sets this + * interpreter directly without showing any Quick Pick UI to the user. + * This prevents the agent from getting stuck waiting for user input in + * autopilot / bypass-approvals mode. + */ + pythonPath?: string; } export class SelectPythonEnvTool extends BaseTool @@ -64,15 +72,48 @@ export class SelectPythonEnvTool extends BaseTool resource: Uri | undefined, token: CancellationToken, ): Promise { + // Fast path: if the caller provided a pythonPath, set it directly without any UI. + if (options.input.pythonPath) { + traceVerbose( + `${SelectPythonEnvTool.toolName}: setting environment directly from pythonPath: ${options.input.pythonPath}`, + ); + await raceCancellationError( + this.api.updateActiveEnvironmentPath(options.input.pythonPath, resource), + token, + ); + const env = await raceCancellationError( + this.api.resolveEnvironment(this.api.getActiveEnvironmentPath(resource)), + token, + ); + if (env) { + return getEnvDetailsForResponse( + env, + this.api, + this.terminalExecutionService, + this.terminalHelper, + resource, + token, + ); + } + return new LanguageModelToolResult([ + new LanguageModelTextPart( + `The provided pythonPath '${options.input.pythonPath}' could not be resolved to a valid Python environment.`, + ), + ]); + } + let selected: boolean | undefined = false; const hasVenvOrCondaEnvInWorkspaceFolder = doesWorkspaceHaveVenvOrCondaEnv(resource, this.api); if (options.input.reason === 'cancelled' || hasVenvOrCondaEnvInWorkspaceFolder) { - const result = (await Promise.resolve( - commands.executeCommand(Commands.Set_Interpreter, { - hideCreateVenv: false, - showBackButton: false, - }), - )) as SelectEnvironmentResult | undefined; + const result = await raceCancellationError( + Promise.resolve( + commands.executeCommand(Commands.Set_Interpreter, { + hideCreateVenv: false, + showBackButton: false, + }), + ) as Promise, + token, + ); if (result?.path) { traceVerbose(`User selected a Python environment ${result.path} in Select Python Tool.`); selected = true; @@ -80,7 +121,10 @@ export class SelectPythonEnvTool extends BaseTool traceWarn(`User did not select a Python environment in Select Python Tool.`); } } else { - selected = await showCreateAndSelectEnvironmentQuickPick(resource, this.serviceContainer); + selected = await raceCancellationError( + showCreateAndSelectEnvironmentQuickPick(resource, this.serviceContainer), + token, + ); if (selected) { traceVerbose(`User selected a Python environment ${selected} in Select Python Tool(2).`); } else { From eec0a99a36342772f335f21f6d25dcb51e6ea1ca Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 6 May 2026 13:16:51 -0700 Subject: [PATCH 2/8] fix feedback --- src/client/chat/configurePythonEnvTool.ts | 7 +++++ src/client/chat/createVirtualEnvTool.ts | 2 +- src/client/chat/selectEnvTool.ts | 9 ++++++ src/client/chat/utils.ts | 35 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/client/chat/configurePythonEnvTool.ts b/src/client/chat/configurePythonEnvTool.ts index 7371e728db80..437d65f1910f 100644 --- a/src/client/chat/configurePythonEnvTool.ts +++ b/src/client/chat/configurePythonEnvTool.ts @@ -23,6 +23,7 @@ import { IResourceReference, isCancellationError, raceCancellationError, + waitForActiveEnvironmentChange, } from './utils'; import { ITerminalHelper } from '../common/terminal/types'; import { IRecommendedEnvironmentService } from '../interpreter/configuration/types'; @@ -133,7 +134,13 @@ export class ConfigurePythonEnvTool extends BaseTool { traceVerbose(`${ConfigurePythonEnvTool.toolName}: setting environment directly from pythonPath: ${pythonPath}`); + // Subscribe to the change event BEFORE triggering the update so we don't miss it. + // updateActiveEnvironmentPath only persists the setting; the active interpreter switch + // is asynchronous, so we wait for the event before resolving env details to avoid + // returning details for the previously-active interpreter. + const activeChanged = waitForActiveEnvironmentChange(this.api, pythonPath, token); await raceCancellationError(this.api.updateActiveEnvironmentPath(pythonPath, resource), token); + await raceCancellationError(activeChanged, token); const envPath = this.api.getActiveEnvironmentPath(resource); const environment = await raceCancellationError(this.api.resolveEnvironment(envPath), token); return getEnvDetailsForResponse( diff --git a/src/client/chat/createVirtualEnvTool.ts b/src/client/chat/createVirtualEnvTool.ts index 9e285e439850..6af825bccbdb 100644 --- a/src/client/chat/createVirtualEnvTool.ts +++ b/src/client/chat/createVirtualEnvTool.ts @@ -45,7 +45,7 @@ import { hideEnvCreation } from '../pythonEnvironments/creation/provider/hideEnv import { BaseTool } from './baseTool'; interface ICreateVirtualEnvToolParams extends IResourceReference { - packageList?: string[]; // Added only because we have ability to create a virtual env with list of packages same tool within the in Python Env extension. + packageList?: string[]; // Added only because we have the ability to create a virtual env with a list of packages using the same tool within the Python Env extension. } export class CreateVirtualEnvTool extends BaseTool diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index 356c5bc73035..0143dd27f81c 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -26,6 +26,7 @@ import { getToolResponseIfNotebook, IResourceReference, raceCancellationError, + waitForActiveEnvironmentChange, } from './utils'; import { ITerminalHelper } from '../common/terminal/types'; import { raceTimeout } from '../common/utils/async'; @@ -72,15 +73,23 @@ export class SelectPythonEnvTool extends BaseTool resource: Uri | undefined, token: CancellationToken, ): Promise { + const notebookResponse = getToolResponseIfNotebook(resource); + if (notebookResponse) { + return notebookResponse; + } + // Fast path: if the caller provided a pythonPath, set it directly without any UI. if (options.input.pythonPath) { traceVerbose( `${SelectPythonEnvTool.toolName}: setting environment directly from pythonPath: ${options.input.pythonPath}`, ); + // Subscribe to the change event BEFORE triggering the update so we don't miss it. + const activeChanged = waitForActiveEnvironmentChange(this.api, options.input.pythonPath, token); await raceCancellationError( this.api.updateActiveEnvironmentPath(options.input.pythonPath, resource), token, ); + await raceCancellationError(activeChanged, token); const env = await raceCancellationError( this.api.resolveEnvironment(this.api.getActiveEnvironmentPath(resource)), token, diff --git a/src/client/chat/utils.ts b/src/client/chat/utils.ts index 2309316bcbdd..9cae30885b27 100644 --- a/src/client/chat/utils.ts +++ b/src/client/chat/utils.ts @@ -58,6 +58,41 @@ export function raceCancellationError(promise: Promise, token: Cancellatio }); } +/** + * Returns a promise that resolves once the active environment path changes to match the + * provided `pythonPath` (matched against either the event's `path` or `id`). Resolves early + * on cancellation or after `timeoutMs` to avoid hanging callers if the event is missed. + * Callers must subscribe via this helper BEFORE invoking `updateActiveEnvironmentPath` to + * avoid a race where the event fires before the listener is attached. + */ +export function waitForActiveEnvironmentChange( + api: PythonExtension['environments'], + pythonPath: string, + token: CancellationToken, + timeoutMs = 5000, +): Promise { + return new Promise((resolve) => { + let settled = false; + const settle = () => { + if (settled) { + return; + } + settled = true; + listener.dispose(); + cancelRef.dispose(); + clearTimeout(timer); + resolve(); + }; + const listener = api.onDidChangeActiveEnvironmentPath((e) => { + if (e.path === pythonPath || e.id === pythonPath) { + settle(); + } + }); + const cancelRef = token.onCancellationRequested(() => settle()); + const timer = setTimeout(() => settle(), timeoutMs); + }); +} + export async function getEnvDisplayName( discovery: IDiscoveryAPI, resource: Uri | undefined, From 82fef402dcb8ea83a17b47b779405f4c70f7e0b9 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 6 May 2026 14:31:27 -0700 Subject: [PATCH 3/8] add testing --- src/client/chat/configurePythonEnvTool.ts | 19 +- src/client/chat/selectEnvTool.ts | 28 +-- src/client/chat/utils.ts | 31 +++ .../chat/setEnvironmentFastPath.unit.test.ts | 224 ++++++++++++++++++ src/test/vscode-mock.ts | 6 + 5 files changed, 277 insertions(+), 31 deletions(-) create mode 100644 src/test/chat/setEnvironmentFastPath.unit.test.ts diff --git a/src/client/chat/configurePythonEnvTool.ts b/src/client/chat/configurePythonEnvTool.ts index 437d65f1910f..ccfef202de9a 100644 --- a/src/client/chat/configurePythonEnvTool.ts +++ b/src/client/chat/configurePythonEnvTool.ts @@ -23,7 +23,7 @@ import { IResourceReference, isCancellationError, raceCancellationError, - waitForActiveEnvironmentChange, + setEnvironmentDirectlyByPath, } from './utils'; import { ITerminalHelper } from '../common/terminal/types'; import { IRecommendedEnvironmentService } from '../interpreter/configuration/types'; @@ -134,23 +134,18 @@ export class ConfigurePythonEnvTool extends BaseTool { traceVerbose(`${ConfigurePythonEnvTool.toolName}: setting environment directly from pythonPath: ${pythonPath}`); - // Subscribe to the change event BEFORE triggering the update so we don't miss it. - // updateActiveEnvironmentPath only persists the setting; the active interpreter switch - // is asynchronous, so we wait for the event before resolving env details to avoid - // returning details for the previously-active interpreter. - const activeChanged = waitForActiveEnvironmentChange(this.api, pythonPath, token); - await raceCancellationError(this.api.updateActiveEnvironmentPath(pythonPath, resource), token); - await raceCancellationError(activeChanged, token); - const envPath = this.api.getActiveEnvironmentPath(resource); - const environment = await raceCancellationError(this.api.resolveEnvironment(envPath), token); - return getEnvDetailsForResponse( - environment, + const result = await setEnvironmentDirectlyByPath( + pythonPath, this.api, this.terminalExecutionService, this.terminalHelper, resource, token, ); + if (result) { + return result; + } + throw new Error(`No environment found for the provided pythonPath '${pythonPath}'.`); } async prepareInvocationImpl( diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index 0143dd27f81c..c22f00323a31 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -26,7 +26,7 @@ import { getToolResponseIfNotebook, IResourceReference, raceCancellationError, - waitForActiveEnvironmentChange, + setEnvironmentDirectlyByPath, } from './utils'; import { ITerminalHelper } from '../common/terminal/types'; import { raceTimeout } from '../common/utils/async'; @@ -83,26 +83,16 @@ export class SelectPythonEnvTool extends BaseTool traceVerbose( `${SelectPythonEnvTool.toolName}: setting environment directly from pythonPath: ${options.input.pythonPath}`, ); - // Subscribe to the change event BEFORE triggering the update so we don't miss it. - const activeChanged = waitForActiveEnvironmentChange(this.api, options.input.pythonPath, token); - await raceCancellationError( - this.api.updateActiveEnvironmentPath(options.input.pythonPath, resource), - token, - ); - await raceCancellationError(activeChanged, token); - const env = await raceCancellationError( - this.api.resolveEnvironment(this.api.getActiveEnvironmentPath(resource)), + const result = await setEnvironmentDirectlyByPath( + options.input.pythonPath, + this.api, + this.terminalExecutionService, + this.terminalHelper, + resource, token, ); - if (env) { - return getEnvDetailsForResponse( - env, - this.api, - this.terminalExecutionService, - this.terminalHelper, - resource, - token, - ); + if (result) { + return result; } return new LanguageModelToolResult([ new LanguageModelTextPart( diff --git a/src/client/chat/utils.ts b/src/client/chat/utils.ts index 9cae30885b27..0bcf3b10e90b 100644 --- a/src/client/chat/utils.ts +++ b/src/client/chat/utils.ts @@ -93,6 +93,37 @@ export function waitForActiveEnvironmentChange( }); } +/** + * Sets the active Python interpreter to `pythonPath` without any UI, waits for the + * asynchronous environment switch to settle (via `onDidChangeActiveEnvironmentPath`), + * resolves the environment, and returns a tool result describing it. + * + * Returns `undefined` if the path cannot be resolved to a valid environment so callers + * can produce a tool-specific error message. + */ +export async function setEnvironmentDirectlyByPath( + pythonPath: string, + api: PythonExtension['environments'], + terminalExecutionService: TerminalCodeExecutionProvider, + terminalHelper: ITerminalHelper, + resource: Uri | undefined, + token: CancellationToken, +): Promise { + // Subscribe to the change event BEFORE triggering the update so we don't miss it. + // updateActiveEnvironmentPath only persists the setting; the active interpreter switch + // is asynchronous, so we wait for the event before resolving env details to avoid + // returning details for the previously-active interpreter. + const activeChanged = waitForActiveEnvironmentChange(api, pythonPath, token); + await raceCancellationError(api.updateActiveEnvironmentPath(pythonPath, resource), token); + await raceCancellationError(activeChanged, token); + const envPath = api.getActiveEnvironmentPath(resource); + const environment = await raceCancellationError(api.resolveEnvironment(envPath), token); + if (!environment) { + return undefined; + } + return getEnvDetailsForResponse(environment, api, terminalExecutionService, terminalHelper, resource, token); +} + export async function getEnvDisplayName( discovery: IDiscoveryAPI, resource: Uri | undefined, diff --git a/src/test/chat/setEnvironmentFastPath.unit.test.ts b/src/test/chat/setEnvironmentFastPath.unit.test.ts new file mode 100644 index 000000000000..0b4076105348 --- /dev/null +++ b/src/test/chat/setEnvironmentFastPath.unit.test.ts @@ -0,0 +1,224 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import * as sinon from 'sinon'; +import { CancellationTokenSource, EventEmitter, Uri } from 'vscode'; +import { instance, mock, when } from 'ts-mockito'; +import { mockedVSCodeNamespaces } from '../vscode-mock'; +import { setEnvironmentDirectlyByPath, waitForActiveEnvironmentChange } from '../../client/chat/utils'; +import { ConfigurePythonEnvTool } from '../../client/chat/configurePythonEnvTool'; +import { SelectPythonEnvTool } from '../../client/chat/selectEnvTool'; +import { PythonExtension } from '../../client/api/types'; +import { IServiceContainer } from '../../client/ioc/types'; +import { ICodeExecutionService } from '../../client/terminals/types'; +import { ITerminalHelper } from '../../client/common/terminal/types'; +import { IRecommendedEnvironmentService } from '../../client/interpreter/configuration/types'; +import { TerminalCodeExecutionProvider } from '../../client/terminals/codeExecution/terminalCodeExecution'; +import { CreateVirtualEnvTool } from '../../client/chat/createVirtualEnvTool'; + +suite('Chat fast-path environment setup', () => { + let tokenSource: CancellationTokenSource; + + setup(() => { + tokenSource = new CancellationTokenSource(); + when(mockedVSCodeNamespaces.workspace!.notebookDocuments).thenReturn([]); + }); + + teardown(() => { + tokenSource.dispose(); + sinon.restore(); + }); + + suite('waitForActiveEnvironmentChange()', () => { + function makeApi(emitter: EventEmitter<{ path: string; id: string; resource: undefined }>) { + return ({ + onDidChangeActiveEnvironmentPath: emitter.event, + } as unknown) as PythonExtension['environments']; + } + + test('resolves when an event matches the requested path', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + const api = makeApi(emitter); + const promise = waitForActiveEnvironmentChange(api, '/usr/bin/python3', tokenSource.token, 5000); + emitter.fire({ path: '/usr/bin/python3', id: 'id-1', resource: undefined }); + await promise; + }); + + test('resolves when an event matches the requested id', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + const api = makeApi(emitter); + const promise = waitForActiveEnvironmentChange(api, 'env-id-42', tokenSource.token, 5000); + emitter.fire({ path: '/some/other/path', id: 'env-id-42', resource: undefined }); + await promise; + }); + + test('resolves on cancellation without firing the event', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + const api = makeApi(emitter); + const promise = waitForActiveEnvironmentChange(api, '/never/fires', tokenSource.token, 60_000); + tokenSource.cancel(); + await promise; + }); + + test('resolves on timeout when the event never fires', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + const api = makeApi(emitter); + await waitForActiveEnvironmentChange(api, '/never/fires', tokenSource.token, 5); + }); + + test('ignores events that do not match', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + const api = makeApi(emitter); + const promise = waitForActiveEnvironmentChange(api, '/want/this', tokenSource.token, 50); + emitter.fire({ path: '/something/else', id: 'wrong-id', resource: undefined }); + // Should fall through to the timeout rather than resolve from the non-matching event. + await promise; + }); + }); + + suite('setEnvironmentDirectlyByPath()', () => { + test('subscribes to env-change BEFORE updateActiveEnvironmentPath, then resolves env', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + let listenerAttached = false; + const calls: string[] = []; + const api = ({ + onDidChangeActiveEnvironmentPath: (handler: (e: any) => void) => { + listenerAttached = true; + return emitter.event(handler); + }, + updateActiveEnvironmentPath: async (p: string) => { + calls.push(`update:${p}`); + expect(listenerAttached, 'listener must be attached before update').to.equal(true); + // Fire the event asynchronously, as the real API would. + setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); + }, + getActiveEnvironmentPath: () => ({ path: '/usr/bin/python3', id: 'id-1' }), + resolveEnvironment: async () => { + calls.push('resolve'); + // Returning undefined keeps this test focused on sequencing without + // exercising getEnvDetailsForResponse internals. + return undefined; + }, + } as unknown) as PythonExtension['environments']; + + const result = await setEnvironmentDirectlyByPath( + '/usr/bin/python3', + api, + instance(mock()), + instance(mock()), + undefined, + tokenSource.token, + ); + + expect(result).to.equal(undefined); + expect(calls).to.deep.equal(['update:/usr/bin/python3', 'resolve']); + }); + + test('returns undefined when resolveEnvironment cannot find the env', async () => { + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + const api = ({ + onDidChangeActiveEnvironmentPath: emitter.event, + updateActiveEnvironmentPath: async (p: string) => { + setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); + }, + getActiveEnvironmentPath: () => ({ path: '/missing', id: 'id-1' }), + resolveEnvironment: async () => undefined, + } as unknown) as PythonExtension['environments']; + + const result = await setEnvironmentDirectlyByPath( + '/missing', + api, + instance(mock()), + instance(mock()), + undefined, + tokenSource.token, + ); + + expect(result).to.equal(undefined); + }); + }); + + suite('ConfigurePythonEnvTool fast path', () => { + test('skips workspace-env / create-venv path when pythonPath is provided', async () => { + const getRecommededEnvironment = sinon.stub().resolves(undefined); + const shouldCreateNewVirtualEnv = sinon.stub().resolves(false); + const serviceContainer = mock(); + when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( + instance(mock()), + ); + when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); + when(serviceContainer.get(IRecommendedEnvironmentService)).thenReturn(({ + getRecommededEnvironment, + } as unknown) as IRecommendedEnvironmentService); + + const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + let updateCalled = false; + const api = ({ + onDidChangeActiveEnvironmentPath: emitter.event, + updateActiveEnvironmentPath: async (p: string) => { + updateCalled = true; + setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); + }, + getActiveEnvironmentPath: () => ({ path: '/usr/bin/python3', id: 'id-1' }), + resolveEnvironment: async () => undefined, + } as unknown) as PythonExtension['environments']; + + const createVenvTool = ({ shouldCreateNewVirtualEnv } as unknown) as CreateVirtualEnvTool; + const tool = new ConfigurePythonEnvTool(api, instance(serviceContainer), createVenvTool); + + try { + await (tool as any).invokeImpl( + { input: { pythonPath: '/usr/bin/python3' } } as any, + Uri.file('/workspace/file.py'), + tokenSource.token, + ); + } catch { + // setEnvironmentDirectly throws when env can't be resolved; that's expected here. + // The behavior we care about is that the fast path was taken. + } + + expect(updateCalled, 'fast path should invoke updateActiveEnvironmentPath').to.equal(true); + // The recommended-env / create-venv branches must not have been consulted. + sinon.assert.notCalled(getRecommededEnvironment); + sinon.assert.notCalled(shouldCreateNewVirtualEnv); + }); + }); + + suite('SelectPythonEnvTool', () => { + test('returns notebook response without setting env when resource is a notebook', async () => { + const serviceContainer = mock(); + when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( + instance(mock()), + ); + when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); + + let updateCalled = false; + const api = ({ + onDidChangeActiveEnvironmentPath: new EventEmitter().event, + updateActiveEnvironmentPath: async () => { + updateCalled = true; + }, + getActiveEnvironmentPath: () => ({ path: '/x', id: 'x' }), + resolveEnvironment: async () => undefined, + } as unknown) as PythonExtension['environments']; + + const tool = new SelectPythonEnvTool(api, instance(serviceContainer)); + + const result = await (tool as any).invokeImpl( + { input: { pythonPath: '/usr/bin/python3' } } as any, + Uri.file('/workspace/notebook.ipynb'), + tokenSource.token, + ); + + expect(updateCalled, 'must NOT update env for notebook resources').to.equal(false); + expect(result, 'notebook resources must produce a tool response').to.not.equal(undefined); + const text = (result.content as any[]) + .map((p) => (p && typeof p.value === 'string' ? p.value : '')) + .join(' '); + expect(text.toLowerCase()).to.include('notebook'); + }); + }); +}); diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index b7ea2bc549a0..ac64384520cf 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -149,6 +149,12 @@ mockedVSCode.TestRunProfileKind = vscodeMocks.TestRunProfileKind; (mockedVSCode as any).StatementCoverage = class StatementCoverage { constructor(public executed: number | boolean, public location: any, public branches?: any) {} }; +(mockedVSCode as any).LanguageModelTextPart = class LanguageModelTextPart { + constructor(public value: string) {} +}; +(mockedVSCode as any).LanguageModelToolResult = class LanguageModelToolResult { + constructor(public content: unknown[]) {} +}; // Mock TestController for vscode.tests namespace function createMockTestController(): vscode.TestController { From d27493149f16dde30c418175d363c62955c2a53c Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 6 May 2026 16:26:55 -0700 Subject: [PATCH 4/8] fixes --- src/client/chat/selectEnvTool.ts | 6 ++ src/client/chat/utils.ts | 32 ++++-- .../chat/setEnvironmentFastPath.unit.test.ts | 97 ++++++++++++++++--- 3 files changed, 115 insertions(+), 20 deletions(-) diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index c22f00323a31..9f9b443dc213 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -163,6 +163,12 @@ export class SelectPythonEnvTool extends BaseTool if (getToolResponseIfNotebook(resource)) { return {}; } + // Fast path: skip the confirmation prompt when the model has already supplied + // a specific interpreter to use. Showing a confirmation here would defeat the + // purpose of the autopilot/bypass-approvals fast path. + if (options.input.pythonPath) { + return {}; + } const hasVenvOrCondaEnvInWorkspaceFolder = doesWorkspaceHaveVenvOrCondaEnv(resource, this.api); if ( diff --git a/src/client/chat/utils.ts b/src/client/chat/utils.ts index 0bcf3b10e90b..6deb53960d26 100644 --- a/src/client/chat/utils.ts +++ b/src/client/chat/utils.ts @@ -73,7 +73,14 @@ export function waitForActiveEnvironmentChange( ): Promise { return new Promise((resolve) => { let settled = false; - const settle = () => { + const listener = api.onDidChangeActiveEnvironmentPath((e) => { + if (e.path === pythonPath || e.id === pythonPath) { + settle(); + } + }); + const cancelRef = token.onCancellationRequested(() => settle()); + const timer = setTimeout(() => settle(), timeoutMs); + function settle() { if (settled) { return; } @@ -82,14 +89,7 @@ export function waitForActiveEnvironmentChange( cancelRef.dispose(); clearTimeout(timer); resolve(); - }; - const listener = api.onDidChangeActiveEnvironmentPath((e) => { - if (e.path === pythonPath || e.id === pythonPath) { - settle(); - } - }); - const cancelRef = token.onCancellationRequested(() => settle()); - const timer = setTimeout(() => settle(), timeoutMs); + } }); } @@ -109,6 +109,14 @@ export async function setEnvironmentDirectlyByPath( resource: Uri | undefined, token: CancellationToken, ): Promise { + // Validate the path resolves to a real environment BEFORE mutating user settings. + // updateActiveEnvironmentPath persists unconditionally, so an invalid path would + // permanently overwrite the user's selected interpreter. + const candidate = await raceCancellationError(api.resolveEnvironment(pythonPath), token); + if (!candidate) { + return undefined; + } + // Subscribe to the change event BEFORE triggering the update so we don't miss it. // updateActiveEnvironmentPath only persists the setting; the active interpreter switch // is asynchronous, so we wait for the event before resolving env details to avoid @@ -116,7 +124,13 @@ export async function setEnvironmentDirectlyByPath( const activeChanged = waitForActiveEnvironmentChange(api, pythonPath, token); await raceCancellationError(api.updateActiveEnvironmentPath(pythonPath, resource), token); await raceCancellationError(activeChanged, token); + + // Verify the active env actually switched. If the change event timed out and the + // active path is still the previous one, don't report success for the wrong env. const envPath = api.getActiveEnvironmentPath(resource); + if (envPath.path !== pythonPath && envPath.id !== pythonPath) { + return undefined; + } const environment = await raceCancellationError(api.resolveEnvironment(envPath), token); if (!environment) { return undefined; diff --git a/src/test/chat/setEnvironmentFastPath.unit.test.ts b/src/test/chat/setEnvironmentFastPath.unit.test.ts index 0b4076105348..fbed453fbca4 100644 --- a/src/test/chat/setEnvironmentFastPath.unit.test.ts +++ b/src/test/chat/setEnvironmentFastPath.unit.test.ts @@ -80,7 +80,7 @@ suite('Chat fast-path environment setup', () => { }); suite('setEnvironmentDirectlyByPath()', () => { - test('subscribes to env-change BEFORE updateActiveEnvironmentPath, then resolves env', async () => { + test('validates path, subscribes BEFORE update, then resolves the active env', async () => { const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); let listenerAttached = false; const calls: string[] = []; @@ -96,10 +96,15 @@ suite('Chat fast-path environment setup', () => { setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); }, getActiveEnvironmentPath: () => ({ path: '/usr/bin/python3', id: 'id-1' }), - resolveEnvironment: async () => { - calls.push('resolve'); - // Returning undefined keeps this test focused on sequencing without - // exercising getEnvDetailsForResponse internals. + resolveEnvironment: async (arg: any) => { + const key = typeof arg === 'string' ? arg : arg?.path; + calls.push(`resolve:${key}`); + // Validation call (string arg) must succeed so the rest of the sequence runs. + // Post-switch call (EnvironmentPath object) returns undefined to keep this test + // focused on sequencing without exercising getEnvDetailsForResponse internals. + if (typeof arg === 'string') { + return ({ id: 'x' } as unknown) as undefined; + } return undefined; }, } as unknown) as PythonExtension['environments']; @@ -114,22 +119,65 @@ suite('Chat fast-path environment setup', () => { ); expect(result).to.equal(undefined); - expect(calls).to.deep.equal(['update:/usr/bin/python3', 'resolve']); + expect(listenerAttached, 'listener must have been attached').to.equal(true); + // Full sequence: validate (resolve) -> update -> resolve active env. + expect(calls).to.deep.equal([ + 'resolve:/usr/bin/python3', + 'update:/usr/bin/python3', + 'resolve:/usr/bin/python3', + ]); }); - test('returns undefined when resolveEnvironment cannot find the env', async () => { + test('does NOT call updateActiveEnvironmentPath when pythonPath cannot be resolved', async () => { + let updateCalled = false; + const api = ({ + onDidChangeActiveEnvironmentPath: new EventEmitter().event, + updateActiveEnvironmentPath: async () => { + updateCalled = true; + }, + getActiveEnvironmentPath: () => ({ path: '/old', id: 'old' }), + resolveEnvironment: async () => undefined, + } as unknown) as PythonExtension['environments']; + + const result = await setEnvironmentDirectlyByPath( + '/bogus/python', + api, + instance(mock()), + instance(mock()), + undefined, + tokenSource.token, + ); + + expect(result).to.equal(undefined); + expect(updateCalled, 'must not mutate user settings for an invalid path').to.equal(false); + }); + + test('returns undefined when active path does not actually switch after update', async () => { const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); + let postSwitchResolveCalled = false; const api = ({ onDidChangeActiveEnvironmentPath: emitter.event, updateActiveEnvironmentPath: async (p: string) => { + // Fire the change event so waitForActiveEnvironmentChange resolves normally + // (no cancellation, no timeout) -- this drives execution past the wait into + // the post-switch verification block. setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); }, - getActiveEnvironmentPath: () => ({ path: '/missing', id: 'id-1' }), - resolveEnvironment: async () => undefined, + // But the getter still reports the OLD interpreter, simulating an inconsistent + // switch where the event fired but the active path didn't actually update. + getActiveEnvironmentPath: () => ({ path: '/previous', id: 'previous' }), + resolveEnvironment: async (arg: any) => { + if (typeof arg === 'string') { + // Validation passes (path is known). + return ({ id: 'x' } as unknown) as undefined; + } + postSwitchResolveCalled = true; + return undefined; + }, } as unknown) as PythonExtension['environments']; const result = await setEnvironmentDirectlyByPath( - '/missing', + '/new/python', api, instance(mock()), instance(mock()), @@ -138,6 +186,9 @@ suite('Chat fast-path environment setup', () => { ); expect(result).to.equal(undefined); + expect(postSwitchResolveCalled, 'must not resolve / report details for the previously-active env').to.equal( + false, + ); }); }); @@ -163,7 +214,10 @@ suite('Chat fast-path environment setup', () => { setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); }, getActiveEnvironmentPath: () => ({ path: '/usr/bin/python3', id: 'id-1' }), - resolveEnvironment: async () => undefined, + // Validation (string arg) succeeds; post-switch resolve returns undefined + // so the helper exits without exercising getEnvDetailsForResponse. + resolveEnvironment: async (arg: any) => + typeof arg === 'string' ? (({ id: 'x' } as unknown) as undefined) : undefined, } as unknown) as PythonExtension['environments']; const createVenvTool = ({ shouldCreateNewVirtualEnv } as unknown) as CreateVirtualEnvTool; @@ -220,5 +274,26 @@ suite('Chat fast-path environment setup', () => { .join(' '); expect(text.toLowerCase()).to.include('notebook'); }); + + test('prepareInvocationImpl skips confirmation when pythonPath is provided', async () => { + const serviceContainer = mock(); + when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( + instance(mock()), + ); + when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); + + const api = ({ + onDidChangeActiveEnvironmentPath: new EventEmitter().event, + } as unknown) as PythonExtension['environments']; + const tool = new SelectPythonEnvTool(api, instance(serviceContainer)); + + const prep = await (tool as any).prepareInvocationImpl( + { input: { pythonPath: '/usr/bin/python3' } }, + Uri.file('/workspace/file.py'), + tokenSource.token, + ); + + expect(prep.confirmationMessages, 'fast path must not show a confirmation prompt').to.equal(undefined); + }); }); }); From 42a887dfb640b9107439d3de6af2dd752a2ab2ad Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:41:18 -0700 Subject: [PATCH 5/8] Tighten non-interactive environment configuration Avoid stale environment results and cross-workspace event races when a tool receives a Python path. Propagate cancellation through interactive and post-creation waits, and strengthen focused unit coverage.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/client/chat/configurePythonEnvTool.ts | 19 +- src/client/chat/createVirtualEnvTool.ts | 6 +- src/client/chat/selectEnvTool.ts | 32 +- src/client/chat/utils.ts | 57 ++- .../chat/setEnvironmentFastPath.unit.test.ts | 423 ++++++++++-------- 5 files changed, 321 insertions(+), 216 deletions(-) diff --git a/src/client/chat/configurePythonEnvTool.ts b/src/client/chat/configurePythonEnvTool.ts index ccfef202de9a..a10e805fd947 100644 --- a/src/client/chat/configurePythonEnvTool.ts +++ b/src/client/chat/configurePythonEnvTool.ts @@ -31,6 +31,7 @@ import { CreateVirtualEnvTool } from './createVirtualEnvTool'; import { ISelectPythonEnvToolArguments, SelectPythonEnvTool } from './selectEnvTool'; import { BaseTool } from './baseTool'; import { traceVerbose } from '../logging'; +import { ErrorWithTelemetrySafeReason } from '../common/errors/errorUtils'; export interface IConfigurePythonEnvToolArguments extends IResourceReference { /** @@ -137,15 +138,25 @@ export class ConfigurePythonEnvTool extends BaseTool // Wait a few secs to ensure the env is selected as the active environment.. // If this doesn't work, then something went wrong. - await raceTimeout(5_000, interpreterChanged); + await raceCancellationError(raceTimeout(5_000, interpreterChanged), token); const stopWatch = new StopWatch(); let env: ResolvedEnvironment | undefined; while (stopWatch.elapsedTime < 5_000 && !env) { - env = await this.api.resolveEnvironment(createdEnvPath); + env = await raceCancellationError(this.api.resolveEnvironment(createdEnvPath), token); if (env) { break; } else { traceVerbose( `${CreateVirtualEnvTool.toolName} tool invoked, env created but not yet resolved, waiting...`, ); - await sleep(200); + await raceCancellationError(sleep(200), token); } } if (!env) { diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index 9f9b443dc213..22120630d351 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -86,13 +86,18 @@ export class SelectPythonEnvTool extends BaseTool const result = await setEnvironmentDirectlyByPath( options.input.pythonPath, this.api, - this.terminalExecutionService, - this.terminalHelper, resource, token, ); if (result) { - return result; + return getEnvDetailsForResponse( + result, + this.api, + this.terminalExecutionService, + this.terminalHelper, + resource, + token, + ); } return new LanguageModelToolResult([ new LanguageModelTextPart( @@ -121,7 +126,7 @@ export class SelectPythonEnvTool extends BaseTool } } else { selected = await raceCancellationError( - showCreateAndSelectEnvironmentQuickPick(resource, this.serviceContainer), + showCreateAndSelectEnvironmentQuickPick(resource, this.serviceContainer, token), token, ); if (selected) { @@ -201,6 +206,7 @@ export class SelectPythonEnvTool extends BaseTool async function showCreateAndSelectEnvironmentQuickPick( uri: Uri | undefined, serviceContainer: IServiceContainer, + token: CancellationToken, ): Promise { const createLabel = `${Octicons.Add} ${InterpreterQuickPickList.create.label}`; const selectLabel = l10n.t('Select an existing Python Environment'); @@ -210,11 +216,15 @@ async function showCreateAndSelectEnvironmentQuickPick( { label: selectLabel }, ]; - const selectedItem = await showQuickPick(items, { - placeHolder: l10n.t('Configure a Python Environment'), - matchOnDescription: true, - ignoreFocusOut: true, - }); + const selectedItem = await showQuickPick( + items, + { + placeHolder: l10n.t('Configure a Python Environment'), + matchOnDescription: true, + ignoreFocusOut: true, + }, + token, + ); if (selectedItem && !Array.isArray(selectedItem) && selectedItem.label === createLabel) { const disposables = new DisposableStore(); @@ -236,7 +246,7 @@ async function showCreateAndSelectEnvironmentQuickPick( ); if (created?.action === 'Back') { - return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer); + return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer, token); } if (created?.action === 'Cancel') { return undefined; @@ -255,7 +265,7 @@ async function showCreateAndSelectEnvironmentQuickPick( commands.executeCommand(Commands.Set_Interpreter, { hideCreateVenv: true, showBackButton: true }), )) as SelectEnvironmentResult | undefined; if (result?.action === 'Back') { - return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer); + return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer, token); } if (result?.action === 'Cancel') { return undefined; diff --git a/src/client/chat/utils.ts b/src/client/chat/utils.ts index 6deb53960d26..ede9f7ad9af4 100644 --- a/src/client/chat/utils.ts +++ b/src/client/chat/utils.ts @@ -20,6 +20,7 @@ import { dirname, join } from 'path'; import { resolveEnvironment, useEnvExtension } from '../envExt/api.internal'; import { ErrorWithTelemetrySafeReason } from '../common/errors/errorUtils'; import { getWorkspaceFolders } from '../common/vscodeApis/workspaceApis'; +import { arePathsSame } from '../common/platform/fs-paths'; export interface IResourceReference { resourcePath?: string; @@ -49,12 +50,24 @@ export function resolveFilePath(filepath?: string): Uri | undefined { * @see {@link raceCancellation} */ export function raceCancellationError(promise: Promise, token: CancellationToken): Promise { + if (token.isCancellationRequested) { + return Promise.reject(new CancellationError()); + } return new Promise((resolve, reject) => { const ref = token.onCancellationRequested(() => { ref.dispose(); reject(new CancellationError()); }); - promise.then(resolve, reject).finally(() => ref.dispose()); + promise.then( + (value) => { + ref.dispose(); + resolve(value); + }, + (error) => { + ref.dispose(); + reject(error); + }, + ); }); } @@ -68,13 +81,17 @@ export function raceCancellationError(promise: Promise, token: Cancellatio export function waitForActiveEnvironmentChange( api: PythonExtension['environments'], pythonPath: string, + resource: Uri | undefined, token: CancellationToken, timeoutMs = 5000, ): Promise { + if (token.isCancellationRequested) { + return Promise.resolve(); + } return new Promise((resolve) => { let settled = false; const listener = api.onDidChangeActiveEnvironmentPath((e) => { - if (e.path === pythonPath || e.id === pythonPath) { + if (isEnvironmentPathMatch(e, pythonPath) && isResourceMatch(e.resource, resource)) { settle(); } }); @@ -93,10 +110,27 @@ export function waitForActiveEnvironmentChange( }); } +function isResourceMatch( + eventResource: { uri: Uri } | Uri | undefined, + requestedResource: Uri | undefined, +): boolean { + const eventUri = eventResource && 'uri' in eventResource ? eventResource.uri : eventResource; + const requestedUri = requestedResource + ? workspace.getWorkspaceFolder(requestedResource)?.uri ?? requestedResource + : undefined; + return eventUri === undefined + ? requestedUri === undefined + : requestedUri !== undefined && arePathsSame(eventUri.fsPath, requestedUri.fsPath); +} + +function isEnvironmentPathMatch(environment: { path: string; id: string }, pythonPath: string): boolean { + return arePathsSame(environment.path, pythonPath) || environment.id === pythonPath; +} + /** * Sets the active Python interpreter to `pythonPath` without any UI, waits for the * asynchronous environment switch to settle (via `onDidChangeActiveEnvironmentPath`), - * resolves the environment, and returns a tool result describing it. + * resolves the environment, and returns it. * * Returns `undefined` if the path cannot be resolved to a valid environment so callers * can produce a tool-specific error message. @@ -104,11 +138,9 @@ export function waitForActiveEnvironmentChange( export async function setEnvironmentDirectlyByPath( pythonPath: string, api: PythonExtension['environments'], - terminalExecutionService: TerminalCodeExecutionProvider, - terminalHelper: ITerminalHelper, resource: Uri | undefined, token: CancellationToken, -): Promise { +): Promise { // Validate the path resolves to a real environment BEFORE mutating user settings. // updateActiveEnvironmentPath persists unconditionally, so an invalid path would // permanently overwrite the user's selected interpreter. @@ -116,26 +148,25 @@ export async function setEnvironmentDirectlyByPath( if (!candidate) { return undefined; } + if (isEnvironmentPathMatch(api.getActiveEnvironmentPath(resource), pythonPath)) { + return candidate; + } // Subscribe to the change event BEFORE triggering the update so we don't miss it. // updateActiveEnvironmentPath only persists the setting; the active interpreter switch // is asynchronous, so we wait for the event before resolving env details to avoid // returning details for the previously-active interpreter. - const activeChanged = waitForActiveEnvironmentChange(api, pythonPath, token); + const activeChanged = waitForActiveEnvironmentChange(api, pythonPath, resource, token); await raceCancellationError(api.updateActiveEnvironmentPath(pythonPath, resource), token); await raceCancellationError(activeChanged, token); // Verify the active env actually switched. If the change event timed out and the // active path is still the previous one, don't report success for the wrong env. const envPath = api.getActiveEnvironmentPath(resource); - if (envPath.path !== pythonPath && envPath.id !== pythonPath) { - return undefined; - } - const environment = await raceCancellationError(api.resolveEnvironment(envPath), token); - if (!environment) { + if (!isEnvironmentPathMatch(envPath, pythonPath)) { return undefined; } - return getEnvDetailsForResponse(environment, api, terminalExecutionService, terminalHelper, resource, token); + return raceCancellationError(api.resolveEnvironment(envPath), token); } export async function getEnvDisplayName( diff --git a/src/test/chat/setEnvironmentFastPath.unit.test.ts b/src/test/chat/setEnvironmentFastPath.unit.test.ts index fbed453fbca4..0fd2b38e2173 100644 --- a/src/test/chat/setEnvironmentFastPath.unit.test.ts +++ b/src/test/chat/setEnvironmentFastPath.unit.test.ts @@ -5,26 +5,41 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; -import { CancellationTokenSource, EventEmitter, Uri } from 'vscode'; +import { + CancellationError, + CancellationTokenSource, + EventEmitter, + LanguageModelToolInvocationOptions, + Uri, +} from 'vscode'; import { instance, mock, when } from 'ts-mockito'; -import { mockedVSCodeNamespaces } from '../vscode-mock'; +import { ConfigurePythonEnvTool, IConfigurePythonEnvToolArguments } from '../../client/chat/configurePythonEnvTool'; +import { ISelectPythonEnvToolArguments, SelectPythonEnvTool } from '../../client/chat/selectEnvTool'; import { setEnvironmentDirectlyByPath, waitForActiveEnvironmentChange } from '../../client/chat/utils'; -import { ConfigurePythonEnvTool } from '../../client/chat/configurePythonEnvTool'; -import { SelectPythonEnvTool } from '../../client/chat/selectEnvTool'; -import { PythonExtension } from '../../client/api/types'; +import { ActiveEnvironmentPathChangeEvent, PythonExtension, ResolvedEnvironment } from '../../client/api/types'; import { IServiceContainer } from '../../client/ioc/types'; import { ICodeExecutionService } from '../../client/terminals/types'; -import { ITerminalHelper } from '../../client/common/terminal/types'; +import { ITerminalHelper, TerminalShellType } from '../../client/common/terminal/types'; import { IRecommendedEnvironmentService } from '../../client/interpreter/configuration/types'; import { TerminalCodeExecutionProvider } from '../../client/terminals/codeExecution/terminalCodeExecution'; import { CreateVirtualEnvTool } from '../../client/chat/createVirtualEnvTool'; +import { mockedVSCodeNamespaces } from '../vscode-mock'; suite('Chat fast-path environment setup', () => { + const pythonPath = '/usr/bin/python3'; + const environment = ({ + id: 'python-env', + path: pythonPath, + executable: { uri: Uri.file(pythonPath), bitness: 64, sysPrefix: '/usr' }, + version: { major: 3, minor: 13, micro: 0, release: { level: 'final', serial: 0 }, sysVersion: '3.13.0' }, + environment: { type: 'Venv' }, + } as unknown) as ResolvedEnvironment; let tokenSource: CancellationTokenSource; setup(() => { tokenSource = new CancellationTokenSource(); when(mockedVSCodeNamespaces.workspace!.notebookDocuments).thenReturn([]); + when(mockedVSCodeNamespaces.workspace!.isTrusted).thenReturn(true); }); teardown(() => { @@ -33,267 +48,305 @@ suite('Chat fast-path environment setup', () => { }); suite('waitForActiveEnvironmentChange()', () => { - function makeApi(emitter: EventEmitter<{ path: string; id: string; resource: undefined }>) { - return ({ - onDidChangeActiveEnvironmentPath: emitter.event, - } as unknown) as PythonExtension['environments']; + function makeApi(emitter: EventEmitter) { + return ({ onDidChangeActiveEnvironmentPath: emitter.event } as unknown) as PythonExtension['environments']; } test('resolves when an event matches the requested path', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - const api = makeApi(emitter); - const promise = waitForActiveEnvironmentChange(api, '/usr/bin/python3', tokenSource.token, 5000); - emitter.fire({ path: '/usr/bin/python3', id: 'id-1', resource: undefined }); - await promise; + const emitter = new EventEmitter(); + const promise = waitForActiveEnvironmentChange(makeApi(emitter), pythonPath, undefined, tokenSource.token); + let settled = false; + void promise.then(() => { + settled = true; + }); + + emitter.fire({ path: pythonPath, id: 'python-env', resource: undefined }); + await Promise.resolve(); + + expect(settled).to.equal(true); }); - test('resolves when an event matches the requested id', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - const api = makeApi(emitter); - const promise = waitForActiveEnvironmentChange(api, 'env-id-42', tokenSource.token, 5000); - emitter.fire({ path: '/some/other/path', id: 'env-id-42', resource: undefined }); - await promise; + test('resolves when an event matches the requested environment id', async () => { + const emitter = new EventEmitter(); + const promise = waitForActiveEnvironmentChange(makeApi(emitter), 'python-env', undefined, tokenSource.token); + let settled = false; + void promise.then(() => { + settled = true; + }); + + emitter.fire({ path: pythonPath, id: 'python-env', resource: undefined }); + await Promise.resolve(); + + expect(settled).to.equal(true); }); - test('resolves on cancellation without firing the event', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - const api = makeApi(emitter); - const promise = waitForActiveEnvironmentChange(api, '/never/fires', tokenSource.token, 60_000); + test('resolves when an event matches an equivalent normalized path', async () => { + const emitter = new EventEmitter(); + const promise = waitForActiveEnvironmentChange(makeApi(emitter), pythonPath, undefined, tokenSource.token); + let settled = false; + void promise.then(() => { + settled = true; + }); + + emitter.fire({ path: '/usr/bin/../bin/python3', id: 'other-env', resource: undefined }); + await Promise.resolve(); + + expect(settled).to.equal(true); + }); + + test('resolves on cancellation', async () => { + const emitter = new EventEmitter(); + const promise = waitForActiveEnvironmentChange( + makeApi(emitter), + pythonPath, + undefined, + tokenSource.token, + 60_000, + ); + let settled = false; + void promise.then(() => { + settled = true; + }); + tokenSource.cancel(); - await promise; + await Promise.resolve(); + + expect(settled).to.equal(true); }); - test('resolves on timeout when the event never fires', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - const api = makeApi(emitter); - await waitForActiveEnvironmentChange(api, '/never/fires', tokenSource.token, 5); + test('ignores non-matching events', async () => { + const emitter = new EventEmitter(); + const promise = waitForActiveEnvironmentChange( + makeApi(emitter), + pythonPath, + undefined, + tokenSource.token, + 60_000, + ); + let settled = false; + void promise.then(() => { + settled = true; + }); + + emitter.fire({ path: '/other/python', id: 'other-env', resource: undefined }); + await Promise.resolve(); + + expect(settled).to.equal(false); + tokenSource.cancel(); + await promise; }); - test('ignores events that do not match', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - const api = makeApi(emitter); - const promise = waitForActiveEnvironmentChange(api, '/want/this', tokenSource.token, 50); - emitter.fire({ path: '/something/else', id: 'wrong-id', resource: undefined }); - // Should fall through to the timeout rather than resolve from the non-matching event. + test('ignores matching environment events for another workspace', async () => { + const emitter = new EventEmitter(); + const resource = Uri.file('/workspace-one'); + const promise = waitForActiveEnvironmentChange( + makeApi(emitter), + pythonPath, + resource, + tokenSource.token, + 60_000, + ); + let settled = false; + void promise.then(() => { + settled = true; + }); + + emitter.fire({ path: pythonPath, id: 'python-env', resource: Uri.file('/workspace-two') }); + await Promise.resolve(); + + expect(settled).to.equal(false); + emitter.fire({ path: pythonPath, id: 'python-env', resource }); await promise; }); }); suite('setEnvironmentDirectlyByPath()', () => { - test('validates path, subscribes BEFORE update, then resolves the active env', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - let listenerAttached = false; + test('validates before updating and returns the newly active environment', async () => { + const emitter = new EventEmitter(); const calls: string[] = []; + let listenerAttached = false; + let activePath = { path: '/old/python', id: 'old-env' }; const api = ({ - onDidChangeActiveEnvironmentPath: (handler: (e: any) => void) => { + onDidChangeActiveEnvironmentPath: (handler: (event: ActiveEnvironmentPathChangeEvent) => void) => { listenerAttached = true; return emitter.event(handler); }, - updateActiveEnvironmentPath: async (p: string) => { - calls.push(`update:${p}`); - expect(listenerAttached, 'listener must be attached before update').to.equal(true); - // Fire the event asynchronously, as the real API would. - setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); + updateActiveEnvironmentPath: async (path: string) => { + expect(listenerAttached).to.equal(true); + calls.push(`update:${path}`); + activePath = { path, id: 'python-env' }; + setImmediate(() => emitter.fire({ path, id: 'python-env', resource: undefined })); }, - getActiveEnvironmentPath: () => ({ path: '/usr/bin/python3', id: 'id-1' }), - resolveEnvironment: async (arg: any) => { - const key = typeof arg === 'string' ? arg : arg?.path; - calls.push(`resolve:${key}`); - // Validation call (string arg) must succeed so the rest of the sequence runs. - // Post-switch call (EnvironmentPath object) returns undefined to keep this test - // focused on sequencing without exercising getEnvDetailsForResponse internals. - if (typeof arg === 'string') { - return ({ id: 'x' } as unknown) as undefined; - } - return undefined; + getActiveEnvironmentPath: () => activePath, + resolveEnvironment: async (value: string | { path: string }) => { + calls.push(`resolve:${typeof value === 'string' ? value : value.path}`); + return environment; }, } as unknown) as PythonExtension['environments']; - const result = await setEnvironmentDirectlyByPath( - '/usr/bin/python3', - api, - instance(mock()), - instance(mock()), - undefined, - tokenSource.token, - ); + const result = await setEnvironmentDirectlyByPath(pythonPath, api, undefined, tokenSource.token); - expect(result).to.equal(undefined); - expect(listenerAttached, 'listener must have been attached').to.equal(true); - // Full sequence: validate (resolve) -> update -> resolve active env. - expect(calls).to.deep.equal([ - 'resolve:/usr/bin/python3', - 'update:/usr/bin/python3', - 'resolve:/usr/bin/python3', - ]); + expect(result).to.equal(environment); + expect(calls).to.deep.equal([`resolve:${pythonPath}`, `update:${pythonPath}`, `resolve:${pythonPath}`]); }); - test('does NOT call updateActiveEnvironmentPath when pythonPath cannot be resolved', async () => { - let updateCalled = false; + test('does not update settings when the path cannot be resolved', async () => { + const update = sinon.stub().resolves(); const api = ({ - onDidChangeActiveEnvironmentPath: new EventEmitter().event, - updateActiveEnvironmentPath: async () => { - updateCalled = true; - }, - getActiveEnvironmentPath: () => ({ path: '/old', id: 'old' }), - resolveEnvironment: async () => undefined, + onDidChangeActiveEnvironmentPath: new EventEmitter().event, + updateActiveEnvironmentPath: update, + getActiveEnvironmentPath: () => ({ path: '/old/python', id: 'old-env' }), + resolveEnvironment: sinon.stub().resolves(undefined), } as unknown) as PythonExtension['environments']; - const result = await setEnvironmentDirectlyByPath( - '/bogus/python', - api, - instance(mock()), - instance(mock()), - undefined, - tokenSource.token, - ); + const result = await setEnvironmentDirectlyByPath('/invalid/python', api, undefined, tokenSource.token); expect(result).to.equal(undefined); - expect(updateCalled, 'must not mutate user settings for an invalid path').to.equal(false); + sinon.assert.notCalled(update); }); - test('returns undefined when active path does not actually switch after update', async () => { - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - let postSwitchResolveCalled = false; + test('does not update settings when the requested environment is already active', async () => { + const update = sinon.stub().resolves(); + const api = ({ + updateActiveEnvironmentPath: update, + getActiveEnvironmentPath: () => ({ path: pythonPath, id: 'python-env' }), + resolveEnvironment: sinon.stub().resolves(environment), + } as unknown) as PythonExtension['environments']; + + const result = await setEnvironmentDirectlyByPath(pythonPath, api, undefined, tokenSource.token); + + expect(result).to.equal(environment); + sinon.assert.notCalled(update); + }); + + test('does not report the previous environment when the active path fails to switch', async () => { + const emitter = new EventEmitter(); + const resolveEnvironment = sinon.stub(); + resolveEnvironment.onFirstCall().resolves(environment); const api = ({ onDidChangeActiveEnvironmentPath: emitter.event, - updateActiveEnvironmentPath: async (p: string) => { - // Fire the change event so waitForActiveEnvironmentChange resolves normally - // (no cancellation, no timeout) -- this drives execution past the wait into - // the post-switch verification block. - setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); - }, - // But the getter still reports the OLD interpreter, simulating an inconsistent - // switch where the event fired but the active path didn't actually update. - getActiveEnvironmentPath: () => ({ path: '/previous', id: 'previous' }), - resolveEnvironment: async (arg: any) => { - if (typeof arg === 'string') { - // Validation passes (path is known). - return ({ id: 'x' } as unknown) as undefined; - } - postSwitchResolveCalled = true; - return undefined; + updateActiveEnvironmentPath: async () => { + setImmediate(() => + emitter.fire({ path: '/new/python', id: 'new-env', resource: undefined }), + ); }, + getActiveEnvironmentPath: () => ({ path: '/old/python', id: 'old-env' }), + resolveEnvironment, } as unknown) as PythonExtension['environments']; const result = await setEnvironmentDirectlyByPath( '/new/python', api, - instance(mock()), - instance(mock()), undefined, tokenSource.token, ); expect(result).to.equal(undefined); - expect(postSwitchResolveCalled, 'must not resolve / report details for the previously-active env').to.equal( - false, - ); + sinon.assert.calledOnce(resolveEnvironment); }); - }); - suite('ConfigurePythonEnvTool fast path', () => { - test('skips workspace-env / create-venv path when pythonPath is provided', async () => { - const getRecommededEnvironment = sinon.stub().resolves(undefined); - const shouldCreateNewVirtualEnv = sinon.stub().resolves(false); - const serviceContainer = mock(); - when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( - instance(mock()), - ); - when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); - when(serviceContainer.get(IRecommendedEnvironmentService)).thenReturn(({ - getRecommededEnvironment, - } as unknown) as IRecommendedEnvironmentService); - - const emitter = new EventEmitter<{ path: string; id: string; resource: undefined }>(); - let updateCalled = false; - const api = ({ - onDidChangeActiveEnvironmentPath: emitter.event, - updateActiveEnvironmentPath: async (p: string) => { - updateCalled = true; - setImmediate(() => emitter.fire({ path: p, id: p, resource: undefined })); - }, - getActiveEnvironmentPath: () => ({ path: '/usr/bin/python3', id: 'id-1' }), - // Validation (string arg) succeeds; post-switch resolve returns undefined - // so the helper exits without exercising getEnvDetailsForResponse. - resolveEnvironment: async (arg: any) => - typeof arg === 'string' ? (({ id: 'x' } as unknown) as undefined) : undefined, - } as unknown) as PythonExtension['environments']; - - const createVenvTool = ({ shouldCreateNewVirtualEnv } as unknown) as CreateVirtualEnvTool; - const tool = new ConfigurePythonEnvTool(api, instance(serviceContainer), createVenvTool); + test('rejects immediately when already cancelled', async () => { + const resolveEnvironment = sinon.stub().resolves(environment); + const api = ({ resolveEnvironment } as unknown) as PythonExtension['environments']; + tokenSource.cancel(); + let error: unknown; try { - await (tool as any).invokeImpl( - { input: { pythonPath: '/usr/bin/python3' } } as any, - Uri.file('/workspace/file.py'), - tokenSource.token, - ); - } catch { - // setEnvironmentDirectly throws when env can't be resolved; that's expected here. - // The behavior we care about is that the fast path was taken. + await setEnvironmentDirectlyByPath(pythonPath, api, undefined, tokenSource.token); + } catch (ex) { + error = ex; } - expect(updateCalled, 'fast path should invoke updateActiveEnvironmentPath').to.equal(true); - // The recommended-env / create-venv branches must not have been consulted. - sinon.assert.notCalled(getRecommededEnvironment); - sinon.assert.notCalled(shouldCreateNewVirtualEnv); + expect(error).to.be.instanceOf(CancellationError); + sinon.assert.notCalled(resolveEnvironment); }); }); + test('configure tool skips interactive environment selection when pythonPath is provided', async () => { + const resource = Uri.file('/workspace/file.py'); + const getRecommendedEnvironment = sinon.stub().resolves(undefined); + const shouldCreateNewVirtualEnv = sinon.stub().resolves(false); + const terminalExecutionService = mock(); + const terminalHelper = mock(); + when(terminalExecutionService.getExecutableInfo(resource)).thenResolve({ command: 'python', args: [] }); + when(terminalHelper.buildCommandForTerminal(TerminalShellType.other, 'python', [])).thenReturn('python'); + const serviceContainer = mock(); + when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( + instance(terminalExecutionService), + ); + when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(terminalHelper)); + when(serviceContainer.get(IRecommendedEnvironmentService)).thenReturn(({ + getRecommededEnvironment: getRecommendedEnvironment, + } as unknown) as IRecommendedEnvironmentService); + const emitter = new EventEmitter(); + let activePath = { path: '/old/python', id: 'old-env' }; + const api = ({ + onDidChangeActiveEnvironmentPath: emitter.event, + updateActiveEnvironmentPath: async (path: string) => { + activePath = { path, id: 'python-env' }; + setImmediate(() => emitter.fire({ path, id: 'python-env', resource })); + }, + getActiveEnvironmentPath: () => activePath, + resolveEnvironment: sinon.stub().resolves(environment), + } as unknown) as PythonExtension['environments']; + const createVenvTool = ({ shouldCreateNewVirtualEnv } as unknown) as CreateVirtualEnvTool; + const tool = new ConfigurePythonEnvTool(api, instance(serviceContainer), createVenvTool); + const options = ({ + input: { pythonPath }, + } as unknown) as LanguageModelToolInvocationOptions; + + const result = await tool.invokeImpl(options, resource, tokenSource.token); + + const text = result.content.map((part) => ('value' in part ? part.value : '')).join(' '); + expect(text).to.include('A Python Environment has been configured'); + sinon.assert.notCalled(getRecommendedEnvironment); + sinon.assert.notCalled(shouldCreateNewVirtualEnv); + }); + suite('SelectPythonEnvTool', () => { - test('returns notebook response without setting env when resource is a notebook', async () => { + function createTool(api: PythonExtension['environments']) { const serviceContainer = mock(); when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( instance(mock()), ); when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); + return new SelectPythonEnvTool(api, instance(serviceContainer)); + } - let updateCalled = false; + test('returns the notebook response without changing the environment', async () => { + const update = sinon.stub().resolves(); const api = ({ - onDidChangeActiveEnvironmentPath: new EventEmitter().event, - updateActiveEnvironmentPath: async () => { - updateCalled = true; - }, - getActiveEnvironmentPath: () => ({ path: '/x', id: 'x' }), - resolveEnvironment: async () => undefined, + onDidChangeActiveEnvironmentPath: new EventEmitter().event, + updateActiveEnvironmentPath: update, } as unknown) as PythonExtension['environments']; + const tool = createTool(api); - const tool = new SelectPythonEnvTool(api, instance(serviceContainer)); - - const result = await (tool as any).invokeImpl( - { input: { pythonPath: '/usr/bin/python3' } } as any, + const result = await tool.invokeImpl( + ({ input: { pythonPath } } as unknown) as LanguageModelToolInvocationOptions< + ISelectPythonEnvToolArguments + >, Uri.file('/workspace/notebook.ipynb'), tokenSource.token, ); - expect(updateCalled, 'must NOT update env for notebook resources').to.equal(false); - expect(result, 'notebook resources must produce a tool response').to.not.equal(undefined); - const text = (result.content as any[]) - .map((p) => (p && typeof p.value === 'string' ? p.value : '')) - .join(' '); + const text = result.content.map((part) => ('value' in part ? part.value : '')).join(' '); expect(text.toLowerCase()).to.include('notebook'); + sinon.assert.notCalled(update); }); - test('prepareInvocationImpl skips confirmation when pythonPath is provided', async () => { - const serviceContainer = mock(); - when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( - instance(mock()), - ); - when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); - + test('skips confirmation when pythonPath is provided', async () => { const api = ({ - onDidChangeActiveEnvironmentPath: new EventEmitter().event, + onDidChangeActiveEnvironmentPath: new EventEmitter().event, } as unknown) as PythonExtension['environments']; - const tool = new SelectPythonEnvTool(api, instance(serviceContainer)); + const tool = createTool(api); - const prep = await (tool as any).prepareInvocationImpl( - { input: { pythonPath: '/usr/bin/python3' } }, + const preparation = await tool.prepareInvocationImpl( + { input: { pythonPath } }, Uri.file('/workspace/file.py'), tokenSource.token, ); - expect(prep.confirmationMessages, 'fast path must not show a confirmation prompt').to.equal(undefined); + expect(preparation.confirmationMessages).to.equal(undefined); }); }); }); From 1638b075770c39d0fc16f790c28ab18ff0eeff58 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 20 Jul 2026 11:59:22 -0700 Subject: [PATCH 6/8] Fix fast-path test typings Provide a complete PythonExecInfo fixture and safely narrow language-model result content in assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../chat/setEnvironmentFastPath.unit.test.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/test/chat/setEnvironmentFastPath.unit.test.ts b/src/test/chat/setEnvironmentFastPath.unit.test.ts index 0fd2b38e2173..8178b25d4725 100644 --- a/src/test/chat/setEnvironmentFastPath.unit.test.ts +++ b/src/test/chat/setEnvironmentFastPath.unit.test.ts @@ -36,6 +36,16 @@ suite('Chat fast-path environment setup', () => { } as unknown) as ResolvedEnvironment; let tokenSource: CancellationTokenSource; + function getToolResultText(content: readonly unknown[]): string { + return content + .map((part) => + typeof part === 'object' && part !== null && 'value' in part && typeof part.value === 'string' + ? part.value + : '', + ) + .join(' '); + } + setup(() => { tokenSource = new CancellationTokenSource(); when(mockedVSCodeNamespaces.workspace!.notebookDocuments).thenReturn([]); @@ -268,7 +278,12 @@ suite('Chat fast-path environment setup', () => { const shouldCreateNewVirtualEnv = sinon.stub().resolves(false); const terminalExecutionService = mock(); const terminalHelper = mock(); - when(terminalExecutionService.getExecutableInfo(resource)).thenResolve({ command: 'python', args: [] }); + when(terminalExecutionService.getExecutableInfo(resource)).thenResolve({ + command: 'python', + args: [], + python: ['python'], + pythonExecutable: 'python', + }); when(terminalHelper.buildCommandForTerminal(TerminalShellType.other, 'python', [])).thenReturn('python'); const serviceContainer = mock(); when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( @@ -297,7 +312,7 @@ suite('Chat fast-path environment setup', () => { const result = await tool.invokeImpl(options, resource, tokenSource.token); - const text = result.content.map((part) => ('value' in part ? part.value : '')).join(' '); + const text = getToolResultText(result.content); expect(text).to.include('A Python Environment has been configured'); sinon.assert.notCalled(getRecommendedEnvironment); sinon.assert.notCalled(shouldCreateNewVirtualEnv); @@ -329,7 +344,7 @@ suite('Chat fast-path environment setup', () => { tokenSource.token, ); - const text = result.content.map((part) => ('value' in part ? part.value : '')).join(' '); + const text = getToolResultText(result.content); expect(text.toLowerCase()).to.include('notebook'); sinon.assert.notCalled(update); }); From 01733eec74b2b583697969f80b8feb2cbea1099e Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 20 Jul 2026 12:43:51 -0700 Subject: [PATCH 7/8] Format environment tool changes Apply the repository Prettier configuration to the non-interactive environment setup implementation and tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/client/chat/configurePythonEnvTool.ts | 7 +------ src/client/chat/selectEnvTool.ts | 7 +------ src/client/chat/utils.ts | 5 +---- .../chat/setEnvironmentFastPath.unit.test.ts | 18 ++++++++---------- 4 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/client/chat/configurePythonEnvTool.ts b/src/client/chat/configurePythonEnvTool.ts index a10e805fd947..2f07e05c4a24 100644 --- a/src/client/chat/configurePythonEnvTool.ts +++ b/src/client/chat/configurePythonEnvTool.ts @@ -135,12 +135,7 @@ export class ConfigurePythonEnvTool extends BaseTool { traceVerbose(`${ConfigurePythonEnvTool.toolName}: setting environment directly from pythonPath: ${pythonPath}`); - const result = await setEnvironmentDirectlyByPath( - pythonPath, - this.api, - resource, - token, - ); + const result = await setEnvironmentDirectlyByPath(pythonPath, this.api, resource, token); if (result) { this.extraTelemetryProperties.resolveOutcome = 'providedEnv'; this.extraTelemetryProperties.envType = getEnvTypeForTelemetry(result); diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index 22120630d351..1d0cf7389410 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -83,12 +83,7 @@ export class SelectPythonEnvTool extends BaseTool traceVerbose( `${SelectPythonEnvTool.toolName}: setting environment directly from pythonPath: ${options.input.pythonPath}`, ); - const result = await setEnvironmentDirectlyByPath( - options.input.pythonPath, - this.api, - resource, - token, - ); + const result = await setEnvironmentDirectlyByPath(options.input.pythonPath, this.api, resource, token); if (result) { return getEnvDetailsForResponse( result, diff --git a/src/client/chat/utils.ts b/src/client/chat/utils.ts index ede9f7ad9af4..f10af5852cad 100644 --- a/src/client/chat/utils.ts +++ b/src/client/chat/utils.ts @@ -110,10 +110,7 @@ export function waitForActiveEnvironmentChange( }); } -function isResourceMatch( - eventResource: { uri: Uri } | Uri | undefined, - requestedResource: Uri | undefined, -): boolean { +function isResourceMatch(eventResource: { uri: Uri } | Uri | undefined, requestedResource: Uri | undefined): boolean { const eventUri = eventResource && 'uri' in eventResource ? eventResource.uri : eventResource; const requestedUri = requestedResource ? workspace.getWorkspaceFolder(requestedResource)?.uri ?? requestedResource diff --git a/src/test/chat/setEnvironmentFastPath.unit.test.ts b/src/test/chat/setEnvironmentFastPath.unit.test.ts index 8178b25d4725..9ca7c93f289a 100644 --- a/src/test/chat/setEnvironmentFastPath.unit.test.ts +++ b/src/test/chat/setEnvironmentFastPath.unit.test.ts @@ -78,7 +78,12 @@ suite('Chat fast-path environment setup', () => { test('resolves when an event matches the requested environment id', async () => { const emitter = new EventEmitter(); - const promise = waitForActiveEnvironmentChange(makeApi(emitter), 'python-env', undefined, tokenSource.token); + const promise = waitForActiveEnvironmentChange( + makeApi(emitter), + 'python-env', + undefined, + tokenSource.token, + ); let settled = false; void promise.then(() => { settled = true; @@ -236,20 +241,13 @@ suite('Chat fast-path environment setup', () => { const api = ({ onDidChangeActiveEnvironmentPath: emitter.event, updateActiveEnvironmentPath: async () => { - setImmediate(() => - emitter.fire({ path: '/new/python', id: 'new-env', resource: undefined }), - ); + setImmediate(() => emitter.fire({ path: '/new/python', id: 'new-env', resource: undefined })); }, getActiveEnvironmentPath: () => ({ path: '/old/python', id: 'old-env' }), resolveEnvironment, } as unknown) as PythonExtension['environments']; - const result = await setEnvironmentDirectlyByPath( - '/new/python', - api, - undefined, - tokenSource.token, - ); + const result = await setEnvironmentDirectlyByPath('/new/python', api, undefined, tokenSource.token); expect(result).to.equal(undefined); sinon.assert.calledOnce(resolveEnvironment); From 00cf18a76879685b7b612251d65700af8a8cc94b Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 20 Jul 2026 12:49:26 -0700 Subject: [PATCH 8/8] Tighten environment configuration scope Keep direct interpreter selection on the public configure tool and preserve legacy package-install behavior. Remove the redundant hidden-tool fast path and its tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- package.json | 6 +-- src/client/chat/createVirtualEnvTool.ts | 1 - src/client/chat/selectEnvTool.ts | 42 ---------------- src/client/chat/utils.ts | 3 ++ .../chat/setEnvironmentFastPath.unit.test.ts | 48 ------------------- 5 files changed, 4 insertions(+), 96 deletions(-) diff --git a/package.json b/package.json index e3441b066420..f51dea6e26e4 100644 --- a/package.json +++ b/package.json @@ -1646,7 +1646,7 @@ { "name": "selectEnvironment", "displayName": "Select a Python Environment", - "modelDescription": "This tool will prompt the user to select an existing Python Environment. If pythonPath is provided, it sets that interpreter directly without showing any UI.", + "modelDescription": "This tool will prompt the user to select an existing Python Environment", "tags": [], "canBeReferencedInPrompt": false, "inputSchema": { @@ -1655,10 +1655,6 @@ "resourcePath": { "type": "string", "description": "The path to the Python file or workspace for which a Python Environment needs to be configured." - }, - "pythonPath": { - "type": "string", - "description": "Optional absolute path to a Python interpreter to use. When provided, the interpreter is set directly without showing any UI picker." } }, "required": [] diff --git a/src/client/chat/createVirtualEnvTool.ts b/src/client/chat/createVirtualEnvTool.ts index ea4ef3c4e52f..096067a8f779 100644 --- a/src/client/chat/createVirtualEnvTool.ts +++ b/src/client/chat/createVirtualEnvTool.ts @@ -109,7 +109,6 @@ export class CreateVirtualEnvTool extends BaseTool createVirtualEnvironment({ interpreter: preferredGlobalPythonEnv.id, workspaceFolder, - installPackages: false, }), token, ); diff --git a/src/client/chat/selectEnvTool.ts b/src/client/chat/selectEnvTool.ts index 1d0cf7389410..834bcd315944 100644 --- a/src/client/chat/selectEnvTool.ts +++ b/src/client/chat/selectEnvTool.ts @@ -26,7 +26,6 @@ import { getToolResponseIfNotebook, IResourceReference, raceCancellationError, - setEnvironmentDirectlyByPath, } from './utils'; import { ITerminalHelper } from '../common/terminal/types'; import { raceTimeout } from '../common/utils/async'; @@ -42,13 +41,6 @@ import { BaseTool } from './baseTool'; export interface ISelectPythonEnvToolArguments extends IResourceReference { reason?: 'cancelled'; - /** - * Optional path to a Python interpreter. When provided, the tool sets this - * interpreter directly without showing any Quick Pick UI to the user. - * This prevents the agent from getting stuck waiting for user input in - * autopilot / bypass-approvals mode. - */ - pythonPath?: string; } export class SelectPythonEnvTool extends BaseTool @@ -73,34 +65,6 @@ export class SelectPythonEnvTool extends BaseTool resource: Uri | undefined, token: CancellationToken, ): Promise { - const notebookResponse = getToolResponseIfNotebook(resource); - if (notebookResponse) { - return notebookResponse; - } - - // Fast path: if the caller provided a pythonPath, set it directly without any UI. - if (options.input.pythonPath) { - traceVerbose( - `${SelectPythonEnvTool.toolName}: setting environment directly from pythonPath: ${options.input.pythonPath}`, - ); - const result = await setEnvironmentDirectlyByPath(options.input.pythonPath, this.api, resource, token); - if (result) { - return getEnvDetailsForResponse( - result, - this.api, - this.terminalExecutionService, - this.terminalHelper, - resource, - token, - ); - } - return new LanguageModelToolResult([ - new LanguageModelTextPart( - `The provided pythonPath '${options.input.pythonPath}' could not be resolved to a valid Python environment.`, - ), - ]); - } - let selected: boolean | undefined = false; const hasVenvOrCondaEnvInWorkspaceFolder = doesWorkspaceHaveVenvOrCondaEnv(resource, this.api); if (options.input.reason === 'cancelled' || hasVenvOrCondaEnvInWorkspaceFolder) { @@ -163,12 +127,6 @@ export class SelectPythonEnvTool extends BaseTool if (getToolResponseIfNotebook(resource)) { return {}; } - // Fast path: skip the confirmation prompt when the model has already supplied - // a specific interpreter to use. Showing a confirmation here would defeat the - // purpose of the autopilot/bypass-approvals fast path. - if (options.input.pythonPath) { - return {}; - } const hasVenvOrCondaEnvInWorkspaceFolder = doesWorkspaceHaveVenvOrCondaEnv(resource, this.api); if ( diff --git a/src/client/chat/utils.ts b/src/client/chat/utils.ts index f10af5852cad..7778fe6b27b5 100644 --- a/src/client/chat/utils.ts +++ b/src/client/chat/utils.ts @@ -138,6 +138,9 @@ export async function setEnvironmentDirectlyByPath( resource: Uri | undefined, token: CancellationToken, ): Promise { + if (token.isCancellationRequested) { + throw new CancellationError(); + } // Validate the path resolves to a real environment BEFORE mutating user settings. // updateActiveEnvironmentPath persists unconditionally, so an invalid path would // permanently overwrite the user's selected interpreter. diff --git a/src/test/chat/setEnvironmentFastPath.unit.test.ts b/src/test/chat/setEnvironmentFastPath.unit.test.ts index 9ca7c93f289a..2377e09577fd 100644 --- a/src/test/chat/setEnvironmentFastPath.unit.test.ts +++ b/src/test/chat/setEnvironmentFastPath.unit.test.ts @@ -14,7 +14,6 @@ import { } from 'vscode'; import { instance, mock, when } from 'ts-mockito'; import { ConfigurePythonEnvTool, IConfigurePythonEnvToolArguments } from '../../client/chat/configurePythonEnvTool'; -import { ISelectPythonEnvToolArguments, SelectPythonEnvTool } from '../../client/chat/selectEnvTool'; import { setEnvironmentDirectlyByPath, waitForActiveEnvironmentChange } from '../../client/chat/utils'; import { ActiveEnvironmentPathChangeEvent, PythonExtension, ResolvedEnvironment } from '../../client/api/types'; import { IServiceContainer } from '../../client/ioc/types'; @@ -315,51 +314,4 @@ suite('Chat fast-path environment setup', () => { sinon.assert.notCalled(getRecommendedEnvironment); sinon.assert.notCalled(shouldCreateNewVirtualEnv); }); - - suite('SelectPythonEnvTool', () => { - function createTool(api: PythonExtension['environments']) { - const serviceContainer = mock(); - when(serviceContainer.get(ICodeExecutionService, 'standard')).thenReturn( - instance(mock()), - ); - when(serviceContainer.get(ITerminalHelper)).thenReturn(instance(mock())); - return new SelectPythonEnvTool(api, instance(serviceContainer)); - } - - test('returns the notebook response without changing the environment', async () => { - const update = sinon.stub().resolves(); - const api = ({ - onDidChangeActiveEnvironmentPath: new EventEmitter().event, - updateActiveEnvironmentPath: update, - } as unknown) as PythonExtension['environments']; - const tool = createTool(api); - - const result = await tool.invokeImpl( - ({ input: { pythonPath } } as unknown) as LanguageModelToolInvocationOptions< - ISelectPythonEnvToolArguments - >, - Uri.file('/workspace/notebook.ipynb'), - tokenSource.token, - ); - - const text = getToolResultText(result.content); - expect(text.toLowerCase()).to.include('notebook'); - sinon.assert.notCalled(update); - }); - - test('skips confirmation when pythonPath is provided', async () => { - const api = ({ - onDidChangeActiveEnvironmentPath: new EventEmitter().event, - } as unknown) as PythonExtension['environments']; - const tool = createTool(api); - - const preparation = await tool.prepareInvocationImpl( - { input: { pythonPath } }, - Uri.file('/workspace/file.py'), - tokenSource.token, - ); - - expect(preparation.confirmationMessages).to.equal(undefined); - }); - }); });