From 8ed11005c1687d5468f3c1d38cc6d5b91af47820 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 12:59:13 -0700 Subject: [PATCH 1/6] Add conservative inline script cache clearing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3cb82ae9-7424-40a4-9156-8c54ac6e0895 --- package.json | 11 + package.nls.json | 1 + src/extension.ts | 29 +- src/features/envCommands.ts | 50 ++ .../builtin/inlineScript/envManager.ts | 346 +++++++++++- src/managers/builtin/inlineScript/main.ts | 5 +- src/test/features/envCommands.unit.test.ts | 119 +++- src/test/features/envManagers.unit.test.ts | 68 +++ .../inlineScript/envManager.unit.test.ts | 507 +++++++++++++++++- .../builtin/inlineScript/main.unit.test.ts | 18 +- src/test/smoke/registration.smoke.test.ts | 39 +- 11 files changed, 1150 insertions(+), 43 deletions(-) diff --git a/package.json b/package.json index dd7cba3cf..fc37a6277 100644 --- a/package.json +++ b/package.json @@ -245,6 +245,13 @@ "category": "Python", "icon": "$(trash)" }, + { + "command": "python-envs.clearInlineScriptCache", + "title": "%python-envs.clearInlineScriptCache.title%", + "category": "Python", + "icon": "$(trash)", + "enablement": "config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true" + }, { "command": "python-envs.runInTerminal", "title": "%python-envs.runInTerminal.title%", @@ -414,6 +421,10 @@ "command": "python-envs.runAsTask", "when": "config.python.useEnvironmentsExtension != false" }, + { + "command": "python-envs.clearInlineScriptCache", + "when": "config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true" + }, { "command": "python-envs.terminal.activate", "when": "pythonTerminalActivation" diff --git a/package.nls.json b/package.nls.json index 483ecfd29..538b5abb7 100644 --- a/package.nls.json +++ b/package.nls.json @@ -35,6 +35,7 @@ "python-envs.refreshPackages.title": "Refresh Packages List", "python-envs.packages.title": "Manage Packages", "python-envs.clearCache.title": "Clear Cache", + "python-envs.clearInlineScriptCache.title": "Clear Script Environment Cache", "python-envs.runInTerminal.title": "Run in Terminal", "python-envs.createTerminal.title": "Create Python Terminal", "python-envs.runAsTask.title": "Run as Task", diff --git a/src/extension.ts b/src/extension.ts index 46f89009b..4c51caf96 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -13,7 +13,7 @@ import { PythonEnvironment, PythonEnvironmentApi, PythonProjectCreator } from '. import { ENVS_EXTENSION_ID } from './common/constants'; import { ensureCorrectVersion } from './common/extVersion'; import { registerLogger, traceError, traceInfo, traceWarn } from './common/logging'; -import { clearPersistentState, setPersistentState } from './common/persistentState'; +import { setPersistentState } from './common/persistentState'; import { newProjectSelection } from './common/pickers/managers'; import { StopWatch } from './common/stopWatch'; import { EventNames } from './common/telemetry/constants'; @@ -44,6 +44,8 @@ import { NewScriptProject } from './features/creators/newScriptProject'; import { ProjectCreatorsImpl } from './features/creators/projectCreators'; import { addPythonProjectCommand, + clearCacheCommand, + clearInlineScriptCacheCommand, copyPathToClipboard, createAnyEnvironmentCommand, createEnvironmentCommand, @@ -96,6 +98,7 @@ import { TemporaryStateManager } from './features/views/temporaryStateManager'; import { PythonEnvTreeItem } from './features/views/treeViewItems'; import { collectEnvironmentInfo, getEnvManagerAndPackageManagerConfigLevels, runPetInTerminalImpl } from './helpers'; import { EnvironmentManagers, ProjectCreators, PythonProjectManager } from './internal.api'; +import type { InlineScriptEnvManager } from './managers/builtin/inlineScript/envManager'; import { registerInlineScriptFeatures } from './managers/builtin/inlineScript/main'; import { registerSystemPythonFeatures } from './managers/builtin/main'; import { SysPythonManager } from './managers/builtin/sysPythonManager'; @@ -191,6 +194,7 @@ export async function activate(context: ExtensionContext): Promise { - await clearPersistentState(); - await envManagers.clearCache(undefined); - await clearShellProfileCache(shellStartupProviders); + await clearCacheCommand(envManagers, () => clearShellProfileCache(shellStartupProviders)); + }), + commands.registerCommand('python-envs.clearInlineScriptCache', async () => { + await clearInlineScriptCacheCommand(() => inlineScriptEnvManager); }), commands.registerCommand('python-envs.runInTerminal', (item) => { return runInTerminalCommand(item, api, terminalManager); @@ -666,13 +671,15 @@ export async function activate(context: ExtensionContext): Promise { + inlineScriptEnvManager = await registerInlineScriptFeatures( + nativeFinder, + context.subscriptions, + outputChannel, + sysMgr, + context.globalStorageUri, + ); + })(), ), safeRegister('shellStartupVars', shellStartupVarsMgr.initialize()), ]); diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index 1de8a13a6..d5c0ef657 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -18,7 +18,10 @@ import { PythonProjectCreator, PythonProjectCreatorOptions, } from '../api'; +import { INLINE_SCRIPT_MANAGER_ID } from '../common/constants'; import { traceError, traceInfo, traceVerbose } from '../common/logging'; +import { clearPersistentState } from '../common/persistentState'; +import type { InlineScriptEnvManager } from '../managers/builtin/inlineScript/envManager'; import { EnvironmentManagers, InternalEnvironmentManager, @@ -26,6 +29,8 @@ import { ProjectCreators, PythonProjectManager, } from '../internal.api'; +import { isInlineScriptsFeatureEnabled } from '../helpers'; +import { waitForEnvManagerId } from './common/managerReady'; import { removePythonProjectSetting, setEnvironmentManager, setPackageManager } from './settings/settingHelpers'; import { valid as pep440Valid } from '@renovatebot/pep440'; @@ -50,6 +55,7 @@ import { showInputBox, showOpenDialog, showQuickPick, + showWarningMessage, withProgress, } from '../common/window.apis'; import { runAsTask } from './execution/runAsTask'; @@ -306,6 +312,50 @@ export async function removeEnvironmentCommand(context: unknown, managers: Envir } } +export async function clearCacheCommand( + envManagers: EnvironmentManagers, + clearShellProfileCache: () => Promise, +): Promise { + await clearPersistentState(); + await envManagers.clearCache(undefined); + await clearShellProfileCache(); +} + +export async function clearInlineScriptCacheCommand( + getManager: () => InlineScriptEnvManager | undefined | Promise, +): Promise { + if (!isInlineScriptsFeatureEnabled()) { + const message = l10n.t( + 'Script environment cache is unavailable because inline script environments are disabled in this window.', + ); + showErrorMessage(message); + throw new Error(message); + } + + await waitForEnvManagerId([INLINE_SCRIPT_MANAGER_ID]); + const manager = await getManager(); + if (!manager) { + const message = l10n.t( + 'Script environment cache is unavailable because the inline script environment manager is not available in this window.', + ); + showErrorMessage(message); + throw new Error(message); + } + + const clearLabel = l10n.t('Clear Cache'); + const confirm = await showWarningMessage( + l10n.t('Delete cached environments created for inline Python scripts?'), + { modal: true }, + clearLabel, + l10n.t('Cancel'), + ); + if (confirm !== clearLabel) { + return; + } + + await manager.clearScriptCache(); +} + export async function handlePackageUninstall(context: unknown, em: EnvironmentManagers) { if (context instanceof PackageTreeItem || context instanceof ProjectPackage) { if (context.pkg.isTransitive) { diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 3390da8ce..3e3d2786f 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -24,6 +24,7 @@ import { getErrorMessage } from '../../../common/errors/utils'; import { computeCacheKey, normalizeDependency } from '../../../common/inlineScript/cacheKey'; import { CacheEnvironmentInspection, + INLINE_SCRIPT_CACHE_DIR_NAME, META_SCHEMA_VERSION, getBaseInterpreterStatus, getScriptEnvCacheRoot, @@ -61,8 +62,10 @@ const BASE_INTERPRETER_MANAGER_IDS = new Set([ PYENV_MANAGER_ID, ]); -const CACHE_LOCK_TIMEOUT_MS = 5 * 60 * 1000; -const CACHE_LOCK_RETRY_MS = 500; +const CACHE_CLEAR_ROOT_LOCK_TIMEOUT_MS = 1_000; +const CACHE_CLEAR_ROOT_LOCK_RETRY_MS = 50; +const CACHE_CREATE_HANDOFF_LOCK_TIMEOUT_MS = 1_000; +const CACHE_CREATE_HANDOFF_LOCK_RETRY_MS = 50; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; /** Workspace-state key for PEP 723 script path to environment executable associations. */ export const INLINE_SCRIPT_ENVS_KEY = `${ENVS_EXTENSION_ID}:inline-script:SCRIPT_ENVIRONMENTS`; @@ -104,6 +107,8 @@ type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; +type CacheLockDisposition = 'retained' | 'active' | 'unknown'; + /** Manages extension-owned PEP 723 script environments. */ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly pendingSetups = new Map>(); @@ -117,6 +122,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly associationRevisions = new Map(); private persistenceQueue: Promise = Promise.resolve(); private selectionQueue: Promise = Promise.resolve(); + private activeCreateCount = 0; + private isClearCacheInProgress = false; private readonly _onDidChangeEnvironments = new EventEmitter(); public readonly onDidChangeEnvironments: Event = @@ -147,6 +154,14 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { scope: CreateEnvironmentScope, options?: CreateEnvironmentOptions, ): Promise { + if (this.isClearCacheInProgress) { + throw this.createCacheOperationConflict( + l10n.t( + 'Cannot create an inline script environment while the script environment cache is being cleared. Retry after the cache clear finishes.', + ), + ); + } + this.activeCreateCount += 1; try { const scriptUri = this.getScriptUri(scope); if (!scriptUri) { @@ -185,9 +200,16 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } } catch (error) { + if (error instanceof InlineScriptCacheOperationError) { + this.sendInlineScriptEnvErrorTelemetry('setup-failure'); + this.log.warn(error.message); + throw error; + } this.sendInlineScriptEnvErrorTelemetry('setup-failure'); this.log.error(`Failed to set up inline-script environment: ${getErrorMessage(error)}`); return undefined; + } finally { + this.activeCreateCount -= 1; } } @@ -283,6 +305,64 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return undefined; } + async clearScriptCache(): Promise { + if (this.isClearCacheInProgress) { + throw this.createCacheOperationConflict( + l10n.t('Script environment cache clear is already in progress.'), + ); + } + this.isClearCacheInProgress = true; + + try { + if (this.activeCreateCount > 0) { + throw this.createCacheOperationConflict( + l10n.t( + 'Cannot clear the script environment cache while another inline script environment operation may still be using it. Close other VS Code windows or restart VS Code, then retry.', + ), + ); + } + + const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); + let rootLock: AcquiredFileLock | undefined = await this.acquireCacheRootLock(cacheRoot, { + timeoutMs: CACHE_CLEAR_ROOT_LOCK_TIMEOUT_MS, + retryIntervalMs: CACHE_CLEAR_ROOT_LOCK_RETRY_MS, + }, 'clear'); + try { + const clearableCacheRoot = await this.getClearableCacheRootPath(cacheRoot); + if (clearableCacheRoot) { + await this.assertNoCacheLocks(clearableCacheRoot); + await this.removeClearableCacheRoot(clearableCacheRoot); + } + + let persistError: unknown; + try { + await this.clearPersistedAssociations(); + } catch (error) { + persistError = error; + } + + this.clearKnownAssociations(); + + if (persistError) { + throw persistError; + } + } finally { + const lockToRelease = rootLock; + rootLock = undefined; + await this.releaseCacheRootLockOrThrow(lockToRelease, cacheRoot.fsPath); + } + } catch (error) { + if (error instanceof InlineScriptCacheOperationError) { + this.log.warn(error.message); + } else { + this.log.error(`Failed to clear inline-script cache: ${getErrorMessage(error)}`); + } + throw error; + } finally { + this.isClearCacheInProgress = false; + } + } + private getScriptUri(scope: CreateEnvironmentScope): Uri | undefined { const uri = scope instanceof Uri ? scope : Array.isArray(scope) && scope.length === 1 ? scope[0] : undefined; return uri?.scheme === 'file' ? uri : undefined; @@ -777,6 +857,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } + private clearPersistedAssociations(): Promise { + return this.enqueuePersistence((state) => state.clear([INLINE_SCRIPT_ENVS_KEY])); + } + private asPersistedAssociations(value: unknown): PersistedInlineScriptEnvironments | undefined { if (!value || typeof value !== 'object' || Array.isArray(value)) { return undefined; @@ -805,6 +889,232 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return run; } + private clearKnownAssociations(): void { + const cleared = [...this.fsPathToEnv.entries()].map(([scriptPath, old]) => ({ + uri: Uri.file(scriptPath), + old, + new: undefined as PythonEnvironment | undefined, + })); + const knownScriptPaths = new Set([ + ...this.associationRevisions.keys(), + ...this.pendingRehydrations.keys(), + ...this.fsPathToPersistedEnvPath.keys(), + ...this.fsPathToEnv.keys(), + ]); + for (const scriptPath of knownScriptPaths) { + this.bumpAssociationRevision(scriptPath); + this.pendingRehydrations.delete(scriptPath); + } + this.fsPathToEnv.clear(); + this.fsPathToPersistedEnvPath.clear(); + this.cachedAssociationValidatedAt.clear(); + + cleared.forEach((event) => this._onDidChangeEnvironment.fire(event)); + } + + private async getClearableCacheRootPath(cacheRoot: Uri): Promise { + const globalStoragePath = path.resolve(this.globalStorageUri.fsPath); + let globalStorageStat: fs.Stats; + try { + globalStorageStat = await fs.lstat(globalStoragePath); + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } + if (!globalStorageStat.isDirectory() || globalStorageStat.isSymbolicLink()) { + throw this.createUnsafeClearTargetError(globalStoragePath); + } + + const resolvedGlobalStorage = await fs.realpath(globalStoragePath); + if (normalizePath(resolvedGlobalStorage) !== normalizePath(globalStoragePath)) { + throw this.createUnsafeClearTargetError(globalStoragePath); + } + + const cacheRootPath = path.resolve(cacheRoot.fsPath); + try { + const cacheRootStat = await fs.lstat(cacheRootPath); + if (!cacheRootStat.isDirectory() || cacheRootStat.isSymbolicLink()) { + throw this.createUnsafeClearTargetError(cacheRootPath); + } + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } + + const resolvedCacheRoot = await resolveCacheEntryPath(Uri.file(globalStoragePath), Uri.file(cacheRootPath)); + const expectedCacheRoot = path.join(resolvedGlobalStorage, INLINE_SCRIPT_CACHE_DIR_NAME); + if (!resolvedCacheRoot || normalizePath(resolvedCacheRoot) !== normalizePath(expectedCacheRoot)) { + throw this.createUnsafeClearTargetError(cacheRootPath); + } + + return resolvedCacheRoot; + } + + private async acquireCacheRootLock( + cacheRoot: Uri, + options: { + timeoutMs: number; + retryIntervalMs: number; + }, + operation: 'create' | 'clear', + ): Promise { + await fs.ensureDir(path.dirname(cacheRoot.fsPath)); + const lockPath = this.getLockPath(cacheRoot.fsPath); + try { + return await acquireFileLock(cacheRoot.fsPath, options); + } catch (error) { + if (this.isBusyLockError(error)) { + throw this.createCacheRootBusyError(operation, lockPath); + } + throw error; + } + } + + private async assertNoCacheLocks(cacheRootPath: string): Promise { + let entries: string[]; + try { + entries = await fs.readdir(cacheRootPath); + } catch (error) { + if (isFileNotFoundError(error)) { + return; + } + throw error; + } + + for (const entry of entries.filter((candidate) => candidate.endsWith('.lock'))) { + const lockPath = path.join(cacheRootPath, entry); + const lockDisposition = await this.inspectCacheLock(lockPath); + if (lockDisposition === 'active') { + throw this.createActiveLockError(lockPath); + } + if (lockDisposition === 'unknown') { + throw this.createUnknownLockError(lockPath); + } + } + } + + private removeClearableCacheRoot(cacheRootPath: string): Promise { + return fs.remove(cacheRootPath); + } + + private async inspectCacheLock(lockPath: string): Promise { + try { + const lockStat = await fs.lstat(lockPath); + if (!lockStat.isDirectory() || lockStat.isSymbolicLink()) { + return 'unknown'; + } + } catch { + return 'unknown'; + } + + const retainedPath = path.join(lockPath, 'retained'); + try { + const retainedStat = await fs.lstat(retainedPath); + if (retainedStat.isFile()) { + return 'retained'; + } + return 'unknown'; + } catch (error) { + if (!isFileNotFoundError(error)) { + return 'unknown'; + } + } + + try { + return (await fs.readdir(lockPath)).some((entry) => entry.startsWith('owner-')) ? 'active' : 'unknown'; + } catch { + return 'unknown'; + } + } + + private createUnsafeClearTargetError(targetPath: string): Error { + return new Error( + l10n.t( + 'Cannot clear the script environment cache because the target could not be proven safe: {0}', + targetPath, + ), + ); + } + + private createCacheOperationConflict(message: string): InlineScriptCacheOperationError { + return new InlineScriptCacheOperationError(message); + } + + private createCacheRootBusyError(operation: 'create' | 'clear', lockPath: string): InlineScriptCacheOperationError { + return this.createCacheOperationConflict( + operation === 'clear' + ? l10n.t( + 'Cannot clear the script environment cache because the cache root lock at {0} may still be active or may have been left by an interrupted operation. Wait for current work to finish or close other VS Code windows and retry. If it persists after restart and no inline script cache operation is using it, remove only this lock path manually.', + lockPath, + ) + : l10n.t( + 'Inline script environment cache is busy because the cache root lock at {0} may still be active or may have been left by an interrupted operation. Wait for current work to finish or close other VS Code windows and retry. If it persists after restart and no inline script cache operation is using it, remove only this lock path manually.', + lockPath, + ), + ); + } + + private createActiveLockError(lockPath: string): InlineScriptCacheOperationError { + return this.createCacheOperationConflict( + l10n.t( + 'Cannot clear the script environment cache because the owner-only lock at {0} may still be active or may have been left by an interrupted operation. Close other VS Code windows and retry. If it persists after restart, manually remove only this lock path after confirming that no inline script cache operation is using it.', + lockPath, + ), + ); + } + + private createUnknownLockError(lockPath: string): InlineScriptCacheOperationError { + return this.createCacheOperationConflict( + l10n.t( + 'Cannot clear the script environment cache because the cache lock at {0} could not be verified as retained. Remove it manually only if you know no inline script environment operation still needs it.', + lockPath, + ), + ); + } + + private createCacheRootReleaseError(lockPath: string): InlineScriptCacheOperationError { + return this.createCacheOperationConflict( + l10n.t( + 'Failed to release the script environment cache root lock at {0}. Close other VS Code windows and retry. If it persists after restart and no inline script cache operation is using it, remove only this lock path manually.', + lockPath, + ), + ); + } + + private async releaseCacheRootLockOrThrow(lock: AcquiredFileLock, cacheRootPath: string): Promise { + const lockPath = this.getLockPath(cacheRootPath); + try { + await lock.release(); + } catch { + throw this.createCacheRootReleaseError(lockPath); + } + } + + private async releaseCacheLock(lock: AcquiredFileLock, label: string): Promise { + try { + await lock.release(); + } catch (error) { + this.log.warn(`Failed to release ${label} lock: ${getErrorMessage(error)}`); + } + } + + private isBusyLockError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + ['ELOCKED', 'ELOCKRETAINED'].includes((error as NodeJS.ErrnoException).code ?? '') + ); + } + + private getLockPath(targetPath: string): string { + return `${path.resolve(targetPath)}.lock`; + } + private async isCacheEntryBusy(envDirPath: string): Promise { return ( this.pendingCreations.has(path.basename(envDirPath)) || @@ -1124,14 +1434,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { const dependencyCount = this.getTelemetryDependencyCount(packages); const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); const envDir = getScriptEnvDir(this.globalStorageUri, cacheKey); - await fs.ensureDir(cacheRoot.fsPath); + let rootLock: AcquiredFileLock | undefined; let lock: AcquiredFileLock | undefined; try { + rootLock = await this.acquireCacheRootLock(cacheRoot, { + timeoutMs: CACHE_CREATE_HANDOFF_LOCK_TIMEOUT_MS, + retryIntervalMs: CACHE_CREATE_HANDOFF_LOCK_RETRY_MS, + }, 'create'); + await fs.ensureDir(cacheRoot.fsPath); lock = await acquireFileLock(envDir.fsPath, { - timeoutMs: CACHE_LOCK_TIMEOUT_MS, - retryIntervalMs: CACHE_LOCK_RETRY_MS, + timeoutMs: CACHE_CREATE_HANDOFF_LOCK_TIMEOUT_MS, + retryIntervalMs: CACHE_CREATE_HANDOFF_LOCK_RETRY_MS, }); + const handoffRootLock = rootLock; + rootLock = undefined; + await this.releaseCacheRootLockOrThrow(handoffRootLock, cacheRoot.fsPath); const cached = await this.inspectCacheEntry(cacheRoot, envDir, metadata, selectedBase); if (cached.kind === 'reusable') { @@ -1172,16 +1490,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } return undefined; } catch (error) { + if (error instanceof InlineScriptCacheOperationError) { + this.sendInlineScriptEnvErrorTelemetry('setup-failure'); + this.log.warn(error.message); + return undefined; + } this.sendInlineScriptEnvErrorTelemetry(this.getCreateOrReuseErrorCategory(error)); this.log.error(`Failed to create or reuse inline-script cache entry: ${getErrorMessage(error)}`); return undefined; } finally { if (lock) { - try { - await lock.release(); - } catch (error) { - this.log.warn(`Failed to release inline-script cache lock: ${getErrorMessage(error)}`); - } + await this.releaseCacheLock(lock, 'inline-script cache entry'); + } + if (rootLock) { + const lockToRelease = rootLock; + rootLock = undefined; + await this.releaseCacheRootLockOrThrow(lockToRelease, cacheRoot.fsPath); } } } @@ -1413,3 +1737,5 @@ interface PendingScriptUpdate extends ScriptReference { readonly needsPersistence: boolean; readonly shouldNotify: boolean; } + +class InlineScriptCacheOperationError extends Error {} diff --git a/src/managers/builtin/inlineScript/main.ts b/src/managers/builtin/inlineScript/main.ts index 8c35fc6ed..94531313f 100644 --- a/src/managers/builtin/inlineScript/main.ts +++ b/src/managers/builtin/inlineScript/main.ts @@ -20,14 +20,15 @@ export async function registerInlineScriptFeatures( log: LogOutputChannel, baseManager: EnvironmentManager, globalStorageUri: Uri, -): Promise { +): Promise { if (!isInlineScriptsFeatureEnabled()) { traceVerbose('Inline-script env manager: skipping registration (internal flag is off)'); - return; + return undefined; } const api: PythonEnvironmentApi = await getPythonApi(); const mgr = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); disposables.push(mgr, api.registerEnvironmentManager(mgr)); traceInfo('Inline-script env manager: registered (internal flag is on)'); + return mgr; } diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index 079ca9d73..6fd6229d6 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/src/test/features/envCommands.unit.test.ts @@ -6,8 +6,19 @@ import { PythonEnvironment, PythonProject } from '../../api'; import * as commandApi from '../../common/command.api'; import * as managerApi from '../../common/pickers/managers'; import * as projectApi from '../../common/pickers/projects'; -import { createAnyEnvironmentCommand, removePythonProject, revealEnvInManagerView } from '../../features/envCommands'; +import * as persistentState from '../../common/persistentState'; +import * as windowApis from '../../common/window.apis'; +import { + clearCacheCommand, + clearInlineScriptCacheCommand, + createAnyEnvironmentCommand, + removePythonProject, + revealEnvInManagerView, +} from '../../features/envCommands'; +import * as managerReady from '../../features/common/managerReady'; import * as settingHelpers from '../../features/settings/settingHelpers'; +import * as helpers from '../../helpers'; +import type { InlineScriptEnvManager } from '../../managers/builtin/inlineScript/envManager'; import { EnvManagerView } from '../../features/views/envManagersView'; import { ProjectEnvironment, ProjectItem } from '../../features/views/treeViewItems'; import { EnvironmentManagers, InternalEnvironmentManager, PythonProjectManager } from '../../internal.api'; @@ -216,6 +227,112 @@ suite('Remove Python Project Command Tests', () => { }); }); +suite('Clear Cache Command Tests', () => { + teardown(() => { + sinon.restore(); + }); + + test('keeps the broad clear handler on the base path', async () => { + const calls: string[] = []; + const envManagers = { + clearCache: sinon.stub().callsFake(async (scope: unknown) => { + calls.push(`managers:${String(scope)}`); + }), + } as unknown as EnvironmentManagers; + const clearShellProfileCache = sinon.stub().callsFake(async () => { + calls.push('shell'); + }); + sinon.stub(persistentState, 'clearPersistentState').callsFake(async () => { + calls.push('state'); + }); + + await clearCacheCommand(envManagers, clearShellProfileCache); + + assert.deepStrictEqual(calls, ['state', 'managers:undefined', 'shell']); + assert.ok((envManagers.clearCache as sinon.SinonStub).calledOnceWithExactly(undefined)); + assert.ok(clearShellProfileCache.calledOnce); + }); +}); + +suite('Clear Inline Script Environment Cache Command Tests', () => { + let clearScriptCacheStub: sinon.SinonStub; + let getManager: sinon.SinonStub; + let showErrorMessageStub: sinon.SinonStub; + let showWarningMessageStub: sinon.SinonStub; + let isInlineScriptsFeatureEnabledStub: sinon.SinonStub; + let waitForEnvManagerIdStub: sinon.SinonStub; + + setup(() => { + clearScriptCacheStub = sinon.stub().resolves(); + getManager = sinon + .stub<[], InlineScriptEnvManager | undefined>() + .returns({ clearScriptCache: clearScriptCacheStub } as unknown as InlineScriptEnvManager); + showErrorMessageStub = sinon.stub(windowApis, 'showErrorMessage'); + showWarningMessageStub = sinon.stub(windowApis, 'showWarningMessage'); + isInlineScriptsFeatureEnabledStub = sinon.stub(helpers, 'isInlineScriptsFeatureEnabled').returns(true); + waitForEnvManagerIdStub = sinon.stub(managerReady, 'waitForEnvManagerId').resolves(); + }); + + teardown(() => { + sinon.restore(); + }); + + test('clears the cache after confirmation', async () => { + showWarningMessageStub.callsFake(async (_message, _options, clearLabel: string) => clearLabel); + + await clearInlineScriptCacheCommand(getManager); + + assert.ok(showWarningMessageStub.calledOnce); + assert.deepStrictEqual(showWarningMessageStub.firstCall.args[1], { modal: true }); + assert.ok(waitForEnvManagerIdStub.calledOnce); + assert.ok(getManager.calledOnce); + assert.ok(clearScriptCacheStub.calledOnce); + assert.strictEqual(showErrorMessageStub.called, false); + }); + + test('does nothing when the confirmation is cancelled', async () => { + showWarningMessageStub.resolves(undefined); + + await clearInlineScriptCacheCommand(getManager); + + assert.ok(showWarningMessageStub.calledOnce); + assert.ok(waitForEnvManagerIdStub.calledOnce); + assert.ok(getManager.calledOnce); + assert.strictEqual(clearScriptCacheStub.called, false); + assert.strictEqual(showErrorMessageStub.called, false); + }); + + test('fails fast when the feature setting is off', async () => { + isInlineScriptsFeatureEnabledStub.returns(false); + showErrorMessageStub.resolves(undefined); + + await assert.rejects( + clearInlineScriptCacheCommand(getManager), + /inline script environments are disabled in this window/i, + ); + + assert.ok(showErrorMessageStub.calledOnce); + assert.strictEqual(waitForEnvManagerIdStub.called, false); + assert.strictEqual(getManager.called, false); + assert.strictEqual(showWarningMessageStub.called, false); + }); + + test('throws a clear error when the manager is unavailable after the readiness wait', async () => { + getManager.returns(undefined); + showErrorMessageStub.resolves(undefined); + + await assert.rejects( + clearInlineScriptCacheCommand(getManager), + /inline script environment manager is not available in this window/i, + ); + + assert.ok(waitForEnvManagerIdStub.calledOnce); + assert.ok(getManager.calledOnce); + assert.ok(showErrorMessageStub.calledOnce); + assert.strictEqual(showWarningMessageStub.called, false); + }); +}); + suite('Reveal Env In Manager View Command Tests', () => { let managerView: typeMoq.IMock; let executeCommandStub: sinon.SinonStub; diff --git a/src/test/features/envManagers.unit.test.ts b/src/test/features/envManagers.unit.test.ts index d642fa948..42a64579e 100644 --- a/src/test/features/envManagers.unit.test.ts +++ b/src/test/features/envManagers.unit.test.ts @@ -7,6 +7,7 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { Uri } from 'vscode'; import { PythonEnvironment } from '../../api'; +import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; import * as frameUtils from '../../common/utils/frameUtils'; import * as workspaceApis from '../../common/workspace.apis'; import { PythonEnvironmentManagers } from '../../features/envManagers'; @@ -336,3 +337,70 @@ suite('PythonEnvironmentManagers - refreshEnvironment', () => { await envManagers.refreshEnvironment(Uri.file('/unknown/path')); }); }); + +suite('PythonEnvironmentManagers - clearCache', () => { + let sandbox: sinon.SinonSandbox; + let envManagers: PythonEnvironmentManagers; + let mockProjectManager: sinon.SinonStubbedInstance; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(frameUtils, 'getCallingExtension').returns('ms-python.python'); + sandbox.stub(workspaceApis, 'getConfiguration').returns({ + get: (key: string, defaultValue?: unknown) => { + if (key === 'defaultEnvManager') { + return 'ms-python.python:system'; + } + if (key === 'pythonProjects') { + return []; + } + return defaultValue; + }, + has: () => false, + inspect: () => undefined, + update: () => Promise.resolve(), + } as any); + + mockProjectManager = { + getProjects: sandbox.stub().returns([]), + get: sandbox.stub().returns(undefined), + } as unknown as sinon.SinonStubbedInstance; + + envManagers = new PythonEnvironmentManagers(mockProjectManager as unknown as PythonProjectManager); + }); + + teardown(() => { + sandbox.restore(); + }); + + function registerFakeManager(managerId: string, clearCache: sinon.SinonStub): void { + envManagers.registerEnvironmentManager( + { + name: managerId.split(':')[1], + displayName: managerId, + preferredPackageManagerId: 'ms-python.python:pip', + clearCache, + get: sandbox.stub().resolves(undefined), + set: sandbox.stub().resolves(), + resolve: sandbox.stub().resolves(undefined), + refresh: sandbox.stub().resolves(), + getEnvironments: sandbox.stub().resolves([]), + onDidChangeEnvironments: sandbox.stub().returns({ dispose: () => {} }), + onDidChangeEnvironment: sandbox.stub().returns({ dispose: () => {} }), + } as any, + { extensionId: 'ms-python.python' }, + ); + } + + test('does not special-case managers during broad cache clears', async () => { + const systemClearCache = sandbox.stub().resolves(); + const inlineClearCache = sandbox.stub().resolves(); + registerFakeManager('ms-python.python:system', systemClearCache); + registerFakeManager(INLINE_SCRIPT_MANAGER_ID, inlineClearCache); + + await envManagers.clearCache(undefined); + + assert.ok(systemClearCache.calledOnce); + assert.ok(inlineClearCache.calledOnce); + }); +}); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 940dbb9ca..7d5f2c9bf 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -102,6 +102,7 @@ suite('InlineScriptEnvManager', () => { let ensureUvForVersionLookupStub: sinon.SinonStub; let globalStorageUri: Uri; let lockStub: sinon.SinonStub; + let log: LogOutputChannel; let manager: InlineScriptEnvManager; let nativeFinder: NativePythonFinder; let promptInstallPythonViaUvStub: sinon.SinonStub; @@ -147,7 +148,11 @@ suite('InlineScriptEnvManager', () => { persistedAssociations = value; } }), - clear: sinon.stub(), + clear: sinon.stub().callsFake(async (keys?: string[]) => { + if (!keys || keys.includes(INLINE_SCRIPT_ENVS_KEY)) { + persistedAssociations = undefined; + } + }), }; sinon.stub(persistentState, 'getWorkspacePersistentState').resolves(workspaceState); @@ -186,7 +191,8 @@ suite('InlineScriptEnvManager', () => { }); clock = sinon.useFakeTimers({ now: NOW, toFake: ['Date'] }); - manager = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, makeFakeLog()); + log = makeFakeLog(); + manager = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); }); teardown(async () => { @@ -203,6 +209,10 @@ suite('InlineScriptEnvManager', () => { return cacheLayout.getScriptEnvDir(globalStorageUri, CACHE_KEY); } + function cacheRoot(): Uri { + return cacheLayout.getScriptEnvCacheRoot(globalStorageUri); + } + function setSidecar(metadata: cacheLayout.InlineScriptEnvMeta): void { inspectMetaStub.resolves({ kind: 'valid', metadata }); } @@ -238,6 +248,7 @@ suite('InlineScriptEnvManager', () => { test('exposes creation but leaves later-phase methods empty', async () => { const asInterface: EnvironmentManager = manager; assert.strictEqual(typeof asInterface.create, 'function'); + assert.strictEqual(asInterface.clearCache, undefined); assert.strictEqual(asInterface.remove, undefined); assert.strictEqual(asInterface.quickCreateConfig, undefined); assert.deepStrictEqual(await manager.getEnvironments('all'), []); @@ -974,16 +985,55 @@ suite('InlineScriptEnvManager', () => { false, 'inline-script cache entries must not be tracked as workspace uv environments', ); - assert.ok(releaseLockStub.calledOnce); - }); + assert.strictEqual(lockStub.callCount, 2); + assert.strictEqual(lockStub.firstCall.args[0], cacheRoot().fsPath); + assert.strictEqual(lockStub.secondCall.args[0], envDir().fsPath); + assert.strictEqual(releaseLockStub.callCount, 2); + }); + + test('acquires the cache root lock before the final cache-entry lock and releases root before build', async () => { + const rootRelease = sinon.stub().resolves(); + const entryRelease = sinon.stub().resolves(); + lockStub.callsFake(async (lockPath: string) => { + if (lockPath === cacheRoot().fsPath) { + return { + retain: sinon.stub().resolves(), + release: rootRelease, + }; + } + return { + retain: sinon.stub().resolves(), + release: entryRelease, + }; + }); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + assert.ok(rootRelease.calledOnce, 'root lock should be released before build starts'); + assert.strictEqual(entryRelease.called, false, 'entry lock should remain held during build'); + const envDir = args[6] as string; + const selectedBase = args[4] as PythonEnvironment; + await fs.outputFile(getVenvPythonPath(envDir), ''); + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + selectedBase.version, + getVenvPythonPath(envDir), + envDir, + ), + }; + }); - test('uses a bounded cross-process lock at the final cache path', async () => { await manager.create(scriptUri()); - assert.strictEqual(lockStub.firstCall.args[0], envDir().fsPath); - const options = lockStub.firstCall.args[1]; - assert.ok(options.timeoutMs > 0); - assert.ok(options.retryIntervalMs > 0); + assert.strictEqual(lockStub.firstCall.args[0], cacheRoot().fsPath); + assert.strictEqual(lockStub.secondCall.args[0], envDir().fsPath); + const rootOptions = lockStub.firstCall.args[1]; + const entryOptions = lockStub.secondCall.args[1]; + assert.strictEqual(rootOptions.timeoutMs, 1_000); + assert.strictEqual(rootOptions.retryIntervalMs, 50); + assert.strictEqual(entryOptions.timeoutMs, 1_000); + assert.strictEqual(entryOptions.retryIntervalMs, 50); + assert.ok(rootRelease.calledOnce); + assert.ok(entryRelease.calledOnce); }); test('coalesces simultaneous same-key creation within one extension host', async () => { @@ -1028,14 +1078,110 @@ suite('InlineScriptEnvManager', () => { const [firstResult, secondResult] = await Promise.all([first, second]); assert.strictEqual(firstResult, secondResult); - assert.strictEqual(lockStub.callCount, 1); + assert.strictEqual(lockStub.callCount, 2); assert.strictEqual(createWithProgressStub.callCount, 1); }); - test('returns undefined without building when the cache lock cannot be acquired', async () => { + test('returns undefined without building when the cache root lock cannot be acquired', async () => { + const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; lockStub.rejects(Object.assign(new Error('already locked'), { code: 'ELOCKED' })); assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(createWithProgressStub.callCount, 0); + sinon.assert.calledWithMatch( + log.warn as sinon.SinonStub, + sinon.match(new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually`, 'i')), + ); + }); + + test('aborts before inspect/build when releasing the cache root lock for handoff fails', async () => { + const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; + const rootRelease = sinon.stub().callsFake(async () => { + await fs.ensureDir(rootLockPath); + throw new Error('root release failed'); + }); + const entryRelease = sinon.stub().resolves(); + lockStub.callsFake(async (lockPath: string) => { + if (lockPath === cacheRoot().fsPath) { + return { + retain: sinon.stub().resolves(), + release: rootRelease, + }; + } + return { + retain: sinon.stub().resolves(), + release: entryRelease, + }; + }); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + assert.strictEqual(createWithProgressStub.callCount, 0); + assert.ok(rootRelease.calledOnce); + assert.ok(entryRelease.calledOnce); + assert.strictEqual(await fs.pathExists(rootLockPath), true); + sinon.assert.calledWithMatch( + log.warn as sinon.SinonStub, + sinon.match(new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually`, 'i')), + ); + }); + + test('allows different cache entries to build concurrently after the root-to-entry handoff', async () => { + const secondCacheKey = 'fedcba9876543210'; + const secondEnvDir = cacheLayout.getScriptEnvDir(globalStorageUri, secondCacheKey); + computeCacheKeyStub.onFirstCall().returns(CACHE_KEY); + computeCacheKeyStub.onSecondCall().returns(secondCacheKey); + + let releaseFirstBuild: (() => void) | undefined; + const firstBuildGate = new Promise((resolve) => { + releaseFirstBuild = resolve; + }); + const secondBuildStarted = sinon.stub(); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + await fs.outputFile(venvPythonPath(target), ''); + if (target === envDir().fsPath) { + await firstBuildGate; + } else if (target === secondEnvDir.fsPath) { + secondBuildStarted(); + } + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(target), + target, + ), + }; + }); + + const first = manager.create(scriptUri('first.py')); + let second: Promise | undefined; + try { + await waitForStubCall(createWithProgressStub); + second = manager.create(scriptUri('second.py')); + await waitForStubCall(secondBuildStarted); + assert.ok(secondBuildStarted.calledOnce); + assert.strictEqual(createWithProgressStub.callCount, 2); + } finally { + releaseFirstBuild?.(); + await Promise.allSettled([first, second ?? Promise.resolve(undefined)]); + } + }); + + test('releases the cache root lock when the per-entry lock cannot be acquired', async () => { + const rootRelease = sinon.stub().resolves(); + lockStub.callsFake(async (lockPath: string) => { + if (lockPath === cacheRoot().fsPath) { + return { + retain: sinon.stub().resolves(), + release: rootRelease, + }; + } + throw Object.assign(new Error('entry locked'), { code: 'ELOCKED' }); + }); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + assert.strictEqual(createWithProgressStub.callCount, 0); + assert.ok(rootRelease.calledOnce); }); }); @@ -1368,7 +1514,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await fs.pathExists(envDir().fsPath), true); assert.strictEqual(writeMetaStub.callCount, 0); assert.ok(retainLockStub.calledOnce); - assert.ok(releaseLockStub.calledOnce); + assert.strictEqual(releaseLockStub.callCount, 2); }); test('keeps a failed lock-retain transition fail-closed', async () => { @@ -1386,7 +1532,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.ok(retainLockStub.calledOnce); - assert.ok(releaseLockStub.calledOnce); + assert.strictEqual(releaseLockStub.callCount, 2); }); test('removes the partial environment when package installation fails', async () => { @@ -1407,7 +1553,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(await fs.pathExists(envDir().fsPath), false); assert.strictEqual(writeMetaStub.callCount, 0); - assert.ok(releaseLockStub.calledOnce); + assert.strictEqual(releaseLockStub.callCount, 2); }); test('removes the new environment when sidecar writing fails', async () => { @@ -1415,7 +1561,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(await fs.pathExists(envDir().fsPath), false); - assert.ok(releaseLockStub.calledOnce); + assert.strictEqual(releaseLockStub.callCount, 2); }); test('removes a partial environment when createWithProgress throws', async () => { @@ -1426,7 +1572,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(await fs.pathExists(envDir().fsPath), false); - assert.ok(releaseLockStub.calledOnce); + assert.strictEqual(releaseLockStub.callCount, 2); }); test('rejects and removes a created environment with a different Python release', async () => { @@ -1471,6 +1617,335 @@ suite('InlineScriptEnvManager', () => { }); }); + suite('clear cache', () => { + test('treats a missing cache root as idempotent and clears persisted associations', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + await manager.set(uri, environment); + listener.resetHistory(); + await fs.remove(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath); + + await manager.clearScriptCache(); + await manager.clearScriptCache(); + + assert.strictEqual(workspaceState.clear.callCount, 2); + assert.deepStrictEqual(workspaceState.clear.firstCall.args[0], [INLINE_SCRIPT_ENVS_KEY]); + assert.strictEqual(persistedAssociations, undefined); + assert.strictEqual(await manager.get(uri), undefined); + sinon.assert.calledOnceWithMatch(listener, { old: environment, new: undefined }); + }); + + test('removes the cache root, clears state, and notifies known associations', async () => { + const firstUri = scriptUri('first.py'); + const secondUri = scriptUri('second.py'); + const firstEnvironment = await createOwnedEnvironment(); + const secondEnvironment = await createOwnedEnvironment('fedcba9876543210'); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + await manager.set([firstUri, secondUri], firstEnvironment); + await manager.set(secondUri, secondEnvironment); + listener.resetHistory(); + + await manager.clearScriptCache(); + + assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), false); + assert.strictEqual(persistedAssociations, undefined); + assert.strictEqual(await manager.get(firstUri), undefined); + assert.strictEqual(await manager.get(secondUri), undefined); + assert.strictEqual(listener.callCount, 2); + assert.strictEqual(listener.firstCall.args[0].old, firstEnvironment); + assert.strictEqual(listener.firstCall.args[0].new, undefined); + assert.strictEqual(listener.secondCall.args[0].old, secondEnvironment); + assert.strictEqual(listener.secondCall.args[0].new, undefined); + }); + + test('refuses to clear while a create is active', async () => { + let releaseMetadata: ((value: metadataReader.InlineScriptMetadata | undefined) => void) | undefined; + readMetadataStub.callsFake( + () => + new Promise((resolve) => { + releaseMetadata = resolve; + }), + ); + + const createPromise = manager.create(scriptUri()); + + await assert.rejects( + manager.clearScriptCache(), + /Close other VS Code windows or restart VS Code, then retry/i, + ); + + releaseMetadata!(VALID_METADATA); + assert.ok(await createPromise); + }); + + test('refuses create requests while a clear is in progress', async () => { + let clearStarted: (() => void) | undefined; + let releaseClear: (() => void) | undefined; + const started = new Promise((resolve) => { + clearStarted = resolve; + }); + const gate = new Promise((resolve) => { + releaseClear = resolve; + }); + const clearManager = manager as unknown as { + getClearableCacheRootPath(cacheRoot: Uri): Promise; + }; + sinon.stub(clearManager, 'getClearableCacheRootPath').callsFake(async () => { + clearStarted!(); + await gate; + return undefined; + }); + + const clearPromise = manager.clearScriptCache(); + await started; + + await assert.rejects(manager.create(scriptUri()), /cache is being cleared/i); + + releaseClear!(); + await clearPromise; + }); + + test('refuses to clear when the cache root lock is already held', async () => { + const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; + lockStub.callsFake(async (lockPath: string) => { + if (lockPath === cacheRoot().fsPath) { + throw Object.assign(new Error('already locked'), { code: 'ELOCKED' }); + } + return { release: releaseLockStub, retain: retainLockStub }; + }); + + await assert.rejects( + manager.clearScriptCache(), + new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*remove only this lock path manually`, 'i'), + ); + assert.strictEqual(workspaceState.clear.callCount, 0); + }); + + test('rejects when cache deletion and state clear succeed but root lock release fails', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; + const rootRelease = sinon.stub().callsFake(async () => { + await fs.ensureDir(rootLockPath); + throw new Error('root release failed'); + }); + await manager.set(uri, environment); + lockStub.callsFake(async () => ({ + retain: sinon.stub().resolves(), + release: rootRelease, + })); + + await assert.rejects( + manager.clearScriptCache(), + new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually`, 'i'), + ); + + assert.strictEqual(await fs.pathExists(cacheRoot().fsPath), false); + assert.strictEqual(persistedAssociations, undefined); + assert.ok(rootRelease.calledOnce); + }); + + test('refuses clear after the root-to-entry handoff because the entry lock is visible on disk', async () => { + const otherManager = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); + const entryLockPath = `${path.resolve(envDir().fsPath)}.lock`; + let releaseBuild: (() => void) | undefined; + const buildGate = new Promise((resolve) => { + releaseBuild = resolve; + }); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + const selectedBase = args[4] as PythonEnvironment; + await fs.outputFile(getVenvPythonPath(target), ''); + await buildGate; + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + selectedBase.version, + getVenvPythonPath(target), + target, + ), + }; + }); + lockStub.callsFake(async (lockPath: string) => { + if (lockPath === envDir().fsPath) { + await fs.ensureDir(entryLockPath); + await fs.outputFile(path.join(entryLockPath, 'owner-1234'), ''); + return { + retain: sinon.stub().resolves(), + release: sinon.stub().callsFake(async () => { + await fs.remove(entryLockPath); + }), + }; + } + return { + retain: sinon.stub().resolves(), + release: sinon.stub().resolves(), + }; + }); + + const createPromise = manager.create(scriptUri()); + try { + await waitForStubCall(createWithProgressStub); + await assert.rejects(otherManager.clearScriptCache(), /owner-only lock/i); + } finally { + releaseBuild!(); + await createPromise; + otherManager.dispose(); + } + }); + + test('allows retained lock directories to be removed with the cache root', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const lockPath = path.join(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath, `${CACHE_KEY}.lock`); + await fs.ensureDir(lockPath); + await fs.outputFile(path.join(lockPath, 'owner-1234'), ''); + await fs.outputFile(path.join(lockPath, 'retained'), ''); + + await manager.clearScriptCache(); + + assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), false); + assert.strictEqual(persistedAssociations, undefined); + }); + + test('rejects active owner lock directories', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const lockPath = path.join(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath, `${CACHE_KEY}.lock`); + await fs.ensureDir(lockPath); + await fs.outputFile(path.join(lockPath, 'owner-1234'), ''); + + await assert.rejects( + manager.clearScriptCache(), + new RegExp(`${lockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually remove`, 'i'), + ); + + assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), true); + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, + }); + }); + + test('rejects orphaned or malformed lock entries', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + const cacheRootPath = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; + const lockPath = path.join(cacheRootPath, `${CACHE_KEY}.lock`); + await manager.set(uri, environment); + + await fs.ensureDir(lockPath); + await assert.rejects(manager.clearScriptCache(), /could not be verified as retained/i); + await fs.remove(lockPath); + + await fs.outputFile(lockPath, 'not a directory'); + await assert.rejects(manager.clearScriptCache(), /could not be verified as retained/i); + + assert.strictEqual(await fs.pathExists(cacheRootPath), true); + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, + }); + }); + + test('fails closed when the cache root is redirected through a symlink or junction', async function () { + const cacheRoot = cacheLayout.getScriptEnvCacheRoot(globalStorageUri); + const externalRoot = path.join(tempRoot, 'external-cache-root'); + const markerPath = path.join(externalRoot, 'keep.txt'); + await fs.ensureDir(globalStorageUri.fsPath); + await fs.remove(cacheRoot.fsPath); + await fs.outputFile(markerPath, 'keep'); + try { + await fs.symlink(externalRoot, cacheRoot.fsPath, isWindows() ? 'junction' : 'dir'); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + this.skip(); + return; + } + throw error; + } + + await assert.rejects(manager.clearScriptCache(), /could not be proven safe/i); + + assert.strictEqual(await fs.readFile(markerPath, 'utf8'), 'keep'); + assert.strictEqual((await fs.lstat(cacheRoot.fsPath)).isSymbolicLink(), true); + }); + + test('surfaces state clear failures after removing the cache root and clearing in-memory state', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + await manager.set(uri, environment); + listener.resetHistory(); + workspaceState.clear.rejects(new Error('Memento unavailable')); + + await assert.rejects(manager.clearScriptCache(), /Memento unavailable/); + + const clearState = manager as unknown as { + fsPathToEnv: Map; + fsPathToPersistedEnvPath: Map; + }; + assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), false); + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, + }); + assert.strictEqual(clearState.fsPathToEnv.size, 0); + assert.strictEqual(clearState.fsPathToPersistedEnvPath.size, 0); + sinon.assert.calledOnceWithMatch(listener, { old: environment, new: undefined }); + }); + + test('surfaces disk deletion failures without clearing state', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const cacheRootPath = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; + const clearManager = manager as unknown as { + removeClearableCacheRoot(cacheRootPath: string): Promise; + }; + sinon.stub(clearManager, 'removeClearableCacheRoot').rejects(new Error('disk busy')); + + await assert.rejects(manager.clearScriptCache(), /disk busy/); + + assert.strictEqual(await fs.pathExists(cacheRootPath), true); + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, + }); + assert.strictEqual(await manager.get(uri), environment); + }); + + test('does not let a pending rehydration repopulate after clear', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + persistedAssociations = { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath }; + + let resolvePending: ((value: PythonEnvironment | undefined) => void) | undefined; + resolveVenvStub.callsFake( + () => + new Promise((resolve) => { + resolvePending = resolve; + }), + ); + + const pendingGet = manager.get(uri); + await waitForStubCall(resolveVenvStub); + + await manager.clearScriptCache(); + resolvePending!(environment); + + assert.strictEqual(await pendingGet, undefined); + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(persistedAssociations, undefined); + }); + }); + suite('events and disposal', () => { test('create does not establish an association or fire later-phase events', async () => { const environmentsListener = sinon.spy(); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index d109e318d..1fec3cd12 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -51,23 +51,37 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { isEnabledStub.returns(false); const disposables: Disposable[] = []; - await registerInlineScriptFeatures(nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri); + const result = await registerInlineScriptFeatures( + nativeFinder, + disposables, + makeFakeLog(), + baseManager, + globalStorageUri, + ); assert.strictEqual(disposables.length, 0, 'no disposables should be added when flag is off'); assert.strictEqual(getPythonApiStub.called, false, 'should not even call getPythonApi when gated off'); assert.strictEqual(registerEnvironmentManagerStub.called, false); + assert.strictEqual(result, undefined); }); test('when the feature flag is TRUE: registers the manager and pushes the disposable', async () => { isEnabledStub.returns(true); const disposables: Disposable[] = []; - await registerInlineScriptFeatures(nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri); + const result = await registerInlineScriptFeatures( + nativeFinder, + disposables, + makeFakeLog(), + baseManager, + globalStorageUri, + ); assert.strictEqual(getPythonApiStub.callCount, 1); assert.strictEqual(registerEnvironmentManagerStub.callCount, 1); assert.strictEqual(disposables.length, 2, 'expected manager + registration disposable'); const manager = registerEnvironmentManagerStub.firstCall.args[0]; + assert.strictEqual(result, manager); assert.ok(disposables.includes(manager), 'manager itself should be disposed'); assert.ok( disposables.includes(registerEnvironmentManagerStub.firstCall.returnValue), diff --git a/src/test/smoke/registration.smoke.test.ts b/src/test/smoke/registration.smoke.test.ts index bd8d469e0..176aba815 100644 --- a/src/test/smoke/registration.smoke.test.ts +++ b/src/test/smoke/registration.smoke.test.ts @@ -26,9 +26,10 @@ suite('Smoke: Registration Checks', function () { this.timeout(MAX_EXTENSION_ACTIVATION_TIME); let api: PythonEnvironmentApi; + let extension: vscode.Extension; suiteSetup(async function () { - const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID)!; assert.ok(extension, `Extension ${ENVS_EXTENSION_ID} not found`); if (!extension.isActive) { @@ -65,6 +66,7 @@ suite('Smoke: Registration Checks', function () { 'python-envs.setPkgManager', 'python-envs.refreshAllManagers', 'python-envs.clearCache', + 'python-envs.clearInlineScriptCache', 'python-envs.searchSettings', // Package management @@ -113,6 +115,41 @@ suite('Smoke: Registration Checks', function () { ); }); + test('Clear cache commands are contributed from package.json', function () { + const clearCacheCommand = extension.packageJSON?.contributes?.commands?.find( + (item: { command: string }) => item.command === 'python-envs.clearCache', + ); + const clearInlineScriptCacheCommand = extension.packageJSON?.contributes?.commands?.find( + (item: { command: string }) => item.command === 'python-envs.clearInlineScriptCache', + ); + const clearInlineScriptCachePaletteEntry = extension.packageJSON?.contributes?.menus?.commandPalette?.find( + (item: { command: string }) => item.command === 'python-envs.clearInlineScriptCache', + ); + + assert.ok(clearCacheCommand, 'python-envs.clearCache should be contributed in package.json'); + assert.strictEqual(clearCacheCommand.category, 'Python'); + assert.strictEqual(clearCacheCommand.title, 'Clear Cache'); + + assert.ok( + clearInlineScriptCacheCommand, + 'python-envs.clearInlineScriptCache should be contributed in package.json', + ); + assert.strictEqual(clearInlineScriptCacheCommand.category, 'Python'); + assert.strictEqual(clearInlineScriptCacheCommand.title, 'Clear Script Environment Cache'); + assert.strictEqual( + clearInlineScriptCacheCommand.enablement, + 'config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true', + ); + assert.ok( + clearInlineScriptCachePaletteEntry, + 'python-envs.clearInlineScriptCache should have a command palette contribution', + ); + assert.strictEqual( + clearInlineScriptCachePaletteEntry.when, + 'config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true', + ); + }); + // ========================================================================= // API METHODS - All API methods must exist and be functions // ========================================================================= From 82f381592f85dc0dcf938fcbe9a9732c0d13aa77 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 13:33:55 -0700 Subject: [PATCH 2/6] Restore complete inline script cache lifecycle cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3cb82ae9-7424-40a4-9156-8c54ac6e0895 --- package.json | 15 +- package.nls.json | 2 +- src/common/lockfile.apis.ts | 94 +- src/extension.ts | 31 +- src/features/envCommands.ts | 123 +-- src/features/projectManager.ts | 5 +- src/features/settings/settingHelpers.ts | 237 ++++- .../builtin/inlineScript/envManager.ts | 722 ++++++++-------- src/managers/builtin/inlineScript/main.ts | 5 +- src/managers/builtin/venvUtils.ts | 4 +- src/test/common/lockfile.apis.unit.test.ts | 67 +- src/test/features/envCommands.unit.test.ts | 319 +++++-- src/test/features/envManagers.unit.test.ts | 68 -- .../projectManager.initialize.unit.test.ts | 159 ++++ .../settings/settingHelpers.unit.test.ts | 408 ++++++++- .../inlineScript/envManager.unit.test.ts | 810 +++++++----------- .../builtin/inlineScript/main.unit.test.ts | 18 +- src/test/smoke/registration.smoke.test.ts | 40 +- 18 files changed, 1949 insertions(+), 1178 deletions(-) diff --git a/package.json b/package.json index fc37a6277..0a9a6abaf 100644 --- a/package.json +++ b/package.json @@ -246,11 +246,10 @@ "icon": "$(trash)" }, { - "command": "python-envs.clearInlineScriptCache", - "title": "%python-envs.clearInlineScriptCache.title%", + "command": "python-envs.clearScriptEnvCache", + "title": "%python-envs.clearScriptEnvCache.title%", "category": "Python", - "icon": "$(trash)", - "enablement": "config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true" + "icon": "$(trash)" }, { "command": "python-envs.runInTerminal", @@ -421,10 +420,6 @@ "command": "python-envs.runAsTask", "when": "config.python.useEnvironmentsExtension != false" }, - { - "command": "python-envs.clearInlineScriptCache", - "when": "config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true" - }, { "command": "python-envs.terminal.activate", "when": "pythonTerminalActivation" @@ -476,6 +471,10 @@ { "command": "python-envs.reportIssue", "when": "config.python.useEnvironmentsExtension != false" + }, + { + "command": "python-envs.clearScriptEnvCache", + "when": "config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true" } ], "view/item/context": [ diff --git a/package.nls.json b/package.nls.json index 538b5abb7..c128863de 100644 --- a/package.nls.json +++ b/package.nls.json @@ -35,7 +35,7 @@ "python-envs.refreshPackages.title": "Refresh Packages List", "python-envs.packages.title": "Manage Packages", "python-envs.clearCache.title": "Clear Cache", - "python-envs.clearInlineScriptCache.title": "Clear Script Environment Cache", + "python-envs.clearScriptEnvCache.title": "Clear Script Environment Cache", "python-envs.runInTerminal.title": "Run in Terminal", "python-envs.createTerminal.title": "Create Python Terminal", "python-envs.runAsTask.title": "Run as Task", diff --git a/src/common/lockfile.apis.ts b/src/common/lockfile.apis.ts index 1d8409056..2fbe10352 100644 --- a/src/common/lockfile.apis.ts +++ b/src/common/lockfile.apis.ts @@ -16,13 +16,31 @@ export interface AcquiredFileLock { readonly retain: () => Promise; } +export const FILE_LOCK_DIR_SUFFIX = '.lock'; +export const FILE_LOCK_OWNER_MARKER_PREFIX = 'owner-'; +export const FILE_LOCK_RETAINED_MARKER = 'retained'; + +export type ProcessLiveness = 'live' | 'dead' | 'unavailable'; +export type FileLockState = 'missing' | 'held' | 'retained' | 'stale' | 'orphaned' | 'malformed' | 'unavailable'; + +export interface InspectFileLockOptions { + readonly checkProcessLiveness?: (pid: number) => Promise; +} + type LockState = 'held' | 'released' | 'retained'; +export function getFileLockPath(filePath: string): string { + return `${path.resolve(filePath)}${FILE_LOCK_DIR_SUFFIX}`; +} + /** Acquire an atomic lock released only explicitly; interrupted operations remain locked. */ export async function acquireFileLock(filePath: string, options: AcquireFileLockOptions): Promise { - const lockPath = `${path.resolve(filePath)}.lock`; - const ownerMarker = path.join(lockPath, `owner-${process.pid}-${crypto.randomBytes(16).toString('hex')}`); - const retainedMarker = path.join(lockPath, 'retained'); + const lockPath = getFileLockPath(filePath); + const ownerMarker = path.join( + lockPath, + `${FILE_LOCK_OWNER_MARKER_PREFIX}${process.pid}-${crypto.randomBytes(16).toString('hex')}`, + ); + const retainedMarker = path.join(lockPath, FILE_LOCK_RETAINED_MARKER); const deadline = Date.now() + options.timeoutMs; while (true) { @@ -100,9 +118,68 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock } } +export async function inspectFileLock(filePath: string, options?: InspectFileLockOptions): Promise { + const lockPath = getFileLockPath(filePath); + + let stat; + try { + stat = await fsapi.lstat(lockPath); + } catch (error) { + if (hasErrorCode(error, 'ENOENT')) { + return 'missing'; + } + throw error; + } + + if (!stat.isDirectory() || stat.isSymbolicLink()) { + return 'malformed'; + } + + const entries = await fsapi.readdir(lockPath); + const ownerEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX)); + const retainedEntries = entries.filter((entry) => entry === FILE_LOCK_RETAINED_MARKER); + const unknownEntries = entries.filter( + (entry) => !entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX) && entry !== FILE_LOCK_RETAINED_MARKER, + ); + + if (unknownEntries.length > 0 || ownerEntries.length > 1 || retainedEntries.length > 1) { + return 'malformed'; + } + if (retainedEntries.length === 1) { + return 'retained'; + } + if (ownerEntries.length === 1) { + const ownerPid = parseOwnerPid(ownerEntries[0]); + if (ownerPid === undefined) { + return 'malformed'; + } + const liveness = await (options?.checkProcessLiveness ?? getProcessLiveness)(ownerPid); + if (liveness === 'dead') { + return 'stale'; + } + return liveness === 'live' ? 'held' : 'unavailable'; + } + return 'orphaned'; +} + +export async function getProcessLiveness(pid: number): Promise { + try { + process.kill(pid, 0); + return 'live'; + } catch (error) { + if (hasErrorCode(error, 'ESRCH')) { + return 'dead'; + } + if (hasErrorCode(error, 'EPERM') || hasErrorCode(error, 'EACCES')) { + return 'unavailable'; + } + return 'unavailable'; + } +} + async function isRetainedLock(lockPath: string): Promise { try { - await fsapi.lstat(path.join(lockPath, 'retained')); + await fsapi.lstat(path.join(lockPath, FILE_LOCK_RETAINED_MARKER)); return true; } catch (error) { if (hasErrorCode(error, 'ENOENT')) { @@ -118,6 +195,15 @@ function hasErrorCode(error: unknown, code: string): boolean { ); } +function parseOwnerPid(entry: string): number | undefined { + const match = entry.match(/^owner-(\d+)-/); + if (!match) { + return undefined; + } + const pid = Number(match[1]); + return Number.isSafeInteger(pid) && pid > 0 ? pid : undefined; +} + function createLockError(message: string, code: string, lockPath: string): NodeJS.ErrnoException { return Object.assign(new Error(message), { code, path: lockPath }); } diff --git a/src/extension.ts b/src/extension.ts index 4c51caf96..e3735b4ad 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -13,7 +13,7 @@ import { PythonEnvironment, PythonEnvironmentApi, PythonProjectCreator } from '. import { ENVS_EXTENSION_ID } from './common/constants'; import { ensureCorrectVersion } from './common/extVersion'; import { registerLogger, traceError, traceInfo, traceWarn } from './common/logging'; -import { setPersistentState } from './common/persistentState'; +import { clearPersistentState, setPersistentState } from './common/persistentState'; import { newProjectSelection } from './common/pickers/managers'; import { StopWatch } from './common/stopWatch'; import { EventNames } from './common/telemetry/constants'; @@ -44,9 +44,8 @@ import { NewScriptProject } from './features/creators/newScriptProject'; import { ProjectCreatorsImpl } from './features/creators/projectCreators'; import { addPythonProjectCommand, - clearCacheCommand, - clearInlineScriptCacheCommand, copyPathToClipboard, + clearScriptEnvironmentCacheCommand, createAnyEnvironmentCommand, createEnvironmentCommand, createTerminalCommand, @@ -98,7 +97,6 @@ import { TemporaryStateManager } from './features/views/temporaryStateManager'; import { PythonEnvTreeItem } from './features/views/treeViewItems'; import { collectEnvironmentInfo, getEnvManagerAndPackageManagerConfigLevels, runPetInTerminalImpl } from './helpers'; import { EnvironmentManagers, ProjectCreators, PythonProjectManager } from './internal.api'; -import type { InlineScriptEnvManager } from './managers/builtin/inlineScript/envManager'; import { registerInlineScriptFeatures } from './managers/builtin/inlineScript/main'; import { registerSystemPythonFeatures } from './managers/builtin/main'; import { SysPythonManager } from './managers/builtin/sysPythonManager'; @@ -194,7 +192,6 @@ export async function activate(context: ExtensionContext): Promise { - await clearCacheCommand(envManagers, () => clearShellProfileCache(shellStartupProviders)); + await clearPersistentState(); + await envManagers.clearCache(undefined); + await clearShellProfileCache(shellStartupProviders); }), - commands.registerCommand('python-envs.clearInlineScriptCache', async () => { - await clearInlineScriptCacheCommand(() => inlineScriptEnvManager); + commands.registerCommand('python-envs.clearScriptEnvCache', async () => { + await clearScriptEnvironmentCacheCommand(envManagers, projectManager); }), commands.registerCommand('python-envs.runInTerminal', (item) => { return runInTerminalCommand(item, api, terminalManager); @@ -671,15 +670,13 @@ export async function activate(context: ExtensionContext): Promise { - inlineScriptEnvManager = await registerInlineScriptFeatures( - nativeFinder, - context.subscriptions, - outputChannel, - sysMgr, - context.globalStorageUri, - ); - })(), + registerInlineScriptFeatures( + nativeFinder, + context.subscriptions, + outputChannel, + sysMgr, + context.globalStorageUri, + ), ), safeRegister('shellStartupVars', shellStartupVarsMgr.initialize()), ]); diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index d5c0ef657..0136539f1 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -18,10 +18,7 @@ import { PythonProjectCreator, PythonProjectCreatorOptions, } from '../api'; -import { INLINE_SCRIPT_MANAGER_ID } from '../common/constants'; import { traceError, traceInfo, traceVerbose } from '../common/logging'; -import { clearPersistentState } from '../common/persistentState'; -import type { InlineScriptEnvManager } from '../managers/builtin/inlineScript/envManager'; import { EnvironmentManagers, InternalEnvironmentManager, @@ -29,9 +26,12 @@ import { ProjectCreators, PythonProjectManager, } from '../internal.api'; -import { isInlineScriptsFeatureEnabled } from '../helpers'; -import { waitForEnvManagerId } from './common/managerReady'; -import { removePythonProjectSetting, setEnvironmentManager, setPackageManager } from './settings/settingHelpers'; +import { + getResolvedPythonProjectSettings, + removePythonProjectSetting, + setEnvironmentManager, + setPackageManager, +} from './settings/settingHelpers'; import { valid as pep440Valid } from '@renovatebot/pep440'; import { executeCommand } from '../common/command.api'; @@ -58,6 +58,8 @@ import { showWarningMessage, withProgress, } from '../common/window.apis'; +import { getWorkspaceFolders } from '../common/workspace.apis'; +import { INLINE_SCRIPT_MANAGER_ID } from '../common/constants'; import { runAsTask } from './execution/runAsTask'; import { runInTerminal } from './terminal/runInTerminal'; import { TerminalManager } from './terminal/terminalManager'; @@ -312,50 +314,6 @@ export async function removeEnvironmentCommand(context: unknown, managers: Envir } } -export async function clearCacheCommand( - envManagers: EnvironmentManagers, - clearShellProfileCache: () => Promise, -): Promise { - await clearPersistentState(); - await envManagers.clearCache(undefined); - await clearShellProfileCache(); -} - -export async function clearInlineScriptCacheCommand( - getManager: () => InlineScriptEnvManager | undefined | Promise, -): Promise { - if (!isInlineScriptsFeatureEnabled()) { - const message = l10n.t( - 'Script environment cache is unavailable because inline script environments are disabled in this window.', - ); - showErrorMessage(message); - throw new Error(message); - } - - await waitForEnvManagerId([INLINE_SCRIPT_MANAGER_ID]); - const manager = await getManager(); - if (!manager) { - const message = l10n.t( - 'Script environment cache is unavailable because the inline script environment manager is not available in this window.', - ); - showErrorMessage(message); - throw new Error(message); - } - - const clearLabel = l10n.t('Clear Cache'); - const confirm = await showWarningMessage( - l10n.t('Delete cached environments created for inline Python scripts?'), - { modal: true }, - clearLabel, - l10n.t('Cancel'), - ); - if (confirm !== clearLabel) { - return; - } - - await manager.clearScriptCache(); -} - export async function handlePackageUninstall(context: unknown, em: EnvironmentManagers) { if (context instanceof PackageTreeItem || context instanceof ProjectPackage) { if (context.pkg.isTransitive) { @@ -712,6 +670,71 @@ export async function removePythonProject( wm.remove(item.project); } +function getInlineScriptProjectEdits(wm: PythonProjectManager) { + const currentProjects = new Map(wm.getProjects().map((project) => [project.uri.toString(), project] as const)); + const edits = new Map(); + for (const workspaceFolder of getWorkspaceFolders() ?? []) { + for (const resolvedSetting of getResolvedPythonProjectSettings(workspaceFolder)) { + if ( + !resolvedSetting.sources.some( + (source) => source.setting.envManager === INLINE_SCRIPT_MANAGER_ID, + ) + ) { + continue; + } + const projectUri = resolvedSetting.uri; + const key = projectUri.toString(); + edits.set(key, { + project: + currentProjects.get(key) ?? + wm.create(path.basename(projectUri.fsPath) || resolvedSetting.effective.setting.path, projectUri), + envManager: INLINE_SCRIPT_MANAGER_ID, + }); + } + } + return { + edits: Array.from(edits.values()), + loadedProjects: currentProjects, + }; +} + +export async function clearScriptEnvironmentCacheCommand( + em: EnvironmentManagers, + wm: PythonProjectManager, +): Promise { + const manager = em.getEnvironmentManager(INLINE_SCRIPT_MANAGER_ID); + if (!manager || !manager.supportsClearCache()) { + throw new Error( + l10n.t('Inline-script environment cache is unavailable because the inline-script manager is not registered.'), + ); + } + + const clearLabel = l10n.t('Clear Cache'); + const confirmation = await showWarningMessage( + l10n.t( + 'This will delete all cached inline-script environments, forget their script associations, and remove inline-script project entries from settings.', + ), + { modal: true }, + clearLabel, + ); + if (confirmation !== clearLabel) { + return; + } + + const { edits, loadedProjects } = getInlineScriptProjectEdits(wm); + await manager.clearCache(); + if (edits.length === 0) { + return; + } + const removedProjects = await removePythonProjectSetting(edits); + const loadedProjectsToRemove = removedProjects + .map((project) => loadedProjects.get(project.uri.toString())) + .filter((project): project is PythonProject => project !== undefined); + if (loadedProjectsToRemove.length > 0) { + wm.remove(loadedProjectsToRemove); + } +} + export async function getPackageCommandOptions( e: unknown, em: EnvironmentManagers, diff --git a/src/features/projectManager.ts b/src/features/projectManager.ts index 9c1cf7bb3..31dd86cd0 100644 --- a/src/features/projectManager.ts +++ b/src/features/projectManager.ts @@ -130,20 +130,17 @@ export class PythonProjectManagerImpl implements PythonProjectManager { // For each override, resolve its path and add as a project if not already present for (const o of overrides) { let uriFromWorkspace: Uri | undefined = undefined; - // if override has a workspace property, resolve the path relative to that workspace if (o.workspace) { - // const workspaceFolder = workspaces.find((ws) => ws.name === o.workspace); if (workspaceFolder) { if (workspaceFolder.uri.toString() !== w.uri.toString()) { - continue; // skip if the workspace is not the same as the current workspace + continue; } uriFromWorkspace = Uri.file(path.resolve(workspaceFolder.uri.fsPath, o.path)); } } const uri = uriFromWorkspace ? uriFromWorkspace : Uri.file(path.resolve(w.uri.fsPath, o.path)); - // Check if the project already exists in the newProjects array if (!newProjects.some((p) => p.uri.toString() === uri.toString())) { newProjects.push(new PythonProjectsImpl(o.path, uri)); } diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 752a1c2c0..25b2eca9d 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -10,6 +10,112 @@ import { sendTelemetryEvent } from '../../common/telemetry/sender'; import * as workspaceApis from '../../common/workspace.apis'; import { PythonProjectManager, PythonProjectSettings } from '../../internal.api'; +export interface ResolvedPythonProjectSettingSource { + readonly setting: PythonProjectSettings; + readonly uri: Uri; + readonly workspaceFolder: WorkspaceFolder; + readonly source: 'workspace' | 'workspaceFolder'; + readonly target: ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder; +} + +export interface ResolvedPythonProjectSetting { + readonly uri: Uri; + readonly workspaceFolder: WorkspaceFolder; + readonly effective: ResolvedPythonProjectSettingSource; + readonly sources: readonly ResolvedPythonProjectSettingSource[]; +} + +function resolvePythonProjectSettingSource( + setting: PythonProjectSettings, + workspaceFolder: WorkspaceFolder, + allWorkspaceFolders: readonly WorkspaceFolder[], + source: 'workspace' | 'workspaceFolder', + target: ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder, +): ResolvedPythonProjectSettingSource | undefined { + const resolvedWorkspaceFolder = setting.workspace + ? allWorkspaceFolders.find((candidate) => candidate.name === setting.workspace) + : workspaceFolder; + if (!resolvedWorkspaceFolder || resolvedWorkspaceFolder.uri.toString() !== workspaceFolder.uri.toString()) { + return undefined; + } + return { + setting, + uri: Uri.file(path.resolve(resolvedWorkspaceFolder.uri.fsPath, setting.path)), + workspaceFolder, + source, + target, + }; +} + +function resolveProjectSettingUri( + setting: PythonProjectSettings, + workspaceFolder: WorkspaceFolder, + allWorkspaceFolders: readonly WorkspaceFolder[] = workspaceApis.getWorkspaceFolders() ?? [workspaceFolder], +): Uri | undefined { + const resolvedWorkspaceFolder = setting.workspace + ? allWorkspaceFolders.find((candidate) => candidate.name === setting.workspace) + : workspaceFolder; + return resolvedWorkspaceFolder + ? Uri.file(path.resolve(resolvedWorkspaceFolder.uri.fsPath, setting.path)) + : undefined; +} + +export function getResolvedPythonProjectSettings( + workspaceFolder: WorkspaceFolder, + config: WorkspaceConfiguration = workspaceApis.getConfiguration('python-envs', workspaceFolder.uri), +): ResolvedPythonProjectSetting[] { + const allWorkspaceFolders = workspaceApis.getWorkspaceFolders() ?? [workspaceFolder]; + const projectsInspect = + typeof config.inspect === 'function' ? config.inspect('pythonProjects') : undefined; + const fallbackSettings = + projectsInspect === undefined ? config.get('pythonProjects', []) : undefined; + const orderedSources: ResolvedPythonProjectSettingSource[] = [ + ...(projectsInspect?.workspaceValue ?? fallbackSettings ?? []) + .map((setting) => + resolvePythonProjectSettingSource( + setting, + workspaceFolder, + allWorkspaceFolders, + 'workspace', + ConfigurationTarget.Workspace, + ), + ) + .filter((setting): setting is ResolvedPythonProjectSettingSource => setting !== undefined), + ...(projectsInspect?.workspaceFolderValue ?? []) + .map((setting) => + resolvePythonProjectSettingSource( + setting, + workspaceFolder, + allWorkspaceFolders, + 'workspaceFolder', + ConfigurationTarget.WorkspaceFolder, + ), + ) + .filter((setting): setting is ResolvedPythonProjectSettingSource => setting !== undefined), + ]; + + const grouped = new Map(); + for (const source of orderedSources) { + const key = source.uri.toString(); + const existing = grouped.get(key); + if (existing) { + grouped.set(key, { + ...existing, + effective: source, + sources: [...existing.sources, source], + }); + } else { + grouped.set(key, { + uri: source.uri, + workspaceFolder, + effective: source, + sources: [source], + }); + } + } + return Array.from(grouped.values()); +} + function getSettings( wm: PythonProjectManager, config: WorkspaceConfiguration, @@ -349,6 +455,46 @@ export interface EditProjectSettings { workspace?: string; } +function matchesProjectSettingEdit( + setting: PythonProjectSettings, + edit: EditProjectSettings, + workspaceFolder: WorkspaceFolder, +): boolean { + const projectPath = normalizePath(edit.project.uri.fsPath); + const settingUri = resolveProjectSettingUri(setting, workspaceFolder); + if (!settingUri || normalizePath(settingUri.fsPath) !== projectPath) { + return false; + } + if (edit.workspace !== undefined && setting.workspace !== edit.workspace) { + return false; + } + if (edit.envManager !== undefined && setting.envManager !== edit.envManager) { + return false; + } + if (edit.packageManager !== undefined && setting.packageManager !== edit.packageManager) { + return false; + } + return true; +} + +function hasProjectSetting( + settings: readonly PythonProjectSettings[], + project: PythonProject, + workspaceFolder: WorkspaceFolder, +): boolean { + const projectPath = normalizePath(project.uri.fsPath); + return settings.some((setting) => { + const settingUri = resolveProjectSettingUri(setting, workspaceFolder); + return settingUri ? normalizePath(settingUri.fsPath) === projectPath : false; + }); +} + +function cloneProjectSettings( + settings: readonly PythonProjectSettings[] | undefined, +): PythonProjectSettings[] | undefined { + return settings?.map((setting) => ({ ...setting })); +} + export async function addPythonProjectSetting(edits: EditProjectSettings[]): Promise { const noWorkspace: EditProjectSettings[] = []; const workspaces = new Map(); @@ -445,7 +591,7 @@ export async function addPythonProjectSetting(edits: EditProjectSettings[]): Pro await Promise.all(promises); } -export async function removePythonProjectSetting(edits: EditProjectSettings[]): Promise { +export async function removePythonProjectSetting(edits: EditProjectSettings[]): Promise { const noWorkspace: EditProjectSettings[] = []; const workspaces = new Map(); edits.forEach((e) => { @@ -461,24 +607,87 @@ export async function removePythonProjectSetting(edits: EditProjectSettings[]): traceError(`Unable to find workspace for ${e.project.uri.fsPath}`); }); + const workspaceEntries = Array.from(workspaces.entries()); + if (workspaceEntries.length === 0) { + return []; + } + + const removedProjects = new Map(); + const folderRemainingSettings = new Map(); + const folderExistingSettings = new Map(); const promises: Thenable[] = []; - workspaces.forEach((es, w) => { + let workspaceConfig: WorkspaceConfiguration | undefined; + let workspaceValueOriginal: PythonProjectSettings[] | undefined; + + workspaceEntries.forEach(([w, es]) => { const config = workspaceApis.getConfiguration('python-envs', w.uri); - const overrides = config.get('pythonProjects', []); - es.forEach((e) => { - const pwPath = normalizePath(e.project.uri.fsPath); - const index = overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath); - if (index >= 0) { - overrides.splice(index, 1); - } - }); - if (overrides.length === 0) { - promises.push(config.update('pythonProjects', undefined, ConfigurationTarget.Workspace)); - } else { - promises.push(config.update('pythonProjects', overrides, ConfigurationTarget.Workspace)); + const projectsInspect = config.inspect('pythonProjects'); + workspaceConfig ??= config; + workspaceValueOriginal ??= cloneProjectSettings(projectsInspect?.workspaceValue); + + const workspaceFolderOriginal = cloneProjectSettings(projectsInspect?.workspaceFolderValue) ?? []; + folderExistingSettings.set(w.uri.toString(), workspaceFolderOriginal); + const workspaceFolderRemaining = workspaceFolderOriginal.filter( + (projectSetting) => !es.some((edit) => matchesProjectSettingEdit(projectSetting, edit, w)), + ); + folderRemainingSettings.set(w.uri.toString(), workspaceFolderRemaining); + + if (workspaceFolderRemaining.length !== workspaceFolderOriginal.length) { + promises.push( + config.update( + 'pythonProjects', + workspaceFolderRemaining.length > 0 ? workspaceFolderRemaining : undefined, + ConfigurationTarget.WorkspaceFolder, + ), + ); } }); + + const aggregatedEdits = workspaceEntries.flatMap(([workspaceFolder, workspaceEdits]) => + workspaceEdits.map((edit) => ({ workspaceFolder, edit })), + ); + const workspaceValueRemaining = + workspaceValueOriginal?.filter( + (projectSetting) => + !aggregatedEdits.some(({ workspaceFolder, edit }) => + matchesProjectSettingEdit(projectSetting, edit, workspaceFolder), + ), + ) ?? []; + + if ( + workspaceConfig && + workspaceValueOriginal !== undefined && + workspaceValueRemaining.length !== workspaceValueOriginal.length + ) { + promises.push( + workspaceConfig.update( + 'pythonProjects', + workspaceValueRemaining.length > 0 ? workspaceValueRemaining : undefined, + ConfigurationTarget.Workspace, + ), + ); + } + + workspaceEntries.forEach(([w, es]) => { + const existingSettings = [ + ...(workspaceValueOriginal ?? []), + ...((folderExistingSettings.get(w.uri.toString()) ?? [])), + ]; + const remainingSettings = [ + ...workspaceValueRemaining, + ...((folderRemainingSettings.get(w.uri.toString()) ?? [])), + ]; + es.filter( + (edit) => + existingSettings.some((projectSetting) => matchesProjectSettingEdit(projectSetting, edit, w)) && + !hasProjectSetting(remainingSettings, edit.project, w), + ).forEach((edit) => { + removedProjects.set(edit.project.uri.toString(), edit.project); + }); + }); + await Promise.all(promises); + return Array.from(removedProjects.values()); } /** diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 3e3d2786f..549228891 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -43,10 +43,17 @@ import { PYENV_MANAGER_ID, SYSTEM_MANAGER_ID, } from '../../../common/constants'; -import { acquireFileLock, AcquiredFileLock } from '../../../common/lockfile.apis'; +import { + acquireFileLock, + AcquiredFileLock, + FILE_LOCK_DIR_SUFFIX, + getFileLockPath, + inspectFileLock, +} from '../../../common/lockfile.apis'; import { getWorkspacePersistentState, PersistentState } from '../../../common/persistentState'; import { EventNames, InlineScriptEnvErrorCategory } from '../../../common/telemetry/constants'; import { sendTelemetryEvent } from '../../../common/telemetry/sender'; +import { createDeferred, Deferred } from '../../../common/utils/deferred'; import { isFileNotFoundError } from '../../../common/utils/filesystem'; import { normalizePath } from '../../../common/utils/pathUtils'; import { compareReleaseSegments, parseReleaseSegments } from '../../../common/utils/pep440Release'; @@ -54,7 +61,12 @@ import { getVenvPythonPath } from '../../../common/utils/virtualEnvironment'; import { NativePythonFinder } from '../../common/nativePythonFinder'; import { resolveSystemPythonEnvironmentPath } from '../utils'; import * as uvPythonInstaller from '../uvPythonInstaller'; -import { createWithProgress, resolveVenvPythonEnvironmentPath } from '../venvUtils'; +import { + createWithProgress, + hasMinimumPathDepth, + isDriveRoot, + resolveVenvPythonEnvironmentPath, +} from '../venvUtils'; const BASE_INTERPRETER_MANAGER_IDS = new Set([ SYSTEM_MANAGER_ID, @@ -62,10 +74,8 @@ const BASE_INTERPRETER_MANAGER_IDS = new Set([ PYENV_MANAGER_ID, ]); -const CACHE_CLEAR_ROOT_LOCK_TIMEOUT_MS = 1_000; -const CACHE_CLEAR_ROOT_LOCK_RETRY_MS = 50; -const CACHE_CREATE_HANDOFF_LOCK_TIMEOUT_MS = 1_000; -const CACHE_CREATE_HANDOFF_LOCK_RETRY_MS = 50; +const CACHE_LOCK_TIMEOUT_MS = 5 * 60 * 1000; +const CACHE_LOCK_RETRY_MS = 500; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; /** Workspace-state key for PEP 723 script path to environment executable associations. */ export const INLINE_SCRIPT_ENVS_KEY = `${ENVS_EXTENSION_ID}:inline-script:SCRIPT_ENVIRONMENTS`; @@ -107,8 +117,6 @@ type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; -type CacheLockDisposition = 'retained' | 'active' | 'unknown'; - /** Manages extension-owned PEP 723 script environments. */ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly pendingSetups = new Map>(); @@ -122,8 +130,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly associationRevisions = new Map(); private persistenceQueue: Promise = Promise.resolve(); private selectionQueue: Promise = Promise.resolve(); - private activeCreateCount = 0; - private isClearCacheInProgress = false; + private cacheMaintenanceQueue: Promise = Promise.resolve(); + private cacheMaintenanceBarrier: Deferred | undefined; + private pendingCacheMaintenances = 0; + private activeCreateOperations = 0; private readonly _onDidChangeEnvironments = new EventEmitter(); public readonly onDidChangeEnvironments: Event = @@ -154,62 +164,54 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { scope: CreateEnvironmentScope, options?: CreateEnvironmentOptions, ): Promise { - if (this.isClearCacheInProgress) { - throw this.createCacheOperationConflict( - l10n.t( - 'Cannot create an inline script environment while the script environment cache is being cleared. Retry after the cache clear finishes.', - ), - ); - } - this.activeCreateCount += 1; + this.activeCreateOperations += 1; try { - const scriptUri = this.getScriptUri(scope); - if (!scriptUri) { - this.log.warn('Inline-script environment creation requires exactly one local file URI.'); - return undefined; - } + return await this.waitForCacheMaintenance(async () => { + try { + const scriptUri = this.getScriptUri(scope); + if (!scriptUri) { + this.log.warn('Inline-script environment creation requires exactly one local file URI.'); + return undefined; + } - const metadata = await readInlineScriptMetadataFromFile(scriptUri); - if (!metadata) { - this.log.warn(`No valid PEP 723 metadata found in ${scriptUri.fsPath}.`); - return undefined; - } + const metadata = await readInlineScriptMetadataFromFile(scriptUri); + if (!metadata) { + this.log.warn(`No valid PEP 723 metadata found in ${scriptUri.fsPath}.`); + return undefined; + } - const packages = [ - ...(metadata.dependencies ?? []), - ...(options?.additionalPackages ?? []), - ].map((value) => value.trim()); - if (packages.some((value) => value.length === 0)) { - this.log.warn(`Inline-script dependencies must not contain empty entries: ${scriptUri.fsPath}.`); - return undefined; - } + const packages = [ + ...(metadata.dependencies ?? []), + ...(options?.additionalPackages ?? []), + ].map((value) => value.trim()); + if (packages.some((value) => value.length === 0)) { + this.log.warn(`Inline-script dependencies must not contain empty entries: ${scriptUri.fsPath}.`); + return undefined; + } - const setupKey = this.getPendingSetupKey(scriptUri, metadata, packages, options); - const pending = this.pendingSetups.get(setupKey); - if (pending) { - return await pending; - } + const setupKey = this.getPendingSetupKey(scriptUri, metadata, packages, options); + const pending = this.pendingSetups.get(setupKey); + if (pending) { + return await pending; + } - const setup = this.createForScript(scriptUri, metadata, packages, options); - this.pendingSetups.set(setupKey, setup); - try { - return await setup; - } finally { - if (this.pendingSetups.get(setupKey) === setup) { - this.pendingSetups.delete(setupKey); + const setup = this.createForScript(scriptUri, metadata, packages, options); + this.pendingSetups.set(setupKey, setup); + try { + return await setup; + } finally { + if (this.pendingSetups.get(setupKey) === setup) { + this.pendingSetups.delete(setupKey); + } + } + } catch (error) { + this.sendInlineScriptEnvErrorTelemetry('setup-failure'); + this.log.error(`Failed to set up inline-script environment: ${getErrorMessage(error)}`); + return undefined; } - } - } catch (error) { - if (error instanceof InlineScriptCacheOperationError) { - this.sendInlineScriptEnvErrorTelemetry('setup-failure'); - this.log.warn(error.message); - throw error; - } - this.sendInlineScriptEnvErrorTelemetry('setup-failure'); - this.log.error(`Failed to set up inline-script environment: ${getErrorMessage(error)}`); - return undefined; + }); } finally { - this.activeCreateCount -= 1; + this.activeCreateOperations -= 1; } } @@ -294,73 +296,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } async set(scope: SetEnvironmentScope, environment?: PythonEnvironment): Promise { - return this.enqueueSelection(() => this.setInternal(scope, environment)); + return this.waitForCacheMaintenance(() => this.enqueueSelection(() => this.setInternal(scope, environment))); } async get(scope: GetEnvironmentScope): Promise { - return this.getInternal(scope); + return this.waitForCacheMaintenance(() => this.getInternal(scope)); } async resolve(_context: ResolveEnvironmentContext): Promise { return undefined; } - async clearScriptCache(): Promise { - if (this.isClearCacheInProgress) { - throw this.createCacheOperationConflict( - l10n.t('Script environment cache clear is already in progress.'), - ); - } - this.isClearCacheInProgress = true; - - try { - if (this.activeCreateCount > 0) { - throw this.createCacheOperationConflict( - l10n.t( - 'Cannot clear the script environment cache while another inline script environment operation may still be using it. Close other VS Code windows or restart VS Code, then retry.', - ), - ); - } - - const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); - let rootLock: AcquiredFileLock | undefined = await this.acquireCacheRootLock(cacheRoot, { - timeoutMs: CACHE_CLEAR_ROOT_LOCK_TIMEOUT_MS, - retryIntervalMs: CACHE_CLEAR_ROOT_LOCK_RETRY_MS, - }, 'clear'); - try { - const clearableCacheRoot = await this.getClearableCacheRootPath(cacheRoot); - if (clearableCacheRoot) { - await this.assertNoCacheLocks(clearableCacheRoot); - await this.removeClearableCacheRoot(clearableCacheRoot); - } - - let persistError: unknown; - try { - await this.clearPersistedAssociations(); - } catch (error) { - persistError = error; - } - - this.clearKnownAssociations(); - - if (persistError) { - throw persistError; - } - } finally { - const lockToRelease = rootLock; - rootLock = undefined; - await this.releaseCacheRootLockOrThrow(lockToRelease, cacheRoot.fsPath); - } - } catch (error) { - if (error instanceof InlineScriptCacheOperationError) { - this.log.warn(error.message); - } else { - this.log.error(`Failed to clear inline-script cache: ${getErrorMessage(error)}`); - } - throw error; - } finally { - this.isClearCacheInProgress = false; - } + async clearCache(): Promise { + const activeCreatesAtStart = this.activeCreateOperations; + return this.enqueueCacheMaintenance(() => + this.enqueueSelection(() => this.clearCacheInternal(activeCreatesAtStart)), + ); } private getScriptUri(scope: CreateEnvironmentScope): Uri | undefined { @@ -857,10 +808,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } - private clearPersistedAssociations(): Promise { - return this.enqueuePersistence((state) => state.clear([INLINE_SCRIPT_ENVS_KEY])); - } - private asPersistedAssociations(value: unknown): PersistedInlineScriptEnvironments | undefined { if (!value || typeof value !== 'object' || Array.isArray(value)) { return undefined; @@ -880,245 +827,46 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return run; } - private enqueueSelection(operation: () => Promise): Promise { - const run = this.selectionQueue.then(operation); - this.selectionQueue = run.then( - () => undefined, - () => undefined, - ); - return run; - } - - private clearKnownAssociations(): void { - const cleared = [...this.fsPathToEnv.entries()].map(([scriptPath, old]) => ({ - uri: Uri.file(scriptPath), - old, - new: undefined as PythonEnvironment | undefined, - })); - const knownScriptPaths = new Set([ - ...this.associationRevisions.keys(), - ...this.pendingRehydrations.keys(), - ...this.fsPathToPersistedEnvPath.keys(), - ...this.fsPathToEnv.keys(), - ]); - for (const scriptPath of knownScriptPaths) { - this.bumpAssociationRevision(scriptPath); - this.pendingRehydrations.delete(scriptPath); - } - this.fsPathToEnv.clear(); - this.fsPathToPersistedEnvPath.clear(); - this.cachedAssociationValidatedAt.clear(); - - cleared.forEach((event) => this._onDidChangeEnvironment.fire(event)); - } - - private async getClearableCacheRootPath(cacheRoot: Uri): Promise { - const globalStoragePath = path.resolve(this.globalStorageUri.fsPath); - let globalStorageStat: fs.Stats; - try { - globalStorageStat = await fs.lstat(globalStoragePath); - } catch (error) { - if (isFileNotFoundError(error)) { - return undefined; - } - throw error; - } - if (!globalStorageStat.isDirectory() || globalStorageStat.isSymbolicLink()) { - throw this.createUnsafeClearTargetError(globalStoragePath); - } - - const resolvedGlobalStorage = await fs.realpath(globalStoragePath); - if (normalizePath(resolvedGlobalStorage) !== normalizePath(globalStoragePath)) { - throw this.createUnsafeClearTargetError(globalStoragePath); - } - - const cacheRootPath = path.resolve(cacheRoot.fsPath); - try { - const cacheRootStat = await fs.lstat(cacheRootPath); - if (!cacheRootStat.isDirectory() || cacheRootStat.isSymbolicLink()) { - throw this.createUnsafeClearTargetError(cacheRootPath); - } - } catch (error) { - if (isFileNotFoundError(error)) { - return undefined; - } - throw error; - } - - const resolvedCacheRoot = await resolveCacheEntryPath(Uri.file(globalStoragePath), Uri.file(cacheRootPath)); - const expectedCacheRoot = path.join(resolvedGlobalStorage, INLINE_SCRIPT_CACHE_DIR_NAME); - if (!resolvedCacheRoot || normalizePath(resolvedCacheRoot) !== normalizePath(expectedCacheRoot)) { - throw this.createUnsafeClearTargetError(cacheRootPath); - } - - return resolvedCacheRoot; - } - - private async acquireCacheRootLock( - cacheRoot: Uri, - options: { - timeoutMs: number; - retryIntervalMs: number; - }, - operation: 'create' | 'clear', - ): Promise { - await fs.ensureDir(path.dirname(cacheRoot.fsPath)); - const lockPath = this.getLockPath(cacheRoot.fsPath); - try { - return await acquireFileLock(cacheRoot.fsPath, options); - } catch (error) { - if (this.isBusyLockError(error)) { - throw this.createCacheRootBusyError(operation, lockPath); - } - throw error; + private async waitForCacheMaintenance(operation: () => Promise): Promise { + const barrier = this.cacheMaintenanceBarrier; + if (barrier) { + await barrier.promise; } + return operation(); } - private async assertNoCacheLocks(cacheRootPath: string): Promise { - let entries: string[]; - try { - entries = await fs.readdir(cacheRootPath); - } catch (error) { - if (isFileNotFoundError(error)) { - return; - } - throw error; + private enqueueCacheMaintenance(operation: () => Promise): Promise { + if (!this.cacheMaintenanceBarrier) { + this.cacheMaintenanceBarrier = createDeferred(); } - - for (const entry of entries.filter((candidate) => candidate.endsWith('.lock'))) { - const lockPath = path.join(cacheRootPath, entry); - const lockDisposition = await this.inspectCacheLock(lockPath); - if (lockDisposition === 'active') { - throw this.createActiveLockError(lockPath); - } - if (lockDisposition === 'unknown') { - throw this.createUnknownLockError(lockPath); - } - } - } - - private removeClearableCacheRoot(cacheRootPath: string): Promise { - return fs.remove(cacheRootPath); - } - - private async inspectCacheLock(lockPath: string): Promise { - try { - const lockStat = await fs.lstat(lockPath); - if (!lockStat.isDirectory() || lockStat.isSymbolicLink()) { - return 'unknown'; - } - } catch { - return 'unknown'; - } - - const retainedPath = path.join(lockPath, 'retained'); - try { - const retainedStat = await fs.lstat(retainedPath); - if (retainedStat.isFile()) { - return 'retained'; - } - return 'unknown'; - } catch (error) { - if (!isFileNotFoundError(error)) { - return 'unknown'; - } - } - - try { - return (await fs.readdir(lockPath)).some((entry) => entry.startsWith('owner-')) ? 'active' : 'unknown'; - } catch { - return 'unknown'; - } - } - - private createUnsafeClearTargetError(targetPath: string): Error { - return new Error( - l10n.t( - 'Cannot clear the script environment cache because the target could not be proven safe: {0}', - targetPath, - ), - ); - } - - private createCacheOperationConflict(message: string): InlineScriptCacheOperationError { - return new InlineScriptCacheOperationError(message); - } - - private createCacheRootBusyError(operation: 'create' | 'clear', lockPath: string): InlineScriptCacheOperationError { - return this.createCacheOperationConflict( - operation === 'clear' - ? l10n.t( - 'Cannot clear the script environment cache because the cache root lock at {0} may still be active or may have been left by an interrupted operation. Wait for current work to finish or close other VS Code windows and retry. If it persists after restart and no inline script cache operation is using it, remove only this lock path manually.', - lockPath, - ) - : l10n.t( - 'Inline script environment cache is busy because the cache root lock at {0} may still be active or may have been left by an interrupted operation. Wait for current work to finish or close other VS Code windows and retry. If it persists after restart and no inline script cache operation is using it, remove only this lock path manually.', - lockPath, - ), - ); - } - - private createActiveLockError(lockPath: string): InlineScriptCacheOperationError { - return this.createCacheOperationConflict( - l10n.t( - 'Cannot clear the script environment cache because the owner-only lock at {0} may still be active or may have been left by an interrupted operation. Close other VS Code windows and retry. If it persists after restart, manually remove only this lock path after confirming that no inline script cache operation is using it.', - lockPath, - ), - ); - } - - private createUnknownLockError(lockPath: string): InlineScriptCacheOperationError { - return this.createCacheOperationConflict( - l10n.t( - 'Cannot clear the script environment cache because the cache lock at {0} could not be verified as retained. Remove it manually only if you know no inline script environment operation still needs it.', - lockPath, - ), - ); - } - - private createCacheRootReleaseError(lockPath: string): InlineScriptCacheOperationError { - return this.createCacheOperationConflict( - l10n.t( - 'Failed to release the script environment cache root lock at {0}. Close other VS Code windows and retry. If it persists after restart and no inline script cache operation is using it, remove only this lock path manually.', - lockPath, - ), + this.pendingCacheMaintenances += 1; + const run = this.cacheMaintenanceQueue.then(operation); + this.cacheMaintenanceQueue = run.then( + () => undefined, + () => undefined, ); + return run.finally(() => { + this.pendingCacheMaintenances -= 1; + if (this.pendingCacheMaintenances === 0) { + this.cacheMaintenanceBarrier?.resolve(); + this.cacheMaintenanceBarrier = undefined; + } + }); } - private async releaseCacheRootLockOrThrow(lock: AcquiredFileLock, cacheRootPath: string): Promise { - const lockPath = this.getLockPath(cacheRootPath); - try { - await lock.release(); - } catch { - throw this.createCacheRootReleaseError(lockPath); - } - } - - private async releaseCacheLock(lock: AcquiredFileLock, label: string): Promise { - try { - await lock.release(); - } catch (error) { - this.log.warn(`Failed to release ${label} lock: ${getErrorMessage(error)}`); - } - } - - private isBusyLockError(error: unknown): boolean { - return ( - typeof error === 'object' && - error !== null && - 'code' in error && - ['ELOCKED', 'ELOCKRETAINED'].includes((error as NodeJS.ErrnoException).code ?? '') + private enqueueSelection(operation: () => Promise): Promise { + const run = this.selectionQueue.then(operation); + this.selectionQueue = run.then( + () => undefined, + () => undefined, ); - } - - private getLockPath(targetPath: string): string { - return `${path.resolve(targetPath)}.lock`; + return run; } private async isCacheEntryBusy(envDirPath: string): Promise { return ( this.pendingCreations.has(path.basename(envDirPath)) || - (await fs.pathExists(`${path.resolve(envDirPath)}.lock`)) + (await fs.pathExists(getFileLockPath(envDirPath))) ); } @@ -1434,22 +1182,14 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { const dependencyCount = this.getTelemetryDependencyCount(packages); const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); const envDir = getScriptEnvDir(this.globalStorageUri, cacheKey); + await fs.ensureDir(cacheRoot.fsPath); - let rootLock: AcquiredFileLock | undefined; let lock: AcquiredFileLock | undefined; try { - rootLock = await this.acquireCacheRootLock(cacheRoot, { - timeoutMs: CACHE_CREATE_HANDOFF_LOCK_TIMEOUT_MS, - retryIntervalMs: CACHE_CREATE_HANDOFF_LOCK_RETRY_MS, - }, 'create'); - await fs.ensureDir(cacheRoot.fsPath); lock = await acquireFileLock(envDir.fsPath, { - timeoutMs: CACHE_CREATE_HANDOFF_LOCK_TIMEOUT_MS, - retryIntervalMs: CACHE_CREATE_HANDOFF_LOCK_RETRY_MS, + timeoutMs: CACHE_LOCK_TIMEOUT_MS, + retryIntervalMs: CACHE_LOCK_RETRY_MS, }); - const handoffRootLock = rootLock; - rootLock = undefined; - await this.releaseCacheRootLockOrThrow(handoffRootLock, cacheRoot.fsPath); const cached = await this.inspectCacheEntry(cacheRoot, envDir, metadata, selectedBase); if (cached.kind === 'reusable') { @@ -1490,22 +1230,16 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } return undefined; } catch (error) { - if (error instanceof InlineScriptCacheOperationError) { - this.sendInlineScriptEnvErrorTelemetry('setup-failure'); - this.log.warn(error.message); - return undefined; - } this.sendInlineScriptEnvErrorTelemetry(this.getCreateOrReuseErrorCategory(error)); this.log.error(`Failed to create or reuse inline-script cache entry: ${getErrorMessage(error)}`); return undefined; } finally { if (lock) { - await this.releaseCacheLock(lock, 'inline-script cache entry'); - } - if (rootLock) { - const lockToRelease = rootLock; - rootLock = undefined; - await this.releaseCacheRootLockOrThrow(lockToRelease, cacheRoot.fsPath); + try { + await lock.release(); + } catch (error) { + this.log.warn(`Failed to release inline-script cache lock: ${getErrorMessage(error)}`); + } } } } @@ -1651,6 +1385,254 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return { environment: result.environment }; } + private async clearCacheInternal(activeCreatesAtStart: number): Promise { + if (activeCreatesAtStart > 0) { + const message = l10n.t( + 'Cannot clear the script environment cache while script environments are being created.', + ); + this.log.error(message); + throw new Error(message); + } + + const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); + const cacheEntryPaths = await this.getClearableCacheEntryPaths(cacheRoot); + const persistedAssociations = await this.getPersistedAssociationSnapshot(); + const scriptPaths = new Set([ + ...Object.keys(persistedAssociations), + ...this.associationRevisions.keys(), + ...this.cachedAssociationValidatedAt.keys(), + ...this.fsPathToEnv.keys(), + ...this.fsPathToPersistedEnvPath.keys(), + ...this.pendingRehydrations.keys(), + ]); + const priorSelections = new Map(); + scriptPaths.forEach((scriptPath) => { + priorSelections.set(scriptPath, this.fsPathToEnv.get(scriptPath)); + }); + + for (const cacheEntryPath of cacheEntryPaths) { + await fs.remove(cacheEntryPath); + } + + let persistenceError: unknown; + try { + const state = await getWorkspacePersistentState(); + await state.clear([INLINE_SCRIPT_ENVS_KEY]); + } catch (error) { + persistenceError = error; + this.log.error(`Failed to clear inline-script environment associations: ${getErrorMessage(error)}`); + } + + scriptPaths.forEach((scriptPath) => this.bumpAssociationRevision(scriptPath)); + this.pendingRehydrations.clear(); + this.fsPathToEnv.clear(); + this.fsPathToPersistedEnvPath.clear(); + this.cachedAssociationValidatedAt.clear(); + + priorSelections.forEach((environment, scriptPath) => { + if (!environment) { + return; + } + this._onDidChangeEnvironment.fire({ + uri: Uri.file(scriptPath), + old: environment, + new: undefined, + }); + }); + + if (persistenceError) { + throw persistenceError; + } + } + + private async getClearableCacheEntryPaths(cacheRoot: Uri): Promise { + const resolvedCacheRootPath = await this.getPhysicalOwnedCacheRootPath(cacheRoot); + if (!resolvedCacheRootPath) { + return []; + } + const cacheRootPath = path.resolve(resolvedCacheRootPath); + const physicalCacheRoot = Uri.file(cacheRootPath); + + const entryNames = await fs.readdir(cacheRootPath); + const lockStates = new Map(); + for (const entryName of entryNames.filter((entry) => entry.endsWith(FILE_LOCK_DIR_SUFFIX))) { + const envName = entryName.slice(0, -FILE_LOCK_DIR_SUFFIX.length); + if (envName.length === 0) { + const message = l10n.t( + 'Refusing to clear the script environment cache because a lock entry is malformed.', + ); + this.log.error(`${message} (${path.join(cacheRootPath, entryName)})`); + throw new Error(message); + } + + const envDirPath = path.join(cacheRootPath, envName); + const lockState = await inspectFileLock(envDirPath); + if (lockState === 'retained' || lockState === 'stale') { + lockStates.set(envDirPath, lockState); + continue; + } + if (lockState === 'held') { + const message = l10n.t( + 'Cannot clear the script environment cache while a cached environment is being created.', + ); + this.log.error(`${message} (${getFileLockPath(envDirPath)})`); + throw new Error(message); + } + if (lockState === 'unavailable') { + const message = l10n.t( + 'Cannot clear the script environment cache because a cached environment lock could not be verified.', + ); + this.log.error(`${message} (${getFileLockPath(envDirPath)})`); + throw new Error(message); + } + + const message = l10n.t( + 'Refusing to clear the script environment cache because a lock entry is incomplete or malformed.', + ); + this.log.error(`${message} (${getFileLockPath(envDirPath)})`); + throw new Error(message); + } + + const pathsToRemove: string[] = []; + const scheduledPaths = new Set(); + + for (const envDirPath of lockStates.keys()) { + const cacheEntryPath = await this.getClearableCacheEntryPath(physicalCacheRoot, envDirPath); + if (cacheEntryPath) { + pathsToRemove.push(cacheEntryPath); + scheduledPaths.add(normalizePath(cacheEntryPath)); + } + } + + for (const envDirPath of lockStates.keys()) { + const lockPath = getFileLockPath(envDirPath); + pathsToRemove.push(lockPath); + scheduledPaths.add(normalizePath(lockPath)); + } + + for (const entryName of entryNames.filter((entry) => !entry.endsWith(FILE_LOCK_DIR_SUFFIX))) { + const cacheEntryPath = await this.getClearableCacheEntryPath(physicalCacheRoot, path.join(cacheRootPath, entryName)); + if (cacheEntryPath && !scheduledPaths.has(normalizePath(cacheEntryPath))) { + pathsToRemove.push(cacheEntryPath); + } + } + + return pathsToRemove; + } + + private async getPhysicalOwnedCacheRootPath(cacheRoot: Uri): Promise { + const globalStoragePath = path.resolve(this.globalStorageUri.fsPath); + const cacheRootPath = path.resolve(cacheRoot.fsPath); + if (path.basename(cacheRootPath) !== INLINE_SCRIPT_CACHE_DIR_NAME || normalizePath(path.dirname(cacheRootPath)) !== normalizePath(globalStoragePath)) { + this.log.error(`Refusing to clear inline-script cache from unsafe root: ${cacheRootPath}`); + throw new Error(l10n.t('Refusing to clear the script environment cache from an unsafe cache root.')); + } + if (isDriveRoot(globalStoragePath) || !hasMinimumPathDepth(cacheRootPath, 3)) { + this.log.error(`Refusing to clear inline-script cache from unsafe root: ${cacheRootPath}`); + throw new Error(l10n.t('Refusing to clear the script environment cache from an unsafe cache root.')); + } + + let globalStorageStat; + try { + globalStorageStat = await fs.lstat(globalStoragePath); + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } + + if (!globalStorageStat.isDirectory() || globalStorageStat.isSymbolicLink()) { + this.log.error(`Refusing to clear inline-script cache from redirected globalStorage root: ${globalStoragePath}`); + throw new Error( + l10n.t('Refusing to clear the script environment cache because the global storage root is not a normal directory.'), + ); + } + + let cacheRootStat; + try { + cacheRootStat = await fs.lstat(cacheRootPath); + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } + + if (!cacheRootStat.isDirectory() || cacheRootStat.isSymbolicLink()) { + this.log.error(`Refusing to clear inline-script cache from redirected cache root: ${cacheRootPath}`); + throw new Error( + l10n.t('Refusing to clear the script environment cache because the cache root is not a normal directory.'), + ); + } + + let resolvedGlobalStoragePath: string; + let resolvedCacheRootPath: string; + try { + [resolvedGlobalStoragePath, resolvedCacheRootPath] = await Promise.all([ + fs.realpath(globalStoragePath), + fs.realpath(cacheRootPath), + ]); + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + this.log.error(`Failed to resolve inline-script cache root physically: ${getErrorMessage(error)}`); + throw new Error( + l10n.t('Refusing to clear the script environment cache because its physical location could not be verified.'), + ); + } + + const expectedResolvedCacheRootPath = path.join(resolvedGlobalStoragePath, INLINE_SCRIPT_CACHE_DIR_NAME); + if ( + normalizePath(resolvedCacheRootPath) !== normalizePath(expectedResolvedCacheRootPath) || + normalizePath(path.dirname(resolvedCacheRootPath)) !== normalizePath(resolvedGlobalStoragePath) + ) { + this.log.error( + `Refusing to clear inline-script cache from redirected physical root: ${resolvedCacheRootPath}`, + ); + throw new Error( + l10n.t('Refusing to clear the script environment cache because the cache root is redirected.'), + ); + } + return resolvedCacheRootPath; + } + + private async getClearableCacheEntryPath(cacheRoot: Uri, entryPath: string): Promise { + let stat; + try { + stat = await fs.lstat(entryPath); + } catch (error) { + if (isFileNotFoundError(error)) { + return undefined; + } + throw error; + } + + if (!stat.isDirectory() || stat.isSymbolicLink()) { + this.log.error(`Refusing to clear inline-script cache entry from unsafe path: ${entryPath}`); + throw new Error( + l10n.t('Refusing to clear the script environment cache because a cache entry is not a normal directory.'), + ); + } + + const resolvedEntryPath = await resolveCacheEntryPath(cacheRoot, Uri.file(entryPath)); + if (!resolvedEntryPath) { + this.log.error(`Refusing to clear inline-script cache entry outside the expected root: ${entryPath}`); + throw new Error( + l10n.t('Refusing to clear the script environment cache because a cache entry is outside the expected root.'), + ); + } + + return resolvedEntryPath; + } + + private async getPersistedAssociationSnapshot(): Promise { + await this.persistenceQueue; + const state = await getWorkspacePersistentState(); + return this.asPersistedAssociations(await state.get(INLINE_SCRIPT_ENVS_KEY)) ?? {}; + } + private async removeCacheEntry(envDir: Uri): Promise { try { await fs.remove(envDir.fsPath); @@ -1737,5 +1719,3 @@ interface PendingScriptUpdate extends ScriptReference { readonly needsPersistence: boolean; readonly shouldNotify: boolean; } - -class InlineScriptCacheOperationError extends Error {} diff --git a/src/managers/builtin/inlineScript/main.ts b/src/managers/builtin/inlineScript/main.ts index 94531313f..8c35fc6ed 100644 --- a/src/managers/builtin/inlineScript/main.ts +++ b/src/managers/builtin/inlineScript/main.ts @@ -20,15 +20,14 @@ export async function registerInlineScriptFeatures( log: LogOutputChannel, baseManager: EnvironmentManager, globalStorageUri: Uri, -): Promise { +): Promise { if (!isInlineScriptsFeatureEnabled()) { traceVerbose('Inline-script env manager: skipping registration (internal flag is off)'); - return undefined; + return; } const api: PythonEnvironmentApi = await getPythonApi(); const mgr = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); disposables.push(mgr, api.registerEnvironmentManager(mgr)); traceInfo('Inline-script env manager: registered (internal flag is on)'); - return mgr; } diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index 2962235e1..e35825866 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -513,7 +513,7 @@ export async function createPythonVenv( return createStepBasedVenvFlow(nativeFinder, api, log, manager, basePythons, venvRoot, options); } -function isDriveRoot(fsPath: string): boolean { +export function isDriveRoot(fsPath: string): boolean { const normalized = path.normalize(fsPath); if (os.platform() === 'win32') { return /^[a-zA-Z]:[\\/]?$/.test(normalized); @@ -521,7 +521,7 @@ function isDriveRoot(fsPath: string): boolean { return normalized === '/'; } -function hasMinimumPathDepth(fsPath: string, minDepth: number = 2): boolean { +export function hasMinimumPathDepth(fsPath: string, minDepth: number = 2): boolean { const normalized = path.normalize(fsPath); const parts = normalized.split(path.sep).filter((p) => p.length > 0 && p !== '.'); diff --git a/src/test/common/lockfile.apis.unit.test.ts b/src/test/common/lockfile.apis.unit.test.ts index a2d343a19..df8c5acc4 100644 --- a/src/test/common/lockfile.apis.unit.test.ts +++ b/src/test/common/lockfile.apis.unit.test.ts @@ -8,7 +8,13 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; import * as sinon from 'sinon'; -import { acquireFileLock, AcquireFileLockOptions } from '../../common/lockfile.apis'; +import { + acquireFileLock, + AcquireFileLockOptions, + FILE_LOCK_OWNER_MARKER_PREFIX, + getFileLockPath, + inspectFileLock, +} from '../../common/lockfile.apis'; const OPTIONS: AcquireFileLockOptions = { timeoutMs: 40, @@ -209,4 +215,63 @@ suite('lockfile APIs', () => { return error.code === 'ELOCKORPHANED' && error.path === `${path.resolve(targetPath)}.lock`; }); }); + + test('classifies a live owner marker as held using the liveness probe', async () => { + const lockPath = getFileLockPath(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `${FILE_LOCK_OWNER_MARKER_PREFIX}${process.pid}-live`), ''); + const checkProcessLiveness = sinon.stub().resolves('live'); + + assert.strictEqual(await inspectFileLock(targetPath, { checkProcessLiveness }), 'held'); + sinon.assert.calledOnceWithExactly(checkProcessLiveness, process.pid); + }); + + test('classifies a retained lock after retain()', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + await lock.retain(); + + assert.strictEqual(await inspectFileLock(targetPath), 'retained'); + }); + + test('classifies a dead owner marker as stale using the liveness probe', async () => { + const lockPath = getFileLockPath(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `${FILE_LOCK_OWNER_MARKER_PREFIX}424242-dead`), ''); + const checkProcessLiveness = sinon.stub().resolves('dead'); + + assert.strictEqual(await inspectFileLock(targetPath, { checkProcessLiveness }), 'stale'); + sinon.assert.calledOnceWithExactly(checkProcessLiveness, 424242); + }); + + test('classifies an unavailable owner probe conservatively', async () => { + const lockPath = getFileLockPath(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `${FILE_LOCK_OWNER_MARKER_PREFIX}${process.pid}-busy`), ''); + const checkProcessLiveness = sinon.stub().resolves('unavailable'); + + assert.strictEqual(await inspectFileLock(targetPath, { checkProcessLiveness }), 'unavailable'); + sinon.assert.calledOnceWithExactly(checkProcessLiveness, process.pid); + }); + + test('classifies an owner-less lock directory as orphaned', async () => { + await fs.ensureDir(getFileLockPath(targetPath)); + + assert.strictEqual(await inspectFileLock(targetPath), 'orphaned'); + }); + + test('classifies a malformed owner marker as malformed', async () => { + const lockPath = getFileLockPath(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `${FILE_LOCK_OWNER_MARKER_PREFIX}not-a-pid-live`), ''); + + assert.strictEqual(await inspectFileLock(targetPath), 'malformed'); + }); + + test('classifies a lock directory with unexpected entries as malformed', async () => { + const lockPath = getFileLockPath(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, 'unexpected.txt'), ''); + + assert.strictEqual(await inspectFileLock(targetPath), 'malformed'); + }); }); diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index 6fd6229d6..e31e98adc 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/src/test/features/envCommands.unit.test.ts @@ -1,24 +1,22 @@ import * as assert from 'assert'; +import * as path from 'path'; import * as sinon from 'sinon'; import * as typeMoq from 'typemoq'; -import { Uri } from 'vscode'; +import { Uri, WorkspaceFolder } from 'vscode'; import { PythonEnvironment, PythonProject } from '../../api'; import * as commandApi from '../../common/command.api'; +import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; import * as managerApi from '../../common/pickers/managers'; import * as projectApi from '../../common/pickers/projects'; -import * as persistentState from '../../common/persistentState'; import * as windowApis from '../../common/window.apis'; +import * as workspaceApis from '../../common/workspace.apis'; import { - clearCacheCommand, - clearInlineScriptCacheCommand, + clearScriptEnvironmentCacheCommand, createAnyEnvironmentCommand, removePythonProject, revealEnvInManagerView, } from '../../features/envCommands'; -import * as managerReady from '../../features/common/managerReady'; import * as settingHelpers from '../../features/settings/settingHelpers'; -import * as helpers from '../../helpers'; -import type { InlineScriptEnvManager } from '../../managers/builtin/inlineScript/envManager'; import { EnvManagerView } from '../../features/views/envManagersView'; import { ProjectEnvironment, ProjectItem } from '../../features/views/treeViewItems'; import { EnvironmentManagers, InternalEnvironmentManager, PythonProjectManager } from '../../internal.api'; @@ -215,6 +213,7 @@ suite('Remove Python Project Command Tests', () => { } as unknown as PythonProjectManager; sinon.stub(settingHelpers, 'removePythonProjectSetting').callsFake(async () => { calls.push('removeSetting'); + return []; }); await removePythonProject(item, projectManager, envManagers); @@ -227,109 +226,241 @@ suite('Remove Python Project Command Tests', () => { }); }); -suite('Clear Cache Command Tests', () => { +suite('Clear Script Environment Cache Command Tests', () => { + const workspacePath = process.platform === 'win32' ? 'C:\\workspace' : '/workspace'; + const workspaceFolder: WorkspaceFolder = { + uri: Uri.file(workspacePath), + name: 'workspace', + index: 0, + }; + teardown(() => { sinon.restore(); }); - test('keeps the broad clear handler on the base path', async () => { - const calls: string[] = []; + test('cancels without clearing the cache or touching project settings', async () => { + const clearCache = sinon.stub().resolves(); const envManagers = { - clearCache: sinon.stub().callsFake(async (scope: unknown) => { - calls.push(`managers:${String(scope)}`); + getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ + supportsClearCache: () => true, + clearCache, }), } as unknown as EnvironmentManagers; - const clearShellProfileCache = sinon.stub().callsFake(async () => { - calls.push('shell'); - }); - sinon.stub(persistentState, 'clearPersistentState').callsFake(async () => { - calls.push('state'); - }); - - await clearCacheCommand(envManagers, clearShellProfileCache); - - assert.deepStrictEqual(calls, ['state', 'managers:undefined', 'shell']); - assert.ok((envManagers.clearCache as sinon.SinonStub).calledOnceWithExactly(undefined)); - assert.ok(clearShellProfileCache.calledOnce); - }); -}); - -suite('Clear Inline Script Environment Cache Command Tests', () => { - let clearScriptCacheStub: sinon.SinonStub; - let getManager: sinon.SinonStub; - let showErrorMessageStub: sinon.SinonStub; - let showWarningMessageStub: sinon.SinonStub; - let isInlineScriptsFeatureEnabledStub: sinon.SinonStub; - let waitForEnvManagerIdStub: sinon.SinonStub; - - setup(() => { - clearScriptCacheStub = sinon.stub().resolves(); - getManager = sinon - .stub<[], InlineScriptEnvManager | undefined>() - .returns({ clearScriptCache: clearScriptCacheStub } as unknown as InlineScriptEnvManager); - showErrorMessageStub = sinon.stub(windowApis, 'showErrorMessage'); - showWarningMessageStub = sinon.stub(windowApis, 'showWarningMessage'); - isInlineScriptsFeatureEnabledStub = sinon.stub(helpers, 'isInlineScriptsFeatureEnabled').returns(true); - waitForEnvManagerIdStub = sinon.stub(managerReady, 'waitForEnvManagerId').resolves(); - }); - - teardown(() => { - sinon.restore(); - }); - - test('clears the cache after confirmation', async () => { - showWarningMessageStub.callsFake(async (_message, _options, clearLabel: string) => clearLabel); + const projectManager = { + getProjects: sinon.stub().returns([]), + create: sinon.stub(), + remove: sinon.stub(), + } as unknown as PythonProjectManager; + sinon.stub(windowApis, 'showWarningMessage').resolves(undefined); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([]); + const removeSettings = sinon.stub(settingHelpers, 'removePythonProjectSetting').resolves([]); - await clearInlineScriptCacheCommand(getManager); + await clearScriptEnvironmentCacheCommand(envManagers, projectManager); - assert.ok(showWarningMessageStub.calledOnce); - assert.deepStrictEqual(showWarningMessageStub.firstCall.args[1], { modal: true }); - assert.ok(waitForEnvManagerIdStub.calledOnce); - assert.ok(getManager.calledOnce); - assert.ok(clearScriptCacheStub.calledOnce); - assert.strictEqual(showErrorMessageStub.called, false); + sinon.assert.notCalled(clearCache); + sinon.assert.notCalled(removeSettings); + sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); }); - test('does nothing when the confirmation is cancelled', async () => { - showWarningMessageStub.resolves(undefined); - - await clearInlineScriptCacheCommand(getManager); - - assert.ok(showWarningMessageStub.calledOnce); - assert.ok(waitForEnvManagerIdStub.calledOnce); - assert.ok(getManager.calledOnce); - assert.strictEqual(clearScriptCacheStub.called, false); - assert.strictEqual(showErrorMessageStub.called, false); + test('clears the cache and removes only inline-script projects returned by the settings cleanup', async () => { + const inlineProject: PythonProject = { + uri: Uri.file(path.join(workspacePath, 'script.py')), + name: 'script.py', + }; + const clearCache = sinon.stub().resolves(); + const envManagers = { + getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ + supportsClearCache: () => true, + clearCache, + }), + } as unknown as EnvironmentManagers; + const projectManager = { + getProjects: sinon.stub().returns([inlineProject]), + create: sinon.stub().callsFake((name: string, uri: Uri) => ({ name, uri })), + remove: sinon.stub(), + } as unknown as PythonProjectManager; + sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (key: string) => + key === 'pythonProjects' + ? [ + { + path: 'script.py', + envManager: INLINE_SCRIPT_MANAGER_ID, + packageManager: 'ms-python.python:pip', + }, + { + path: 'other.py', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ] + : [], + inspect: (key: string) => + key === 'pythonProjects' + ? { + workspaceValue: [ + { + path: 'script.py', + envManager: INLINE_SCRIPT_MANAGER_ID, + packageManager: 'ms-python.python:pip', + }, + { + path: 'other.py', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ], + workspaceFolderValue: undefined, + } + : undefined, + } as never); + const removeSettings = sinon.stub(settingHelpers, 'removePythonProjectSetting').resolves([inlineProject]); + + await clearScriptEnvironmentCacheCommand(envManagers, projectManager); + + sinon.assert.calledOnce(clearCache); + sinon.assert.calledOnceWithExactly(removeSettings, [ + { + project: inlineProject, + envManager: INLINE_SCRIPT_MANAGER_ID, + }, + ]); + sinon.assert.calledOnceWithExactly(projectManager.remove as sinon.SinonStub, [inlineProject]); }); - test('fails fast when the feature setting is off', async () => { - isInlineScriptsFeatureEnabledStub.returns(false); - showErrorMessageStub.resolves(undefined); - - await assert.rejects( - clearInlineScriptCacheCommand(getManager), - /inline script environments are disabled in this window/i, + test('includes inline-script projects without a .py extension', async () => { + const inlineProjectUri = Uri.file(path.join(workspacePath, 'runner')); + const clearCache = sinon.stub().resolves(); + const envManagers = { + getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ + supportsClearCache: () => true, + clearCache, + }), + } as unknown as EnvironmentManagers; + const projectManager = { + getProjects: sinon.stub().returns([]), + create: sinon.stub().callsFake((name: string, uri: Uri) => ({ name, uri })), + remove: sinon.stub(), + } as unknown as PythonProjectManager; + sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (key: string) => + key === 'pythonProjects' + ? [ + { + path: 'runner', + envManager: INLINE_SCRIPT_MANAGER_ID, + packageManager: 'ms-python.python:pip', + }, + { + path: 'unrelated', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ] + : [], + inspect: (key: string) => + key === 'pythonProjects' + ? { + workspaceValue: [ + { + path: 'runner', + envManager: INLINE_SCRIPT_MANAGER_ID, + packageManager: 'ms-python.python:pip', + }, + { + path: 'unrelated', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ], + workspaceFolderValue: undefined, + } + : undefined, + } as never); + const removeSettings = sinon + .stub(settingHelpers, 'removePythonProjectSetting') + .callsFake(async (edits) => [edits[0].project]); + + await clearScriptEnvironmentCacheCommand(envManagers, projectManager); + + assert.strictEqual(removeSettings.callCount, 1); + assert.strictEqual(removeSettings.firstCall.args[0].length, 1); + assert.strictEqual(removeSettings.firstCall.args[0][0].envManager, INLINE_SCRIPT_MANAGER_ID); + assert.strictEqual(removeSettings.firstCall.args[0][0].project.uri.fsPath, inlineProjectUri.fsPath); + assert.strictEqual((projectManager.create as sinon.SinonStub).callCount, 1); + assert.strictEqual((projectManager.create as sinon.SinonStub).firstCall.args[0], 'runner'); + assert.strictEqual( + (projectManager.create as sinon.SinonStub).firstCall.args[1].fsPath.toLowerCase(), + inlineProjectUri.fsPath.toLowerCase(), ); - - assert.ok(showErrorMessageStub.calledOnce); - assert.strictEqual(waitForEnvManagerIdStub.called, false); - assert.strictEqual(getManager.called, false); - assert.strictEqual(showWarningMessageStub.called, false); + sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); }); - test('throws a clear error when the manager is unavailable after the readiness wait', async () => { - getManager.returns(undefined); - showErrorMessageStub.resolves(undefined); - - await assert.rejects( - clearInlineScriptCacheCommand(getManager), - /inline script environment manager is not available in this window/i, - ); - - assert.ok(waitForEnvManagerIdStub.calledOnce); - assert.ok(getManager.calledOnce); - assert.ok(showErrorMessageStub.calledOnce); - assert.strictEqual(showWarningMessageStub.called, false); + test('removes a hidden inline workspace entry when a folder override exists for the same URI', async () => { + const projectUri = Uri.file(path.join(workspacePath, 'script.py')); + const visibleProject: PythonProject = { + uri: projectUri, + name: 'script.py', + }; + const clearCache = sinon.stub().resolves(); + const envManagers = { + getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ + supportsClearCache: () => true, + clearCache, + }), + } as unknown as EnvironmentManagers; + const projectManager = { + getProjects: sinon.stub().returns([visibleProject]), + create: sinon.stub(), + remove: sinon.stub(), + } as unknown as PythonProjectManager; + sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); + sinon.stub(workspaceApis, 'getConfiguration').returns({ + get: (key: string) => + key === 'pythonProjects' + ? [ + { + path: 'script.py', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ] + : [], + inspect: (key: string) => + key === 'pythonProjects' + ? { + workspaceValue: [ + { + path: 'script.py', + envManager: INLINE_SCRIPT_MANAGER_ID, + packageManager: 'ms-python.python:pip', + }, + ], + workspaceFolderValue: [ + { + path: 'script.py', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ], + } + : undefined, + } as never); + const removeSettings = sinon + .stub(settingHelpers, 'removePythonProjectSetting') + .callsFake(async (edits) => [edits[0].project]); + + await clearScriptEnvironmentCacheCommand(envManagers, projectManager); + + assert.strictEqual(removeSettings.callCount, 1); + assert.strictEqual(removeSettings.firstCall.args[0].length, 1); + assert.strictEqual(removeSettings.firstCall.args[0][0].project.uri.fsPath, visibleProject.uri.fsPath); + assert.strictEqual(removeSettings.firstCall.args[0][0].envManager, INLINE_SCRIPT_MANAGER_ID); }); }); diff --git a/src/test/features/envManagers.unit.test.ts b/src/test/features/envManagers.unit.test.ts index 42a64579e..d642fa948 100644 --- a/src/test/features/envManagers.unit.test.ts +++ b/src/test/features/envManagers.unit.test.ts @@ -7,7 +7,6 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import { Uri } from 'vscode'; import { PythonEnvironment } from '../../api'; -import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; import * as frameUtils from '../../common/utils/frameUtils'; import * as workspaceApis from '../../common/workspace.apis'; import { PythonEnvironmentManagers } from '../../features/envManagers'; @@ -337,70 +336,3 @@ suite('PythonEnvironmentManagers - refreshEnvironment', () => { await envManagers.refreshEnvironment(Uri.file('/unknown/path')); }); }); - -suite('PythonEnvironmentManagers - clearCache', () => { - let sandbox: sinon.SinonSandbox; - let envManagers: PythonEnvironmentManagers; - let mockProjectManager: sinon.SinonStubbedInstance; - - setup(() => { - sandbox = sinon.createSandbox(); - sandbox.stub(frameUtils, 'getCallingExtension').returns('ms-python.python'); - sandbox.stub(workspaceApis, 'getConfiguration').returns({ - get: (key: string, defaultValue?: unknown) => { - if (key === 'defaultEnvManager') { - return 'ms-python.python:system'; - } - if (key === 'pythonProjects') { - return []; - } - return defaultValue; - }, - has: () => false, - inspect: () => undefined, - update: () => Promise.resolve(), - } as any); - - mockProjectManager = { - getProjects: sandbox.stub().returns([]), - get: sandbox.stub().returns(undefined), - } as unknown as sinon.SinonStubbedInstance; - - envManagers = new PythonEnvironmentManagers(mockProjectManager as unknown as PythonProjectManager); - }); - - teardown(() => { - sandbox.restore(); - }); - - function registerFakeManager(managerId: string, clearCache: sinon.SinonStub): void { - envManagers.registerEnvironmentManager( - { - name: managerId.split(':')[1], - displayName: managerId, - preferredPackageManagerId: 'ms-python.python:pip', - clearCache, - get: sandbox.stub().resolves(undefined), - set: sandbox.stub().resolves(), - resolve: sandbox.stub().resolves(undefined), - refresh: sandbox.stub().resolves(), - getEnvironments: sandbox.stub().resolves([]), - onDidChangeEnvironments: sandbox.stub().returns({ dispose: () => {} }), - onDidChangeEnvironment: sandbox.stub().returns({ dispose: () => {} }), - } as any, - { extensionId: 'ms-python.python' }, - ); - } - - test('does not special-case managers during broad cache clears', async () => { - const systemClearCache = sandbox.stub().resolves(); - const inlineClearCache = sandbox.stub().resolves(); - registerFakeManager('ms-python.python:system', systemClearCache); - registerFakeManager(INLINE_SCRIPT_MANAGER_ID, inlineClearCache); - - await envManagers.clearCache(undefined); - - assert.ok(systemClearCache.calledOnce); - assert.ok(inlineClearCache.calledOnce); - }); -}); diff --git a/src/test/features/projectManager.initialize.unit.test.ts b/src/test/features/projectManager.initialize.unit.test.ts index 84e0c9fc7..f89a325ac 100644 --- a/src/test/features/projectManager.initialize.unit.test.ts +++ b/src/test/features/projectManager.initialize.unit.test.ts @@ -1,8 +1,10 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import * as assert from 'assert'; +import * as path from 'path'; import * as sinon from 'sinon'; import { Disposable, EventEmitter, Uri, WorkspaceFolder } from 'vscode'; import * as workspaceApis from '../../common/workspace.apis'; +import { normalizePath } from '../../common/utils/pathUtils'; import { PythonProjectManagerImpl } from '../../features/projectManager'; import * as settingHelpers from '../../features/settings/settingHelpers'; import { PythonProjectSettings } from '../../internal.api'; @@ -304,6 +306,163 @@ suite('Project Manager Initialization - Settings Preservation', () => { pm.dispose(); }); + + test('config refresh drops only the project removed from workspaceValue and preserves workspaceFolder entries', async () => { + let workspaceValueProjects: PythonProjectSettings[] = [ + { + path: 'script.py', + envManager: 'ms-python.python:inline-script', + packageManager: 'ms-python.python:pip', + workspace: workspaceFolder.name, + }, + ]; + let workspaceFolderProjects: PythonProjectSettings[] = [ + { + path: 'keep.py', + envManager: 'ms-python.python:venv', + packageManager: 'ms-python.python:pip', + }, + ]; + const mockConfig = new MockWorkspaceConfiguration(); + (mockConfig as any).get = (key: string, defaultValue?: T): T | undefined => { + if (key === 'pythonProjects') { + return [...workspaceValueProjects, ...workspaceFolderProjects] as unknown as T; + } + if (key === 'defaultEnvManager') { + return 'ms-python.python:venv' as T; + } + if (key === 'defaultPackageManager') { + return 'ms-python.python:pip' as T; + } + return defaultValue; + }; + mockConfig.update = () => Promise.resolve(); + sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + + const pm = new PythonProjectManagerImpl(); + pm.initialize(); + await clock.tickAsync(150); + + assert.ok( + pm.getProjects().some( + (project) => + normalizePath(project.uri.fsPath) === normalizePath(path.join(workspacePath, 'script.py')), + ), + 'workspaceValue project should be loaded initially', + ); + assert.ok( + pm.getProjects().some( + (project) => normalizePath(project.uri.fsPath) === normalizePath(path.join(workspacePath, 'keep.py')), + ), + 'workspaceFolder project should be loaded initially', + ); + + workspaceValueProjects = []; + configChangeEmitter.fire({ + affectsConfiguration: (section: string) => section === 'python-envs.pythonProjects', + }); + await clock.tickAsync(150); + + assert.strictEqual( + pm.getProjects().some( + (project) => + normalizePath(project.uri.fsPath) === normalizePath(path.join(workspacePath, 'script.py')), + ), + false, + 'workspaceValue project should be removed after config refresh', + ); + assert.ok( + pm.getProjects().some( + (project) => normalizePath(project.uri.fsPath) === normalizePath(path.join(workspacePath, 'keep.py')), + ), + 'workspaceFolder project should remain after config refresh', + ); + assertNoSettingsWritten('Config refresh after project removal'); + + pm.dispose(); + }); + + test('shared workspaceValue removals do not resurrect projects after a multi-root refresh', async () => { + const secondWorkspacePath = process.platform === 'win32' ? 'C:\\workspace2' : '/workspace2'; + const secondWorkspaceFolder: WorkspaceFolder = { + uri: Uri.file(secondWorkspacePath), + name: 'workspace2', + index: 1, + }; + let sharedWorkspaceProjects: PythonProjectSettings[] = [ + { + path: 'first', + envManager: 'ms-python.python:inline-script', + packageManager: 'ms-python.python:pip', + workspace: workspaceFolder.name, + }, + { + path: 'second', + envManager: 'ms-python.python:inline-script', + packageManager: 'ms-python.python:pip', + workspace: secondWorkspaceFolder.name, + }, + ]; + (workspaceApis.getWorkspaceFolders as sinon.SinonStub).returns([workspaceFolder, secondWorkspaceFolder]); + const mockConfig = new MockWorkspaceConfiguration(); + (mockConfig as any).get = (key: string, defaultValue?: T): T | undefined => { + if (key === 'pythonProjects') { + return sharedWorkspaceProjects as unknown as T; + } + if (key === 'defaultEnvManager') { + return 'ms-python.python:venv' as T; + } + if (key === 'defaultPackageManager') { + return 'ms-python.python:pip' as T; + } + return defaultValue; + }; + mockConfig.update = () => Promise.resolve(); + sinon.stub(workspaceApis, 'getConfiguration').returns(mockConfig); + + const pm = new PythonProjectManagerImpl(); + pm.initialize(); + await clock.tickAsync(150); + + assert.ok( + pm.getProjects().some( + (project) => normalizePath(project.uri.fsPath) === normalizePath(path.join(workspacePath, 'first')), + ), + 'first shared workspace project should be loaded initially', + ); + assert.ok( + pm.getProjects().some( + (project) => + normalizePath(project.uri.fsPath) === normalizePath(path.join(secondWorkspacePath, 'second')), + ), + 'second shared workspace project should be loaded initially', + ); + + sharedWorkspaceProjects = []; + configChangeEmitter.fire({ + affectsConfiguration: (section: string) => section === 'python-envs.pythonProjects', + }); + await clock.tickAsync(150); + + assert.strictEqual( + pm.getProjects().some( + (project) => normalizePath(project.uri.fsPath) === normalizePath(path.join(workspacePath, 'first')), + ), + false, + 'first shared workspace project should stay removed after refresh', + ); + assert.strictEqual( + pm.getProjects().some( + (project) => + normalizePath(project.uri.fsPath) === normalizePath(path.join(secondWorkspacePath, 'second')), + ), + false, + 'second shared workspace project should stay removed after refresh', + ); + assertNoSettingsWritten('Shared workspace refresh'); + + pm.dispose(); + }); }); suite('Workspace Folder Changes - No Settings Writes', () => { diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index ef195addd..09835fab7 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -9,12 +9,14 @@ import * as sender from '../../../common/telemetry/sender'; import * as workspaceApis from '../../../common/workspace.apis'; import { addPythonProjectSetting, + getResolvedPythonProjectSettings, migrateGlobalDefaultEnvManagerSetting, + removePythonProjectSetting, setAllManagerSettings, setEnvironmentManager, setPackageManager, } from '../../../features/settings/settingHelpers'; -import { PythonProjectsImpl } from '../../../internal.api'; +import { PythonProjectSettings, PythonProjectsImpl } from '../../../internal.api'; import { MockWorkspaceConfiguration } from '../../mocks/mockWorkspaceConfig'; /** @@ -617,6 +619,410 @@ suite('Setting Helpers - Empty Path Migration', () => { }); }); +suite('Setting Helpers - Exact Project Removal', () => { + const INLINE_MANAGER_ID = 'ms-python.python:inline-script'; + const VENV_MANAGER_ID = 'ms-python.python:venv'; + const PIP_MANAGER_ID = 'ms-python.python:pip'; + const firstWorkspacePath = getTestWorkspacePath(); + const firstWorkspaceUri = Uri.file(firstWorkspacePath); + const firstWorkspace: WorkspaceFolder = { + uri: firstWorkspaceUri, + name: 'workspace', + index: 0, + }; + const secondWorkspaceUri = Uri.file(process.platform === 'win32' ? 'C:\\workspace2' : '/workspace2'); + const secondWorkspace: WorkspaceFolder = { + uri: secondWorkspaceUri, + name: 'workspace2', + index: 1, + }; + + let updateCalls: Array<{ + workspace: string; + key: string; + value: unknown; + target: boolean | ConfigurationTarget | undefined; + }>; + + setup(() => { + updateCalls = []; + }); + + teardown(() => { + sinon.restore(); + }); + + function createProjectConfig(options: { + workspaceName: string; + workspaceValue?: PythonProjectSettings[]; + workspaceFolderValue?: PythonProjectSettings[]; + }): MockWorkspaceConfiguration { + const mockConfig = new MockWorkspaceConfiguration(); + const mergedProjects = [...(options.workspaceValue ?? []), ...(options.workspaceFolderValue ?? [])]; + (mockConfig as any).get = (key: string, defaultValue?: T): T | undefined => + key === 'pythonProjects' ? (mergedProjects as unknown as T) : defaultValue; + (mockConfig as any).inspect = (key: string) => + key === 'pythonProjects' + ? { + workspaceValue: options.workspaceValue, + workspaceFolderValue: options.workspaceFolderValue, + } + : undefined; + mockConfig.update = ( + section: string, + value: unknown, + configurationTarget?: boolean | ConfigurationTarget, + ): Promise => { + updateCalls.push({ + workspace: options.workspaceName, + key: section, + value, + target: configurationTarget, + }); + return Promise.resolve(); + }; + return mockConfig; + } + + function cloneSettings(settings: PythonProjectSettings[] | undefined): PythonProjectSettings[] { + return (settings ?? []).map((setting) => ({ ...setting })); + } + + function createSharedWorkspaceConfigs(options: { + workspaceValue: PythonProjectSettings[]; + firstWorkspaceFolderValue?: PythonProjectSettings[]; + secondWorkspaceFolderValue?: PythonProjectSettings[]; + }): { firstConfig: MockWorkspaceConfiguration; secondConfig: MockWorkspaceConfiguration; getWorkspaceValue: () => PythonProjectSettings[] } { + let sharedWorkspaceValue = cloneSettings(options.workspaceValue); + const workspaceFolderValues = new Map([ + [firstWorkspace.name, cloneSettings(options.firstWorkspaceFolderValue)], + [secondWorkspace.name, cloneSettings(options.secondWorkspaceFolderValue)], + ]); + + function createConfigForWorkspace(workspace: WorkspaceFolder): MockWorkspaceConfiguration { + const mockConfig = new MockWorkspaceConfiguration(); + (mockConfig as any).get = (key: string, defaultValue?: T): T | undefined => + key === 'pythonProjects' + ? ([...sharedWorkspaceValue, ...workspaceFolderValues.get(workspace.name)!] as unknown as T) + : defaultValue; + (mockConfig as any).inspect = (key: string) => + key === 'pythonProjects' + ? { + workspaceValue: cloneSettings(sharedWorkspaceValue), + workspaceFolderValue: cloneSettings(workspaceFolderValues.get(workspace.name)), + } + : undefined; + mockConfig.update = ( + section: string, + value: unknown, + configurationTarget?: boolean | ConfigurationTarget, + ): Promise => { + updateCalls.push({ + workspace: workspace.name, + key: section, + value, + target: configurationTarget, + }); + const updatedSettings = cloneSettings(value as PythonProjectSettings[] | undefined); + if (configurationTarget === ConfigurationTarget.Workspace) { + sharedWorkspaceValue = updatedSettings; + } else if (configurationTarget === ConfigurationTarget.WorkspaceFolder) { + workspaceFolderValues.set(workspace.name, updatedSettings); + } + return Promise.resolve(); + }; + return mockConfig; + } + + return { + firstConfig: createConfigForWorkspace(firstWorkspace), + secondConfig: createConfigForWorkspace(secondWorkspace), + getWorkspaceValue: () => cloneSettings(sharedWorkspaceValue), + }; + } + + test('removes only the matching inline-script entry and preserves unrelated duplicates', async () => { + const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'other.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); + + const removedProjects = await removePythonProjectSetting([{ project, envManager: INLINE_MANAGER_ID }]); + + assert.deepStrictEqual(removedProjects, [], 'Project should stay because another entry still targets the same path'); + assert.strictEqual(updateCalls.length, 1, 'Should update pythonProjects once'); + assert.strictEqual(updateCalls[0].workspace, firstWorkspace.name); + assert.strictEqual(updateCalls[0].key, 'pythonProjects'); + assert.strictEqual(updateCalls[0].target, ConfigurationTarget.Workspace); + assert.deepStrictEqual(updateCalls[0].value, [ + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'other.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ]); + }); + + test('dedupes duplicate project URIs with workspaceFolder precedence while keeping both sources visible', () => { + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceFolderValue: [ + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + + const resolved = getResolvedPythonProjectSettings(firstWorkspace, config); + + assert.strictEqual(resolved.length, 1); + assert.strictEqual(resolved[0].effective.source, 'workspaceFolder'); + assert.strictEqual(resolved[0].effective.setting.envManager, VENV_MANAGER_ID); + assert.deepStrictEqual( + resolved[0].sources.map((source) => source.setting.envManager), + [INLINE_MANAGER_ID, VENV_MANAGER_ID], + ); + }); + + test('removes only one of two roots that share the same relative path', async () => { + const firstProject = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const secondProject = new PythonProjectsImpl( + 'script.py', + Uri.file(path.join(secondWorkspaceUri.fsPath, 'script.py')), + ); + const firstConfig = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceFolderValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + const secondConfig = createProjectConfig({ + workspaceName: secondWorkspace.name, + workspaceFolderValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); + + const removedProjects = await removePythonProjectSetting([{ project: firstProject, envManager: INLINE_MANAGER_ID }]); + + assert.deepStrictEqual(removedProjects.map((project) => project.uri.fsPath), [firstProject.uri.fsPath]); + assert.strictEqual(updateCalls.length, 1, 'Only the matching workspace folder should be updated'); + assert.strictEqual(updateCalls[0].workspace, firstWorkspace.name); + assert.strictEqual(updateCalls[0].target, ConfigurationTarget.WorkspaceFolder); + assert.strictEqual(updateCalls[0].value, undefined); + assert.strictEqual(secondProject.uri.fsPath, path.join(secondWorkspaceUri.fsPath, 'script.py')); + }); + + test('removes a hidden shared inline entry while preserving a folder override for the same URI', async () => { + const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { + path: 'script.py', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: firstWorkspace.name, + }, + ], + workspaceFolderValue: [ + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); + + const removedProjects = await removePythonProjectSetting([{ project, envManager: INLINE_MANAGER_ID }]); + + assert.deepStrictEqual(removedProjects, [], 'Folder override should keep the project configured'); + const workspaceUpdate = updateCalls.find((call) => call.target === ConfigurationTarget.Workspace); + const folderUpdate = updateCalls.find((call) => call.target === ConfigurationTarget.WorkspaceFolder); + assert.ok(workspaceUpdate, 'WorkspaceValue source should be updated'); + assert.strictEqual(workspaceUpdate!.value, undefined); + assert.strictEqual(folderUpdate, undefined, 'Folder override should not be rewritten'); + }); + + test('aggregates shared workspaceValue removals across folders into one update', async () => { + const firstProject = new PythonProjectsImpl('first', Uri.file(path.join(firstWorkspacePath, 'first'))); + const secondProject = new PythonProjectsImpl('second', Uri.file(path.join(secondWorkspaceUri.fsPath, 'second'))); + const { firstConfig, secondConfig, getWorkspaceValue } = createSharedWorkspaceConfigs({ + workspaceValue: [ + { + path: 'first', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: firstWorkspace.name, + }, + { + path: 'second', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); + + const removedProjects = await removePythonProjectSetting([ + { project: firstProject, envManager: INLINE_MANAGER_ID }, + { project: secondProject, envManager: INLINE_MANAGER_ID }, + ]); + + assert.deepStrictEqual( + removedProjects.map((project) => project.uri.fsPath).sort(), + [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), + ); + assert.strictEqual( + updateCalls.filter((call) => call.target === ConfigurationTarget.Workspace).length, + 1, + 'Shared workspaceValue should be written once', + ); + assert.deepStrictEqual(getWorkspaceValue(), []); + }); + + test('removes a subset from the shared workspace array without resurrecting siblings', async () => { + const firstProject = new PythonProjectsImpl('first', Uri.file(path.join(firstWorkspacePath, 'first'))); + const secondProject = new PythonProjectsImpl('second', Uri.file(path.join(secondWorkspaceUri.fsPath, 'second'))); + const { firstConfig, secondConfig, getWorkspaceValue } = createSharedWorkspaceConfigs({ + workspaceValue: [ + { + path: 'first', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: firstWorkspace.name, + }, + { + path: 'second', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + { + path: 'keep', + envManager: VENV_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); + + const removedProjects = await removePythonProjectSetting([{ project: firstProject, envManager: INLINE_MANAGER_ID }]); + + assert.deepStrictEqual(removedProjects.map((project) => project.uri.fsPath), [firstProject.uri.fsPath]); + assert.strictEqual( + updateCalls.filter((call) => call.target === ConfigurationTarget.Workspace).length, + 1, + 'Shared workspaceValue should still be written once', + ); + assert.deepStrictEqual(getWorkspaceValue(), [ + { + path: 'second', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + { + path: 'keep', + envManager: VENV_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + ]); + assert.strictEqual(secondProject.uri.fsPath, path.join(secondWorkspaceUri.fsPath, 'second')); + }); + + test('removes matching inline-script projects independently in a multi-root workspace', async () => { + const firstProject = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const secondProject = new PythonProjectsImpl( + 'script.py', + Uri.file(path.join(secondWorkspaceUri.fsPath, 'script.py')), + ); + const firstConfig = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceFolderValue: [ + { path: 'keep-folder.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + const secondConfig = createProjectConfig({ + workspaceName: secondWorkspace.name, + workspaceFolderValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); + + const removedProjects = await removePythonProjectSetting([ + { project: firstProject, envManager: INLINE_MANAGER_ID }, + { project: secondProject, envManager: INLINE_MANAGER_ID }, + ]); + + assert.deepStrictEqual( + removedProjects.map((project) => project.uri.fsPath).sort(), + [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), + ); + assert.strictEqual(updateCalls.length, 2, 'Should update each workspace independently'); + const firstWorkspaceUpdate = updateCalls.find((call) => call.workspace === firstWorkspace.name); + const secondWorkspaceUpdate = updateCalls.find((call) => call.workspace === secondWorkspace.name); + assert.ok(firstWorkspaceUpdate, 'First workspace should receive an update'); + assert.ok(secondWorkspaceUpdate, 'Second workspace should receive an update'); + assert.strictEqual(firstWorkspaceUpdate!.value, undefined); + assert.deepStrictEqual(secondWorkspaceUpdate!.value, [ + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ]); + assert.ok( + updateCalls.some((call) => call.workspace === firstWorkspace.name && call.target === ConfigurationTarget.Workspace) && + updateCalls.some( + (call) => + call.workspace === secondWorkspace.name && + call.target === ConfigurationTarget.WorkspaceFolder, + ), + 'Should update the same configuration scope that originally contained each project entry', + ); + }); +}); + suite('Setting Helpers - migrateGlobalDefaultEnvManagerSetting', () => { const SYSTEM_MANAGER_ID = 'ms-python.python:system'; const VENV_MANAGER_ID = 'ms-python.python:venv'; diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 7d5f2c9bf..f9054d104 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -102,7 +102,6 @@ suite('InlineScriptEnvManager', () => { let ensureUvForVersionLookupStub: sinon.SinonStub; let globalStorageUri: Uri; let lockStub: sinon.SinonStub; - let log: LogOutputChannel; let manager: InlineScriptEnvManager; let nativeFinder: NativePythonFinder; let promptInstallPythonViaUvStub: sinon.SinonStub; @@ -191,8 +190,7 @@ suite('InlineScriptEnvManager', () => { }); clock = sinon.useFakeTimers({ now: NOW, toFake: ['Date'] }); - log = makeFakeLog(); - manager = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); + manager = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, makeFakeLog()); }); teardown(async () => { @@ -209,10 +207,6 @@ suite('InlineScriptEnvManager', () => { return cacheLayout.getScriptEnvDir(globalStorageUri, CACHE_KEY); } - function cacheRoot(): Uri { - return cacheLayout.getScriptEnvCacheRoot(globalStorageUri); - } - function setSidecar(metadata: cacheLayout.InlineScriptEnvMeta): void { inspectMetaStub.resolves({ kind: 'valid', metadata }); } @@ -248,7 +242,6 @@ suite('InlineScriptEnvManager', () => { test('exposes creation but leaves later-phase methods empty', async () => { const asInterface: EnvironmentManager = manager; assert.strictEqual(typeof asInterface.create, 'function'); - assert.strictEqual(asInterface.clearCache, undefined); assert.strictEqual(asInterface.remove, undefined); assert.strictEqual(asInterface.quickCreateConfig, undefined); assert.deepStrictEqual(await manager.getEnvironments('all'), []); @@ -985,55 +978,16 @@ suite('InlineScriptEnvManager', () => { false, 'inline-script cache entries must not be tracked as workspace uv environments', ); - assert.strictEqual(lockStub.callCount, 2); - assert.strictEqual(lockStub.firstCall.args[0], cacheRoot().fsPath); - assert.strictEqual(lockStub.secondCall.args[0], envDir().fsPath); - assert.strictEqual(releaseLockStub.callCount, 2); - }); - - test('acquires the cache root lock before the final cache-entry lock and releases root before build', async () => { - const rootRelease = sinon.stub().resolves(); - const entryRelease = sinon.stub().resolves(); - lockStub.callsFake(async (lockPath: string) => { - if (lockPath === cacheRoot().fsPath) { - return { - retain: sinon.stub().resolves(), - release: rootRelease, - }; - } - return { - retain: sinon.stub().resolves(), - release: entryRelease, - }; - }); - createWithProgressStub.callsFake(async (...args: unknown[]) => { - assert.ok(rootRelease.calledOnce, 'root lock should be released before build starts'); - assert.strictEqual(entryRelease.called, false, 'entry lock should remain held during build'); - const envDir = args[6] as string; - const selectedBase = args[4] as PythonEnvironment; - await fs.outputFile(getVenvPythonPath(envDir), ''); - return { - environment: makeEnvironment( - 'ms-python.python:inline-script', - selectedBase.version, - getVenvPythonPath(envDir), - envDir, - ), - }; - }); + assert.ok(releaseLockStub.calledOnce); + }); + test('uses a bounded cross-process lock at the final cache path', async () => { await manager.create(scriptUri()); - assert.strictEqual(lockStub.firstCall.args[0], cacheRoot().fsPath); - assert.strictEqual(lockStub.secondCall.args[0], envDir().fsPath); - const rootOptions = lockStub.firstCall.args[1]; - const entryOptions = lockStub.secondCall.args[1]; - assert.strictEqual(rootOptions.timeoutMs, 1_000); - assert.strictEqual(rootOptions.retryIntervalMs, 50); - assert.strictEqual(entryOptions.timeoutMs, 1_000); - assert.strictEqual(entryOptions.retryIntervalMs, 50); - assert.ok(rootRelease.calledOnce); - assert.ok(entryRelease.calledOnce); + assert.strictEqual(lockStub.firstCall.args[0], envDir().fsPath); + const options = lockStub.firstCall.args[1]; + assert.ok(options.timeoutMs > 0); + assert.ok(options.retryIntervalMs > 0); }); test('coalesces simultaneous same-key creation within one extension host', async () => { @@ -1078,110 +1032,14 @@ suite('InlineScriptEnvManager', () => { const [firstResult, secondResult] = await Promise.all([first, second]); assert.strictEqual(firstResult, secondResult); - assert.strictEqual(lockStub.callCount, 2); + assert.strictEqual(lockStub.callCount, 1); assert.strictEqual(createWithProgressStub.callCount, 1); }); - test('returns undefined without building when the cache root lock cannot be acquired', async () => { - const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; + test('returns undefined without building when the cache lock cannot be acquired', async () => { lockStub.rejects(Object.assign(new Error('already locked'), { code: 'ELOCKED' })); assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(createWithProgressStub.callCount, 0); - sinon.assert.calledWithMatch( - log.warn as sinon.SinonStub, - sinon.match(new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually`, 'i')), - ); - }); - - test('aborts before inspect/build when releasing the cache root lock for handoff fails', async () => { - const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; - const rootRelease = sinon.stub().callsFake(async () => { - await fs.ensureDir(rootLockPath); - throw new Error('root release failed'); - }); - const entryRelease = sinon.stub().resolves(); - lockStub.callsFake(async (lockPath: string) => { - if (lockPath === cacheRoot().fsPath) { - return { - retain: sinon.stub().resolves(), - release: rootRelease, - }; - } - return { - retain: sinon.stub().resolves(), - release: entryRelease, - }; - }); - - assert.strictEqual(await manager.create(scriptUri()), undefined); - assert.strictEqual(createWithProgressStub.callCount, 0); - assert.ok(rootRelease.calledOnce); - assert.ok(entryRelease.calledOnce); - assert.strictEqual(await fs.pathExists(rootLockPath), true); - sinon.assert.calledWithMatch( - log.warn as sinon.SinonStub, - sinon.match(new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually`, 'i')), - ); - }); - - test('allows different cache entries to build concurrently after the root-to-entry handoff', async () => { - const secondCacheKey = 'fedcba9876543210'; - const secondEnvDir = cacheLayout.getScriptEnvDir(globalStorageUri, secondCacheKey); - computeCacheKeyStub.onFirstCall().returns(CACHE_KEY); - computeCacheKeyStub.onSecondCall().returns(secondCacheKey); - - let releaseFirstBuild: (() => void) | undefined; - const firstBuildGate = new Promise((resolve) => { - releaseFirstBuild = resolve; - }); - const secondBuildStarted = sinon.stub(); - createWithProgressStub.callsFake(async (...args: unknown[]) => { - const target = args[6] as string; - await fs.outputFile(venvPythonPath(target), ''); - if (target === envDir().fsPath) { - await firstBuildGate; - } else if (target === secondEnvDir.fsPath) { - secondBuildStarted(); - } - return { - environment: makeEnvironment( - 'ms-python.python:inline-script', - '3.12.4', - venvPythonPath(target), - target, - ), - }; - }); - - const first = manager.create(scriptUri('first.py')); - let second: Promise | undefined; - try { - await waitForStubCall(createWithProgressStub); - second = manager.create(scriptUri('second.py')); - await waitForStubCall(secondBuildStarted); - assert.ok(secondBuildStarted.calledOnce); - assert.strictEqual(createWithProgressStub.callCount, 2); - } finally { - releaseFirstBuild?.(); - await Promise.allSettled([first, second ?? Promise.resolve(undefined)]); - } - }); - - test('releases the cache root lock when the per-entry lock cannot be acquired', async () => { - const rootRelease = sinon.stub().resolves(); - lockStub.callsFake(async (lockPath: string) => { - if (lockPath === cacheRoot().fsPath) { - return { - retain: sinon.stub().resolves(), - release: rootRelease, - }; - } - throw Object.assign(new Error('entry locked'), { code: 'ELOCKED' }); - }); - - assert.strictEqual(await manager.create(scriptUri()), undefined); - assert.strictEqual(createWithProgressStub.callCount, 0); - assert.ok(rootRelease.calledOnce); }); }); @@ -1514,7 +1372,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await fs.pathExists(envDir().fsPath), true); assert.strictEqual(writeMetaStub.callCount, 0); assert.ok(retainLockStub.calledOnce); - assert.strictEqual(releaseLockStub.callCount, 2); + assert.ok(releaseLockStub.calledOnce); }); test('keeps a failed lock-retain transition fail-closed', async () => { @@ -1532,7 +1390,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.ok(retainLockStub.calledOnce); - assert.strictEqual(releaseLockStub.callCount, 2); + assert.ok(releaseLockStub.calledOnce); }); test('removes the partial environment when package installation fails', async () => { @@ -1553,7 +1411,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(await fs.pathExists(envDir().fsPath), false); assert.strictEqual(writeMetaStub.callCount, 0); - assert.strictEqual(releaseLockStub.callCount, 2); + assert.ok(releaseLockStub.calledOnce); }); test('removes the new environment when sidecar writing fails', async () => { @@ -1561,7 +1419,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(await fs.pathExists(envDir().fsPath), false); - assert.strictEqual(releaseLockStub.callCount, 2); + assert.ok(releaseLockStub.calledOnce); }); test('removes a partial environment when createWithProgress throws', async () => { @@ -1572,7 +1430,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.create(scriptUri()), undefined); assert.strictEqual(await fs.pathExists(envDir().fsPath), false); - assert.strictEqual(releaseLockStub.callCount, 2); + assert.ok(releaseLockStub.calledOnce); }); test('rejects and removes a created environment with a different Python release', async () => { @@ -1617,335 +1475,6 @@ suite('InlineScriptEnvManager', () => { }); }); - suite('clear cache', () => { - test('treats a missing cache root as idempotent and clears persisted associations', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - const listener = sinon.spy(); - manager.onDidChangeEnvironment(listener); - - await manager.set(uri, environment); - listener.resetHistory(); - await fs.remove(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath); - - await manager.clearScriptCache(); - await manager.clearScriptCache(); - - assert.strictEqual(workspaceState.clear.callCount, 2); - assert.deepStrictEqual(workspaceState.clear.firstCall.args[0], [INLINE_SCRIPT_ENVS_KEY]); - assert.strictEqual(persistedAssociations, undefined); - assert.strictEqual(await manager.get(uri), undefined); - sinon.assert.calledOnceWithMatch(listener, { old: environment, new: undefined }); - }); - - test('removes the cache root, clears state, and notifies known associations', async () => { - const firstUri = scriptUri('first.py'); - const secondUri = scriptUri('second.py'); - const firstEnvironment = await createOwnedEnvironment(); - const secondEnvironment = await createOwnedEnvironment('fedcba9876543210'); - const listener = sinon.spy(); - manager.onDidChangeEnvironment(listener); - - await manager.set([firstUri, secondUri], firstEnvironment); - await manager.set(secondUri, secondEnvironment); - listener.resetHistory(); - - await manager.clearScriptCache(); - - assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), false); - assert.strictEqual(persistedAssociations, undefined); - assert.strictEqual(await manager.get(firstUri), undefined); - assert.strictEqual(await manager.get(secondUri), undefined); - assert.strictEqual(listener.callCount, 2); - assert.strictEqual(listener.firstCall.args[0].old, firstEnvironment); - assert.strictEqual(listener.firstCall.args[0].new, undefined); - assert.strictEqual(listener.secondCall.args[0].old, secondEnvironment); - assert.strictEqual(listener.secondCall.args[0].new, undefined); - }); - - test('refuses to clear while a create is active', async () => { - let releaseMetadata: ((value: metadataReader.InlineScriptMetadata | undefined) => void) | undefined; - readMetadataStub.callsFake( - () => - new Promise((resolve) => { - releaseMetadata = resolve; - }), - ); - - const createPromise = manager.create(scriptUri()); - - await assert.rejects( - manager.clearScriptCache(), - /Close other VS Code windows or restart VS Code, then retry/i, - ); - - releaseMetadata!(VALID_METADATA); - assert.ok(await createPromise); - }); - - test('refuses create requests while a clear is in progress', async () => { - let clearStarted: (() => void) | undefined; - let releaseClear: (() => void) | undefined; - const started = new Promise((resolve) => { - clearStarted = resolve; - }); - const gate = new Promise((resolve) => { - releaseClear = resolve; - }); - const clearManager = manager as unknown as { - getClearableCacheRootPath(cacheRoot: Uri): Promise; - }; - sinon.stub(clearManager, 'getClearableCacheRootPath').callsFake(async () => { - clearStarted!(); - await gate; - return undefined; - }); - - const clearPromise = manager.clearScriptCache(); - await started; - - await assert.rejects(manager.create(scriptUri()), /cache is being cleared/i); - - releaseClear!(); - await clearPromise; - }); - - test('refuses to clear when the cache root lock is already held', async () => { - const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; - lockStub.callsFake(async (lockPath: string) => { - if (lockPath === cacheRoot().fsPath) { - throw Object.assign(new Error('already locked'), { code: 'ELOCKED' }); - } - return { release: releaseLockStub, retain: retainLockStub }; - }); - - await assert.rejects( - manager.clearScriptCache(), - new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*remove only this lock path manually`, 'i'), - ); - assert.strictEqual(workspaceState.clear.callCount, 0); - }); - - test('rejects when cache deletion and state clear succeed but root lock release fails', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - const rootLockPath = `${path.resolve(cacheRoot().fsPath)}.lock`; - const rootRelease = sinon.stub().callsFake(async () => { - await fs.ensureDir(rootLockPath); - throw new Error('root release failed'); - }); - await manager.set(uri, environment); - lockStub.callsFake(async () => ({ - retain: sinon.stub().resolves(), - release: rootRelease, - })); - - await assert.rejects( - manager.clearScriptCache(), - new RegExp(`${rootLockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually`, 'i'), - ); - - assert.strictEqual(await fs.pathExists(cacheRoot().fsPath), false); - assert.strictEqual(persistedAssociations, undefined); - assert.ok(rootRelease.calledOnce); - }); - - test('refuses clear after the root-to-entry handoff because the entry lock is visible on disk', async () => { - const otherManager = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); - const entryLockPath = `${path.resolve(envDir().fsPath)}.lock`; - let releaseBuild: (() => void) | undefined; - const buildGate = new Promise((resolve) => { - releaseBuild = resolve; - }); - createWithProgressStub.callsFake(async (...args: unknown[]) => { - const target = args[6] as string; - const selectedBase = args[4] as PythonEnvironment; - await fs.outputFile(getVenvPythonPath(target), ''); - await buildGate; - return { - environment: makeEnvironment( - 'ms-python.python:inline-script', - selectedBase.version, - getVenvPythonPath(target), - target, - ), - }; - }); - lockStub.callsFake(async (lockPath: string) => { - if (lockPath === envDir().fsPath) { - await fs.ensureDir(entryLockPath); - await fs.outputFile(path.join(entryLockPath, 'owner-1234'), ''); - return { - retain: sinon.stub().resolves(), - release: sinon.stub().callsFake(async () => { - await fs.remove(entryLockPath); - }), - }; - } - return { - retain: sinon.stub().resolves(), - release: sinon.stub().resolves(), - }; - }); - - const createPromise = manager.create(scriptUri()); - try { - await waitForStubCall(createWithProgressStub); - await assert.rejects(otherManager.clearScriptCache(), /owner-only lock/i); - } finally { - releaseBuild!(); - await createPromise; - otherManager.dispose(); - } - }); - - test('allows retained lock directories to be removed with the cache root', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - await manager.set(uri, environment); - const lockPath = path.join(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath, `${CACHE_KEY}.lock`); - await fs.ensureDir(lockPath); - await fs.outputFile(path.join(lockPath, 'owner-1234'), ''); - await fs.outputFile(path.join(lockPath, 'retained'), ''); - - await manager.clearScriptCache(); - - assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), false); - assert.strictEqual(persistedAssociations, undefined); - }); - - test('rejects active owner lock directories', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - await manager.set(uri, environment); - const lockPath = path.join(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath, `${CACHE_KEY}.lock`); - await fs.ensureDir(lockPath); - await fs.outputFile(path.join(lockPath, 'owner-1234'), ''); - - await assert.rejects( - manager.clearScriptCache(), - new RegExp(`${lockPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*manually remove`, 'i'), - ); - - assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), true); - assert.deepStrictEqual(persistedAssociations, { - [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, - }); - }); - - test('rejects orphaned or malformed lock entries', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - const cacheRootPath = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; - const lockPath = path.join(cacheRootPath, `${CACHE_KEY}.lock`); - await manager.set(uri, environment); - - await fs.ensureDir(lockPath); - await assert.rejects(manager.clearScriptCache(), /could not be verified as retained/i); - await fs.remove(lockPath); - - await fs.outputFile(lockPath, 'not a directory'); - await assert.rejects(manager.clearScriptCache(), /could not be verified as retained/i); - - assert.strictEqual(await fs.pathExists(cacheRootPath), true); - assert.deepStrictEqual(persistedAssociations, { - [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, - }); - }); - - test('fails closed when the cache root is redirected through a symlink or junction', async function () { - const cacheRoot = cacheLayout.getScriptEnvCacheRoot(globalStorageUri); - const externalRoot = path.join(tempRoot, 'external-cache-root'); - const markerPath = path.join(externalRoot, 'keep.txt'); - await fs.ensureDir(globalStorageUri.fsPath); - await fs.remove(cacheRoot.fsPath); - await fs.outputFile(markerPath, 'keep'); - try { - await fs.symlink(externalRoot, cacheRoot.fsPath, isWindows() ? 'junction' : 'dir'); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === 'EPERM' || code === 'EACCES') { - this.skip(); - return; - } - throw error; - } - - await assert.rejects(manager.clearScriptCache(), /could not be proven safe/i); - - assert.strictEqual(await fs.readFile(markerPath, 'utf8'), 'keep'); - assert.strictEqual((await fs.lstat(cacheRoot.fsPath)).isSymbolicLink(), true); - }); - - test('surfaces state clear failures after removing the cache root and clearing in-memory state', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - const listener = sinon.spy(); - manager.onDidChangeEnvironment(listener); - - await manager.set(uri, environment); - listener.resetHistory(); - workspaceState.clear.rejects(new Error('Memento unavailable')); - - await assert.rejects(manager.clearScriptCache(), /Memento unavailable/); - - const clearState = manager as unknown as { - fsPathToEnv: Map; - fsPathToPersistedEnvPath: Map; - }; - assert.strictEqual(await fs.pathExists(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath), false); - assert.deepStrictEqual(persistedAssociations, { - [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, - }); - assert.strictEqual(clearState.fsPathToEnv.size, 0); - assert.strictEqual(clearState.fsPathToPersistedEnvPath.size, 0); - sinon.assert.calledOnceWithMatch(listener, { old: environment, new: undefined }); - }); - - test('surfaces disk deletion failures without clearing state', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - await manager.set(uri, environment); - const cacheRootPath = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; - const clearManager = manager as unknown as { - removeClearableCacheRoot(cacheRootPath: string): Promise; - }; - sinon.stub(clearManager, 'removeClearableCacheRoot').rejects(new Error('disk busy')); - - await assert.rejects(manager.clearScriptCache(), /disk busy/); - - assert.strictEqual(await fs.pathExists(cacheRootPath), true); - assert.deepStrictEqual(persistedAssociations, { - [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, - }); - assert.strictEqual(await manager.get(uri), environment); - }); - - test('does not let a pending rehydration repopulate after clear', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - persistedAssociations = { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath }; - - let resolvePending: ((value: PythonEnvironment | undefined) => void) | undefined; - resolveVenvStub.callsFake( - () => - new Promise((resolve) => { - resolvePending = resolve; - }), - ); - - const pendingGet = manager.get(uri); - await waitForStubCall(resolveVenvStub); - - await manager.clearScriptCache(); - resolvePending!(environment); - - assert.strictEqual(await pendingGet, undefined); - assert.strictEqual(await manager.get(uri), undefined); - assert.strictEqual(persistedAssociations, undefined); - }); - }); - suite('events and disposal', () => { test('create does not establish an association or fire later-phase events', async () => { const environmentsListener = sinon.spy(); @@ -2994,4 +2523,313 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.get(Uri.parse('untitled:script.py')), undefined); }); }); + + suite('clear cache', () => { + test('clears cached environments, persisted associations, and in-memory selections', async () => { + const first = scriptUri('first.py'); + const second = scriptUri('second.py'); + const environment = await createOwnedEnvironment(); + await manager.set([first, second], environment); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + await manager.clearCache(); + + assert.strictEqual(await fs.pathExists(envDir().fsPath), false); + assert.strictEqual(persistedAssociations, undefined); + assert.strictEqual(await manager.get(first), undefined); + assert.strictEqual(await manager.get(second), undefined); + assert.deepStrictEqual( + listener.getCalls().map((call) => normalizePath(call.args[0].uri.fsPath)).sort(), + [first.fsPath, second.fsPath].map((value) => normalizePath(value)).sort(), + ); + assert.deepStrictEqual( + listener.getCalls().map((call) => call.args[0].old), + [environment, environment], + ); + assert.ok(listener.getCalls().every((call) => call.args[0].new === undefined)); + }); + + test('clears associations even when the cache directory is already missing', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + await fs.remove(cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath); + + await manager.clearCache(); + + assert.strictEqual(persistedAssociations, undefined); + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(listener.callCount, 1); + assert.strictEqual(normalizePath(listener.firstCall.args[0].uri.fsPath), normalizePath(uri.fsPath)); + assert.strictEqual(listener.firstCall.args[0].old, environment); + assert.strictEqual(listener.firstCall.args[0].new, undefined); + }); + + test('is idempotent when the cache and associations are already absent', async () => { + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + await manager.clearCache(); + await manager.clearCache(); + + assert.strictEqual(persistedAssociations, undefined); + assert.strictEqual(listener.callCount, 0); + }); + + test('refuses to clear from an unsafe cache root', async function () { + if (isWindows() && !process.env.SystemDrive) { + this.skip(); + } + const unsafeManager = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + Uri.file(process.platform === 'win32' ? `${process.env.SystemDrive ?? 'C:'}\\` : '/'), + makeFakeLog(), + ); + + await assert.rejects( + unsafeManager.clearCache(), + /unsafe cache root/, + ); + + unsafeManager.dispose(); + }); + + test('refuses to clear a symlinked cache root', async function () { + const symlinkStorageUri = Uri.file(path.join(tempRoot, 'symlink-storage')); + const symlinkManager = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + symlinkStorageUri, + makeFakeLog(), + ); + const realCacheRoot = cacheLayout.getScriptEnvCacheRoot(symlinkStorageUri).fsPath; + const externalCacheRoot = path.join(tempRoot, 'external-cache-root'); + await fs.ensureDir(symlinkStorageUri.fsPath); + await fs.ensureDir(externalCacheRoot); + try { + await fs.symlink(externalCacheRoot, realCacheRoot, process.platform === 'win32' ? 'junction' : 'dir'); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + this.skip(); + } + throw error; + } + + await assert.rejects( + symlinkManager.clearCache(), + /not a normal directory/, + ); + + symlinkManager.dispose(); + }); + + test('refuses to clear when globalStorage is redirected through a symlink or junction', async function () { + const physicalStoragePath = path.join(tempRoot, 'physical-storage'); + const redirectedStoragePath = path.join(tempRoot, 'redirected-storage'); + await fs.ensureDir(physicalStoragePath); + await fs.ensureDir(redirectedStoragePath); + const redirectedManager = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + Uri.file(redirectedStoragePath), + makeFakeLog(), + ); + try { + await fs.remove(redirectedStoragePath); + await fs.symlink( + physicalStoragePath, + redirectedStoragePath, + process.platform === 'win32' ? 'junction' : 'dir', + ); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + this.skip(); + } + throw error; + } + + await assert.rejects(redirectedManager.clearCache(), /global storage root is not a normal directory/); + + redirectedManager.dispose(); + }); + + test('fails closed when physical cache verification reports a redirected root', async () => { + const internalManager = manager as unknown as { + getPhysicalOwnedCacheRootPath(cacheRoot: Uri): Promise; + }; + const original = internalManager.getPhysicalOwnedCacheRootPath.bind(manager); + internalManager.getPhysicalOwnedCacheRootPath = async () => { + throw new Error('Refusing to clear the script environment cache because the cache root is redirected.'); + }; + try { + await assert.rejects(manager.clearCache(), /cache root is redirected/); + } finally { + internalManager.getPhysicalOwnedCacheRootPath = original; + } + }); + + test('refuses to clear while a cached environment is locked', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const lockPath = lockfileApis.getFileLockPath(environment.sysPrefix); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `owner-${process.pid}-test`), ''); + + await assert.rejects(manager.clearCache(), /being created/); + + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, + }); + assert.strictEqual(await manager.get(uri), environment); + }); + + test('clears a retained lock and its corresponding cache entry', async () => { + const retainedCacheDir = envDir().fsPath; + const retainedLockPath = lockfileApis.getFileLockPath(retainedCacheDir); + await fs.outputFile(venvPythonPath(retainedCacheDir), ''); + await fs.ensureDir(retainedLockPath); + await fs.writeFile(path.join(retainedLockPath, 'retained'), ''); + + await manager.clearCache(); + + assert.strictEqual(await fs.pathExists(retainedCacheDir), false); + assert.strictEqual(await fs.pathExists(retainedLockPath), false); + }); + + test('clears a stale owner lock and its corresponding cache entry', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const staleLockPath = lockfileApis.getFileLockPath(environment.sysPrefix); + await fs.ensureDir(staleLockPath); + await fs.writeFile(path.join(staleLockPath, 'owner-424242-dead'), ''); + const originalInspectFileLock = lockfileApis.inspectFileLock; + sinon.stub(lockfileApis, 'inspectFileLock').callsFake(async (filePath, options) => { + if (normalizePath(filePath) === normalizePath(environment.sysPrefix)) { + return 'stale'; + } + return originalInspectFileLock(filePath, options); + }); + + await manager.clearCache(); + + assert.strictEqual(await fs.pathExists(environment.sysPrefix), false); + assert.strictEqual(await fs.pathExists(staleLockPath), false); + assert.strictEqual(await manager.get(uri), undefined); + }); + + test('rejects an orphaned lock directory conservatively', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await fs.ensureDir(lockfileApis.getFileLockPath(environment.sysPrefix)); + + await assert.rejects(manager.clearCache(), /incomplete or malformed/); + + assert.strictEqual(await fs.pathExists(environment.sysPrefix), true); + assert.strictEqual(await manager.get(uri), environment); + }); + + test('surfaces a persistence failure after clearing disk and memory state', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + workspaceState.clear.onFirstCall().rejects(new Error('Memento unavailable')); + + await assert.rejects(manager.clearCache(), /Memento unavailable/); + + assert.strictEqual(await fs.pathExists(envDir().fsPath), false); + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(listener.callCount, 1); + assert.strictEqual(normalizePath(listener.firstCall.args[0].uri.fsPath), normalizePath(uri.fsPath)); + assert.strictEqual(listener.firstCall.args[0].old, environment); + assert.strictEqual(listener.firstCall.args[0].new, undefined); + }); + + test('does not let a pending rehydration restore an association after clear cache', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + persistedAssociations = { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath }; + let resolveRehydration: ((value: PythonEnvironment | undefined) => void) | undefined; + resolveVenvStub.callsFake( + () => + new Promise((resolve) => { + resolveRehydration = resolve; + }), + ); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + const pendingGet = manager.get(uri); + await waitForStubCall(resolveVenvStub); + await manager.clearCache(); + resolveRehydration!(environment); + + assert.strictEqual(await pendingGet, undefined); + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(listener.callCount, 0); + }); + + test('rejects clear when creation started before the clear request', async () => { + const uri = scriptUri(); + let resolveMetadata: ((value: metadataReader.InlineScriptMetadata | undefined) => void) | undefined; + readMetadataStub.callsFake( + () => + new Promise((resolve) => { + resolveMetadata = resolve; + }), + ); + + const createPromise = manager.create(uri); + + await assert.rejects(manager.clearCache(), /being created/); + resolveMetadata!(VALID_METADATA); + assert.ok(await createPromise); + assert.strictEqual(await fs.pathExists(envDir().fsPath), true); + }); + + test('queues create behind a clear request that started first', async () => { + const uri = scriptUri(); + let releaseClear: (() => void) | undefined; + let signalClearStarted: (() => void) | undefined; + const clearStarted = new Promise((resolve) => { + signalClearStarted = resolve; + }); + workspaceState.clear.callsFake( + async (keys?: string[]) => + new Promise((resolve) => { + signalClearStarted!(); + releaseClear = () => { + if (!keys || keys.includes(INLINE_SCRIPT_ENVS_KEY)) { + persistedAssociations = undefined; + } + resolve(); + }; + }), + ); + + const clearPromise = manager.clearCache(); + const createPromise = manager.create(uri); + + await clearStarted; + assert.strictEqual(readMetadataStub.callCount, 0); + releaseClear!(); + await clearPromise; + + assert.ok(await createPromise); + assert.ok(readMetadataStub.calledOnce); + }); + }); }); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index 1fec3cd12..d109e318d 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -51,37 +51,23 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { isEnabledStub.returns(false); const disposables: Disposable[] = []; - const result = await registerInlineScriptFeatures( - nativeFinder, - disposables, - makeFakeLog(), - baseManager, - globalStorageUri, - ); + await registerInlineScriptFeatures(nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri); assert.strictEqual(disposables.length, 0, 'no disposables should be added when flag is off'); assert.strictEqual(getPythonApiStub.called, false, 'should not even call getPythonApi when gated off'); assert.strictEqual(registerEnvironmentManagerStub.called, false); - assert.strictEqual(result, undefined); }); test('when the feature flag is TRUE: registers the manager and pushes the disposable', async () => { isEnabledStub.returns(true); const disposables: Disposable[] = []; - const result = await registerInlineScriptFeatures( - nativeFinder, - disposables, - makeFakeLog(), - baseManager, - globalStorageUri, - ); + await registerInlineScriptFeatures(nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri); assert.strictEqual(getPythonApiStub.callCount, 1); assert.strictEqual(registerEnvironmentManagerStub.callCount, 1); assert.strictEqual(disposables.length, 2, 'expected manager + registration disposable'); const manager = registerEnvironmentManagerStub.firstCall.args[0]; - assert.strictEqual(result, manager); assert.ok(disposables.includes(manager), 'manager itself should be disposed'); assert.ok( disposables.includes(registerEnvironmentManagerStub.firstCall.returnValue), diff --git a/src/test/smoke/registration.smoke.test.ts b/src/test/smoke/registration.smoke.test.ts index 176aba815..727bf2bc0 100644 --- a/src/test/smoke/registration.smoke.test.ts +++ b/src/test/smoke/registration.smoke.test.ts @@ -26,10 +26,9 @@ suite('Smoke: Registration Checks', function () { this.timeout(MAX_EXTENSION_ACTIVATION_TIME); let api: PythonEnvironmentApi; - let extension: vscode.Extension; suiteSetup(async function () { - extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID)!; + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); assert.ok(extension, `Extension ${ENVS_EXTENSION_ID} not found`); if (!extension.isActive) { @@ -66,7 +65,7 @@ suite('Smoke: Registration Checks', function () { 'python-envs.setPkgManager', 'python-envs.refreshAllManagers', 'python-envs.clearCache', - 'python-envs.clearInlineScriptCache', + 'python-envs.clearScriptEnvCache', 'python-envs.searchSettings', // Package management @@ -115,41 +114,6 @@ suite('Smoke: Registration Checks', function () { ); }); - test('Clear cache commands are contributed from package.json', function () { - const clearCacheCommand = extension.packageJSON?.contributes?.commands?.find( - (item: { command: string }) => item.command === 'python-envs.clearCache', - ); - const clearInlineScriptCacheCommand = extension.packageJSON?.contributes?.commands?.find( - (item: { command: string }) => item.command === 'python-envs.clearInlineScriptCache', - ); - const clearInlineScriptCachePaletteEntry = extension.packageJSON?.contributes?.menus?.commandPalette?.find( - (item: { command: string }) => item.command === 'python-envs.clearInlineScriptCache', - ); - - assert.ok(clearCacheCommand, 'python-envs.clearCache should be contributed in package.json'); - assert.strictEqual(clearCacheCommand.category, 'Python'); - assert.strictEqual(clearCacheCommand.title, 'Clear Cache'); - - assert.ok( - clearInlineScriptCacheCommand, - 'python-envs.clearInlineScriptCache should be contributed in package.json', - ); - assert.strictEqual(clearInlineScriptCacheCommand.category, 'Python'); - assert.strictEqual(clearInlineScriptCacheCommand.title, 'Clear Script Environment Cache'); - assert.strictEqual( - clearInlineScriptCacheCommand.enablement, - 'config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true', - ); - assert.ok( - clearInlineScriptCachePaletteEntry, - 'python-envs.clearInlineScriptCache should have a command palette contribution', - ); - assert.strictEqual( - clearInlineScriptCachePaletteEntry.when, - 'config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true', - ); - }); - // ========================================================================= // API METHODS - All API methods must exist and be functions // ========================================================================= From 1471f87deca746046c1407bc29f716ed24a6104b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 14:20:53 -0700 Subject: [PATCH 3/6] Hide inline script cleanup while preview is disabled Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3cb82ae9-7424-40a4-9156-8c54ac6e0895 --- package.json | 10 - package.nls.json | 1 - src/extension.ts | 17 +- src/features/envCommands.ts | 40 +- src/features/settings/settingHelpers.ts | 213 ++++--- src/test/features/envCommands.unit.test.ts | 214 ++----- .../settings/settingHelpers.unit.test.ts | 545 ++++++++++-------- src/test/smoke/registration.smoke.test.ts | 33 +- 8 files changed, 510 insertions(+), 563 deletions(-) diff --git a/package.json b/package.json index 0a9a6abaf..dd7cba3cf 100644 --- a/package.json +++ b/package.json @@ -245,12 +245,6 @@ "category": "Python", "icon": "$(trash)" }, - { - "command": "python-envs.clearScriptEnvCache", - "title": "%python-envs.clearScriptEnvCache.title%", - "category": "Python", - "icon": "$(trash)" - }, { "command": "python-envs.runInTerminal", "title": "%python-envs.runInTerminal.title%", @@ -471,10 +465,6 @@ { "command": "python-envs.reportIssue", "when": "config.python.useEnvironmentsExtension != false" - }, - { - "command": "python-envs.clearScriptEnvCache", - "when": "config.python.useEnvironmentsExtension != false && config.python-envs.inlineScripts.enabled == true" } ], "view/item/context": [ diff --git a/package.nls.json b/package.nls.json index c128863de..483ecfd29 100644 --- a/package.nls.json +++ b/package.nls.json @@ -35,7 +35,6 @@ "python-envs.refreshPackages.title": "Refresh Packages List", "python-envs.packages.title": "Manage Packages", "python-envs.clearCache.title": "Clear Cache", - "python-envs.clearScriptEnvCache.title": "Clear Script Environment Cache", "python-envs.runInTerminal.title": "Run in Terminal", "python-envs.createTerminal.title": "Create Python Terminal", "python-envs.runAsTask.title": "Run as Task", diff --git a/src/extension.ts b/src/extension.ts index e3735b4ad..f45d46aa2 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -95,7 +95,12 @@ import { PythonStatusBarImpl } from './features/views/pythonStatusBar'; import { updateViewsAndStatus } from './features/views/revealHandler'; import { TemporaryStateManager } from './features/views/temporaryStateManager'; import { PythonEnvTreeItem } from './features/views/treeViewItems'; -import { collectEnvironmentInfo, getEnvManagerAndPackageManagerConfigLevels, runPetInTerminalImpl } from './helpers'; +import { + collectEnvironmentInfo, + getEnvManagerAndPackageManagerConfigLevels, + isInlineScriptsFeatureEnabled, + runPetInTerminalImpl, +} from './helpers'; import { EnvironmentManagers, ProjectCreators, PythonProjectManager } from './internal.api'; import { registerInlineScriptFeatures } from './managers/builtin/inlineScript/main'; import { registerSystemPythonFeatures } from './managers/builtin/main'; @@ -387,9 +392,13 @@ export async function activate(context: ExtensionContext): Promise { - await clearScriptEnvironmentCacheCommand(envManagers, projectManager); - }), + ...(isInlineScriptsFeatureEnabled() + ? [ + commands.registerCommand('python-envs.clearScriptEnvCache', async () => { + await clearScriptEnvironmentCacheCommand(envManagers, projectManager); + }), + ] + : []), commands.registerCommand('python-envs.runInTerminal', (item) => { return runInTerminalCommand(item, api, terminalManager); }), diff --git a/src/features/envCommands.ts b/src/features/envCommands.ts index 0136539f1..fafb6fbcd 100644 --- a/src/features/envCommands.ts +++ b/src/features/envCommands.ts @@ -27,7 +27,7 @@ import { PythonProjectManager, } from '../internal.api'; import { - getResolvedPythonProjectSettings, + removeInlineScriptPythonProjectSettings, removePythonProjectSetting, setEnvironmentManager, setPackageManager, @@ -58,7 +58,6 @@ import { showWarningMessage, withProgress, } from '../common/window.apis'; -import { getWorkspaceFolders } from '../common/workspace.apis'; import { INLINE_SCRIPT_MANAGER_ID } from '../common/constants'; import { runAsTask } from './execution/runAsTask'; import { runInTerminal } from './terminal/runInTerminal'; @@ -670,34 +669,6 @@ export async function removePythonProject( wm.remove(item.project); } -function getInlineScriptProjectEdits(wm: PythonProjectManager) { - const currentProjects = new Map(wm.getProjects().map((project) => [project.uri.toString(), project] as const)); - const edits = new Map(); - for (const workspaceFolder of getWorkspaceFolders() ?? []) { - for (const resolvedSetting of getResolvedPythonProjectSettings(workspaceFolder)) { - if ( - !resolvedSetting.sources.some( - (source) => source.setting.envManager === INLINE_SCRIPT_MANAGER_ID, - ) - ) { - continue; - } - const projectUri = resolvedSetting.uri; - const key = projectUri.toString(); - edits.set(key, { - project: - currentProjects.get(key) ?? - wm.create(path.basename(projectUri.fsPath) || resolvedSetting.effective.setting.path, projectUri), - envManager: INLINE_SCRIPT_MANAGER_ID, - }); - } - } - return { - edits: Array.from(edits.values()), - loadedProjects: currentProjects, - }; -} - export async function clearScriptEnvironmentCacheCommand( em: EnvironmentManagers, wm: PythonProjectManager, @@ -721,15 +692,8 @@ export async function clearScriptEnvironmentCacheCommand( return; } - const { edits, loadedProjects } = getInlineScriptProjectEdits(wm); await manager.clearCache(); - if (edits.length === 0) { - return; - } - const removedProjects = await removePythonProjectSetting(edits); - const loadedProjectsToRemove = removedProjects - .map((project) => loadedProjects.get(project.uri.toString())) - .filter((project): project is PythonProject => project !== undefined); + const loadedProjectsToRemove = await removeInlineScriptPythonProjectSettings(wm.getProjects()); if (loadedProjectsToRemove.length > 0) { wm.remove(loadedProjectsToRemove); } diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 25b2eca9d..4d84e4321 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -1,7 +1,12 @@ import * as path from 'path'; import { ConfigurationScope, ConfigurationTarget, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode'; import { PythonProject } from '../../api'; -import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID, SYSTEM_MANAGER_ID } from '../../common/constants'; +import { + DEFAULT_ENV_MANAGER_ID, + DEFAULT_PACKAGE_MANAGER_ID, + INLINE_SCRIPT_MANAGER_ID, + SYSTEM_MANAGER_ID, +} from '../../common/constants'; import { traceError, traceInfo, traceVerbose, traceWarn } from '../../common/logging'; import { getGlobalPersistentState } from '../../common/persistentState'; import { normalizePath } from '../../common/utils/pathUtils'; @@ -10,7 +15,7 @@ import { sendTelemetryEvent } from '../../common/telemetry/sender'; import * as workspaceApis from '../../common/workspace.apis'; import { PythonProjectManager, PythonProjectSettings } from '../../internal.api'; -export interface ResolvedPythonProjectSettingSource { +interface ResolvedPythonProjectSettingSource { readonly setting: PythonProjectSettings; readonly uri: Uri; readonly workspaceFolder: WorkspaceFolder; @@ -18,7 +23,7 @@ export interface ResolvedPythonProjectSettingSource { readonly target: ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder; } -export interface ResolvedPythonProjectSetting { +interface ResolvedPythonProjectSetting { readonly uri: Uri; readonly workspaceFolder: WorkspaceFolder; readonly effective: ResolvedPythonProjectSettingSource; @@ -60,7 +65,7 @@ function resolveProjectSettingUri( : undefined; } -export function getResolvedPythonProjectSettings( +function getResolvedPythonProjectSettings( workspaceFolder: WorkspaceFolder, config: WorkspaceConfiguration = workspaceApis.getConfiguration('python-envs', workspaceFolder.uri), ): ResolvedPythonProjectSetting[] { @@ -495,6 +500,115 @@ function cloneProjectSettings( return settings?.map((setting) => ({ ...setting })); } +export async function removeInlineScriptPythonProjectSettings( + currentProjects: readonly PythonProject[], +): Promise { + const currentProjectsByUri = new Map(currentProjects.map((project) => [project.uri.toString(), project] as const)); + const workspaceEntries: Array = []; + for (const workspaceFolder of workspaceApis.getWorkspaceFolders() ?? []) { + const edits: EditProjectSettings[] = getResolvedPythonProjectSettings(workspaceFolder) + .filter((resolvedSetting) => + resolvedSetting.sources.some((source) => source.setting.envManager === INLINE_SCRIPT_MANAGER_ID), + ) + .map((resolvedSetting) => ({ + project: + currentProjectsByUri.get(resolvedSetting.uri.toString()) ?? { + name: path.basename(resolvedSetting.uri.fsPath) || resolvedSetting.effective.setting.path, + uri: resolvedSetting.uri, + }, + envManager: INLINE_SCRIPT_MANAGER_ID, + })); + + if (edits.length > 0) { + workspaceEntries.push([workspaceFolder, edits]); + } + } + + if (workspaceEntries.length === 0) { + return []; + } + + const removedProjects = new Map(); + const folderRemainingSettings = new Map(); + const folderExistingSettings = new Map(); + const promises: Thenable[] = []; + let workspaceConfig: WorkspaceConfiguration | undefined; + let workspaceValueOriginal: PythonProjectSettings[] | undefined; + + workspaceEntries.forEach(([workspaceFolder, edits]) => { + const config = workspaceApis.getConfiguration('python-envs', workspaceFolder.uri); + const projectsInspect = config.inspect('pythonProjects'); + workspaceConfig ??= config; + workspaceValueOriginal ??= cloneProjectSettings(projectsInspect?.workspaceValue); + + const workspaceFolderOriginal = cloneProjectSettings(projectsInspect?.workspaceFolderValue) ?? []; + folderExistingSettings.set(workspaceFolder.uri.toString(), workspaceFolderOriginal); + const workspaceFolderRemaining = workspaceFolderOriginal.filter( + (projectSetting) => !edits.some((edit) => matchesProjectSettingEdit(projectSetting, edit, workspaceFolder)), + ); + folderRemainingSettings.set(workspaceFolder.uri.toString(), workspaceFolderRemaining); + + if (workspaceFolderRemaining.length !== workspaceFolderOriginal.length) { + promises.push( + config.update( + 'pythonProjects', + workspaceFolderRemaining.length > 0 ? workspaceFolderRemaining : undefined, + ConfigurationTarget.WorkspaceFolder, + ), + ); + } + }); + + const aggregatedEdits = workspaceEntries.flatMap(([workspaceFolder, edits]) => + edits.map((edit) => ({ workspaceFolder, edit })), + ); + const workspaceValueRemaining = + workspaceValueOriginal?.filter( + (projectSetting) => + !aggregatedEdits.some(({ workspaceFolder, edit }) => + matchesProjectSettingEdit(projectSetting, edit, workspaceFolder), + ), + ) ?? []; + + if ( + workspaceConfig && + workspaceValueOriginal !== undefined && + workspaceValueRemaining.length !== workspaceValueOriginal.length + ) { + promises.push( + workspaceConfig.update( + 'pythonProjects', + workspaceValueRemaining.length > 0 ? workspaceValueRemaining : undefined, + ConfigurationTarget.Workspace, + ), + ); + } + + workspaceEntries.forEach(([workspaceFolder, edits]) => { + const existingSettings = [ + ...(workspaceValueOriginal ?? []), + ...((folderExistingSettings.get(workspaceFolder.uri.toString()) ?? [])), + ]; + const remainingSettings = [ + ...workspaceValueRemaining, + ...((folderRemainingSettings.get(workspaceFolder.uri.toString()) ?? [])), + ]; + edits.filter( + (edit) => + existingSettings.some((projectSetting) => matchesProjectSettingEdit(projectSetting, edit, workspaceFolder)) && + !hasProjectSetting(remainingSettings, edit.project, workspaceFolder), + ).forEach((edit) => { + removedProjects.set(edit.project.uri.toString(), edit.project); + }); + }); + + await Promise.all(promises); + + return Array.from(removedProjects.values()) + .map((project) => currentProjectsByUri.get(project.uri.toString())) + .filter((project): project is PythonProject => project !== undefined); +} + export async function addPythonProjectSetting(edits: EditProjectSettings[]): Promise { const noWorkspace: EditProjectSettings[] = []; const workspaces = new Map(); @@ -591,7 +705,7 @@ export async function addPythonProjectSetting(edits: EditProjectSettings[]): Pro await Promise.all(promises); } -export async function removePythonProjectSetting(edits: EditProjectSettings[]): Promise { +export async function removePythonProjectSetting(edits: EditProjectSettings[]): Promise { const noWorkspace: EditProjectSettings[] = []; const workspaces = new Map(); edits.forEach((e) => { @@ -607,87 +721,24 @@ export async function removePythonProjectSetting(edits: EditProjectSettings[]): traceError(`Unable to find workspace for ${e.project.uri.fsPath}`); }); - const workspaceEntries = Array.from(workspaces.entries()); - if (workspaceEntries.length === 0) { - return []; - } - - const removedProjects = new Map(); - const folderRemainingSettings = new Map(); - const folderExistingSettings = new Map(); const promises: Thenable[] = []; - let workspaceConfig: WorkspaceConfiguration | undefined; - let workspaceValueOriginal: PythonProjectSettings[] | undefined; - - workspaceEntries.forEach(([w, es]) => { + workspaces.forEach((es, w) => { const config = workspaceApis.getConfiguration('python-envs', w.uri); - const projectsInspect = config.inspect('pythonProjects'); - workspaceConfig ??= config; - workspaceValueOriginal ??= cloneProjectSettings(projectsInspect?.workspaceValue); - - const workspaceFolderOriginal = cloneProjectSettings(projectsInspect?.workspaceFolderValue) ?? []; - folderExistingSettings.set(w.uri.toString(), workspaceFolderOriginal); - const workspaceFolderRemaining = workspaceFolderOriginal.filter( - (projectSetting) => !es.some((edit) => matchesProjectSettingEdit(projectSetting, edit, w)), - ); - folderRemainingSettings.set(w.uri.toString(), workspaceFolderRemaining); - - if (workspaceFolderRemaining.length !== workspaceFolderOriginal.length) { - promises.push( - config.update( - 'pythonProjects', - workspaceFolderRemaining.length > 0 ? workspaceFolderRemaining : undefined, - ConfigurationTarget.WorkspaceFolder, - ), - ); - } - }); - - const aggregatedEdits = workspaceEntries.flatMap(([workspaceFolder, workspaceEdits]) => - workspaceEdits.map((edit) => ({ workspaceFolder, edit })), - ); - const workspaceValueRemaining = - workspaceValueOriginal?.filter( - (projectSetting) => - !aggregatedEdits.some(({ workspaceFolder, edit }) => - matchesProjectSettingEdit(projectSetting, edit, workspaceFolder), - ), - ) ?? []; - - if ( - workspaceConfig && - workspaceValueOriginal !== undefined && - workspaceValueRemaining.length !== workspaceValueOriginal.length - ) { - promises.push( - workspaceConfig.update( - 'pythonProjects', - workspaceValueRemaining.length > 0 ? workspaceValueRemaining : undefined, - ConfigurationTarget.Workspace, - ), - ); - } - - workspaceEntries.forEach(([w, es]) => { - const existingSettings = [ - ...(workspaceValueOriginal ?? []), - ...((folderExistingSettings.get(w.uri.toString()) ?? [])), - ]; - const remainingSettings = [ - ...workspaceValueRemaining, - ...((folderRemainingSettings.get(w.uri.toString()) ?? [])), - ]; - es.filter( - (edit) => - existingSettings.some((projectSetting) => matchesProjectSettingEdit(projectSetting, edit, w)) && - !hasProjectSetting(remainingSettings, edit.project, w), - ).forEach((edit) => { - removedProjects.set(edit.project.uri.toString(), edit.project); + const overrides = config.get('pythonProjects', []); + es.forEach((e) => { + const pwPath = normalizePath(e.project.uri.fsPath); + const index = overrides.findIndex((s) => normalizePath(path.resolve(w.uri.fsPath, s.path)) === pwPath); + if (index >= 0) { + overrides.splice(index, 1); + } }); + if (overrides.length === 0) { + promises.push(config.update('pythonProjects', undefined, ConfigurationTarget.Workspace)); + } else { + promises.push(config.update('pythonProjects', overrides, ConfigurationTarget.Workspace)); + } }); - await Promise.all(promises); - return Array.from(removedProjects.values()); } /** diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index e31e98adc..f47ceb15a 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/src/test/features/envCommands.unit.test.ts @@ -1,15 +1,13 @@ import * as assert from 'assert'; -import * as path from 'path'; import * as sinon from 'sinon'; import * as typeMoq from 'typemoq'; -import { Uri, WorkspaceFolder } from 'vscode'; +import { Uri } from 'vscode'; import { PythonEnvironment, PythonProject } from '../../api'; import * as commandApi from '../../common/command.api'; import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; import * as managerApi from '../../common/pickers/managers'; import * as projectApi from '../../common/pickers/projects'; import * as windowApis from '../../common/window.apis'; -import * as workspaceApis from '../../common/workspace.apis'; import { clearScriptEnvironmentCacheCommand, createAnyEnvironmentCommand, @@ -213,7 +211,6 @@ suite('Remove Python Project Command Tests', () => { } as unknown as PythonProjectManager; sinon.stub(settingHelpers, 'removePythonProjectSetting').callsFake(async () => { calls.push('removeSetting'); - return []; }); await removePythonProject(item, projectManager, envManagers); @@ -227,13 +224,6 @@ suite('Remove Python Project Command Tests', () => { }); suite('Clear Script Environment Cache Command Tests', () => { - const workspacePath = process.platform === 'win32' ? 'C:\\workspace' : '/workspace'; - const workspaceFolder: WorkspaceFolder = { - uri: Uri.file(workspacePath), - name: 'workspace', - index: 0, - }; - teardown(() => { sinon.restore(); }); @@ -248,26 +238,27 @@ suite('Clear Script Environment Cache Command Tests', () => { } as unknown as EnvironmentManagers; const projectManager = { getProjects: sinon.stub().returns([]), - create: sinon.stub(), remove: sinon.stub(), } as unknown as PythonProjectManager; sinon.stub(windowApis, 'showWarningMessage').resolves(undefined); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([]); - const removeSettings = sinon.stub(settingHelpers, 'removePythonProjectSetting').resolves([]); + const removeInlineSettings = sinon.stub(settingHelpers, 'removeInlineScriptPythonProjectSettings').resolves([]); await clearScriptEnvironmentCacheCommand(envManagers, projectManager); sinon.assert.notCalled(clearCache); - sinon.assert.notCalled(removeSettings); + sinon.assert.notCalled(removeInlineSettings); sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); }); - test('clears the cache and removes only inline-script projects returned by the settings cleanup', async () => { + test('clears cache before inline settings cleanup and unloads removed projects', async () => { + const calls: string[] = []; const inlineProject: PythonProject = { - uri: Uri.file(path.join(workspacePath, 'script.py')), + uri: Uri.file('/workspace/script.py'), name: 'script.py', }; - const clearCache = sinon.stub().resolves(); + const clearCache = sinon.stub().callsFake(async () => { + calls.push('clearCache'); + }); const envManagers = { getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ supportsClearCache: () => true, @@ -275,136 +266,35 @@ suite('Clear Script Environment Cache Command Tests', () => { }), } as unknown as EnvironmentManagers; const projectManager = { - getProjects: sinon.stub().returns([inlineProject]), - create: sinon.stub().callsFake((name: string, uri: Uri) => ({ name, uri })), - remove: sinon.stub(), + getProjects: sinon.stub().callsFake(() => { + calls.push('getProjects'); + return [inlineProject]; + }), + remove: sinon.stub().callsFake(() => { + calls.push('removeProjects'); + }), } as unknown as PythonProjectManager; sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); - sinon.stub(workspaceApis, 'getConfiguration').returns({ - get: (key: string) => - key === 'pythonProjects' - ? [ - { - path: 'script.py', - envManager: INLINE_SCRIPT_MANAGER_ID, - packageManager: 'ms-python.python:pip', - }, - { - path: 'other.py', - envManager: 'ms-python.python:venv', - packageManager: 'ms-python.python:pip', - }, - ] - : [], - inspect: (key: string) => - key === 'pythonProjects' - ? { - workspaceValue: [ - { - path: 'script.py', - envManager: INLINE_SCRIPT_MANAGER_ID, - packageManager: 'ms-python.python:pip', - }, - { - path: 'other.py', - envManager: 'ms-python.python:venv', - packageManager: 'ms-python.python:pip', - }, - ], - workspaceFolderValue: undefined, - } - : undefined, - } as never); - const removeSettings = sinon.stub(settingHelpers, 'removePythonProjectSetting').resolves([inlineProject]); + const removeInlineSettings = sinon + .stub(settingHelpers, 'removeInlineScriptPythonProjectSettings') + .callsFake(async (projects) => { + calls.push('removeInlineSettings'); + assert.deepStrictEqual(projects, [inlineProject]); + return [inlineProject]; + }); await clearScriptEnvironmentCacheCommand(envManagers, projectManager); sinon.assert.calledOnce(clearCache); - sinon.assert.calledOnceWithExactly(removeSettings, [ - { - project: inlineProject, - envManager: INLINE_SCRIPT_MANAGER_ID, - }, - ]); + sinon.assert.calledOnce(removeInlineSettings); sinon.assert.calledOnceWithExactly(projectManager.remove as sinon.SinonStub, [inlineProject]); + assert.deepStrictEqual(calls, ['clearCache', 'getProjects', 'removeInlineSettings', 'removeProjects']); }); - test('includes inline-script projects without a .py extension', async () => { - const inlineProjectUri = Uri.file(path.join(workspacePath, 'runner')); - const clearCache = sinon.stub().resolves(); - const envManagers = { - getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ - supportsClearCache: () => true, - clearCache, - }), - } as unknown as EnvironmentManagers; - const projectManager = { - getProjects: sinon.stub().returns([]), - create: sinon.stub().callsFake((name: string, uri: Uri) => ({ name, uri })), - remove: sinon.stub(), - } as unknown as PythonProjectManager; - sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); - sinon.stub(workspaceApis, 'getConfiguration').returns({ - get: (key: string) => - key === 'pythonProjects' - ? [ - { - path: 'runner', - envManager: INLINE_SCRIPT_MANAGER_ID, - packageManager: 'ms-python.python:pip', - }, - { - path: 'unrelated', - envManager: 'ms-python.python:venv', - packageManager: 'ms-python.python:pip', - }, - ] - : [], - inspect: (key: string) => - key === 'pythonProjects' - ? { - workspaceValue: [ - { - path: 'runner', - envManager: INLINE_SCRIPT_MANAGER_ID, - packageManager: 'ms-python.python:pip', - }, - { - path: 'unrelated', - envManager: 'ms-python.python:venv', - packageManager: 'ms-python.python:pip', - }, - ], - workspaceFolderValue: undefined, - } - : undefined, - } as never); - const removeSettings = sinon - .stub(settingHelpers, 'removePythonProjectSetting') - .callsFake(async (edits) => [edits[0].project]); - - await clearScriptEnvironmentCacheCommand(envManagers, projectManager); - - assert.strictEqual(removeSettings.callCount, 1); - assert.strictEqual(removeSettings.firstCall.args[0].length, 1); - assert.strictEqual(removeSettings.firstCall.args[0][0].envManager, INLINE_SCRIPT_MANAGER_ID); - assert.strictEqual(removeSettings.firstCall.args[0][0].project.uri.fsPath, inlineProjectUri.fsPath); - assert.strictEqual((projectManager.create as sinon.SinonStub).callCount, 1); - assert.strictEqual((projectManager.create as sinon.SinonStub).firstCall.args[0], 'runner'); - assert.strictEqual( - (projectManager.create as sinon.SinonStub).firstCall.args[1].fsPath.toLowerCase(), - inlineProjectUri.fsPath.toLowerCase(), - ); - sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); - }); - - test('removes a hidden inline workspace entry when a folder override exists for the same URI', async () => { - const projectUri = Uri.file(path.join(workspacePath, 'script.py')); - const visibleProject: PythonProject = { - uri: projectUri, - name: 'script.py', + test('keeps loaded projects when inline settings cleanup leaves them configured', async () => { + const inlineProject: PythonProject = { + uri: Uri.file('/workspace/runner'), + name: 'runner', }; const clearCache = sinon.stub().resolves(); const envManagers = { @@ -414,53 +304,17 @@ suite('Clear Script Environment Cache Command Tests', () => { }), } as unknown as EnvironmentManagers; const projectManager = { - getProjects: sinon.stub().returns([visibleProject]), - create: sinon.stub(), + getProjects: sinon.stub().returns([inlineProject]), remove: sinon.stub(), } as unknown as PythonProjectManager; sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]); - sinon.stub(workspaceApis, 'getConfiguration').returns({ - get: (key: string) => - key === 'pythonProjects' - ? [ - { - path: 'script.py', - envManager: 'ms-python.python:venv', - packageManager: 'ms-python.python:pip', - }, - ] - : [], - inspect: (key: string) => - key === 'pythonProjects' - ? { - workspaceValue: [ - { - path: 'script.py', - envManager: INLINE_SCRIPT_MANAGER_ID, - packageManager: 'ms-python.python:pip', - }, - ], - workspaceFolderValue: [ - { - path: 'script.py', - envManager: 'ms-python.python:venv', - packageManager: 'ms-python.python:pip', - }, - ], - } - : undefined, - } as never); - const removeSettings = sinon - .stub(settingHelpers, 'removePythonProjectSetting') - .callsFake(async (edits) => [edits[0].project]); + const removeInlineSettings = sinon.stub(settingHelpers, 'removeInlineScriptPythonProjectSettings').resolves([]); await clearScriptEnvironmentCacheCommand(envManagers, projectManager); - assert.strictEqual(removeSettings.callCount, 1); - assert.strictEqual(removeSettings.firstCall.args[0].length, 1); - assert.strictEqual(removeSettings.firstCall.args[0][0].project.uri.fsPath, visibleProject.uri.fsPath); - assert.strictEqual(removeSettings.firstCall.args[0][0].envManager, INLINE_SCRIPT_MANAGER_ID); + sinon.assert.calledOnce(clearCache); + sinon.assert.calledOnceWithExactly(removeInlineSettings, [inlineProject]); + sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); }); }); diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index 09835fab7..35c63f330 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -9,8 +9,8 @@ import * as sender from '../../../common/telemetry/sender'; import * as workspaceApis from '../../../common/workspace.apis'; import { addPythonProjectSetting, - getResolvedPythonProjectSettings, migrateGlobalDefaultEnvManagerSetting, + removeInlineScriptPythonProjectSettings, removePythonProjectSetting, setAllManagerSettings, setEnvironmentManager, @@ -619,7 +619,7 @@ suite('Setting Helpers - Empty Path Migration', () => { }); }); -suite('Setting Helpers - Exact Project Removal', () => { +suite('Setting Helpers - Project Removal', () => { const INLINE_MANAGER_ID = 'ms-python.python:inline-script'; const VENV_MANAGER_ID = 'ms-python.python:venv'; const PIP_MANAGER_ID = 'ms-python.python:pip'; @@ -741,285 +741,334 @@ suite('Setting Helpers - Exact Project Removal', () => { }; } - test('removes only the matching inline-script entry and preserves unrelated duplicates', async () => { - const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); - const config = createProjectConfig({ - workspaceName: firstWorkspace.name, - workspaceValue: [ - { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + suite('removePythonProjectSetting (bde7cf8-equivalent generic behavior)', () => { + test('rewrites the merged effective array back to workspace scope', async () => { + const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceFolderValue: [ + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); + + await removePythonProjectSetting([{ project }]); + + assert.strictEqual(updateCalls.length, 1, 'Should update pythonProjects once'); + assert.deepStrictEqual(updateCalls[0], { + workspace: firstWorkspace.name, + key: 'pythonProjects', + value: [{ path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }], + target: ConfigurationTarget.Workspace, + }); + }); + + test('ignores envManager metadata and removes the first same-path entry', async () => { + const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'other.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); + + await removePythonProjectSetting([{ project, envManager: VENV_MANAGER_ID }]); + + assert.strictEqual(updateCalls.length, 1, 'Should update pythonProjects once'); + assert.deepStrictEqual(updateCalls[0].value, [ { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, { path: 'other.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], + ]); + assert.strictEqual(updateCalls[0].target, ConfigurationTarget.Workspace); }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); - sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); - sinon.stub(workspaceApis, 'getConfiguration').returns(config); - - const removedProjects = await removePythonProjectSetting([{ project, envManager: INLINE_MANAGER_ID }]); - - assert.deepStrictEqual(removedProjects, [], 'Project should stay because another entry still targets the same path'); - assert.strictEqual(updateCalls.length, 1, 'Should update pythonProjects once'); - assert.strictEqual(updateCalls[0].workspace, firstWorkspace.name); - assert.strictEqual(updateCalls[0].key, 'pythonProjects'); - assert.strictEqual(updateCalls[0].target, ConfigurationTarget.Workspace); - assert.deepStrictEqual(updateCalls[0].value, [ - { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - { path: 'other.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ]); }); - test('dedupes duplicate project URIs with workspaceFolder precedence while keeping both sources visible', () => { - const config = createProjectConfig({ - workspaceName: firstWorkspace.name, - workspaceValue: [ - { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], - workspaceFolderValue: [ + suite('removeInlineScriptPythonProjectSettings', () => { + test('removes all inline-script entries while preserving non-inline duplicates', async () => { + const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const otherProject = new PythonProjectsImpl('other.py', Uri.file(path.join(firstWorkspacePath, 'other.py'))); + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'other.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); + + const removedProjects = await removeInlineScriptPythonProjectSettings([project, otherProject]); + + assert.deepStrictEqual( + removedProjects.map((entry) => entry.uri.fsPath), + [otherProject.uri.fsPath], + 'Only projects left without any non-inline setting should be removed from memory', + ); + assert.strictEqual(updateCalls.length, 1, 'Should update pythonProjects once'); + assert.strictEqual(updateCalls[0].workspace, firstWorkspace.name); + assert.strictEqual(updateCalls[0].key, 'pythonProjects'); + assert.strictEqual(updateCalls[0].target, ConfigurationTarget.Workspace); + assert.deepStrictEqual(updateCalls[0].value, [ { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], + ]); }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); - const resolved = getResolvedPythonProjectSettings(firstWorkspace, config); + test('removes inline-script settings even when the project is not loaded', async () => { + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'runner', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'keep', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); - assert.strictEqual(resolved.length, 1); - assert.strictEqual(resolved[0].effective.source, 'workspaceFolder'); - assert.strictEqual(resolved[0].effective.setting.envManager, VENV_MANAGER_ID); - assert.deepStrictEqual( - resolved[0].sources.map((source) => source.setting.envManager), - [INLINE_MANAGER_ID, VENV_MANAGER_ID], - ); - }); + const removedProjects = await removeInlineScriptPythonProjectSettings([]); - test('removes only one of two roots that share the same relative path', async () => { - const firstProject = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); - const secondProject = new PythonProjectsImpl( - 'script.py', - Uri.file(path.join(secondWorkspaceUri.fsPath, 'script.py')), - ); - const firstConfig = createProjectConfig({ - workspaceName: firstWorkspace.name, - workspaceFolderValue: [ - { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], - }); - const secondConfig = createProjectConfig({ - workspaceName: secondWorkspace.name, - workspaceFolderValue: [ - { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], - }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); - sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => - uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, - ); - sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { - const uri = scope as Uri; - return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + assert.deepStrictEqual(removedProjects, [], 'No loaded project should be returned for memory cleanup'); + assert.strictEqual(updateCalls.length, 1, 'Should update pythonProjects once'); + assert.deepStrictEqual(updateCalls[0].value, [ + { path: 'keep', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ]); }); - const removedProjects = await removePythonProjectSetting([{ project: firstProject, envManager: INLINE_MANAGER_ID }]); + test('removes only one of two roots that share the same relative path', async () => { + const firstProject = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const secondProject = new PythonProjectsImpl( + 'script.py', + Uri.file(path.join(secondWorkspaceUri.fsPath, 'script.py')), + ); + const firstConfig = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceFolderValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + const secondConfig = createProjectConfig({ + workspaceName: secondWorkspace.name, + workspaceFolderValue: [ + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); - assert.deepStrictEqual(removedProjects.map((project) => project.uri.fsPath), [firstProject.uri.fsPath]); - assert.strictEqual(updateCalls.length, 1, 'Only the matching workspace folder should be updated'); - assert.strictEqual(updateCalls[0].workspace, firstWorkspace.name); - assert.strictEqual(updateCalls[0].target, ConfigurationTarget.WorkspaceFolder); - assert.strictEqual(updateCalls[0].value, undefined); - assert.strictEqual(secondProject.uri.fsPath, path.join(secondWorkspaceUri.fsPath, 'script.py')); - }); + const removedProjects = await removeInlineScriptPythonProjectSettings([firstProject, secondProject]); - test('removes a hidden shared inline entry while preserving a folder override for the same URI', async () => { - const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); - const config = createProjectConfig({ - workspaceName: firstWorkspace.name, - workspaceValue: [ - { - path: 'script.py', - envManager: INLINE_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: firstWorkspace.name, - }, - ], - workspaceFolderValue: [ - { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], + assert.deepStrictEqual(removedProjects.map((project) => project.uri.fsPath), [firstProject.uri.fsPath]); + assert.strictEqual(updateCalls.length, 1, 'Only the matching workspace folder should be updated'); + assert.strictEqual(updateCalls[0].workspace, firstWorkspace.name); + assert.strictEqual(updateCalls[0].target, ConfigurationTarget.WorkspaceFolder); + assert.strictEqual(updateCalls[0].value, undefined); + assert.strictEqual(secondProject.uri.fsPath, path.join(secondWorkspaceUri.fsPath, 'script.py')); }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); - sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); - sinon.stub(workspaceApis, 'getConfiguration').returns(config); - - const removedProjects = await removePythonProjectSetting([{ project, envManager: INLINE_MANAGER_ID }]); - - assert.deepStrictEqual(removedProjects, [], 'Folder override should keep the project configured'); - const workspaceUpdate = updateCalls.find((call) => call.target === ConfigurationTarget.Workspace); - const folderUpdate = updateCalls.find((call) => call.target === ConfigurationTarget.WorkspaceFolder); - assert.ok(workspaceUpdate, 'WorkspaceValue source should be updated'); - assert.strictEqual(workspaceUpdate!.value, undefined); - assert.strictEqual(folderUpdate, undefined, 'Folder override should not be rewritten'); - }); - test('aggregates shared workspaceValue removals across folders into one update', async () => { - const firstProject = new PythonProjectsImpl('first', Uri.file(path.join(firstWorkspacePath, 'first'))); - const secondProject = new PythonProjectsImpl('second', Uri.file(path.join(secondWorkspaceUri.fsPath, 'second'))); - const { firstConfig, secondConfig, getWorkspaceValue } = createSharedWorkspaceConfigs({ - workspaceValue: [ - { - path: 'first', - envManager: INLINE_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: firstWorkspace.name, - }, - { - path: 'second', - envManager: INLINE_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: secondWorkspace.name, - }, - ], + test('removes a hidden shared inline entry while preserving a folder override for the same URI', async () => { + const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const config = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { + path: 'script.py', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: firstWorkspace.name, + }, + ], + workspaceFolderValue: [ + { path: 'script.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(firstWorkspace); + sinon.stub(workspaceApis, 'getConfiguration').returns(config); + + const removedProjects = await removeInlineScriptPythonProjectSettings([project]); + + assert.deepStrictEqual(removedProjects, [], 'Folder override should keep the project configured'); + const workspaceUpdate = updateCalls.find((call) => call.target === ConfigurationTarget.Workspace); + const folderUpdate = updateCalls.find((call) => call.target === ConfigurationTarget.WorkspaceFolder); + assert.ok(workspaceUpdate, 'WorkspaceValue source should be updated'); + assert.strictEqual(workspaceUpdate!.value, undefined); + assert.strictEqual(folderUpdate, undefined, 'Folder override should not be rewritten'); }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); - sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => - uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, - ); - sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { - const uri = scope as Uri; - return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + + test('aggregates shared workspaceValue removals across folders into one update', async () => { + const firstProject = new PythonProjectsImpl('first', Uri.file(path.join(firstWorkspacePath, 'first'))); + const secondProject = new PythonProjectsImpl('second', Uri.file(path.join(secondWorkspaceUri.fsPath, 'second'))); + const { firstConfig, secondConfig, getWorkspaceValue } = createSharedWorkspaceConfigs({ + workspaceValue: [ + { + path: 'first', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: firstWorkspace.name, + }, + { + path: 'second', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); + + const removedProjects = await removeInlineScriptPythonProjectSettings([firstProject, secondProject]); + + assert.deepStrictEqual( + removedProjects.map((project) => project.uri.fsPath).sort(), + [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), + ); + assert.strictEqual( + updateCalls.filter((call) => call.target === ConfigurationTarget.Workspace).length, + 1, + 'Shared workspaceValue should be written once', + ); + assert.deepStrictEqual(getWorkspaceValue(), []); }); - const removedProjects = await removePythonProjectSetting([ - { project: firstProject, envManager: INLINE_MANAGER_ID }, - { project: secondProject, envManager: INLINE_MANAGER_ID }, - ]); + test('removes every inline shared entry without resurrecting non-inline siblings', async () => { + const firstProject = new PythonProjectsImpl('first', Uri.file(path.join(firstWorkspacePath, 'first'))); + const secondProject = new PythonProjectsImpl( + 'second', + Uri.file(path.join(secondWorkspaceUri.fsPath, 'second')), + ); + const { firstConfig, secondConfig, getWorkspaceValue } = createSharedWorkspaceConfigs({ + workspaceValue: [ + { + path: 'first', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: firstWorkspace.name, + }, + { + path: 'second', + envManager: INLINE_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + { + path: 'keep', + envManager: VENV_MANAGER_ID, + packageManager: PIP_MANAGER_ID, + workspace: secondWorkspace.name, + }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); - assert.deepStrictEqual( - removedProjects.map((project) => project.uri.fsPath).sort(), - [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), - ); - assert.strictEqual( - updateCalls.filter((call) => call.target === ConfigurationTarget.Workspace).length, - 1, - 'Shared workspaceValue should be written once', - ); - assert.deepStrictEqual(getWorkspaceValue(), []); - }); + const removedProjects = await removeInlineScriptPythonProjectSettings([firstProject, secondProject]); - test('removes a subset from the shared workspace array without resurrecting siblings', async () => { - const firstProject = new PythonProjectsImpl('first', Uri.file(path.join(firstWorkspacePath, 'first'))); - const secondProject = new PythonProjectsImpl('second', Uri.file(path.join(secondWorkspaceUri.fsPath, 'second'))); - const { firstConfig, secondConfig, getWorkspaceValue } = createSharedWorkspaceConfigs({ - workspaceValue: [ - { - path: 'first', - envManager: INLINE_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: firstWorkspace.name, - }, - { - path: 'second', - envManager: INLINE_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: secondWorkspace.name, - }, + assert.deepStrictEqual( + removedProjects.map((project) => project.uri.fsPath).sort(), + [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), + ); + assert.strictEqual( + updateCalls.filter((call) => call.target === ConfigurationTarget.Workspace).length, + 1, + 'Shared workspaceValue should still be written once', + ); + assert.deepStrictEqual(getWorkspaceValue(), [ { path: 'keep', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID, workspace: secondWorkspace.name, }, - ], - }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); - sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => - uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, - ); - sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { - const uri = scope as Uri; - return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + ]); + assert.strictEqual(secondProject.uri.fsPath, path.join(secondWorkspaceUri.fsPath, 'second')); }); - const removedProjects = await removePythonProjectSetting([{ project: firstProject, envManager: INLINE_MANAGER_ID }]); + test('removes matching inline-script projects independently in a multi-root workspace', async () => { + const firstProject = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); + const secondProject = new PythonProjectsImpl( + 'script.py', + Uri.file(path.join(secondWorkspaceUri.fsPath, 'script.py')), + ); + const firstConfig = createProjectConfig({ + workspaceName: firstWorkspace.name, + workspaceValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceFolderValue: [ + { path: 'keep-folder.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + const secondConfig = createProjectConfig({ + workspaceName: secondWorkspace.name, + workspaceFolderValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); - assert.deepStrictEqual(removedProjects.map((project) => project.uri.fsPath), [firstProject.uri.fsPath]); - assert.strictEqual( - updateCalls.filter((call) => call.target === ConfigurationTarget.Workspace).length, - 1, - 'Shared workspaceValue should still be written once', - ); - assert.deepStrictEqual(getWorkspaceValue(), [ - { - path: 'second', - envManager: INLINE_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: secondWorkspace.name, - }, - { - path: 'keep', - envManager: VENV_MANAGER_ID, - packageManager: PIP_MANAGER_ID, - workspace: secondWorkspace.name, - }, - ]); - assert.strictEqual(secondProject.uri.fsPath, path.join(secondWorkspaceUri.fsPath, 'second')); - }); + const removedProjects = await removeInlineScriptPythonProjectSettings([firstProject, secondProject]); - test('removes matching inline-script projects independently in a multi-root workspace', async () => { - const firstProject = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); - const secondProject = new PythonProjectsImpl( - 'script.py', - Uri.file(path.join(secondWorkspaceUri.fsPath, 'script.py')), - ); - const firstConfig = createProjectConfig({ - workspaceName: firstWorkspace.name, - workspaceValue: [ - { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], - workspaceFolderValue: [ - { path: 'keep-folder.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], - }); - const secondConfig = createProjectConfig({ - workspaceName: secondWorkspace.name, - workspaceFolderValue: [ - { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + assert.deepStrictEqual( + removedProjects.map((project) => project.uri.fsPath).sort(), + [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), + ); + assert.strictEqual(updateCalls.length, 2, 'Should update each workspace independently'); + const firstWorkspaceUpdate = updateCalls.find((call) => call.workspace === firstWorkspace.name); + const secondWorkspaceUpdate = updateCalls.find((call) => call.workspace === secondWorkspace.name); + assert.ok(firstWorkspaceUpdate, 'First workspace should receive an update'); + assert.ok(secondWorkspaceUpdate, 'Second workspace should receive an update'); + assert.strictEqual(firstWorkspaceUpdate!.value, undefined); + assert.deepStrictEqual(secondWorkspaceUpdate!.value, [ { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ], - }); - sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); - sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => - uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, - ); - sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { - const uri = scope as Uri; - return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + ]); + assert.ok( + updateCalls.some((call) => call.workspace === firstWorkspace.name && call.target === ConfigurationTarget.Workspace) && + updateCalls.some( + (call) => + call.workspace === secondWorkspace.name && + call.target === ConfigurationTarget.WorkspaceFolder, + ), + 'Should update the same configuration scope that originally contained each project entry', + ); }); - - const removedProjects = await removePythonProjectSetting([ - { project: firstProject, envManager: INLINE_MANAGER_ID }, - { project: secondProject, envManager: INLINE_MANAGER_ID }, - ]); - - assert.deepStrictEqual( - removedProjects.map((project) => project.uri.fsPath).sort(), - [firstProject.uri.fsPath, secondProject.uri.fsPath].sort(), - ); - assert.strictEqual(updateCalls.length, 2, 'Should update each workspace independently'); - const firstWorkspaceUpdate = updateCalls.find((call) => call.workspace === firstWorkspace.name); - const secondWorkspaceUpdate = updateCalls.find((call) => call.workspace === secondWorkspace.name); - assert.ok(firstWorkspaceUpdate, 'First workspace should receive an update'); - assert.ok(secondWorkspaceUpdate, 'Second workspace should receive an update'); - assert.strictEqual(firstWorkspaceUpdate!.value, undefined); - assert.deepStrictEqual(secondWorkspaceUpdate!.value, [ - { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, - ]); - assert.ok( - updateCalls.some((call) => call.workspace === firstWorkspace.name && call.target === ConfigurationTarget.Workspace) && - updateCalls.some( - (call) => - call.workspace === secondWorkspace.name && - call.target === ConfigurationTarget.WorkspaceFolder, - ), - 'Should update the same configuration scope that originally contained each project entry', - ); }); }); diff --git a/src/test/smoke/registration.smoke.test.ts b/src/test/smoke/registration.smoke.test.ts index 727bf2bc0..56d0b2944 100644 --- a/src/test/smoke/registration.smoke.test.ts +++ b/src/test/smoke/registration.smoke.test.ts @@ -65,7 +65,6 @@ suite('Smoke: Registration Checks', function () { 'python-envs.setPkgManager', 'python-envs.refreshAllManagers', 'python-envs.clearCache', - 'python-envs.clearScriptEnvCache', 'python-envs.searchSettings', // Package management @@ -114,6 +113,38 @@ suite('Smoke: Registration Checks', function () { ); }); + test('Internal inline clear command is not publicly contributed', function () { + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + assert.ok(extension, `Extension ${ENVS_EXTENSION_ID} not found`); + + const contributedCommands = (extension.packageJSON?.contributes?.commands ?? []) as Array<{ command: string }>; + const commandPaletteEntries = (extension.packageJSON?.contributes?.menus?.commandPalette ?? []) as Array<{ + command: string; + }>; + + assert.ok( + !contributedCommands.some((entry) => entry.command === 'python-envs.clearScriptEnvCache'), + 'python-envs.clearScriptEnvCache should not be publicly contributed before rollout', + ); + assert.ok( + !commandPaletteEntries.some((entry) => entry.command === 'python-envs.clearScriptEnvCache'), + 'python-envs.clearScriptEnvCache should not appear in contributed menus before rollout', + ); + }); + + test('Internal inline clear command is not registered while the feature flag is off', async function () { + const allCommands = await vscode.commands.getCommands(true); + + assert.ok( + !allCommands.includes('python-envs.clearScriptEnvCache'), + 'python-envs.clearScriptEnvCache should not be registered by default', + ); + await assert.rejects( + () => Promise.resolve(vscode.commands.executeCommand('python-envs.clearScriptEnvCache')), + /not found/i, + ); + }); + // ========================================================================= // API METHODS - All API methods must exist and be functions // ========================================================================= From 2ad83af5752ef71f9ffcd01574c487377d59c2ac Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 17:22:21 -0700 Subject: [PATCH 4/6] Harden inline script cache cleanup Coordinate per-entry deletion locks, keep partial failures consistent, and clean inline project settings safely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2 --- src/common/lockfile.apis.ts | 31 +- src/features/envManagers.ts | 8 +- src/features/settings/settingHelpers.ts | 42 ++- .../builtin/inlineScript/envManager.ts | 331 +++++++++++++----- src/test/features/envCommands.unit.test.ts | 22 ++ src/test/features/envManagers.unit.test.ts | 58 +++ .../settings/settingHelpers.unit.test.ts | 89 ++++- .../inlineScript/envManager.unit.test.ts | 95 +++++ 8 files changed, 583 insertions(+), 93 deletions(-) diff --git a/src/common/lockfile.apis.ts b/src/common/lockfile.apis.ts index 2fbe10352..62dc6aaeb 100644 --- a/src/common/lockfile.apis.ts +++ b/src/common/lockfile.apis.ts @@ -162,6 +162,31 @@ export async function inspectFileLock(filePath: string, options?: InspectFileLoc return 'orphaned'; } +/** + * Move a stale or retained lock out of the lock name before a replacement owner is acquired. + * The rename prevents a newly-created lock from being removed based on an earlier inspection. + */ +export async function reclaimFileLock(filePath: string): Promise { + const lockPath = getFileLockPath(filePath); + const state = await inspectFileLock(filePath); + if (state !== 'stale' && state !== 'retained') { + return false; + } + + const quarantinedLockPath = `${lockPath}.reclaimed-${process.pid}-${crypto.randomBytes(16).toString('hex')}`; + try { + await fsapi.rename(lockPath, quarantinedLockPath); + } catch (error) { + if (hasErrorCode(error, 'ENOENT')) { + return false; + } + throw error; + } + + await fsapi.remove(quarantinedLockPath); + return true; +} + export async function getProcessLiveness(pid: number): Promise { try { process.kill(pid, 0); @@ -196,7 +221,7 @@ function hasErrorCode(error: unknown, code: string): boolean { } function parseOwnerPid(entry: string): number | undefined { - const match = entry.match(/^owner-(\d+)-/); + const match = entry.match(new RegExp(`^${escapeRegExp(FILE_LOCK_OWNER_MARKER_PREFIX)}(\\d+)-`)); if (!match) { return undefined; } @@ -204,6 +229,10 @@ function parseOwnerPid(entry: string): number | undefined { return Number.isSafeInteger(pid) && pid > 0 ? pid : undefined; } +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + function createLockError(message: string, code: string, lockPath: string): NodeJS.ErrnoException { return Object.assign(new Error(message), { code, path: lockPath }); } diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index 9fa545d3b..d68973c9c 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -320,12 +320,16 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { public async clearCache(scope: EnvironmentManagerScope): Promise { if (scope === undefined) { - await Promise.all(this.managers.map((m) => m.clearCache())); + await Promise.all( + this.managers + .filter((manager) => manager.id !== INLINE_SCRIPT_MANAGER_ID) + .map((manager) => manager.clearCache()), + ); return; } const manager = this.getEnvironmentManager(scope); - if (manager) { + if (manager && manager.id !== INLINE_SCRIPT_MANAGER_ID) { await manager.clearCache(); } } diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 4d84e4321..24189aacb 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -19,8 +19,8 @@ interface ResolvedPythonProjectSettingSource { readonly setting: PythonProjectSettings; readonly uri: Uri; readonly workspaceFolder: WorkspaceFolder; - readonly source: 'workspace' | 'workspaceFolder'; - readonly target: ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder; + readonly source: 'global' | 'workspace' | 'workspaceFolder'; + readonly target: ConfigurationTarget.Global | ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder; } interface ResolvedPythonProjectSetting { @@ -34,8 +34,8 @@ function resolvePythonProjectSettingSource( setting: PythonProjectSettings, workspaceFolder: WorkspaceFolder, allWorkspaceFolders: readonly WorkspaceFolder[], - source: 'workspace' | 'workspaceFolder', - target: ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder, + source: 'global' | 'workspace' | 'workspaceFolder', + target: ConfigurationTarget.Global | ConfigurationTarget.Workspace | ConfigurationTarget.WorkspaceFolder, ): ResolvedPythonProjectSettingSource | undefined { const resolvedWorkspaceFolder = setting.workspace ? allWorkspaceFolders.find((candidate) => candidate.name === setting.workspace) @@ -75,6 +75,17 @@ function getResolvedPythonProjectSettings( const fallbackSettings = projectsInspect === undefined ? config.get('pythonProjects', []) : undefined; const orderedSources: ResolvedPythonProjectSettingSource[] = [ + ...(projectsInspect?.globalValue ?? []) + .map((setting) => + resolvePythonProjectSettingSource( + setting, + workspaceFolder, + allWorkspaceFolders, + 'global', + ConfigurationTarget.Global, + ), + ) + .filter((setting): setting is ResolvedPythonProjectSettingSource => setting !== undefined), ...(projectsInspect?.workspaceValue ?? fallbackSettings ?? []) .map((setting) => resolvePythonProjectSettingSource( @@ -532,12 +543,16 @@ export async function removeInlineScriptPythonProjectSettings( const folderRemainingSettings = new Map(); const folderExistingSettings = new Map(); const promises: Thenable[] = []; + let globalConfig: WorkspaceConfiguration | undefined; + let globalValueOriginal: PythonProjectSettings[] | undefined; let workspaceConfig: WorkspaceConfiguration | undefined; let workspaceValueOriginal: PythonProjectSettings[] | undefined; workspaceEntries.forEach(([workspaceFolder, edits]) => { const config = workspaceApis.getConfiguration('python-envs', workspaceFolder.uri); const projectsInspect = config.inspect('pythonProjects'); + globalConfig ??= config; + globalValueOriginal ??= cloneProjectSettings(projectsInspect?.globalValue); workspaceConfig ??= config; workspaceValueOriginal ??= cloneProjectSettings(projectsInspect?.workspaceValue); @@ -562,6 +577,13 @@ export async function removeInlineScriptPythonProjectSettings( const aggregatedEdits = workspaceEntries.flatMap(([workspaceFolder, edits]) => edits.map((edit) => ({ workspaceFolder, edit })), ); + const globalValueRemaining = + globalValueOriginal?.filter( + (projectSetting) => + !aggregatedEdits.some(({ workspaceFolder, edit }) => + matchesProjectSettingEdit(projectSetting, edit, workspaceFolder), + ), + ) ?? []; const workspaceValueRemaining = workspaceValueOriginal?.filter( (projectSetting) => @@ -570,6 +592,16 @@ export async function removeInlineScriptPythonProjectSettings( ), ) ?? []; + if (globalConfig && globalValueOriginal !== undefined && globalValueRemaining.length !== globalValueOriginal.length) { + promises.push( + globalConfig.update( + 'pythonProjects', + globalValueRemaining.length > 0 ? globalValueRemaining : undefined, + ConfigurationTarget.Global, + ), + ); + } + if ( workspaceConfig && workspaceValueOriginal !== undefined && @@ -586,10 +618,12 @@ export async function removeInlineScriptPythonProjectSettings( workspaceEntries.forEach(([workspaceFolder, edits]) => { const existingSettings = [ + ...(globalValueOriginal ?? []), ...(workspaceValueOriginal ?? []), ...((folderExistingSettings.get(workspaceFolder.uri.toString()) ?? [])), ]; const remainingSettings = [ + ...globalValueRemaining, ...workspaceValueRemaining, ...((folderRemainingSettings.get(workspaceFolder.uri.toString()) ?? [])), ]; diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 549228891..9ce918387 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -49,6 +49,7 @@ import { FILE_LOCK_DIR_SUFFIX, getFileLockPath, inspectFileLock, + reclaimFileLock, } from '../../../common/lockfile.apis'; import { getWorkspacePersistentState, PersistentState } from '../../../common/persistentState'; import { EventNames, InlineScriptEnvErrorCategory } from '../../../common/telemetry/constants'; @@ -1395,7 +1396,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); - const cacheEntryPaths = await this.getClearableCacheEntryPaths(cacheRoot); + const physicalCacheRootPath = await this.getPhysicalOwnedCacheRootPath(cacheRoot); const persistedAssociations = await this.getPersistedAssociationSnapshot(); const scriptPaths = new Set([ ...Object.keys(persistedAssociations), @@ -1410,114 +1411,171 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { priorSelections.set(scriptPath, this.fsPathToEnv.get(scriptPath)); }); - for (const cacheEntryPath of cacheEntryPaths) { - await fs.remove(cacheEntryPath); - } - - let persistenceError: unknown; - try { - const state = await getWorkspacePersistentState(); - await state.clear([INLINE_SCRIPT_ENVS_KEY]); - } catch (error) { - persistenceError = error; - this.log.error(`Failed to clear inline-script environment associations: ${getErrorMessage(error)}`); - } + const removedCacheEntries = new Set(); + const deletionErrors: unknown[] = []; + if (physicalCacheRootPath) { + let entryNames: string[]; + try { + entryNames = await fs.readdir(physicalCacheRootPath); + } catch (error) { + if (isFileNotFoundError(error)) { + entryNames = []; + } else { + throw error; + } + } - scriptPaths.forEach((scriptPath) => this.bumpAssociationRevision(scriptPath)); - this.pendingRehydrations.clear(); - this.fsPathToEnv.clear(); - this.fsPathToPersistedEnvPath.clear(); - this.cachedAssociationValidatedAt.clear(); + const cacheEntryNames = new Set(); + for (const entryName of entryNames) { + if (entryName.endsWith(FILE_LOCK_DIR_SUFFIX)) { + const envName = entryName.slice(0, -FILE_LOCK_DIR_SUFFIX.length); + if (envName.length === 0) { + const message = l10n.t( + 'Refusing to clear the script environment cache because a lock entry is malformed.', + ); + this.log.error(`${message} (${path.join(physicalCacheRootPath, entryName)})`); + throw new Error(message); + } + cacheEntryNames.add(envName); + } else { + cacheEntryNames.add(entryName); + } + } - priorSelections.forEach((environment, scriptPath) => { - if (!environment) { - return; + for (const entryName of cacheEntryNames) { + try { + const removed = await this.removeCacheEntryForClear( + cacheRoot, + physicalCacheRootPath, + entryName, + ); + if (removed) { + removedCacheEntries.add(normalizePath(removed)); + } + } catch (error) { + deletionErrors.push(error); + this.log.error( + `Failed to remove inline-script cache entry ${path.join(physicalCacheRootPath, entryName)}: ${getErrorMessage(error)}`, + ); + } } - this._onDidChangeEnvironment.fire({ - uri: Uri.file(scriptPath), - old: environment, - new: undefined, - }); - }); + } + const invalidatedScriptPaths = await this.getInvalidatedAssociationPaths( + scriptPaths, + persistedAssociations, + removedCacheEntries, + ); + const persistenceError = await this.clearInvalidatedAssociations( + invalidatedScriptPaths, + persistedAssociations, + priorSelections, + ); if (persistenceError) { - throw persistenceError; + deletionErrors.push(persistenceError); } - } - - private async getClearableCacheEntryPaths(cacheRoot: Uri): Promise { - const resolvedCacheRootPath = await this.getPhysicalOwnedCacheRootPath(cacheRoot); - if (!resolvedCacheRootPath) { - return []; + if (deletionErrors.length > 0) { + throw new Error( + `Failed to completely clear the inline-script environment cache: ${deletionErrors + .map((error) => getErrorMessage(error)) + .join('; ')}`, + ); } - const cacheRootPath = path.resolve(resolvedCacheRootPath); - const physicalCacheRoot = Uri.file(cacheRootPath); - - const entryNames = await fs.readdir(cacheRootPath); - const lockStates = new Map(); - for (const entryName of entryNames.filter((entry) => entry.endsWith(FILE_LOCK_DIR_SUFFIX))) { - const envName = entryName.slice(0, -FILE_LOCK_DIR_SUFFIX.length); - if (envName.length === 0) { - const message = l10n.t( - 'Refusing to clear the script environment cache because a lock entry is malformed.', - ); - this.log.error(`${message} (${path.join(cacheRootPath, entryName)})`); - throw new Error(message); - } + } - const envDirPath = path.join(cacheRootPath, envName); - const lockState = await inspectFileLock(envDirPath); - if (lockState === 'retained' || lockState === 'stale') { - lockStates.set(envDirPath, lockState); - continue; + private async removeCacheEntryForClear( + cacheRoot: Uri, + originalPhysicalCacheRootPath: string, + entryName: string, + ): Promise { + const envDirPath = path.join(originalPhysicalCacheRootPath, entryName); + let lock: AcquiredFileLock | undefined; + try { + lock = await this.acquireCacheEntryLockForClear(envDirPath); + const currentPhysicalCacheRootPath = await this.getPhysicalOwnedCacheRootPath(cacheRoot); + if (!currentPhysicalCacheRootPath) { + return undefined; } - if (lockState === 'held') { + if ( + normalizePath(currentPhysicalCacheRootPath) !== normalizePath(originalPhysicalCacheRootPath) + ) { const message = l10n.t( - 'Cannot clear the script environment cache while a cached environment is being created.', + 'Refusing to clear the script environment cache because its physical root changed during cleanup.', ); - this.log.error(`${message} (${getFileLockPath(envDirPath)})`); - throw new Error(message); - } - if (lockState === 'unavailable') { - const message = l10n.t( - 'Cannot clear the script environment cache because a cached environment lock could not be verified.', + this.log.error( + `${message} (${originalPhysicalCacheRootPath} -> ${currentPhysicalCacheRootPath})`, ); - this.log.error(`${message} (${getFileLockPath(envDirPath)})`); throw new Error(message); } - const message = l10n.t( - 'Refusing to clear the script environment cache because a lock entry is incomplete or malformed.', + const entryPath = await this.getClearableCacheEntryPath( + Uri.file(currentPhysicalCacheRootPath), + path.join(currentPhysicalCacheRootPath, entryName), ); - this.log.error(`${message} (${getFileLockPath(envDirPath)})`); - throw new Error(message); + if (!entryPath) { + return undefined; + } + await this.deleteCacheEntryForClear(entryPath); + return entryPath; + } finally { + if (lock) { + await lock.release(); + } } + } - const pathsToRemove: string[] = []; - const scheduledPaths = new Set(); - - for (const envDirPath of lockStates.keys()) { - const cacheEntryPath = await this.getClearableCacheEntryPath(physicalCacheRoot, envDirPath); - if (cacheEntryPath) { - pathsToRemove.push(cacheEntryPath); - scheduledPaths.add(normalizePath(cacheEntryPath)); + private async acquireCacheEntryLockForClear(envDirPath: string): Promise { + for (let attempt = 0; attempt < 3; attempt += 1) { + try { + return await acquireFileLock(envDirPath, { timeoutMs: 0, retryIntervalMs: CACHE_LOCK_RETRY_MS }); + } catch (error) { + if (!this.isLockContentionError(error)) { + throw error; + } + const lockState = await inspectFileLock(envDirPath); + if (lockState === 'stale' || lockState === 'retained') { + await reclaimFileLock(envDirPath); + continue; + } + if (lockState === 'missing') { + continue; + } + this.throwClearCacheLockError(envDirPath, lockState); } } - for (const envDirPath of lockStates.keys()) { - const lockPath = getFileLockPath(envDirPath); - pathsToRemove.push(lockPath); - scheduledPaths.add(normalizePath(lockPath)); - } + const lockState = await inspectFileLock(envDirPath); + this.throwClearCacheLockError(envDirPath, lockState); + } - for (const entryName of entryNames.filter((entry) => !entry.endsWith(FILE_LOCK_DIR_SUFFIX))) { - const cacheEntryPath = await this.getClearableCacheEntryPath(physicalCacheRoot, path.join(cacheRootPath, entryName)); - if (cacheEntryPath && !scheduledPaths.has(normalizePath(cacheEntryPath))) { - pathsToRemove.push(cacheEntryPath); - } + private isLockContentionError(error: unknown): boolean { + const code = typeof error === 'object' && error !== null && 'code' in error + ? (error as NodeJS.ErrnoException).code + : undefined; + return code === 'ELOCKED' || code === 'ELOCKRETAINED'; + } + + private throwClearCacheLockError(envDirPath: string, lockState: string): never { + if (lockState === 'held') { + const message = l10n.t( + 'Cannot clear the script environment cache while a cached environment is being created.', + ); + this.log.error(`${message} (${getFileLockPath(envDirPath)})`); + throw new Error(message); + } + if (lockState === 'unavailable') { + const message = l10n.t( + 'Cannot clear the script environment cache because a cached environment lock could not be verified.', + ); + this.log.error(`${message} (${getFileLockPath(envDirPath)})`); + throw new Error(message); } - return pathsToRemove; + const message = l10n.t( + 'Refusing to clear the script environment cache because a lock entry is incomplete or malformed.', + ); + this.log.error(`${message} (${getFileLockPath(envDirPath)})`); + throw new Error(message); } private async getPhysicalOwnedCacheRootPath(cacheRoot: Uri): Promise { @@ -1627,6 +1685,109 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return resolvedEntryPath; } + private deleteCacheEntryForClear(entryPath: string): Promise { + return fs.remove(entryPath); + } + + private async getInvalidatedAssociationPaths( + scriptPaths: ReadonlySet, + persistedAssociations: PersistedInlineScriptEnvironments, + removedCacheEntries: ReadonlySet, + ): Promise> { + const invalidatedScriptPaths = new Set(); + for (const scriptPath of scriptPaths) { + const environmentPaths = [ + persistedAssociations[scriptPath], + this.fsPathToPersistedEnvPath.get(scriptPath), + this.fsPathToEnv.get(scriptPath)?.environmentPath.fsPath, + ].filter((value): value is string => value !== undefined); + const states = await Promise.all( + environmentPaths.map((environmentPath) => + this.isRemovedOrMissingCacheAssociation(environmentPath, removedCacheEntries), + ), + ); + if (states.some((state) => state)) { + invalidatedScriptPaths.add(scriptPath); + } + } + return invalidatedScriptPaths; + } + + private async isRemovedOrMissingCacheAssociation( + environmentPath: string, + removedCacheEntries: ReadonlySet, + ): Promise { + const envDirPath = path.dirname(path.dirname(environmentPath)); + if (removedCacheEntries.has(normalizePath(envDirPath))) { + return true; + } + try { + return !(await fs.pathExists(environmentPath)); + } catch (error) { + this.log.warn( + `Unable to verify inline-script environment association ${environmentPath}: ${getErrorMessage(error)}`, + ); + return false; + } + } + + private async clearInvalidatedAssociations( + invalidatedScriptPaths: ReadonlySet, + persistedAssociations: PersistedInlineScriptEnvironments, + priorSelections: ReadonlyMap, + ): Promise { + if (invalidatedScriptPaths.size === 0) { + if (Object.keys(persistedAssociations).length > 0) { + return undefined; + } + try { + await this.enqueuePersistence(async (state) => state.clear([INLINE_SCRIPT_ENVS_KEY])); + return undefined; + } catch (error) { + this.log.error(`Failed to clear inline-script environment associations: ${getErrorMessage(error)}`); + return error; + } + } + + let persistenceError: unknown; + const persistedPathsToClear = Array.from(invalidatedScriptPaths).filter( + (scriptPath) => persistedAssociations[scriptPath] !== undefined, + ); + try { + if (persistedPathsToClear.length === Object.keys(persistedAssociations).length) { + await this.enqueuePersistence(async (state) => state.clear([INLINE_SCRIPT_ENVS_KEY])); + } else if (persistedPathsToClear.length > 0) { + await this.updatePersistedAssociations( + persistedPathsToClear.map((scriptPath) => ({ + scriptPath, + expectedEnvironmentPath: persistedAssociations[scriptPath], + })), + ); + } + } catch (error) { + persistenceError = error; + this.log.error(`Failed to clear inline-script environment associations: ${getErrorMessage(error)}`); + } + + for (const scriptPath of invalidatedScriptPaths) { + this.bumpAssociationRevision(scriptPath); + this.pendingRehydrations.delete(scriptPath); + this.fsPathToEnv.delete(scriptPath); + this.fsPathToPersistedEnvPath.delete(scriptPath); + this.cachedAssociationValidatedAt.delete(scriptPath); + + const environment = priorSelections.get(scriptPath); + if (environment) { + this._onDidChangeEnvironment.fire({ + uri: Uri.file(scriptPath), + old: environment, + new: undefined, + }); + } + } + return persistenceError; + } + private async getPersistedAssociationSnapshot(): Promise { await this.persistenceQueue; const state = await getWorkspacePersistentState(); diff --git a/src/test/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index f47ceb15a..ae2a1c54f 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/src/test/features/envCommands.unit.test.ts @@ -316,6 +316,28 @@ suite('Clear Script Environment Cache Command Tests', () => { sinon.assert.calledOnceWithExactly(removeInlineSettings, [inlineProject]); sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); }); + + test('preserves project settings when cache cleanup reports a partial failure', async () => { + const clearCache = sinon.stub().rejects(new Error('one cache entry could not be deleted')); + const envManagers = { + getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ + supportsClearCache: () => true, + clearCache, + }), + } as unknown as EnvironmentManagers; + const projectManager = { + getProjects: sinon.stub().returns([]), + remove: sinon.stub(), + } as unknown as PythonProjectManager; + sinon.stub(windowApis, 'showWarningMessage').resolves('Clear Cache' as never); + const removeInlineSettings = sinon.stub(settingHelpers, 'removeInlineScriptPythonProjectSettings').resolves([]); + + await assert.rejects(clearScriptEnvironmentCacheCommand(envManagers, projectManager), /could not be deleted/); + + sinon.assert.calledOnce(clearCache); + sinon.assert.notCalled(removeInlineSettings); + sinon.assert.notCalled(projectManager.remove as sinon.SinonStub); + }); }); suite('Reveal Env In Manager View Command Tests', () => { diff --git a/src/test/features/envManagers.unit.test.ts b/src/test/features/envManagers.unit.test.ts index d642fa948..aafe89f27 100644 --- a/src/test/features/envManagers.unit.test.ts +++ b/src/test/features/envManagers.unit.test.ts @@ -336,3 +336,61 @@ suite('PythonEnvironmentManagers - refreshEnvironment', () => { await envManagers.refreshEnvironment(Uri.file('/unknown/path')); }); }); + +suite('PythonEnvironmentManagers - clearCache', () => { + let sandbox: sinon.SinonSandbox; + let envManagers: PythonEnvironmentManagers; + + setup(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(frameUtils, 'getCallingExtension').returns('ms-python.python'); + envManagers = new PythonEnvironmentManagers({ + get: sandbox.stub().returns(undefined), + getProjects: sandbox.stub().returns([]), + } as unknown as PythonProjectManager); + }); + + teardown(() => { + sandbox.restore(); + }); + + function registerManager(name: string, clearCache: sinon.SinonStub): void { + envManagers.registerEnvironmentManager( + { + name, + displayName: name, + preferredPackageManagerId: 'ms-python.python:pip', + get: sandbox.stub().resolves(undefined), + set: sandbox.stub().resolves(), + resolve: sandbox.stub().resolves(undefined), + refresh: sandbox.stub().resolves(), + getEnvironments: sandbox.stub().resolves([]), + clearCache, + onDidChangeEnvironments: sandbox.stub().returns({ dispose: () => {} }), + onDidChangeEnvironment: sandbox.stub().returns({ dispose: () => {} }), + } as any, + { extensionId: 'ms-python.python' }, + ); + } + + test('clears every existing manager when the inline preview manager is absent', async () => { + const systemClearCache = sandbox.stub().resolves(); + registerManager('system', systemClearCache); + + await envManagers.clearCache(undefined); + + sinon.assert.calledOnce(systemClearCache); + }); + + test('does not clear the preview inline manager through the generic command path', async () => { + const systemClearCache = sandbox.stub().resolves(); + const inlineClearCache = sandbox.stub().resolves(); + registerManager('system', systemClearCache); + registerManager('inline-script', inlineClearCache); + + await envManagers.clearCache(undefined); + + sinon.assert.calledOnce(systemClearCache); + sinon.assert.notCalled(inlineClearCache); + }); +}); diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index 35c63f330..f75bc5b3c 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -654,16 +654,22 @@ suite('Setting Helpers - Project Removal', () => { function createProjectConfig(options: { workspaceName: string; + globalValue?: PythonProjectSettings[]; workspaceValue?: PythonProjectSettings[]; workspaceFolderValue?: PythonProjectSettings[]; }): MockWorkspaceConfiguration { const mockConfig = new MockWorkspaceConfiguration(); - const mergedProjects = [...(options.workspaceValue ?? []), ...(options.workspaceFolderValue ?? [])]; + const mergedProjects = [ + ...(options.globalValue ?? []), + ...(options.workspaceValue ?? []), + ...(options.workspaceFolderValue ?? []), + ]; (mockConfig as any).get = (key: string, defaultValue?: T): T | undefined => key === 'pythonProjects' ? (mergedProjects as unknown as T) : defaultValue; (mockConfig as any).inspect = (key: string) => key === 'pythonProjects' ? { + globalValue: options.globalValue, workspaceValue: options.workspaceValue, workspaceFolderValue: options.workspaceFolderValue, } @@ -1069,6 +1075,87 @@ suite('Setting Helpers - Project Removal', () => { 'Should update the same configuration scope that originally contained each project entry', ); }); + + test('removes global inline entries once while preserving higher-precedence non-inline entries', async () => { + const globalProject = new PythonProjectsImpl( + 'global.py', + Uri.file(path.join(firstWorkspacePath, 'global.py')), + ); + const workspaceProject = new PythonProjectsImpl( + 'workspace.py', + Uri.file(path.join(firstWorkspacePath, 'workspace.py')), + ); + const folderProject = new PythonProjectsImpl( + 'folder.py', + Uri.file(path.join(secondWorkspaceUri.fsPath, 'folder.py')), + ); + const firstConfig = createProjectConfig({ + workspaceName: firstWorkspace.name, + globalValue: [ + { path: 'global.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceValue: [ + { path: 'workspace.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceFolderValue: [ + { path: 'global.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + const secondConfig = createProjectConfig({ + workspaceName: secondWorkspace.name, + globalValue: [ + { path: 'global.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + workspaceFolderValue: [ + { path: 'folder.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns([firstWorkspace, secondWorkspace]); + sinon.stub(workspaceApis, 'getWorkspaceFolder').callsFake((uri) => + uri.fsPath.startsWith(secondWorkspaceUri.fsPath) ? secondWorkspace : firstWorkspace, + ); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + const uri = scope as Uri; + return uri.fsPath === secondWorkspaceUri.fsPath ? secondConfig : firstConfig; + }); + + const removedProjects = await removeInlineScriptPythonProjectSettings([ + globalProject, + workspaceProject, + folderProject, + ]); + + assert.deepStrictEqual( + removedProjects.map((project) => project.uri.fsPath).sort(), + [workspaceProject.uri.fsPath, folderProject.uri.fsPath].sort(), + 'The folder-level non-inline entry keeps the global project loaded', + ); + const globalUpdates = updateCalls.filter((call) => call.target === ConfigurationTarget.Global); + assert.strictEqual(globalUpdates.length, 1, 'Global settings should be updated exactly once'); + assert.deepStrictEqual(globalUpdates[0].value, [ + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ]); + assert.ok( + updateCalls.some( + (call) => + call.workspace === firstWorkspace.name && + call.target === ConfigurationTarget.Workspace && + call.value === undefined, + ), + 'Workspace-scoped inline entry should be removed at its source', + ); + assert.ok( + updateCalls.some( + (call) => + call.workspace === secondWorkspace.name && + call.target === ConfigurationTarget.WorkspaceFolder && + call.value === undefined, + ), + 'Folder-scoped inline entry should be removed at its source', + ); + }); }); }); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index f9054d104..9b30eb4ad 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -2678,6 +2678,7 @@ suite('InlineScriptEnvManager', () => { }); test('refuses to clear while a cached environment is locked', async () => { + lockStub.restore(); const uri = scriptUri(); const environment = await createOwnedEnvironment(); await manager.set(uri, environment); @@ -2694,6 +2695,7 @@ suite('InlineScriptEnvManager', () => { }); test('clears a retained lock and its corresponding cache entry', async () => { + lockStub.restore(); const retainedCacheDir = envDir().fsPath; const retainedLockPath = lockfileApis.getFileLockPath(retainedCacheDir); await fs.outputFile(venvPythonPath(retainedCacheDir), ''); @@ -2707,6 +2709,7 @@ suite('InlineScriptEnvManager', () => { }); test('clears a stale owner lock and its corresponding cache entry', async () => { + lockStub.restore(); const uri = scriptUri(); const environment = await createOwnedEnvironment(); await manager.set(uri, environment); @@ -2728,7 +2731,50 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.get(uri), undefined); }); + test('does not delete an entry when another host acquires a new lock after stale lock reclamation', async () => { + lockStub.restore(); + const environment = await createOwnedEnvironment(); + const lockPath = lockfileApis.getFileLockPath(environment.sysPrefix); + const quarantinedLockPath = `${lockPath}.reclaimed-for-test`; + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, 'owner-424242-dead'), ''); + sinon.stub(lockfileApis, 'inspectFileLock').onFirstCall().resolves('stale').onSecondCall().resolves('held'); + sinon.stub(lockfileApis, 'reclaimFileLock').callsFake(async () => { + await fs.rename(lockPath, quarantinedLockPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `owner-${process.pid}-live`), ''); + return true; + }); + + await assert.rejects(manager.clearCache(), /being created/); + + assert.strictEqual(await fs.pathExists(environment.sysPrefix), true); + assert.strictEqual(await fs.pathExists(lockPath), true); + }); + + test('holds the entry lock through deletion', async () => { + lockStub.restore(); + const environment = await createOwnedEnvironment(); + const lockPath = lockfileApis.getFileLockPath(environment.sysPrefix); + const internalManager = manager as unknown as { + deleteCacheEntryForClear(entryPath: string): Promise; + }; + const removeStub = sinon.stub(internalManager, 'deleteCacheEntryForClear').callThrough(); + removeStub.callsFake(async (target) => { + if (normalizePath(target) === normalizePath(environment.sysPrefix)) { + assert.strictEqual(await fs.pathExists(lockPath), true, 'entry lock must protect deletion'); + } + await fs.remove(target); + }); + + await manager.clearCache(); + + sinon.assert.calledWith(removeStub, environment.sysPrefix); + assert.strictEqual(await fs.pathExists(environment.sysPrefix), false); + }); + test('rejects an orphaned lock directory conservatively', async () => { + lockStub.restore(); const uri = scriptUri(); const environment = await createOwnedEnvironment(); await manager.set(uri, environment); @@ -2758,6 +2804,55 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(listener.firstCall.args[0].new, undefined); }); + test('preserves associations and emits events only for entries removed before a partial failure', async () => { + const firstUri = scriptUri('first.py'); + const secondUri = scriptUri('second.py'); + const firstEnvironment = await createOwnedEnvironment(); + const secondEnvironment = await createOwnedEnvironment('fedcba9876543210'); + await manager.set(firstUri, firstEnvironment); + await manager.set(secondUri, secondEnvironment); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + const internalManager = manager as unknown as { + deleteCacheEntryForClear(entryPath: string): Promise; + }; + sinon.stub(internalManager, 'deleteCacheEntryForClear').callsFake(async (target) => { + if (normalizePath(target) === normalizePath(secondEnvironment.sysPrefix)) { + throw new Error('second entry is busy'); + } + await fs.remove(target); + }); + + await assert.rejects(manager.clearCache(), /Failed to completely clear/); + + assert.strictEqual(await fs.pathExists(firstEnvironment.sysPrefix), false); + assert.strictEqual(await fs.pathExists(secondEnvironment.sysPrefix), true); + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(secondUri.fsPath)]: secondEnvironment.environmentPath.fsPath, + }); + assert.strictEqual(await manager.get(firstUri), undefined); + assert.strictEqual(await manager.get(secondUri), secondEnvironment); + sinon.assert.calledOnce(listener); + assert.strictEqual(normalizePath(listener.firstCall.args[0].uri.fsPath), normalizePath(firstUri.fsPath)); + assert.strictEqual(listener.firstCall.args[0].old, firstEnvironment); + assert.strictEqual(listener.firstCall.args[0].new, undefined); + }); + + test('stops before deletion when the physical cache root changes', async () => { + const environment = await createOwnedEnvironment(); + const otherPhysicalRoot = path.join(tempRoot, 'other-cache-root'); + await fs.ensureDir(otherPhysicalRoot); + const internalManager = manager as unknown as { + getPhysicalOwnedCacheRootPath(cacheRoot: Uri): Promise; + }; + const rootStub = sinon.stub(internalManager, 'getPhysicalOwnedCacheRootPath').callThrough(); + rootStub.onSecondCall().resolves(otherPhysicalRoot); + + await assert.rejects(manager.clearCache(), /physical root changed/); + + assert.strictEqual(await fs.pathExists(environment.sysPrefix), true); + }); + test('does not let a pending rehydration restore an association after clear cache', async () => { const uri = scriptUri(); const environment = await createOwnedEnvironment(); From e0655327bd439bafd8d0f773080f296554e14d13 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 20:00:01 -0700 Subject: [PATCH 5/6] Make lock reclamation generation-safe Claim exact stale or retained lock markers before inline cache cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2 --- src/common/lockfile.apis.ts | 124 ++++++++++++------ src/test/common/lockfile.apis.unit.test.ts | 81 +++++++++++- .../inlineScript/envManager.unit.test.ts | 24 +++- 3 files changed, 176 insertions(+), 53 deletions(-) diff --git a/src/common/lockfile.apis.ts b/src/common/lockfile.apis.ts index 62dc6aaeb..cb8ff8032 100644 --- a/src/common/lockfile.apis.ts +++ b/src/common/lockfile.apis.ts @@ -18,6 +18,8 @@ export interface AcquiredFileLock { export const FILE_LOCK_DIR_SUFFIX = '.lock'; export const FILE_LOCK_OWNER_MARKER_PREFIX = 'owner-'; +export const FILE_LOCK_RETAINED_MARKER_PREFIX = 'retained-'; +/** Legacy retained marker. It remains recognizable but cannot be safely reclaimed. */ export const FILE_LOCK_RETAINED_MARKER = 'retained'; export type ProcessLiveness = 'live' | 'dead' | 'unavailable'; @@ -40,7 +42,7 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock lockPath, `${FILE_LOCK_OWNER_MARKER_PREFIX}${process.pid}-${crypto.randomBytes(16).toString('hex')}`, ); - const retainedMarker = path.join(lockPath, FILE_LOCK_RETAINED_MARKER); + const retainedMarker = path.join(lockPath, getRetainedMarkerName(path.basename(ownerMarker))); const deadline = Date.now() + options.timeoutMs; while (true) { @@ -69,22 +71,9 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock } state = 'retained'; try { - await fsapi.writeFile(retainedMarker, '', { flag: 'wx' }); - } catch (error) { - if (hasErrorCode(error, 'EEXIST')) { - return; - } - try { - await fsapi.rename(ownerMarker, retainedMarker); - } catch (renameError) { - if (!hasErrorCode(renameError, 'EEXIST')) { - throw createLockError( - 'Failed to mark the lock as retained', - 'ERETAINFAILED', - lockPath, - ); - } - } + await fsapi.rename(ownerMarker, retainedMarker); + } catch (_error) { + throw createLockError('Failed to mark the lock as retained', 'ERETAINFAILED', lockPath); } }, release: async () => { @@ -119,6 +108,19 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock } export async function inspectFileLock(filePath: string, options?: InspectFileLockOptions): Promise { + return (await inspectFileLockSnapshot(filePath, options)).state; +} + +interface FileLockSnapshot { + readonly state: FileLockState; + readonly marker?: string; + readonly markerKind?: 'owner' | 'retained'; +} + +async function inspectFileLockSnapshot( + filePath: string, + options?: InspectFileLockOptions, +): Promise { const lockPath = getFileLockPath(filePath); let stat; @@ -126,65 +128,97 @@ export async function inspectFileLock(filePath: string, options?: InspectFileLoc stat = await fsapi.lstat(lockPath); } catch (error) { if (hasErrorCode(error, 'ENOENT')) { - return 'missing'; + return { state: 'missing' }; } throw error; } if (!stat.isDirectory() || stat.isSymbolicLink()) { - return 'malformed'; + return { state: 'malformed' }; } const entries = await fsapi.readdir(lockPath); const ownerEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX)); + const generationRetainedEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_RETAINED_MARKER_PREFIX)); const retainedEntries = entries.filter((entry) => entry === FILE_LOCK_RETAINED_MARKER); const unknownEntries = entries.filter( - (entry) => !entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX) && entry !== FILE_LOCK_RETAINED_MARKER, + (entry) => + !entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX) && + !entry.startsWith(FILE_LOCK_RETAINED_MARKER_PREFIX) && + entry !== FILE_LOCK_RETAINED_MARKER, ); - if (unknownEntries.length > 0 || ownerEntries.length > 1 || retainedEntries.length > 1) { - return 'malformed'; + if ( + unknownEntries.length > 0 || + ownerEntries.length > 1 || + generationRetainedEntries.length > 1 || + retainedEntries.length > 1 || + generationRetainedEntries.length + retainedEntries.length > 1 || + generationRetainedEntries.length + ownerEntries.length > 1 + ) { + return { state: 'malformed' }; } if (retainedEntries.length === 1) { - return 'retained'; + return { state: 'retained' }; + } + if (generationRetainedEntries.length === 1) { + const retainedPid = parseMarkerPid(generationRetainedEntries[0], FILE_LOCK_RETAINED_MARKER_PREFIX); + if (retainedPid === undefined) { + return { state: 'malformed' }; + } + return { state: 'retained', marker: generationRetainedEntries[0], markerKind: 'retained' }; } if (ownerEntries.length === 1) { - const ownerPid = parseOwnerPid(ownerEntries[0]); + const ownerPid = parseMarkerPid(ownerEntries[0], FILE_LOCK_OWNER_MARKER_PREFIX); if (ownerPid === undefined) { - return 'malformed'; + return { state: 'malformed' }; } const liveness = await (options?.checkProcessLiveness ?? getProcessLiveness)(ownerPid); if (liveness === 'dead') { - return 'stale'; + return { state: 'stale', marker: ownerEntries[0], markerKind: 'owner' }; } - return liveness === 'live' ? 'held' : 'unavailable'; + return { state: liveness === 'live' ? 'held' : 'unavailable', marker: ownerEntries[0], markerKind: 'owner' }; } - return 'orphaned'; + return { state: 'orphaned' }; } /** - * Move a stale or retained lock out of the lock name before a replacement owner is acquired. - * The rename prevents a newly-created lock from being removed based on an earlier inspection. + * Claim and remove the exact observed stale or retained generation without releasing the lock directory. */ -export async function reclaimFileLock(filePath: string): Promise { +export async function reclaimFileLock(filePath: string, options?: InspectFileLockOptions): Promise { const lockPath = getFileLockPath(filePath); - const state = await inspectFileLock(filePath); - if (state !== 'stale' && state !== 'retained') { + const snapshot = await inspectFileLockSnapshot(filePath, options); + if ( + (snapshot.state !== 'stale' && snapshot.state !== 'retained') || + !snapshot.marker || + !snapshot.markerKind + ) { return false; } - const quarantinedLockPath = `${lockPath}.reclaimed-${process.pid}-${crypto.randomBytes(16).toString('hex')}`; + const claimedMarker = path.join( + lockPath, + `.reclaim-${process.pid}-${crypto.randomBytes(16).toString('hex')}-${snapshot.marker}`, + ); try { - await fsapi.rename(lockPath, quarantinedLockPath); + await fsapi.rename(path.join(lockPath, snapshot.marker), claimedMarker); } catch (error) { - if (hasErrorCode(error, 'ENOENT')) { + if (hasErrorCode(error, 'ENOENT') || hasErrorCode(error, 'EEXIST')) { return false; } throw error; } - await fsapi.remove(quarantinedLockPath); - return true; + try { + await fsapi.unlink(claimedMarker); + await fsapi.rmdir(lockPath); + return true; + } catch (error) { + if (hasErrorCode(error, 'ENOENT') || hasErrorCode(error, 'ENOTEMPTY')) { + return false; + } + throw error; + } } export async function getProcessLiveness(pid: number): Promise { @@ -204,8 +238,10 @@ export async function getProcessLiveness(pid: number): Promise async function isRetainedLock(lockPath: string): Promise { try { - await fsapi.lstat(path.join(lockPath, FILE_LOCK_RETAINED_MARKER)); - return true; + const entries = await fsapi.readdir(lockPath); + return entries.some( + (entry) => entry === FILE_LOCK_RETAINED_MARKER || entry.startsWith(FILE_LOCK_RETAINED_MARKER_PREFIX), + ); } catch (error) { if (hasErrorCode(error, 'ENOENT')) { return false; @@ -220,8 +256,12 @@ function hasErrorCode(error: unknown, code: string): boolean { ); } -function parseOwnerPid(entry: string): number | undefined { - const match = entry.match(new RegExp(`^${escapeRegExp(FILE_LOCK_OWNER_MARKER_PREFIX)}(\\d+)-`)); +function getRetainedMarkerName(ownerMarker: string): string { + return `${FILE_LOCK_RETAINED_MARKER_PREFIX}${ownerMarker.slice(FILE_LOCK_OWNER_MARKER_PREFIX.length)}`; +} + +function parseMarkerPid(entry: string, prefix: string): number | undefined { + const match = entry.match(new RegExp(`^${escapeRegExp(prefix)}(\\d+)-.+$`)); if (!match) { return undefined; } diff --git a/src/test/common/lockfile.apis.unit.test.ts b/src/test/common/lockfile.apis.unit.test.ts index df8c5acc4..a0230a230 100644 --- a/src/test/common/lockfile.apis.unit.test.ts +++ b/src/test/common/lockfile.apis.unit.test.ts @@ -12,8 +12,11 @@ import { acquireFileLock, AcquireFileLockOptions, FILE_LOCK_OWNER_MARKER_PREFIX, + FILE_LOCK_RETAINED_MARKER, + FILE_LOCK_RETAINED_MARKER_PREFIX, getFileLockPath, inspectFileLock, + reclaimFileLock, } from '../../common/lockfile.apis'; const OPTIONS: AcquireFileLockOptions = { @@ -171,29 +174,29 @@ suite('lockfile APIs', () => { assert.ok(Date.now() - startedAt < 1_000); const lockPath = `${path.resolve(targetPath)}.lock`; const retainedEntries = await fs.readdir(lockPath); - assert.ok(retainedEntries.includes('retained')); - assert.strictEqual(retainedEntries.filter((entry) => entry.startsWith('owner-')).length, 1); + assert.strictEqual(retainedEntries.length, 1); + assert.ok(retainedEntries[0].startsWith(`${FILE_LOCK_RETAINED_MARKER_PREFIX}${process.pid}-`)); await lock.release(); assert.deepStrictEqual(await fs.readdir(lockPath), retainedEntries); }); - test('falls back to renaming the owner marker when the retained sentinel cannot be written', async () => { + test('atomically converts the owner marker into a generation-specific retained marker', async () => { const lock = await acquireFileLock(targetPath, OPTIONS); - sinon.stub(fsExtra, 'writeFile').rejects(Object.assign(new Error('write failed'), { code: 'EACCES' })); await lock.retain(); const lockPath = `${path.resolve(targetPath)}.lock`; - assert.deepStrictEqual(await fs.readdir(lockPath), ['retained']); + const entries = await fs.readdir(lockPath); + assert.strictEqual(entries.length, 1); + assert.ok(entries[0].startsWith(`${FILE_LOCK_RETAINED_MARKER_PREFIX}${process.pid}-`)); await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { return error.code === 'ELOCKRETAINED'; }); }); - test('remains fail-closed when neither retained-marker strategy succeeds', async () => { + test('remains fail-closed when retaining the generation marker fails', async () => { const lock = await acquireFileLock(targetPath, OPTIONS); - sinon.stub(fsExtra, 'writeFile').rejects(Object.assign(new Error('write failed'), { code: 'EACCES' })); sinon.stub(fsExtra, 'rename').rejects(Object.assign(new Error('rename failed'), { code: 'EBUSY' })); await assert.rejects(lock.retain(), (error: NodeJS.ErrnoException) => error.code === 'ERETAINFAILED'); @@ -233,6 +236,70 @@ suite('lockfile APIs', () => { assert.strictEqual(await inspectFileLock(targetPath), 'retained'); }); + test('reclaims a generation-specific retained lock', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + await lock.retain(); + + assert.strictEqual(await reclaimFileLock(targetPath), true); + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + const replacement = await acquireFileLock(targetPath, OPTIONS); + await replacement.release(); + }); + + test('refuses to reclaim the ambiguous legacy retained marker', async () => { + const lockPath = getFileLockPath(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, `${FILE_LOCK_OWNER_MARKER_PREFIX}${process.pid}-legacy`), ''); + await fs.writeFile(path.join(lockPath, FILE_LOCK_RETAINED_MARKER), ''); + + assert.strictEqual(await inspectFileLock(targetPath), 'retained'); + assert.strictEqual(await reclaimFileLock(targetPath), false); + assert.strictEqual(await fs.pathExists(path.join(lockPath, FILE_LOCK_RETAINED_MARKER)), true); + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKRETAINED'; + }); + }); + + test('does not touch a new generation when a delayed reclaimer loses its marker claim', async () => { + const lockPath = getFileLockPath(targetPath); + const staleMarker = `${FILE_LOCK_OWNER_MARKER_PREFIX}424242-dead`; + await fs.ensureDir(targetPath); + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, staleMarker), ''); + const rename = fsExtra.rename; + let releaseFirstClaim: (() => void) | undefined; + let firstClaimStarted: (() => void) | undefined; + const firstClaim = new Promise((resolve) => { + firstClaimStarted = resolve; + }); + const releaseClaim = new Promise((resolve) => { + releaseFirstClaim = resolve; + }); + let renameCount = 0; + sinon.stub(fsExtra, 'rename').callsFake(async (source, destination) => { + renameCount += 1; + if (renameCount === 1) { + firstClaimStarted!(); + await releaseClaim; + } + await rename(source, destination); + }); + + const staleInspection = { checkProcessLiveness: sinon.stub().resolves('dead') }; + const delayedReclaimer = reclaimFileLock(targetPath, staleInspection); + await firstClaim; + assert.strictEqual(await reclaimFileLock(targetPath, staleInspection), true); + const replacement = await acquireFileLock(targetPath, OPTIONS); + const replacementEntries = await fs.readdir(lockPath); + + releaseFirstClaim!(); + assert.strictEqual(await delayedReclaimer, false); + assert.deepStrictEqual(await fs.readdir(lockPath), replacementEntries); + assert.strictEqual(await fs.pathExists(targetPath), true); + + await replacement.release(); + }); + test('classifies a dead owner marker as stale using the liveness probe', async () => { const lockPath = getFileLockPath(targetPath); await fs.ensureDir(lockPath); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 9b30eb4ad..5dcefdb41 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -2694,7 +2694,23 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.get(uri), environment); }); - test('clears a retained lock and its corresponding cache entry', async () => { + test('clears a generation-specific retained lock and its corresponding cache entry', async () => { + lockStub.restore(); + const retainedCacheDir = envDir().fsPath; + await fs.outputFile(venvPythonPath(retainedCacheDir), ''); + const lock = await lockfileApis.acquireFileLock(retainedCacheDir, { + timeoutMs: 0, + retryIntervalMs: 1, + }); + await lock.retain(); + + await manager.clearCache(); + + assert.strictEqual(await fs.pathExists(retainedCacheDir), false); + assert.strictEqual(await fs.pathExists(lockfileApis.getFileLockPath(retainedCacheDir)), false); + }); + + test('refuses to clear a legacy retained lock conservatively', async () => { lockStub.restore(); const retainedCacheDir = envDir().fsPath; const retainedLockPath = lockfileApis.getFileLockPath(retainedCacheDir); @@ -2702,10 +2718,10 @@ suite('InlineScriptEnvManager', () => { await fs.ensureDir(retainedLockPath); await fs.writeFile(path.join(retainedLockPath, 'retained'), ''); - await manager.clearCache(); + await assert.rejects(manager.clearCache(), /incomplete or malformed/); - assert.strictEqual(await fs.pathExists(retainedCacheDir), false); - assert.strictEqual(await fs.pathExists(retainedLockPath), false); + assert.strictEqual(await fs.pathExists(retainedCacheDir), true); + assert.strictEqual(await fs.pathExists(retainedLockPath), true); }); test('clears a stale owner lock and its corresponding cache entry', async () => { From 4826e87786db85cd9c57c21af08b69d08f82c217 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Wed, 19 Aug 2026 10:34:21 -0700 Subject: [PATCH 6/6] Clear global inline projects without workspace Handle user-scope project cleanup independently of open workspace folders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2 --- src/features/settings/settingHelpers.ts | 43 +++++++++---------- .../settings/settingHelpers.unit.test.ts | 27 ++++++++++++ 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/src/features/settings/settingHelpers.ts b/src/features/settings/settingHelpers.ts index 24189aacb..bbea301db 100644 --- a/src/features/settings/settingHelpers.ts +++ b/src/features/settings/settingHelpers.ts @@ -515,8 +515,26 @@ export async function removeInlineScriptPythonProjectSettings( currentProjects: readonly PythonProject[], ): Promise { const currentProjectsByUri = new Map(currentProjects.map((project) => [project.uri.toString(), project] as const)); + const workspaceFolders = workspaceApis.getWorkspaceFolders() ?? []; + const globalConfig = workspaceApis.getConfiguration('python-envs', workspaceFolders[0]?.uri); + const globalValueOriginal = cloneProjectSettings( + globalConfig.inspect('pythonProjects')?.globalValue, + ); + const globalValueRemaining = + globalValueOriginal?.filter((projectSetting) => projectSetting.envManager !== INLINE_SCRIPT_MANAGER_ID) ?? []; + const promises: Thenable[] = []; + if (globalValueOriginal !== undefined && globalValueRemaining.length !== globalValueOriginal.length) { + promises.push( + globalConfig.update( + 'pythonProjects', + globalValueRemaining.length > 0 ? globalValueRemaining : undefined, + ConfigurationTarget.Global, + ), + ); + } + const workspaceEntries: Array = []; - for (const workspaceFolder of workspaceApis.getWorkspaceFolders() ?? []) { + for (const workspaceFolder of workspaceFolders) { const edits: EditProjectSettings[] = getResolvedPythonProjectSettings(workspaceFolder) .filter((resolvedSetting) => resolvedSetting.sources.some((source) => source.setting.envManager === INLINE_SCRIPT_MANAGER_ID), @@ -536,23 +554,19 @@ export async function removeInlineScriptPythonProjectSettings( } if (workspaceEntries.length === 0) { + await Promise.all(promises); return []; } const removedProjects = new Map(); const folderRemainingSettings = new Map(); const folderExistingSettings = new Map(); - const promises: Thenable[] = []; - let globalConfig: WorkspaceConfiguration | undefined; - let globalValueOriginal: PythonProjectSettings[] | undefined; let workspaceConfig: WorkspaceConfiguration | undefined; let workspaceValueOriginal: PythonProjectSettings[] | undefined; workspaceEntries.forEach(([workspaceFolder, edits]) => { const config = workspaceApis.getConfiguration('python-envs', workspaceFolder.uri); const projectsInspect = config.inspect('pythonProjects'); - globalConfig ??= config; - globalValueOriginal ??= cloneProjectSettings(projectsInspect?.globalValue); workspaceConfig ??= config; workspaceValueOriginal ??= cloneProjectSettings(projectsInspect?.workspaceValue); @@ -577,13 +591,6 @@ export async function removeInlineScriptPythonProjectSettings( const aggregatedEdits = workspaceEntries.flatMap(([workspaceFolder, edits]) => edits.map((edit) => ({ workspaceFolder, edit })), ); - const globalValueRemaining = - globalValueOriginal?.filter( - (projectSetting) => - !aggregatedEdits.some(({ workspaceFolder, edit }) => - matchesProjectSettingEdit(projectSetting, edit, workspaceFolder), - ), - ) ?? []; const workspaceValueRemaining = workspaceValueOriginal?.filter( (projectSetting) => @@ -592,16 +599,6 @@ export async function removeInlineScriptPythonProjectSettings( ), ) ?? []; - if (globalConfig && globalValueOriginal !== undefined && globalValueRemaining.length !== globalValueOriginal.length) { - promises.push( - globalConfig.update( - 'pythonProjects', - globalValueRemaining.length > 0 ? globalValueRemaining : undefined, - ConfigurationTarget.Global, - ), - ); - } - if ( workspaceConfig && workspaceValueOriginal !== undefined && diff --git a/src/test/features/settings/settingHelpers.unit.test.ts b/src/test/features/settings/settingHelpers.unit.test.ts index f75bc5b3c..b454de792 100644 --- a/src/test/features/settings/settingHelpers.unit.test.ts +++ b/src/test/features/settings/settingHelpers.unit.test.ts @@ -800,6 +800,33 @@ suite('Setting Helpers - Project Removal', () => { }); suite('removeInlineScriptPythonProjectSettings', () => { + test('removes global inline entries when no workspace folders are open', async () => { + const config = createProjectConfig({ + workspaceName: 'global', + globalValue: [ + { path: 'script.py', envManager: INLINE_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + { path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }, + ], + }); + sinon.stub(workspaceApis, 'getWorkspaceFolders').returns(undefined); + sinon.stub(workspaceApis, 'getConfiguration').callsFake((_section?: string, scope?: unknown) => { + assert.strictEqual(scope, undefined); + return config; + }); + + const removedProjects = await removeInlineScriptPythonProjectSettings([]); + + assert.deepStrictEqual(removedProjects, []); + assert.deepStrictEqual(updateCalls, [ + { + workspace: 'global', + key: 'pythonProjects', + value: [{ path: 'keep.py', envManager: VENV_MANAGER_ID, packageManager: PIP_MANAGER_ID }], + target: ConfigurationTarget.Global, + }, + ]); + }); + test('removes all inline-script entries while preserving non-inline duplicates', async () => { const project = new PythonProjectsImpl('script.py', Uri.file(path.join(firstWorkspacePath, 'script.py'))); const otherProject = new PythonProjectsImpl('other.py', Uri.file(path.join(firstWorkspacePath, 'other.py')));