Address PR review comments: task listener cleanup, use wrappers, add await, handle empty envs

Author: karthiknadig

src/common/tasks.apis.ts

@@ -1,5 +1,13 @@
-import { Task, TaskExecution, tasks } from 'vscode';
+import { Disposable, Task, TaskExecution, TaskProcessEndEvent, tasks } from 'vscode';
 
 export async function executeTask(task: Task): Promise<TaskExecution> {
     return tasks.executeTask(task);
 }
+
+export function onDidEndTaskProcess(
+    listener: (e: TaskProcessEndEvent) => unknown,
+    thisArgs?: unknown,
+    disposables?: Disposable[],
+): Disposable {
+    return tasks.onDidEndTaskProcess(listener, thisArgs, disposables);
+}

src/managers/builtin/sysPythonManager.ts

@@ -75,7 +75,7 @@ export class SysPythonManager implements EnvironmentManager {
 
         // If no Python environments were found, offer to install via uv
         if (this.collection.length === 0) {
-            promptInstallPythonViaUv('activation', this.api, this.log);
+            await promptInstallPythonViaUv('activation', this.api, this.log);
         }
 
         this._initialized.resolve();

src/managers/builtin/uvPythonInstaller.ts

@@ -1,25 +1,15 @@
-import {
-    LogOutputChannel,
-    ProgressLocation,
-    ShellExecution,
-    Task,
-    TaskPanelKind,
-    TaskRevealKind,
-    TaskScope,
-    tasks,
-    window,
-} from 'vscode';
+import { LogOutputChannel, ProgressLocation, ShellExecution, Task, TaskPanelKind, TaskRevealKind, TaskScope } from 'vscode';
 import { PythonEnvironmentApi } from '../../api';
 import { spawnProcess } from '../../common/childProcess.apis';
 import { UvInstallStrings } from '../../common/localize';
 import { traceError, traceInfo, traceLog } from '../../common/logging';
 import { getGlobalPersistentState } from '../../common/persistentState';
-import { executeTask } from '../../common/tasks.apis';
+import { executeTask, onDidEndTaskProcess } from '../../common/tasks.apis';
 import { EventNames } from '../../common/telemetry/constants';
 import { sendTelemetryEvent } from '../../common/telemetry/sender';
 import { createDeferred } from '../../common/utils/deferred';
 import { isWindows } from '../../common/utils/platformUtils';
-import { showInformationMessage } from '../../common/window.apis';
+import { showErrorMessage, showInformationMessage, withProgress } from '../../common/window.apis';
 import { isUvInstalled, resetUvInstallationCache } from './helpers';
 
 const UV_INSTALL_PYTHON_DONT_ASK_KEY = 'python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK';
@@ -72,6 +62,9 @@ async function getUvInstallCommand(): Promise<{ executable: string; args: string
     };
 }
 
+// Timeout for task completion (5 minutes)
+const TASK_TIMEOUT_MS = 5 * 60 * 1000;
+
 /**
  * Runs a shell command as a visible VS Code task and waits for completion.
  * @param name Task name displayed in the UI
@@ -92,20 +85,29 @@ async function runTaskAndWait(name: string, executable: string, args: string[]):
 
     const deferred = createDeferred<boolean>();
 
-    const disposable = tasks.onDidEndTaskProcess((e) => {
+    const disposable = onDidEndTaskProcess((e) => {
         if (e.execution.task === task) {
-            disposable.dispose();
             deferred.resolve(e.exitCode === 0);
         }
     });
 
+    // Set up timeout to prevent indefinite waiting
+    const timeoutId = setTimeout(() => {
+        if (!deferred.completed) {
+            traceError(`Task "${name}" timed out after ${TASK_TIMEOUT_MS / 1000} seconds`);
+            deferred.resolve(false);
+        }
+    }, TASK_TIMEOUT_MS);
+
     try {
         await executeTask(task);
         return await deferred.promise;
     } catch (err) {
-        disposable.dispose();
         traceError(`Task "${name}" failed:`, err);
         return false;
+    } finally {
+        clearTimeout(timeoutId);
+        disposable.dispose();
     }
 }
 
@@ -218,7 +220,7 @@ export async function installPythonWithUv(api: PythonEnvironmentApi, log?: LogOu
 
     sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_STARTED, undefined, { uvAlreadyInstalled: uvInstalled });
 
-    return await window.withProgress(
+    return await withProgress(
         {
             location: ProgressLocation.Notification,
             title: UvInstallStrings.installingPython,
@@ -232,7 +234,7 @@ export async function installPythonWithUv(api: PythonEnvironmentApi, log?: LogOu
                 const uvSuccess = await installUv(log);
                 if (!uvSuccess) {
                     sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_FAILED, undefined, { stage: 'uvInstall' });
-                    window.showErrorMessage(UvInstallStrings.uvInstallFailed);
+                    showErrorMessage(UvInstallStrings.uvInstallFailed);
                     return false;
                 }
             }
@@ -241,7 +243,7 @@ export async function installPythonWithUv(api: PythonEnvironmentApi, log?: LogOu
             const pythonSuccess = await installPythonViaUv(log);
             if (!pythonSuccess) {
                 sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_FAILED, undefined, { stage: 'pythonInstall' });
-                window.showErrorMessage(UvInstallStrings.installFailed);
+                showErrorMessage(UvInstallStrings.installFailed);
                 return false;
             }
 
@@ -250,7 +252,7 @@ export async function installPythonWithUv(api: PythonEnvironmentApi, log?: LogOu
             await api.refreshEnvironments(undefined);
 
             sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_COMPLETED);
-            window.showInformationMessage(UvInstallStrings.installComplete);
+            showInformationMessage(UvInstallStrings.installComplete);
 
             return true;
         },

src/managers/builtin/venvManager.ts

@@ -151,8 +151,13 @@ export class VenvManager implements EnvironmentManager {
                 if (installed) {
                     // Re-fetch environments after installation
                     globals = await this.api.getEnvironments('global');
-                    // Update globalEnv reference
-                    this.globalEnv = getLatest(globals.filter((e) => e.version.startsWith('3.')));
+                    // Update globalEnv reference if we found any Python 3.x environments
+                    const python3Envs = globals.filter((e) => e.version.startsWith('3.'));
+                    if (python3Envs.length === 0) {
+                        this.log.warn('Python installed via uv but no Python 3.x global environments were detected.');
+                    } else {
+                        this.globalEnv = getLatest(python3Envs);
+                    }
                 }
             }
 

src/test/managers/builtin/uvPythonInstaller.unit.test.ts

@@ -168,3 +168,8 @@ suite('uvPythonInstaller - isDontAskAgainSet and clearDontAskAgain', () => {
         assert(mockState.set.calledWith('python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK', false), 'Should clear the flag');
     });
 });
+
+// NOTE: Installation functions (installUv, installPythonViaUv, installPythonWithUv) require
+// VS Code's Task API which cannot be fully mocked in unit tests.
+// These should be tested via integration tests in a real VS Code environment.
+

src/test/mocks/vsc/extHostedTypes.ts

@@ -1706,7 +1706,9 @@ export class ShellExecution implements vscode.ShellExecution {
         //     }
         // }
         // return hash.digest('hex');
-        throw new Error('Not spported');
+        // Return a simple unique ID based on command
+        const cmd = typeof this._command === 'string' ? this._command : this._command?.value ?? '';
+        return `shell-${cmd}-${Date.now()}`;
     }
 }