diff --git a/src/features/common/managerReady.ts b/src/features/common/managerReady.ts index a0b71c26..335e5176 100644 --- a/src/features/common/managerReady.ts +++ b/src/features/common/managerReady.ts @@ -1,5 +1,5 @@ import { Disposable, l10n, Uri } from 'vscode'; -import { allExtensions, getExtension } from '../../common/extension.apis'; +import { getExtension } from '../../common/extension.apis'; import { WorkbenchStrings } from '../../common/localize'; import { traceError, traceInfo, traceWarn } from '../../common/logging'; import { EventNames } from '../../common/telemetry/constants'; @@ -30,7 +30,8 @@ function getExtensionId(managerId: string): string | undefined { /** * Wraps a deferred with a timeout so a missing/dead manager cannot block the API forever. * On timeout the deferred is resolved (not rejected) so callers proceed with degraded results - * instead of hanging. + * instead of hanging. If the manager's extension is genuinely not installed when the timeout + * fires, the user is prompted to install it. */ export function withManagerTimeout( deferred: Deferred, @@ -40,6 +41,7 @@ export function withManagerTimeout( if (deferred.completed) { return deferred.promise; } + return new Promise((resolve) => { const timer = setTimeout(() => { if (!deferred.completed) { @@ -53,6 +55,9 @@ export function withManagerTimeout( managerKind: kind, }); deferred.resolve(); + void promptInstallExtensionIfMissing(managerId).catch((err) => { + traceError(`Failed to prompt installation for manager extension "${managerId}".`, err); + }); } }, MANAGER_READY_TIMEOUT_MS); @@ -69,6 +74,74 @@ export function withManagerTimeout( }); } +/** + * Shows an install prompt only if the extension for the given manager ID + * is genuinely not available. Called after the timeout expires, giving the + * extension host ample time to initialize. Deduplicated per extension ID + * so each missing extension only prompts once per session. + */ +const prompted: Set = new Set(); +async function promptInstallExtensionIfMissing(managerId: string): Promise { + const extId = getExtensionId(managerId); + if (!extId) { + showErrorMessage(l10n.t(`Extension for {0} is not installed or enabled for this workspace.`, managerId)); + return; + } + + const ext = getExtension(extId); + if (ext) { + // Extension is installed but the manager never registered — don't prompt to install. + // Attempt activation as a recovery step since the extension host is reliable at this point. + if (!ext.isActive) { + traceWarn( + `Extension ${extId} is installed but manager "${managerId}" never registered. Attempting activation...`, + ); + try { + await ext.activate(); + traceInfo(`Extension ${extId} activated post-timeout for manager "${managerId}".`); + } catch (err) { + traceError(`Failed to activate extension ${extId} post-timeout for: ${managerId}`, err); + } + } else { + traceWarn( + `Extension ${extId} is installed and active but manager "${managerId}" never registered. ` + + `The extension may have failed during manager registration.`, + ); + } + return; + } + + if (prompted.has(extId)) { + return; + } + prompted.add(extId); + + traceError(`Extension for manager ${managerId} is not installed. Looked up extId="${extId}" via getExtension().`); + const result = await showErrorMessage( + l10n.t(`Do you want to install extension {0} to enable {1} support?`, extId, managerId), + WorkbenchStrings.installExtension, + ); + if (result === WorkbenchStrings.installExtension) { + traceInfo(`Installing extension: ${extId}`); + try { + await installExtension(extId); + traceInfo(`Extension ${extId} installed.`); + } catch (err) { + traceError(`Failed to install extension: ${extId}`, err); + } + + try { + const installedExt = getExtension(extId); + if (installedExt && !installedExt.isActive) { + traceInfo(`Extension for manager ${managerId} is not active: Activating...`); + await installedExt.activate(); + } + } catch (err) { + traceError(`Failed to activate extension ${extId}, required for: ${managerId}`, err); + } + } +} + class ManagerReadyImpl implements ManagerReady { private readonly envManagers: Map> = new Map(); private readonly pkgManagers: Map> = new Map(); @@ -101,8 +174,12 @@ class ManagerReadyImpl implements ManagerReady { ); } - private checkExtension(managerId: string) { - const installed = allExtensions().some((ext) => managerId.startsWith(`${ext.id}:`)); + /** + * Best-effort activation nudge for the extension that provides the given manager. + * Uses setImmediate so the extension host has a chance to finish initializing before + * we query it. Deduplicated per manager ID to avoid redundant activation calls. + */ + private checkExtension(managerId: string): void { if (this.checked.has(managerId)) { return; } @@ -110,46 +187,17 @@ class ManagerReadyImpl implements ManagerReady { const extId = getExtensionId(managerId); if (extId) { setImmediate(async () => { - if (installed) { - const ext = getExtension(extId); - if (ext && !ext.isActive) { - traceInfo(`Extension for manager ${managerId} is not active: Activating...`); - try { - await ext.activate(); - traceInfo(`Extension for manager ${managerId} is now active.`); - } catch (err) { - traceError(`Failed to activate extension ${extId}, required for: ${managerId}`, err); - } - } - } else { - traceError(`Extension for manager ${managerId} is not installed.`); - const result = await showErrorMessage( - l10n.t(`Do you want to install extension {0} to enable {1} support.`, extId, managerId), - WorkbenchStrings.installExtension, - ); - if (result === WorkbenchStrings.installExtension) { - traceInfo(`Installing extension: ${extId}`); - try { - await installExtension(extId); - traceInfo(`Extension ${extId} installed.`); - } catch (err) { - traceError(`Failed to install extension: ${extId}`, err); - } - - try { - const ext = getExtension(extId); - if (ext && !ext.isActive) { - traceInfo(`Extension for manager ${managerId} is not active: Activating...`); - await ext.activate(); - } - } catch (err) { - traceError(`Failed to activate extension ${extId}, required for: ${managerId}`, err); - } + const ext = getExtension(extId); + if (ext && !ext.isActive) { + traceInfo(`Extension for manager ${managerId} is not active: Activating...`); + try { + await ext.activate(); + traceInfo(`Extension for manager ${managerId} is now active.`); + } catch (err) { + traceError(`Failed to activate extension ${extId}, required for: ${managerId}`, err); } } }); - } else { - showErrorMessage(l10n.t(`Extension for {0} is not installed or enabled for this workspace.`, managerId)); } } @@ -251,6 +299,15 @@ export function createManagerReady(em: EnvironmentManagers, pm: PythonProjectMan } } +/** + * Reset the module-level singleton so `createManagerReady` can be called again. + * Only for use in tests. + */ +export function _resetManagerReadyForTesting(): void { + _deferred = createDeferred(); + prompted.clear(); +} + export async function waitForEnvManager(uris?: Uri[]): Promise { const mr = await _deferred.promise; return mr.waitForEnvManager(uris); diff --git a/src/test/features/common/managerReady.unit.test.ts b/src/test/features/common/managerReady.unit.test.ts index 7cef4b54..c861c7de 100644 --- a/src/test/features/common/managerReady.unit.test.ts +++ b/src/test/features/common/managerReady.unit.test.ts @@ -1,10 +1,29 @@ import assert from 'assert'; import * as sinon from 'sinon'; +import { Disposable, EventEmitter } from 'vscode'; +import * as extensionApis from '../../../common/extension.apis'; import * as logging from '../../../common/logging'; import { EventNames } from '../../../common/telemetry/constants'; import * as telemetrySender from '../../../common/telemetry/sender'; import { createDeferred } from '../../../common/utils/deferred'; -import { MANAGER_READY_TIMEOUT_MS, withManagerTimeout } from '../../../features/common/managerReady'; +import * as windowApis from '../../../common/window.apis'; +import { + MANAGER_READY_TIMEOUT_MS, + _resetManagerReadyForTesting, + createManagerReady, + waitForEnvManagerId, + waitForPkgManagerId, + withManagerTimeout, +} from '../../../features/common/managerReady'; +import * as settingHelpers from '../../../features/settings/settingHelpers'; +import { + DidChangeEnvironmentManagerEventArgs, + DidChangePackageManagerEventArgs, + EnvironmentManagers, + InternalEnvironmentManager, + InternalPackageManager, + PythonProjectManager, +} from '../../../internal.api'; suite('withManagerTimeout', () => { let clock: sinon.SinonFakeTimers; @@ -14,7 +33,11 @@ suite('withManagerTimeout', () => { setup(() => { clock = sinon.useFakeTimers(); traceWarnStub = sinon.stub(logging, 'traceWarn'); + sinon.stub(logging, 'traceError'); sendTelemetryStub = sinon.stub(telemetrySender, 'sendTelemetryEvent'); + // Stub dependencies used by promptInstallExtensionIfMissing (called on timeout) + sinon.stub(extensionApis, 'getExtension').returns(undefined); + sinon.stub(windowApis, 'showErrorMessage').returns(Promise.resolve(undefined)); }); teardown(() => { @@ -104,3 +127,149 @@ suite('withManagerTimeout', () => { assert.strictEqual(properties.managerKind, 'package'); }); }); + +suite('ManagerReady - race condition handling', () => { + let envManagerEmitter: EventEmitter; + let pkgManagerEmitter: EventEmitter; + let clock: sinon.SinonFakeTimers; + let disposables: Disposable[]; + + setup(() => { + clock = sinon.useFakeTimers(); + disposables = []; + + _resetManagerReadyForTesting(); + + envManagerEmitter = new EventEmitter(); + pkgManagerEmitter = new EventEmitter(); + + // Stub logging and telemetry to keep test output clean + sinon.stub(logging, 'traceWarn'); + sinon.stub(logging, 'traceError'); + sinon.stub(logging, 'traceInfo'); + sinon.stub(telemetrySender, 'sendTelemetryEvent'); + sinon.stub(windowApis, 'showErrorMessage').returns(Promise.resolve(undefined)); + sinon.stub(extensionApis, 'getExtension').returns({ + id: 'ms-python.python', + isActive: true, + } as unknown as ReturnType); + sinon.stub(settingHelpers, 'getDefaultEnvManagerSetting').returns('ms-python.python:venv'); + sinon.stub(settingHelpers, 'getDefaultPkgManagerSetting').returns('ms-python.python:pip'); + + const mockEm = { + onDidChangeEnvironmentManager: envManagerEmitter.event, + onDidChangePackageManager: pkgManagerEmitter.event, + } as unknown as EnvironmentManagers; + + const mockPm = { + getProjects: () => [], + } as unknown as PythonProjectManager; + + createManagerReady(mockEm, mockPm, disposables); + }); + + teardown(() => { + clock.restore(); + disposables.forEach((d) => d.dispose()); + envManagerEmitter.dispose(); + pkgManagerEmitter.dispose(); + sinon.restore(); + _resetManagerReadyForTesting(); + }); + + test('no install prompt when manager registers before timeout', async () => { + const waitPromise = waitForEnvManagerId(['ms-python.python:venv']); + // Flush microtasks so the internal await _deferred.promise completes + // and the timeout/deferred is set up + await clock.tickAsync(0); + + // Manager registers before timeout + envManagerEmitter.fire({ + kind: 'registered', + manager: { id: 'ms-python.python:venv' } as unknown as InternalEnvironmentManager, + }); + + await clock.tickAsync(0); + await waitPromise; + + // Advance past timeout to ensure no late prompt + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(!showErrorStub.called, 'should not prompt to install when manager registered successfully'); + }); + + test('no install prompt on timeout when extension is installed but manager never registered', async () => { + // Extension IS installed (getExtension returns it), but manager never fires registration event + const waitPromise = waitForEnvManagerId(['ms-python.python:venv']); + // Flush microtasks so internal await completes and timeout is armed + await clock.tickAsync(0); + + // Advance past timeout — manager never registers + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + await waitPromise; + + // Should NOT prompt to install because getExtension finds the extension + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(!showErrorStub.called, 'should not prompt to install when extension is installed'); + + // Should log a warning instead + const traceWarnStub = logging.traceWarn as sinon.SinonStub; + const warnAboutManager = traceWarnStub.getCalls().find( + (c: sinon.SinonSpyCall) => typeof c.args[0] === 'string' && c.args[0].includes('never registered'), + ); + assert.ok(warnAboutManager, 'should warn that manager never registered despite extension being installed'); + }); + + test('install prompt shown on timeout only when extension is genuinely not installed', async () => { + const getExtensionStub = extensionApis.getExtension as sinon.SinonStub; + getExtensionStub.returns(undefined); // Extension not installed + + const waitPromise = waitForEnvManagerId(['ms-python.python:venv']); + // Flush microtasks so internal await completes and timeout is armed + await clock.tickAsync(0); + + // Advance past timeout — manager never registers + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + await waitPromise; + + // NOW the install prompt should appear (after 30s, not immediately) + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(showErrorStub.called, 'should prompt to install after timeout when extension is missing'); + }); + + test('manager registered before wait resolves immediately without prompt', async () => { + envManagerEmitter.fire({ + kind: 'registered', + manager: { id: 'ms-python.python:venv' } as unknown as InternalEnvironmentManager, + }); + + await clock.tickAsync(0); + + // Wait should resolve immediately since the manager already registered + const waitPromise = waitForEnvManagerId(['ms-python.python:venv']); + await clock.tickAsync(0); + await waitPromise; + + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(!showErrorStub.called, 'should not prompt when manager already registered'); + }); + + test('pkg manager wait resolves when registration event fires', async () => { + const waitPromise = waitForPkgManagerId(['ms-python.python:pip']); + + pkgManagerEmitter.fire({ + kind: 'registered', + manager: { id: 'ms-python.python:pip' } as unknown as InternalPackageManager, + }); + + await clock.tickAsync(0); + await waitPromise; + + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(!showErrorStub.called, 'should not prompt when pkg manager registered'); + }); +});