Merge branch 'main' into copilot/fix-env-file-notification-bug

eleanorjboyd · web-flow · commit 080efaf44095 · 2026-03-27T08:39:33.000-07:00
diff --git a/src/extension.ts b/src/extension.ts
@@ -447,12 +447,40 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
         ),
         commands.registerCommand('python-envs.reportIssue', async () => {
             try {
+                // Prompt for issue title
+                const rawTitle = await window.showInputBox({
+                    title: l10n.t('Report Issue - Title'),
+                    prompt: l10n.t('Enter a brief title for the issue'),
+                    placeHolder: l10n.t('e.g., Environment not detected, activation fails, etc.'),
+                    ignoreFocusOut: true,
+                });
+                const title = rawTitle?.trim();
+
+                if (!title) {
+                    // User cancelled or provided empty title
+                    return;
+                }
+
+                // Prompt for issue description
+                const rawDescription = await window.showInputBox({
+                    title: l10n.t('Report Issue - Description'),
+                    prompt: l10n.t('Describe the issue in more detail'),
+                    placeHolder: l10n.t('Provide additional context about what happened...'),
+                    ignoreFocusOut: true,
+                });
+                const description = rawDescription?.trim();
+
+                if (!description) {
+                    // User cancelled or provided empty description
+                    return;
+                }
+
                 const issueData = await collectEnvironmentInfo(context, envManagers, projectManager);
 
                 await commands.executeCommand('workbench.action.openIssueReporter', {
                     extensionId: 'ms-python.vscode-python-envs',
-                    issueTitle: '[Python Environments] ',
-                    issueBody: `<!-- Please describe the issue you're experiencing -->\n\n<!-- The following information was automatically generated -->\n\n<details>\n<summary>Environment Information</summary>\n\n\`\`\`\n${issueData}\n\`\`\`\n\n</details>`,
+                    issueTitle: `[Python Environments] ${title}`,
+                    issueBody: `## Description\n${description}\n\n## Steps to Reproduce\n1. \n2. \n3. \n\n## Expected Behavior\n\n\n## Actual Behavior\n\n\n<!-- The following information was automatically generated -->\n\n<details>\n<summary>Environment Information</summary>\n\n\`\`\`\n${issueData}\n\`\`\`\n\n</details>`,
                 });
             } catch (error) {
                 window.showErrorMessage(`Failed to open issue reporter: ${error}`);
diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts
@@ -61,23 +61,32 @@ export async function clearVenvCache(): Promise<void> {
 }
 
 export async function getVenvForWorkspace(fsPath: string): Promise<string | undefined> {
-    if (process.env.VIRTUAL_ENV) {
-        return process.env.VIRTUAL_ENV;
-    }
-
-    const state = await getWorkspacePersistentState();
-    const data: { [key: string]: string } | undefined = await state.get(VENV_WORKSPACE_KEY);
-    if (data) {
-        try {
+    // Check persisted user selection first — this should always take priority
+    // over process.env.VIRTUAL_ENV so that explicit selections survive reload.
+    try {
+        const state = await getWorkspacePersistentState();
+        const data: { [key: string]: string } | undefined = await state.get(VENV_WORKSPACE_KEY);
+        if (data) {
             const envPath = data[fsPath];
-            if (await fsapi.pathExists(envPath)) {
+            if (envPath && (await fsapi.pathExists(envPath))) {
                 return envPath;
             }
-            setVenvForWorkspace(fsPath, undefined);
-        } catch {
-            return undefined;
+            if (envPath) {
+                await setVenvForWorkspace(fsPath, undefined);
+            }
+        }
+    } catch {
+        // fall through to VIRTUAL_ENV check
+    }
+
+    // Fall back to VIRTUAL_ENV only if it points to a path inside this workspace.
+    if (process.env.VIRTUAL_ENV) {
+        const rel = path.relative(fsPath, process.env.VIRTUAL_ENV);
+        if (rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel))) {
+            return process.env.VIRTUAL_ENV;
         }
     }
+
     return undefined;
 }
 
diff --git a/src/managers/conda/condaUtils.ts b/src/managers/conda/condaUtils.ts
@@ -82,19 +82,21 @@ export function getCondaPathSetting(): string | undefined {
 }
 
 export async function getCondaForWorkspace(fsPath: string): Promise<string | undefined> {
+    // Check persisted user selection first so explicit choices survive restarts
+    const state = await getWorkspacePersistentState();
+    const data = await state.get<unknown>(CONDA_WORKSPACE_KEY);
+    if (data && typeof data === 'object') {
+        const workspaceSelections = data as { [key: string]: string };
+        if (Object.prototype.hasOwnProperty.call(workspaceSelections, fsPath)) {
+            return workspaceSelections[fsPath];
+        }
+    }
+
+    // Fall back to CONDA_PREFIX only when no explicit selection exists
     if (process.env.CONDA_PREFIX) {
         return process.env.CONDA_PREFIX;
     }
 
-    const state = await getWorkspacePersistentState();
-    const data: { [key: string]: string } | undefined = await state.get(CONDA_WORKSPACE_KEY);
-    if (data) {
-        try {
-            return data[fsPath];
-        } catch {
-            return undefined;
-        }
-    }
     return undefined;
 }
 
diff --git a/src/test/integration/pythonProjects.integration.test.ts b/src/test/integration/pythonProjects.integration.test.ts
@@ -23,6 +23,47 @@ import { PythonEnvironment, PythonEnvironmentApi } from '../../api';
 import { ENVS_EXTENSION_ID } from '../constants';
 import { TestEventHandler, waitForCondition } from '../testUtils';
 
+const ENV_CHANGE_TIMEOUT_MS = 15_000;
+
+function getDifferentEnvironment(
+    environments: PythonEnvironment[],
+    currentEnv: PythonEnvironment | undefined,
+): PythonEnvironment | undefined {
+    return environments.find((env) => env.envId.id !== currentEnv?.envId.id);
+}
+
+async function setEnvironmentAndWaitForChange(
+    api: PythonEnvironmentApi,
+    projectUri: vscode.Uri,
+    env: PythonEnvironment,
+    timeoutMs = ENV_CHANGE_TIMEOUT_MS,
+): Promise<void> {
+    await new Promise<void>((resolve, reject) => {
+        let subscription: vscode.Disposable | undefined;
+        const timeout = setTimeout(() => {
+            subscription?.dispose();
+            reject(
+                new Error(`onDidChangeEnvironment did not fire within ${timeoutMs}ms. Expected envId: ${env.envId.id}`),
+            );
+        }, timeoutMs);
+
+        subscription = api.onDidChangeEnvironment((e) => {
+            if (e.uri?.toString() === projectUri.toString() && e.new?.envId.id === env.envId.id) {
+                clearTimeout(timeout);
+                subscription?.dispose();
+                resolve();
+            }
+        });
+
+        // Set environment after subscribing so we don't miss the event.
+        api.setEnvironment(projectUri, env).catch((err) => {
+            clearTimeout(timeout);
+            subscription?.dispose();
+            reject(err);
+        });
+    });
+}
+
 suite('Integration: Python Projects', function () {
     this.timeout(60_000);
 
@@ -162,54 +203,29 @@ suite('Integration: Python Projects', function () {
 
         const environments = await api.getEnvironments('all');
 
-        if (environments.length === 0) {
+        if (environments.length < 2) {
             this.skip();
             return;
         }
 
         const project = projects[0];
-        const env = environments[0];
-
-        // Diagnostic logging for CI debugging
-        console.log(`[TEST DEBUG] Project URI: ${project.uri.fsPath}`);
-        console.log(`[TEST DEBUG] Setting environment with envId: ${env.envId.id}`);
-        console.log(`[TEST DEBUG] Environment path: ${env.environmentPath?.fsPath}`);
-        console.log(`[TEST DEBUG] Total environments available: ${environments.length}`);
-        environments.forEach((e, i) => {
-            console.log(`[TEST DEBUG]   env[${i}]: ${e.envId.id} (${e.displayName})`);
-        });
 
-        // Set environment for project
-        await api.setEnvironment(project.uri, env);
-
-        // Track what getEnvironment returns during polling for diagnostics
-        let pollCount = 0;
-        let lastRetrievedId: string | undefined;
-        let lastRetrievedManagerId: string | undefined;
-
-        // Wait for the environment to be retrievable with the correct ID
-        // This handles async persistence across platforms
-        // Use 15s timeout - CI runners (especially macos) can be slow with settings persistence
-        await waitForCondition(
-            async () => {
-                const retrieved = await api.getEnvironment(project.uri);
-                pollCount++;
-                const retrievedId = retrieved?.envId?.id;
-                lastRetrievedManagerId = retrieved?.envId?.managerId;
-                if (retrievedId !== lastRetrievedId) {
-                    console.log(
-                        `[TEST DEBUG] Poll #${pollCount}: getEnvironment returned envId=${retrievedId ?? 'undefined'}, managerId=${lastRetrievedManagerId ?? 'undefined'}`,
-                    );
-                    lastRetrievedId = retrievedId;
-                }
-                return retrieved !== undefined && retrieved.envId.id === env.envId.id;
-            },
-            15_000,
-            () =>
-                `Environment was not set correctly. Expected envId: ${env.envId.id} (manager: ${env.envId.managerId}), last retrieved: ${lastRetrievedId ?? 'undefined'} (manager: ${lastRetrievedManagerId ?? 'undefined'}) after ${pollCount} polls`,
-        );
-
-        // Final verification
+        // Pick an environment different from the current one so setEnvironment
+        // actually triggers a change event. If all candidates map to the same env,
+        // skip instead of hanging on an event that will never fire.
+        const currentEnv = await api.getEnvironment(project.uri);
+        const env = getDifferentEnvironment(environments, currentEnv);
+        if (!env) {
+            this.skip();
+            return;
+        }
+
+        // Using an event-driven approach instead of polling avoids a race condition where
+        // setEnvironment's async settings write hasn't landed by the time getEnvironment
+        // reads back the manager from settings.
+        await setEnvironmentAndWaitForChange(api, project.uri, env);
+
+        // Verify getEnvironment returns the correct value now that setEnvironment has fully completed
         const retrievedEnv = await api.getEnvironment(project.uri);
         assert.ok(retrievedEnv, 'Should get environment after setting');
         assert.strictEqual(retrievedEnv.envId.id, env.envId.id, 'Retrieved environment should match set environment');
@@ -232,13 +248,12 @@ suite('Integration: Python Projects', function () {
 
         const project = projects[0];
 
-        // Get current environment to pick a different one
+        // Pick an environment different from the current one so a change event is guaranteed.
         const currentEnv = await api.getEnvironment(project.uri);
-
-        // Pick an environment different from current
-        let targetEnv = environments[0];
-        if (currentEnv && currentEnv.envId.id === targetEnv.envId.id) {
-            targetEnv = environments[1];
+        const targetEnv = getDifferentEnvironment(environments, currentEnv);
+        if (!targetEnv) {
+            this.skip();
+            return;
         }
 
         // Register handler BEFORE making the change
@@ -275,30 +290,23 @@ suite('Integration: Python Projects', function () {
         const projects = api.getPythonProjects();
         const environments = await api.getEnvironments('all');
 
-        if (projects.length === 0 || environments.length === 0) {
+        if (projects.length === 0 || environments.length < 2) {
             this.skip();
             return;
         }
 
         const project = projects[0];
-        const env = environments[0];
-
-        // Set environment first
-        await api.setEnvironment(project.uri, env);
-
-        // Wait for it to be set
-        // Use 15s timeout - CI runners can be slow with settings persistence
-        let clearTestLastId: string | undefined;
-        await waitForCondition(
-            async () => {
-                const retrieved = await api.getEnvironment(project.uri);
-                clearTestLastId = retrieved?.envId?.id;
-                return retrieved !== undefined && retrieved.envId.id === env.envId.id;
-            },
-            15_000,
-            () =>
-                `Environment was not set before clearing. Expected: ${env.envId.id} (manager: ${env.envId.managerId}), got: ${clearTestLastId ?? 'undefined'}`,
-        );
+
+        // Pick an environment different from the current one to guarantee a change event
+        const currentEnv = await api.getEnvironment(project.uri);
+        const env = getDifferentEnvironment(environments, currentEnv);
+        if (!env) {
+            this.skip();
+            return;
+        }
+
+        // Set environment first, using event-driven wait.
+        await setEnvironmentAndWaitForChange(api, project.uri, env);
 
         // Verify it was set
         const beforeClear = await api.getEnvironment(project.uri);
@@ -337,32 +345,23 @@ suite('Integration: Python Projects', function () {
         const projects = api.getPythonProjects();
         const environments = await api.getEnvironments('all');
 
-        if (projects.length === 0 || environments.length === 0) {
+        if (projects.length === 0 || environments.length < 2) {
             this.skip();
             return;
         }
 
         const project = projects[0];
-        const env = environments[0];
-
-        // Set environment for project
-        await api.setEnvironment(project.uri, env);
-
-        // Wait for it to be set
-        // Use 15s timeout - CI runners can be slow with settings persistence
-        let fileTestLastId: string | undefined;
-        let fileTestLastManagerId: string | undefined;
-        await waitForCondition(
-            async () => {
-                const retrieved = await api.getEnvironment(project.uri);
-                fileTestLastId = retrieved?.envId?.id;
-                fileTestLastManagerId = retrieved?.envId?.managerId;
-                return retrieved !== undefined && retrieved.envId.id === env.envId.id;
-            },
-            15_000,
-            () =>
-                `Environment was not set for project. Expected: ${env.envId.id} (manager: ${env.envId.managerId}), got: ${fileTestLastId ?? 'undefined'} (manager: ${fileTestLastManagerId ?? 'undefined'})`,
-        );
+
+        // Pick an environment different from the current one to guarantee a change event
+        const currentEnv = await api.getEnvironment(project.uri);
+        const env = getDifferentEnvironment(environments, currentEnv);
+        if (!env) {
+            this.skip();
+            return;
+        }
+
+        // Set environment for project, using event-driven wait.
+        await setEnvironmentAndWaitForChange(api, project.uri, env);
 
         // Create a hypothetical file path inside the project
         const fileUri = vscode.Uri.joinPath(project.uri, 'some_script.py');
diff --git a/src/test/managers/builtin/venvUtils.getVenvForWorkspace.unit.test.ts b/src/test/managers/builtin/venvUtils.getVenvForWorkspace.unit.test.ts
diff --git a/src/test/managers/conda/condaUtils.getCondaForWorkspace.unit.test.ts b/src/test/managers/conda/condaUtils.getCondaForWorkspace.unit.test.ts
