Skip to content

Commit 42a887d

Browse files
committed
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>
1 parent d274931 commit 42a887d

5 files changed

Lines changed: 321 additions & 216 deletions

File tree

src/client/chat/configurePythonEnvTool.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { CreateVirtualEnvTool } from './createVirtualEnvTool';
3131
import { ISelectPythonEnvToolArguments, SelectPythonEnvTool } from './selectEnvTool';
3232
import { BaseTool } from './baseTool';
3333
import { traceVerbose } from '../logging';
34+
import { ErrorWithTelemetrySafeReason } from '../common/errors/errorUtils';
3435

3536
export interface IConfigurePythonEnvToolArguments extends IResourceReference {
3637
/**
@@ -137,15 +138,25 @@ export class ConfigurePythonEnvTool extends BaseTool<IConfigurePythonEnvToolArgu
137138
const result = await setEnvironmentDirectlyByPath(
138139
pythonPath,
139140
this.api,
140-
this.terminalExecutionService,
141-
this.terminalHelper,
142141
resource,
143142
token,
144143
);
145144
if (result) {
146-
return result;
145+
this.extraTelemetryProperties.resolveOutcome = 'providedEnv';
146+
this.extraTelemetryProperties.envType = getEnvTypeForTelemetry(result);
147+
return getEnvDetailsForResponse(
148+
result,
149+
this.api,
150+
this.terminalExecutionService,
151+
this.terminalHelper,
152+
resource,
153+
token,
154+
);
147155
}
148-
throw new Error(`No environment found for the provided pythonPath '${pythonPath}'.`);
156+
throw new ErrorWithTelemetrySafeReason(
157+
`No environment found for the provided pythonPath '${pythonPath}'.`,
158+
'noEnvFound',
159+
);
149160
}
150161

151162
async prepareInvocationImpl(

src/client/chat/createVirtualEnvTool.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,19 +122,19 @@ export class CreateVirtualEnvTool extends BaseTool<ICreateVirtualEnvToolParams>
122122

123123
// Wait a few secs to ensure the env is selected as the active environment..
124124
// If this doesn't work, then something went wrong.
125-
await raceTimeout(5_000, interpreterChanged);
125+
await raceCancellationError(raceTimeout(5_000, interpreterChanged), token);
126126

127127
const stopWatch = new StopWatch();
128128
let env: ResolvedEnvironment | undefined;
129129
while (stopWatch.elapsedTime < 5_000 && !env) {
130-
env = await this.api.resolveEnvironment(createdEnvPath);
130+
env = await raceCancellationError(this.api.resolveEnvironment(createdEnvPath), token);
131131
if (env) {
132132
break;
133133
} else {
134134
traceVerbose(
135135
`${CreateVirtualEnvTool.toolName} tool invoked, env created but not yet resolved, waiting...`,
136136
);
137-
await sleep(200);
137+
await raceCancellationError(sleep(200), token);
138138
}
139139
}
140140
if (!env) {

src/client/chat/selectEnvTool.ts

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,18 @@ export class SelectPythonEnvTool extends BaseTool<ISelectPythonEnvToolArguments>
8686
const result = await setEnvironmentDirectlyByPath(
8787
options.input.pythonPath,
8888
this.api,
89-
this.terminalExecutionService,
90-
this.terminalHelper,
9189
resource,
9290
token,
9391
);
9492
if (result) {
95-
return result;
93+
return getEnvDetailsForResponse(
94+
result,
95+
this.api,
96+
this.terminalExecutionService,
97+
this.terminalHelper,
98+
resource,
99+
token,
100+
);
96101
}
97102
return new LanguageModelToolResult([
98103
new LanguageModelTextPart(
@@ -121,7 +126,7 @@ export class SelectPythonEnvTool extends BaseTool<ISelectPythonEnvToolArguments>
121126
}
122127
} else {
123128
selected = await raceCancellationError(
124-
showCreateAndSelectEnvironmentQuickPick(resource, this.serviceContainer),
129+
showCreateAndSelectEnvironmentQuickPick(resource, this.serviceContainer, token),
125130
token,
126131
);
127132
if (selected) {
@@ -201,6 +206,7 @@ export class SelectPythonEnvTool extends BaseTool<ISelectPythonEnvToolArguments>
201206
async function showCreateAndSelectEnvironmentQuickPick(
202207
uri: Uri | undefined,
203208
serviceContainer: IServiceContainer,
209+
token: CancellationToken,
204210
): Promise<boolean | undefined> {
205211
const createLabel = `${Octicons.Add} ${InterpreterQuickPickList.create.label}`;
206212
const selectLabel = l10n.t('Select an existing Python Environment');
@@ -210,11 +216,15 @@ async function showCreateAndSelectEnvironmentQuickPick(
210216
{ label: selectLabel },
211217
];
212218

213-
const selectedItem = await showQuickPick(items, {
214-
placeHolder: l10n.t('Configure a Python Environment'),
215-
matchOnDescription: true,
216-
ignoreFocusOut: true,
217-
});
219+
const selectedItem = await showQuickPick(
220+
items,
221+
{
222+
placeHolder: l10n.t('Configure a Python Environment'),
223+
matchOnDescription: true,
224+
ignoreFocusOut: true,
225+
},
226+
token,
227+
);
218228

219229
if (selectedItem && !Array.isArray(selectedItem) && selectedItem.label === createLabel) {
220230
const disposables = new DisposableStore();
@@ -236,7 +246,7 @@ async function showCreateAndSelectEnvironmentQuickPick(
236246
);
237247

238248
if (created?.action === 'Back') {
239-
return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer);
249+
return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer, token);
240250
}
241251
if (created?.action === 'Cancel') {
242252
return undefined;
@@ -255,7 +265,7 @@ async function showCreateAndSelectEnvironmentQuickPick(
255265
commands.executeCommand(Commands.Set_Interpreter, { hideCreateVenv: true, showBackButton: true }),
256266
)) as SelectEnvironmentResult | undefined;
257267
if (result?.action === 'Back') {
258-
return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer);
268+
return showCreateAndSelectEnvironmentQuickPick(uri, serviceContainer, token);
259269
}
260270
if (result?.action === 'Cancel') {
261271
return undefined;

src/client/chat/utils.ts

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { dirname, join } from 'path';
2020
import { resolveEnvironment, useEnvExtension } from '../envExt/api.internal';
2121
import { ErrorWithTelemetrySafeReason } from '../common/errors/errorUtils';
2222
import { getWorkspaceFolders } from '../common/vscodeApis/workspaceApis';
23+
import { arePathsSame } from '../common/platform/fs-paths';
2324

2425
export interface IResourceReference {
2526
resourcePath?: string;
@@ -49,12 +50,24 @@ export function resolveFilePath(filepath?: string): Uri | undefined {
4950
* @see {@link raceCancellation}
5051
*/
5152
export function raceCancellationError<T>(promise: Promise<T>, token: CancellationToken): Promise<T> {
53+
if (token.isCancellationRequested) {
54+
return Promise.reject(new CancellationError());
55+
}
5256
return new Promise((resolve, reject) => {
5357
const ref = token.onCancellationRequested(() => {
5458
ref.dispose();
5559
reject(new CancellationError());
5660
});
57-
promise.then(resolve, reject).finally(() => ref.dispose());
61+
promise.then(
62+
(value) => {
63+
ref.dispose();
64+
resolve(value);
65+
},
66+
(error) => {
67+
ref.dispose();
68+
reject(error);
69+
},
70+
);
5871
});
5972
}
6073

@@ -68,13 +81,17 @@ export function raceCancellationError<T>(promise: Promise<T>, token: Cancellatio
6881
export function waitForActiveEnvironmentChange(
6982
api: PythonExtension['environments'],
7083
pythonPath: string,
84+
resource: Uri | undefined,
7185
token: CancellationToken,
7286
timeoutMs = 5000,
7387
): Promise<void> {
88+
if (token.isCancellationRequested) {
89+
return Promise.resolve();
90+
}
7491
return new Promise<void>((resolve) => {
7592
let settled = false;
7693
const listener = api.onDidChangeActiveEnvironmentPath((e) => {
77-
if (e.path === pythonPath || e.id === pythonPath) {
94+
if (isEnvironmentPathMatch(e, pythonPath) && isResourceMatch(e.resource, resource)) {
7895
settle();
7996
}
8097
});
@@ -93,49 +110,63 @@ export function waitForActiveEnvironmentChange(
93110
});
94111
}
95112

113+
function isResourceMatch(
114+
eventResource: { uri: Uri } | Uri | undefined,
115+
requestedResource: Uri | undefined,
116+
): boolean {
117+
const eventUri = eventResource && 'uri' in eventResource ? eventResource.uri : eventResource;
118+
const requestedUri = requestedResource
119+
? workspace.getWorkspaceFolder(requestedResource)?.uri ?? requestedResource
120+
: undefined;
121+
return eventUri === undefined
122+
? requestedUri === undefined
123+
: requestedUri !== undefined && arePathsSame(eventUri.fsPath, requestedUri.fsPath);
124+
}
125+
126+
function isEnvironmentPathMatch(environment: { path: string; id: string }, pythonPath: string): boolean {
127+
return arePathsSame(environment.path, pythonPath) || environment.id === pythonPath;
128+
}
129+
96130
/**
97131
* Sets the active Python interpreter to `pythonPath` without any UI, waits for the
98132
* asynchronous environment switch to settle (via `onDidChangeActiveEnvironmentPath`),
99-
* resolves the environment, and returns a tool result describing it.
133+
* resolves the environment, and returns it.
100134
*
101135
* Returns `undefined` if the path cannot be resolved to a valid environment so callers
102136
* can produce a tool-specific error message.
103137
*/
104138
export async function setEnvironmentDirectlyByPath(
105139
pythonPath: string,
106140
api: PythonExtension['environments'],
107-
terminalExecutionService: TerminalCodeExecutionProvider,
108-
terminalHelper: ITerminalHelper,
109141
resource: Uri | undefined,
110142
token: CancellationToken,
111-
): Promise<LanguageModelToolResult | undefined> {
143+
): Promise<ResolvedEnvironment | undefined> {
112144
// Validate the path resolves to a real environment BEFORE mutating user settings.
113145
// updateActiveEnvironmentPath persists unconditionally, so an invalid path would
114146
// permanently overwrite the user's selected interpreter.
115147
const candidate = await raceCancellationError(api.resolveEnvironment(pythonPath), token);
116148
if (!candidate) {
117149
return undefined;
118150
}
151+
if (isEnvironmentPathMatch(api.getActiveEnvironmentPath(resource), pythonPath)) {
152+
return candidate;
153+
}
119154

120155
// Subscribe to the change event BEFORE triggering the update so we don't miss it.
121156
// updateActiveEnvironmentPath only persists the setting; the active interpreter switch
122157
// is asynchronous, so we wait for the event before resolving env details to avoid
123158
// returning details for the previously-active interpreter.
124-
const activeChanged = waitForActiveEnvironmentChange(api, pythonPath, token);
159+
const activeChanged = waitForActiveEnvironmentChange(api, pythonPath, resource, token);
125160
await raceCancellationError(api.updateActiveEnvironmentPath(pythonPath, resource), token);
126161
await raceCancellationError(activeChanged, token);
127162

128163
// Verify the active env actually switched. If the change event timed out and the
129164
// active path is still the previous one, don't report success for the wrong env.
130165
const envPath = api.getActiveEnvironmentPath(resource);
131-
if (envPath.path !== pythonPath && envPath.id !== pythonPath) {
132-
return undefined;
133-
}
134-
const environment = await raceCancellationError(api.resolveEnvironment(envPath), token);
135-
if (!environment) {
166+
if (!isEnvironmentPathMatch(envPath, pythonPath)) {
136167
return undefined;
137168
}
138-
return getEnvDetailsForResponse(environment, api, terminalExecutionService, terminalHelper, resource, token);
169+
return raceCancellationError(api.resolveEnvironment(envPath), token);
139170
}
140171

141172
export async function getEnvDisplayName(

0 commit comments

Comments
 (0)