From b1c55e959bb693c13ad17640e17ca088f61a41e3 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 27 Feb 2026 08:02:12 -0800 Subject: [PATCH 1/3] fix: deduplicate workspaceSearchPaths error and skip PET resolve for unresolved variables (Fixes #1289) --- src/features/interpreterSelection.ts | 43 +++++++++++++------ src/managers/common/nativePythonFinder.ts | 12 +++++- .../interpreterSelection.unit.test.ts | 30 +++++++++++++ ...Finder.getAllExtraSearchPaths.unit.test.ts | 32 +++++++++++++- 4 files changed, 102 insertions(+), 15 deletions(-) diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts index a9bc179e..98d8619b 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -107,20 +107,37 @@ async function resolvePriorityChainCore( const userInterpreterPath = getUserConfiguredSetting('python', 'defaultInterpreterPath', scope); if (userInterpreterPath) { const expandedInterpreterPath = resolveVariables(userInterpreterPath, scope); - const resolved = await tryResolveInterpreterPath(nativeFinder, api, expandedInterpreterPath, envManagers); - if (resolved) { - traceVerbose(`${logPrefix} Priority 3: Using defaultInterpreterPath: ${userInterpreterPath}`); - return { result: resolved, errors }; + if (expandedInterpreterPath.includes('${')) { + traceWarn( + `${logPrefix} defaultInterpreterPath '${userInterpreterPath}' contains unresolved variables, falling back to auto-discovery`, + ); + const error: SettingResolutionError = { + setting: 'defaultInterpreterPath', + configuredValue: userInterpreterPath, + reason: `Path contains unresolved variables`, + }; + errors.push(error); + } else { + const resolved = await tryResolveInterpreterPath( + nativeFinder, + api, + expandedInterpreterPath, + envManagers, + ); + if (resolved) { + traceVerbose(`${logPrefix} Priority 3: Using defaultInterpreterPath: ${userInterpreterPath}`); + return { result: resolved, errors }; + } + const error: SettingResolutionError = { + setting: 'defaultInterpreterPath', + configuredValue: userInterpreterPath, + reason: `Could not resolve interpreter path '${userInterpreterPath}'`, + }; + errors.push(error); + traceWarn( + `${logPrefix} defaultInterpreterPath '${userInterpreterPath}' unresolvable, falling back to auto-discovery`, + ); } - const error: SettingResolutionError = { - setting: 'defaultInterpreterPath', - configuredValue: userInterpreterPath, - reason: `Could not resolve interpreter path '${userInterpreterPath}'`, - }; - errors.push(error); - traceWarn( - `${logPrefix} defaultInterpreterPath '${userInterpreterPath}' unresolvable, falling back to auto-discovery`, - ); } // PRIORITY 4: Auto-discovery (no user-configured settings matched) diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index efdfb246..8264b93d 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -842,6 +842,15 @@ function getGlobalSearchPaths(): string[] { } } +let workspaceSearchPathsGlobalWarningShown = false; + +/** + * Resets the error flag for testing purposes. + */ +export function resetWorkspaceSearchPathsErrorFlag(): void { + workspaceSearchPathsGlobalWarningShown = false; +} + /** * Gets the most specific workspace-level setting available for workspaceSearchPaths. * Supports glob patterns which are expanded by PET. @@ -851,7 +860,8 @@ function getWorkspaceSearchPaths(): string[] { const envConfig = getConfiguration('python-envs'); const inspection = envConfig.inspect('workspaceSearchPaths'); - if (inspection?.globalValue) { + if (inspection?.globalValue && !workspaceSearchPathsGlobalWarningShown) { + workspaceSearchPathsGlobalWarningShown = true; traceError( 'python-envs.workspaceSearchPaths is set at the user/global level, but this setting can only be set at the workspace or workspace folder level.', ); diff --git a/src/test/features/interpreterSelection.unit.test.ts b/src/test/features/interpreterSelection.unit.test.ts index 4c5fcbf3..f223a202 100644 --- a/src/test/features/interpreterSelection.unit.test.ts +++ b/src/test/features/interpreterSelection.unit.test.ts @@ -264,6 +264,36 @@ suite('Interpreter Selection - Priority Chain', () => { assert.ok(mockNativeFinder.resolve.calledOnceWithExactly(expandedInterpreterPath)); }); + test('should skip native resolution when defaultInterpreterPath has unresolved variables', async () => { + // When resolveVariables can't resolve ${workspaceFolder} (e.g., global scope with no workspace), + // the path still contains '${' and should be skipped without calling nativeFinder.resolve + sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); + sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([]); + sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(undefined); + sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => { + if (section === 'python' && key === 'defaultInterpreterPath') { + return '${workspaceFolder}/.venv/bin/python3'; + } + return undefined; + }); + mockVenvManager.get.resolves(mockVenvEnv); + + const result = await resolveEnvironmentByPriority( + testUri, + mockEnvManagers as unknown as EnvironmentManagers, + mockProjectManager as unknown as PythonProjectManager, + mockNativeFinder as unknown as NativePythonFinder, + mockApi as unknown as PythonEnvironmentApi, + ); + + // Should fall through to auto-discovery without calling nativeFinder.resolve + assert.strictEqual(result.source, 'autoDiscovery'); + assert.ok( + mockNativeFinder.resolve.notCalled, + 'nativeFinder.resolve should not be called with unresolved variables', + ); + }); + test('should fall through to Priority 4 when defaultInterpreterPath cannot be resolved', async () => { sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration); sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([]); diff --git a/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts b/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts index 43a7fe63..0f5b10d1 100644 --- a/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts +++ b/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts @@ -6,7 +6,7 @@ import * as pathUtils from '../../../common/utils/pathUtils'; import * as workspaceApis from '../../../common/workspace.apis'; // Import the function under test -import { getAllExtraSearchPaths } from '../../../managers/common/nativePythonFinder'; +import { getAllExtraSearchPaths, resetWorkspaceSearchPathsErrorFlag } from '../../../managers/common/nativePythonFinder'; interface MockWorkspaceConfig { get: sinon.SinonStub; @@ -26,6 +26,8 @@ suite('getAllExtraSearchPaths Integration Tests', () => { let envConfig: MockWorkspaceConfig; setup(() => { + resetWorkspaceSearchPathsErrorFlag(); + // Mock VS Code workspace APIs mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration'); mockGetWorkspaceFolders = sinon.stub(workspaceApis, 'getWorkspaceFolders'); @@ -87,6 +89,7 @@ suite('getAllExtraSearchPaths Integration Tests', () => { teardown(() => { sinon.restore(); + resetWorkspaceSearchPathsErrorFlag(); }); suite('Legacy Path Consolidation Tests', () => { @@ -332,6 +335,33 @@ suite('getAllExtraSearchPaths Integration Tests', () => { ); }); + test('Global workspace setting warning is only logged once across multiple calls', async () => { + // Mock → Workspace setting incorrectly set at global level + pythonConfig.get.withArgs('venvPath').returns(undefined); + pythonConfig.get.withArgs('venvFolders').returns(undefined); + envConfig.inspect.withArgs('globalSearchPaths').returns({ globalValue: [] }); + envConfig.inspect.withArgs('workspaceSearchPaths').returns({ + globalValue: ['should-be-ignored'], + }); + + // Run multiple times + await getAllExtraSearchPaths(); + await getAllExtraSearchPaths(); + await getAllExtraSearchPaths(); + + // Assert - error should only be logged once, not three times + const matchingCalls = mockTraceError + .getCalls() + .filter((call: sinon.SinonSpyCall) => + /workspaceSearchPaths.*global.*level/i.test(String(call.args[0])), + ); + assert.strictEqual( + matchingCalls.length, + 1, + `Expected exactly 1 warning about workspaceSearchPaths global level, got ${matchingCalls.length}`, + ); + }); + test('Configuration read errors return empty arrays', async () => { // Mock → Configuration throws errors pythonConfig.get.withArgs('venvPath').returns(undefined); From 2b2aee9bc2a37616e16876f9dcf5812fea6c3927 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 27 Feb 2026 08:20:49 -0800 Subject: [PATCH 2/3] fix: address review feedback - consistent naming for reset flag (PR #1290) --- src/managers/common/nativePythonFinder.ts | 4 ++-- ...nativePythonFinder.getAllExtraSearchPaths.unit.test.ts | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index 8264b93d..33f690e7 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -845,9 +845,9 @@ function getGlobalSearchPaths(): string[] { let workspaceSearchPathsGlobalWarningShown = false; /** - * Resets the error flag for testing purposes. + * @internal Test-only helper to reset the workspaceSearchPaths global-level warning flag. */ -export function resetWorkspaceSearchPathsErrorFlag(): void { +export function resetWorkspaceSearchPathsGlobalWarningFlag(): void { workspaceSearchPathsGlobalWarningShown = false; } diff --git a/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts b/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts index 0f5b10d1..6159fdc2 100644 --- a/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts +++ b/src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts @@ -6,7 +6,7 @@ import * as pathUtils from '../../../common/utils/pathUtils'; import * as workspaceApis from '../../../common/workspace.apis'; // Import the function under test -import { getAllExtraSearchPaths, resetWorkspaceSearchPathsErrorFlag } from '../../../managers/common/nativePythonFinder'; +import { getAllExtraSearchPaths, resetWorkspaceSearchPathsGlobalWarningFlag } from '../../../managers/common/nativePythonFinder'; interface MockWorkspaceConfig { get: sinon.SinonStub; @@ -26,7 +26,7 @@ suite('getAllExtraSearchPaths Integration Tests', () => { let envConfig: MockWorkspaceConfig; setup(() => { - resetWorkspaceSearchPathsErrorFlag(); + resetWorkspaceSearchPathsGlobalWarningFlag(); // Mock VS Code workspace APIs mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration'); @@ -89,7 +89,7 @@ suite('getAllExtraSearchPaths Integration Tests', () => { teardown(() => { sinon.restore(); - resetWorkspaceSearchPathsErrorFlag(); + resetWorkspaceSearchPathsGlobalWarningFlag(); }); suite('Legacy Path Consolidation Tests', () => { @@ -358,7 +358,7 @@ suite('getAllExtraSearchPaths Integration Tests', () => { assert.strictEqual( matchingCalls.length, 1, - `Expected exactly 1 warning about workspaceSearchPaths global level, got ${matchingCalls.length}`, + `Expected exactly 1 error about workspaceSearchPaths global level, got ${matchingCalls.length}`, ); }); From e41b013aa90b54f5f789d6fad3ed2b1537024973 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Fri, 27 Feb 2026 08:51:01 -0800 Subject: [PATCH 3/3] fix: localize SettingResolutionError reason for unresolved variables (PR #1290) --- src/features/interpreterSelection.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/features/interpreterSelection.ts b/src/features/interpreterSelection.ts index 98d8619b..d5780b69 100644 --- a/src/features/interpreterSelection.ts +++ b/src/features/interpreterSelection.ts @@ -114,16 +114,11 @@ async function resolvePriorityChainCore( const error: SettingResolutionError = { setting: 'defaultInterpreterPath', configuredValue: userInterpreterPath, - reason: `Path contains unresolved variables`, + reason: l10n.t('Path contains unresolved variables'), }; errors.push(error); } else { - const resolved = await tryResolveInterpreterPath( - nativeFinder, - api, - expandedInterpreterPath, - envManagers, - ); + const resolved = await tryResolveInterpreterPath(nativeFinder, api, expandedInterpreterPath, envManagers); if (resolved) { traceVerbose(`${logPrefix} Priority 3: Using defaultInterpreterPath: ${userInterpreterPath}`); return { result: resolved, errors };