Skip to content

Commit 6138122

Browse files
Respect terminal cwd when resolving local venvs (#1708)
## Summary - resolve terminal environments from the terminal cwd before using workspace-wide fallbacks - reject a selected local venv when it belongs to a sibling directory, then use one nearest, unambiguous venv that contains the cwd - fail closed instead of activating another project's environment when the cwd project has no safe match - keep terminal activation menu context synchronized with the currently active terminal Fixes #1631 ## Context The issue uses a single VS Code workspace folder as a container for several independent Python projects: ```text AILab/ ProjectA/.venv ProjectB/.venv ProjectC/.venv ``` With the default `workspaceSearchPaths` (`[".venv", "*/.venv"]`), all three environments are discovered. However, the workspace root is automatically registered as the only Python project unless the nested projects are explicitly added. A terminal opened in ProjectC could therefore receive ProjectA's activation command. Besides activating the wrong dependencies and Python version, this affects every integrated terminal opened by other extensions because auto-activation runs on the terminal-open event. ## Root cause The failure was caused by two scope-collapsing decisions: 1. Nested environment paths resolve to the automatically registered workspace-root project through ancestor project lookup. `VenvManager` can therefore associate one sorted or persisted sibling venv with that root. 2. `getEnvironmentForTerminal()` previously preferred project-wide environment consensus before checking terminal cwd. With exactly one registered project, it returned the root's selected environment immediately, so the cwd heuristic never ran. Simply moving the existing cwd lookup earlier is insufficient: `api.getEnvironment(ProjectC)` still maps ProjectC back to the root project and returns the same sibling environment. ## Fix ### Cwd-aware terminal resolution The terminal resolver now: 1. reads shell-integration cwd first, then terminal creation cwd 2. finds the most-specific registered project containing that cwd 3. preserves the project's selected environment unless it is a built-in venv located on a divergent sibling branch 4. for a sibling venv only, searches already-known environments for candidates that: - belong to the built-in venv manager - are valid and have an absolute `sysPrefix` - live inside the containing project - have an owner directory that is an ancestor of the terminal cwd 5. selects exactly one nearest candidate If no candidate exists, multiple candidates are equally near, lookup fails, or lookup exceeds one second, the resolver returns no environment rather than activating the wrong one. The losing timeout is cancelled after a successful lookup. A known cwd project with no selected environment also stops resolution, preventing another project's sole environment from leaking through the previous multi-project fallback. ### Compatibility boundaries This is intentionally terminal-only. It does not change central environment selection, persisted manager state, debugger/Pylance resolution, status-bar selection, task execution, or the public environment API. | Scenario | Behavior | | --- | --- | | `workspace/.venv` with a terminal in `workspace/src` | Existing root venv remains selected | | Registered nested project with an explicit environment | Existing project environment remains selected | | Conda/Poetry/Pyenv or another external environment | Existing selected environment remains selected | | Terminal cwd outside all registered projects | Existing fallback behavior remains | | Terminal cwd unavailable | Existing fallback behavior remains | | Selected venv is under ProjectA and cwd is under ProjectC | Use ProjectC's unique nearest venv, or activate nothing | | Cwd project has no selected environment | Activate nothing; do not borrow another project's environment | ### Terminal activation UI Safe resolution can intentionally return `undefined`. The activation menu contexts are now cleared in that case instead of retaining the previous terminal's Activate/Deactivate state. Async context updates also verify that the originating terminal is still active, preventing slower lookups or background activation events from overwriting the currently focused terminal's global menu state. ## Tests Added direct coverage for: - normal workspace-root venvs and nested cwd paths - sibling ProjectA/ProjectB/ProjectC venv resolution - shell-integration cwd precedence - ambiguous equally near venvs - missing local venvs - global venv exclusion - workspace-root terminals with a selected descendant venv - projects with no selected environment - preventing another project's environment from leaking into the cwd project - environment-list rejection and timeout - external environments, explicit nested projects, outside-workspace cwd, and unavailable cwd - clearing activation menu contexts when no environment resolves Validation performed: - `npm run compile-tests` - `npm run compile` - ESLint on all changed TypeScript files - all 97 tracked unit-test files: **1,525 passing, 5 expected pending** ## Out of scope The shell-execution timeout also reported in #1631 is a separate activation-state issue. This PR addresses only selecting the correct environment for a terminal cwd. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2f537bc5-b389-4ee2-aac2-e2b83be47f5c
1 parent dd372a8 commit 6138122

5 files changed

Lines changed: 462 additions & 13 deletions

File tree

‎src/extension.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,13 +523,15 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
523523
}
524524
}),
525525
terminalActivation.onDidChangeTerminalActivationState(async (e) => {
526-
await setActivateMenuButtonContext(e.terminal, e.environment, e.activated);
526+
if (activeTerminal() === e.terminal) {
527+
await setActivateMenuButtonContext(e.terminal, e.environment, e.activated);
528+
}
527529
}),
528530
onDidChangeActiveTerminal(async (t) => {
529531
if (t) {
530532
const env = terminalActivation.getEnvironment(t) ?? (await getEnvironmentForTerminal(api, t));
531-
if (env) {
532-
await setActivateMenuButtonContext(t, env, terminalActivation.isActivated(t));
533+
if (activeTerminal() === t) {
534+
await setActivateMenuButtonContext(t, env, env ? terminalActivation.isActivated(t) : false);
533535
}
534536
}
535537
}),

‎src/features/terminal/activateMenuButton.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import { isTaskTerminal } from './utils';
66

77
export async function setActivateMenuButtonContext(
88
terminal: Terminal,
9-
env: PythonEnvironment,
9+
env: PythonEnvironment | undefined,
1010
activated?: boolean,
1111
): Promise<void> {
12-
const activatable = !isTaskTerminal(terminal) && isActivatableEnvironment(env);
12+
const activatable = !!env && !isTaskTerminal(terminal) && isActivatableEnvironment(env);
1313
await executeCommand('setContext', 'pythonTerminalActivation', activatable);
1414

15-
if (activated !== undefined) {
16-
await executeCommand('setContext', 'pythonTerminalActivated', activated);
15+
if (!env || activated !== undefined) {
16+
await executeCommand('setContext', 'pythonTerminalActivated', env ? activated : false);
1717
}
1818
}

‎src/features/terminal/utils.ts‎

Lines changed: 151 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import * as path from 'path';
22
import { Disposable, env, ExtensionTerminalOptions, tasks, Terminal, TerminalOptions, Uri } from 'vscode';
3-
import { PythonEnvironment, PythonProject, PythonProjectEnvironmentApi, PythonProjectGetterApi } from '../../api';
4-
import { traceVerbose } from '../../common/logging';
3+
import {
4+
PythonEnvironment,
5+
PythonEnvironmentApi,
6+
PythonProject,
7+
PythonProjectEnvironmentApi,
8+
PythonProjectGetterApi,
9+
} from '../../api';
10+
import { VENV_MANAGER_ID } from '../../common/constants';
11+
import { traceError, traceVerbose } from '../../common/logging';
512
import { timeout } from '../../common/utils/asyncUtils';
613
import { createSimpleDebounce } from '../../common/utils/debounce';
714
import { onDidChangeTerminalShellIntegration, onDidWriteTerminalData } from '../../common/window.apis';
@@ -10,6 +17,7 @@ import { identifyTerminalShell } from '../common/shellDetector';
1017
import { shellIntegrationSupportedShells } from './shells/common/shellUtils';
1118

1219
export const SHELL_INTEGRATION_TIMEOUT = 500; // 0.5 seconds
20+
const LOCAL_VENV_LOOKUP_TIMEOUT_MS = 1000;
1321

1422
/**
1523
* Use `terminal.integrated.shellIntegration.timeout` setting if available.
@@ -191,13 +199,152 @@ async function getDistinctProjectEnvs(
191199
return envs;
192200
}
193201

202+
function isSameOrParentPath(parentPath: string, candidatePath: string): boolean {
203+
const relative = path.relative(path.resolve(parentPath), path.resolve(candidatePath));
204+
return relative === '' || (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative));
205+
}
206+
207+
function getProjectForCwd(projects: readonly PythonProject[], cwd: string): PythonProject | undefined {
208+
return [...projects]
209+
.filter((project) => isSameOrParentPath(project.uri.fsPath, cwd))
210+
.sort((a, b) => b.uri.fsPath.length - a.uri.fsPath.length)[0];
211+
}
212+
213+
function getLocalVenvOwner(environment: PythonEnvironment): string | undefined {
214+
if (
215+
environment.envId.managerId !== VENV_MANAGER_ID ||
216+
environment.error ||
217+
!path.isAbsolute(environment.sysPrefix)
218+
) {
219+
return undefined;
220+
}
221+
return path.dirname(path.resolve(environment.sysPrefix));
222+
}
223+
224+
function isSiblingVenv(environment: PythonEnvironment, project: PythonProject, cwd: string): boolean {
225+
const owner = getLocalVenvOwner(environment);
226+
return (
227+
!!owner &&
228+
isSameOrParentPath(project.uri.fsPath, owner) &&
229+
!isSameOrParentPath(owner, cwd) &&
230+
!isSameOrParentPath(cwd, owner)
231+
);
232+
}
233+
234+
async function getLocalVenvForCwd(
235+
api: PythonEnvironmentApi,
236+
project: PythonProject,
237+
cwd: string,
238+
): Promise<PythonEnvironment | undefined> {
239+
const environmentLookup = api
240+
.getEnvironments('all')
241+
.then((environments) => ({ environments }))
242+
.catch((error) => {
243+
traceError('Failed to get environments for terminal cwd resolution', error);
244+
return { environments: undefined };
245+
});
246+
let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
247+
const lookupTimeout = new Promise<{ environments: undefined }>((resolve) => {
248+
timeoutHandle = setTimeout(() => {
249+
traceVerbose(`Timed out looking for a local virtual environment for terminal cwd: ${cwd}`);
250+
resolve({ environments: undefined });
251+
}, LOCAL_VENV_LOOKUP_TIMEOUT_MS);
252+
});
253+
const lookupResult = await Promise.race([environmentLookup, lookupTimeout]).finally(() => {
254+
if (timeoutHandle) {
255+
clearTimeout(timeoutHandle);
256+
}
257+
});
258+
if (!lookupResult.environments) {
259+
return undefined;
260+
}
261+
262+
const candidates = lookupResult.environments
263+
.map((environment) => ({ environment, owner: getLocalVenvOwner(environment) }))
264+
.filter(
265+
(candidate): candidate is { environment: PythonEnvironment; owner: string } =>
266+
candidate.owner !== undefined &&
267+
isSameOrParentPath(project.uri.fsPath, candidate.owner) &&
268+
isSameOrParentPath(candidate.owner, cwd),
269+
)
270+
.map((candidate) => ({
271+
...candidate,
272+
distance: path
273+
.relative(path.resolve(candidate.owner), path.resolve(cwd))
274+
.split(path.sep)
275+
.filter(Boolean).length,
276+
}));
277+
278+
if (candidates.length === 0) {
279+
return undefined;
280+
}
281+
282+
const nearestDistance = Math.min(...candidates.map((candidate) => candidate.distance));
283+
const nearest = candidates.filter((candidate) => candidate.distance === nearestDistance);
284+
const unique = Array.from(
285+
new Map(nearest.map((candidate) => [candidate.environment.envId.id, candidate.environment])).values(),
286+
);
287+
288+
if (unique.length !== 1) {
289+
traceVerbose(`Multiple local virtual environments match terminal cwd: ${cwd}`);
290+
return undefined;
291+
}
292+
293+
return unique[0];
294+
}
295+
296+
interface CwdEnvironmentResolution {
297+
environment?: PythonEnvironment;
298+
stopFallback: boolean;
299+
}
300+
301+
async function getEnvironmentForCwd(
302+
api: PythonEnvironmentApi,
303+
projects: readonly PythonProject[],
304+
cwd: string,
305+
): Promise<CwdEnvironmentResolution> {
306+
const project = getProjectForCwd(projects, cwd);
307+
if (!project) {
308+
return { stopFallback: false };
309+
}
310+
311+
const projectEnvironment = await api.getEnvironment(project.uri);
312+
if (projectEnvironment && !isSiblingVenv(projectEnvironment, project, cwd)) {
313+
return { environment: projectEnvironment, stopFallback: true };
314+
}
315+
316+
if (!projectEnvironment) {
317+
return { stopFallback: true };
318+
}
319+
320+
traceVerbose(`Ignoring virtual environment outside terminal cwd: ${projectEnvironment.environmentPath.fsPath}`);
321+
322+
const localEnvironment = await getLocalVenvForCwd(api, project, cwd);
323+
if (localEnvironment) {
324+
traceVerbose(`Using local virtual environment for terminal cwd: ${localEnvironment.environmentPath.fsPath}`);
325+
return { environment: localEnvironment, stopFallback: true };
326+
}
327+
328+
return {
329+
stopFallback: true,
330+
};
331+
}
332+
194333
export async function getEnvironmentForTerminal(
195-
api: PythonProjectGetterApi & PythonProjectEnvironmentApi,
334+
api: PythonEnvironmentApi,
196335
terminal?: Terminal,
197336
): Promise<PythonEnvironment | undefined> {
198337
let env: PythonEnvironment | undefined;
199338

200339
const projects = api.getPythonProjects();
340+
const terminalCwd = terminal ? getTerminalCwd(terminal) : undefined;
341+
if (terminalCwd) {
342+
const cwdResolution = await getEnvironmentForCwd(api, projects, terminalCwd);
343+
if (cwdResolution.environment || cwdResolution.stopFallback) {
344+
return cwdResolution.environment;
345+
}
346+
}
347+
201348
if (projects.length === 0) {
202349
env = await api.getEnvironment(undefined);
203350
} else if (projects.length === 1) {
@@ -217,10 +364,9 @@ export async function getEnvironmentForTerminal(
217364
if (env) {
218365
return env;
219366
}
220-
221367
// This is a heuristic approach to attempt to find the environment for this terminal.
222368
// This is not guaranteed to work, but is better than nothing.
223-
const terminalCwd = terminal ? getTerminalCwd(terminal) : undefined;
369+
// This is not guaranteed to work, but is better than nothing.
224370
if (terminalCwd) {
225371
env = await api.getEnvironment(Uri.file(path.resolve(terminalCwd)));
226372
} else {

‎src/test/features/terminal/activateMenuButton.unit.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,19 @@ suite('Terminal - Activate Menu Button', () => {
5454
'Should set pythonTerminalActivation to false for task terminal',
5555
);
5656
});
57+
58+
test('should clear activation contexts when no environment is resolved', async () => {
59+
await setActivateMenuButtonContext(mockTerminal, undefined);
60+
61+
assert.ok(
62+
executeCommandStub.calledWith('setContext', 'pythonTerminalActivation', false),
63+
'Should hide the activation button',
64+
);
65+
assert.ok(
66+
executeCommandStub.calledWith('setContext', 'pythonTerminalActivated', false),
67+
'Should clear the activated state',
68+
);
69+
assert.ok(isTaskTerminalStub.notCalled, 'Should not inspect terminal type without an environment');
70+
assert.ok(isActivatableEnvironmentStub.notCalled, 'Should not inspect an undefined environment');
71+
});
5772
});

0 commit comments

Comments
 (0)