diff --git a/src/common/constants.ts b/src/common/constants.ts index 31894953..087834ae 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -4,6 +4,7 @@ export const ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs'; export const PYTHON_EXTENSION_ID = 'ms-python.python'; export const CONDA_MANAGER_ID = `${PYTHON_EXTENSION_ID}:conda`; export const INLINE_SCRIPT_MANAGER_ID = `${PYTHON_EXTENSION_ID}:inline-script`; +export const INLINE_SCRIPT_ENVS_KEY = `${ENVS_EXTENSION_ID}:inline-script:SCRIPT_ENVIRONMENTS`; export const PYENV_MANAGER_ID = `${PYTHON_EXTENSION_ID}:pyenv`; export const JUPYTER_EXTENSION_ID = 'ms-toolsai.jupyter'; export const EXTENSION_ROOT_DIR = path.dirname(__dirname); diff --git a/src/common/lockfile.apis.ts b/src/common/lockfile.apis.ts index cb8ff803..4fbcc043 100644 --- a/src/common/lockfile.apis.ts +++ b/src/common/lockfile.apis.ts @@ -19,6 +19,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-'; +export const FILE_LOCK_RELEASE_MARKER_PREFIX = '.release-'; +export const FILE_LOCK_RETIRED_DIR_INFIX = '.retired-'; /** Legacy retained marker. It remains recognizable but cannot be safely reclaimed. */ export const FILE_LOCK_RETAINED_MARKER = 'retained'; @@ -29,7 +31,9 @@ export interface InspectFileLockOptions { readonly checkProcessLiveness?: (pid: number) => Promise; } -type LockState = 'held' | 'released' | 'retained'; +type LockState = 'held' | 'releasing' | 'released' | 'retained'; +const LOCK_RETIRE_MAX_ATTEMPTS = 3; +const LOCK_RETIRE_RETRY_MS = 10; export function getFileLockPath(filePath: string): string { return `${path.resolve(filePath)}${FILE_LOCK_DIR_SUFFIX}`; @@ -43,6 +47,8 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock `${FILE_LOCK_OWNER_MARKER_PREFIX}${process.pid}-${crypto.randomBytes(16).toString('hex')}`, ); const retainedMarker = path.join(lockPath, getRetainedMarkerName(path.basename(ownerMarker))); + const releaseMarkerName = + `${FILE_LOCK_RELEASE_MARKER_PREFIX}${process.pid}-${crypto.randomBytes(16).toString('hex')}-${path.basename(ownerMarker)}`; const deadline = Date.now() + options.timeoutMs; while (true) { @@ -64,6 +70,56 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock } let state: LockState = 'held'; + let releaseInFlight: Promise | undefined; + const performRelease = async (): Promise => { + if (state === 'released' || state === 'retained') { + return; + } + const releaseMarkerPath = path.join(lockPath, releaseMarkerName); + if (state === 'held') { + try { + await fsapi.rename(ownerMarker, releaseMarkerPath); + } catch (error) { + if (hasErrorCode(error, 'ENOENT')) { + throw createLockError('Lock ownership was compromised', 'ECOMPROMISED', lockPath); + } + throw error; + } + state = 'releasing'; + } + // state === 'releasing': the release marker exists; retire the canonical + // directory. Resumable: if retirement fails and ownership cannot be restored, + // the handle stays 'releasing' so a later release() retries retirement instead + // of leaving the handle unable to make progress. + const retiredPath = getRetiredLockPath(lockPath); + try { + await retireCanonicalLockDirectory(lockPath, retiredPath); + } catch (error) { + const restored = await fsapi + .rename(releaseMarkerPath, ownerMarker) + .then( + () => true, + () => false, + ); + if (restored) { + state = 'held'; + throw createLockError( + 'Failed to retire the lock directory; ownership was restored', + 'ELOCKRELEASEFAILED', + lockPath, + error, + ); + } + throw createLockError( + 'Failed to retire the lock directory; release can be retried', + 'ELOCKRELEASEFAILED', + lockPath, + error, + ); + } + state = 'released'; + await cleanupRetiredLock(retiredPath, releaseMarkerName); + }; return { retain: async () => { if (state !== 'held') { @@ -76,20 +132,18 @@ export async function acquireFileLock(filePath: string, options: AcquireFileLock throw createLockError('Failed to mark the lock as retained', 'ERETAINFAILED', lockPath); } }, - release: async () => { - if (state !== 'held') { - return; - } - state = 'released'; - try { - await fsapi.unlink(ownerMarker); - } catch (error) { - if (hasErrorCode(error, 'ENOENT')) { - throw createLockError('Lock ownership was compromised', 'ECOMPROMISED', lockPath); - } - throw error; + release: () => { + // Serialize concurrent release() calls on the same handle: without this, + // two callers can both observe state === 'held' before either owner-marker + // rename completes, and the loser sees ENOENT and reports ECOMPROMISED even + // though the lock was validly released. Sharing one in-flight promise de-dupes + // concurrent calls; clearing it on settle preserves retry-after-failure. + if (!releaseInFlight) { + releaseInFlight = performRelease().finally(() => { + releaseInFlight = undefined; + }); } - await fsapi.rmdir(lockPath); + return releaseInFlight; }, }; } catch (error) { @@ -114,7 +168,8 @@ export async function inspectFileLock(filePath: string, options?: InspectFileLoc interface FileLockSnapshot { readonly state: FileLockState; readonly marker?: string; - readonly markerKind?: 'owner' | 'retained'; + readonly markerKind?: 'owner' | 'retained' | 'release'; + readonly generationMarker?: string; } async function inspectFileLockSnapshot( @@ -137,14 +192,24 @@ async function inspectFileLockSnapshot( return { state: 'malformed' }; } - const entries = await fsapi.readdir(lockPath); + let entries: string[]; + try { + entries = await fsapi.readdir(lockPath); + } catch (error) { + if (hasErrorCode(error, 'ENOENT')) { + return { state: 'missing' }; + } + throw error; + } 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 releaseEntries = entries.filter((entry) => entry.startsWith(FILE_LOCK_RELEASE_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.startsWith(FILE_LOCK_RETAINED_MARKER_PREFIX) && + !entry.startsWith(FILE_LOCK_RELEASE_MARKER_PREFIX) && entry !== FILE_LOCK_RETAINED_MARKER, ); @@ -152,9 +217,11 @@ async function inspectFileLockSnapshot( unknownEntries.length > 0 || ownerEntries.length > 1 || generationRetainedEntries.length > 1 || + releaseEntries.length > 1 || retainedEntries.length > 1 || - generationRetainedEntries.length + retainedEntries.length > 1 || - generationRetainedEntries.length + ownerEntries.length > 1 + (retainedEntries.length === 1 && generationRetainedEntries.length + releaseEntries.length > 0) || + (retainedEntries.length === 0 && + ownerEntries.length + generationRetainedEntries.length + releaseEntries.length > 1) ) { return { state: 'malformed' }; } @@ -168,6 +235,27 @@ async function inspectFileLockSnapshot( } return { state: 'retained', marker: generationRetainedEntries[0], markerKind: 'retained' }; } + if (releaseEntries.length === 1) { + const releaseMarker = parseTransitionMarker(releaseEntries[0], FILE_LOCK_RELEASE_MARKER_PREFIX); + if (!releaseMarker) { + return { state: 'malformed' }; + } + const liveness = await (options?.checkProcessLiveness ?? getProcessLiveness)(releaseMarker.pid); + if (liveness === 'dead') { + return { + state: 'stale', + marker: releaseEntries[0], + markerKind: 'release', + generationMarker: releaseMarker.generationMarker, + }; + } + return { + state: liveness === 'live' ? 'held' : 'unavailable', + marker: releaseEntries[0], + markerKind: 'release', + generationMarker: releaseMarker.generationMarker, + }; + } if (ownerEntries.length === 1) { const ownerPid = parseMarkerPid(ownerEntries[0], FILE_LOCK_OWNER_MARKER_PREFIX); if (ownerPid === undefined) { @@ -269,12 +357,80 @@ function parseMarkerPid(entry: string, prefix: string): number | undefined { return Number.isSafeInteger(pid) && pid > 0 ? pid : undefined; } +function parseTransitionMarker( + entry: string, + transitionPrefix: string, +): { readonly pid: number; readonly generationMarker: string } | undefined { + const match = entry.match( + new RegExp( + `^${escapeRegExp(transitionPrefix)}(\\d+)-[0-9a-f]{32}-((?:${escapeRegExp(FILE_LOCK_OWNER_MARKER_PREFIX)}|${escapeRegExp(FILE_LOCK_RETAINED_MARKER_PREFIX)})\\d+-.+)$`, + ), + ); + if (!match) { + return undefined; + } + const pid = Number(match[1]); + const generationMarker = match[2]; + const generationPrefix = generationMarker.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX) + ? FILE_LOCK_OWNER_MARKER_PREFIX + : FILE_LOCK_RETAINED_MARKER_PREFIX; + return Number.isSafeInteger(pid) && + pid > 0 && + parseMarkerPid(generationMarker, generationPrefix) !== undefined + ? { pid, generationMarker } + : undefined; +} + +function getRetiredLockPath(lockPath: string): string { + return `${lockPath}${FILE_LOCK_RETIRED_DIR_INFIX}${process.pid}-${crypto.randomBytes(16).toString('hex')}`; +} + +async function cleanupRetiredLock(retiredPath: string, markerName: string): Promise { + try { + await fsapi.unlink(path.join(retiredPath, markerName)); + await fsapi.rmdir(retiredPath); + } catch { + await fsapi.remove(retiredPath).catch(() => undefined); + } +} + +async function retireCanonicalLockDirectory(lockPath: string, retiredPath: string): Promise { + for (let attempt = 0; attempt < LOCK_RETIRE_MAX_ATTEMPTS; attempt += 1) { + try { + await fsapi.rename(lockPath, retiredPath); + return; + } catch (error) { + if (!isRetirementContentionError(error) || attempt === LOCK_RETIRE_MAX_ATTEMPTS - 1) { + throw error; + } + await delay(LOCK_RETIRE_RETRY_MS); + } + } +} + +function isRetirementContentionError(error: unknown): boolean { + return ( + hasErrorCode(error, 'EPERM') || + hasErrorCode(error, 'EBUSY') || + hasErrorCode(error, 'EACCES') + ); +} + 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 }); +function createLockError( + message: string, + code: string, + lockPath: string, + cause?: unknown, +): NodeJS.ErrnoException { + return Object.assign(new Error(message), { + code, + path: lockPath, + ...(cause === undefined ? {} : { cause }), + }); } async function delay(milliseconds: number): Promise { diff --git a/src/extension.ts b/src/extension.ts index 7465f997..86b1324a 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'; @@ -45,6 +45,7 @@ import { ProjectCreatorsImpl } from './features/creators/projectCreators'; import { addPythonProjectCommand, copyPathToClipboard, + clearEnvironmentCachesCommand, clearScriptEnvironmentCacheCommand, createAnyEnvironmentCommand, createEnvironmentCommand, @@ -78,11 +79,7 @@ import { registerCompletionProvider } from './features/settings/settingCompletio import { migrateGlobalDefaultEnvManagerSetting } from './features/settings/settingHelpers'; import { setActivateMenuButtonContext } from './features/terminal/activateMenuButton'; import { normalizeShellPath } from './features/terminal/shells/common/shellUtils'; -import { - clearShellProfileCache, - createShellEnvProviders, - createShellStartupProviders, -} from './features/terminal/shells/providers'; +import { createShellEnvProviders, createShellStartupProviders } from './features/terminal/shells/providers'; import { ShellStartupActivationVariablesManagerImpl } from './features/terminal/shellStartupActivationVariablesManager'; import { cleanupStartupScripts } from './features/terminal/shellStartupSetupHandlers'; import { TerminalActivationImpl } from './features/terminal/terminalActivationState'; @@ -405,9 +402,7 @@ export async function activate(context: ExtensionContext): Promise { - await clearPersistentState(); - await envManagers.clearCache(undefined); - await clearShellProfileCache(shellStartupProviders); + await clearEnvironmentCachesCommand(envManagers, shellStartupProviders, context.workspaceState); }), ...(isInlineScriptsFeatureEnabled() ? [ @@ -704,6 +699,7 @@ export async function activate(context: ExtensionContext): Promise { + // Preserve the inline-script association key without changing the shared PersistentState + // implementation: clear every current workspace key except the inline key by passing an + // explicit filtered list to the existing `clear(keys)`, alongside the existing global clear. + const [workspacePersistentState, globalPersistentState] = await Promise.all([ + persistentState.getWorkspacePersistentState(), + persistentState.getGlobalPersistentState(), + ]); + const workspaceKeys = workspaceState.keys().filter((key) => key !== INLINE_SCRIPT_ENVS_KEY); + await Promise.all([workspacePersistentState.clear(workspaceKeys), globalPersistentState.clear()]); + await em.clearCache(undefined); + await shellProviders.clearShellProfileCache(startupProviders); +} + export async function clearScriptEnvironmentCacheCommand( em: EnvironmentManagers, wm: PythonProjectManager, diff --git a/src/managers/builtin/inlineScript/associationStore.ts b/src/managers/builtin/inlineScript/associationStore.ts new file mode 100644 index 00000000..e189ec79 --- /dev/null +++ b/src/managers/builtin/inlineScript/associationStore.ts @@ -0,0 +1,89 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Memento } from 'vscode'; +import { INLINE_SCRIPT_ENVS_KEY } from '../../../common/constants'; +import { traceError } from '../../../common/logging'; + +/** + * Accessor bound to the single inline-script association key ({@link INLINE_SCRIPT_ENVS_KEY}). + * + * It is handed to callers inside a serialized {@link InlineScriptAssociationStore.runExclusive} + * transaction so a read and its dependent write execute as one atomic queue entry. Callers must + * only use it within the transaction they were given it in — enqueuing another store operation + * from inside a transaction would deadlock the queue on itself. + */ +export interface InlineAssociationAccessor { + /** Raw read of the association key. */ + get(): Promise; + /** + * Verified write of the association key. Mirrors `PersistentState.set`: after the update it + * reads the value back and, on a JSON mismatch, clears the key and logs. A rejected update + * propagates to the caller. + */ + update(value: T): Promise; +} + +/** + * Inline-script-owned persistence for PEP 723 script-to-environment associations. + * + * The store owns exactly one workspace-state key ({@link INLINE_SCRIPT_ENVS_KEY}) and never + * exposes arbitrary keys. Every high-level read/mutation/deletion runs on an internal + * failure-isolated FIFO queue: operations execute in invocation order, each caller receives its + * own operation's success or failure, and a rejected operation still advances the queue so later + * operations run. + * + * The store depends only on the injected {@link Memento}; it never touches the shared + * `PersistentState` clear gate, so a wedged generic "Clear Cache" cannot block inline association + * work, and a dedicated inline deletion cannot be coalesced onto (and dropped by) a generic clear. + */ +export class InlineScriptAssociationStore { + private tail: Promise = Promise.resolve(); + private readonly accessor: InlineAssociationAccessor; + + constructor(private readonly memento: Memento) { + this.accessor = { + get: async (): Promise => this.memento.get(INLINE_SCRIPT_ENVS_KEY), + update: async (value: T): Promise => { + await this.memento.update(INLINE_SCRIPT_ENVS_KEY, value); + const before = JSON.stringify(value); + const after = JSON.stringify(await this.memento.get(INLINE_SCRIPT_ENVS_KEY)); + if (before !== after) { + await this.memento.update(INLINE_SCRIPT_ENVS_KEY, undefined); + traceError('Error while updating state for key:', INLINE_SCRIPT_ENVS_KEY); + } + }, + }; + } + + /** + * Serialize `operation` on the FIFO queue. The operation receives an accessor bound to the + * single association key so it can perform a read-modify-write as one atomic transaction. The + * caller receives the operation's own result or rejection; a rejection still advances the + * queue tail so subsequent operations run. + */ + runExclusive(operation: (state: InlineAssociationAccessor) => Promise): Promise { + const run = this.tail.then(() => operation(this.accessor)); + this.tail = run.then( + () => undefined, + () => undefined, + ); + return run; + } + + /** Queued raw read of the association key. */ + read(): Promise { + return this.runExclusive((state) => state.get()); + } + + /** + * Queued dedicated deletion of the association key via a direct key update to `undefined`. + * + * Because this is an ordinary queued write on the inline-owned queue (never a shared + * `PersistentState.clear`), it cannot be coalesced onto an in-flight generic clear and then + * dropped; it runs strictly in invocation order and always writes. + */ + clear(): Promise { + return this.runExclusive((state) => state.update(undefined)); + } +} diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 04505bba..38a8ee64 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -5,7 +5,7 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import type { Stats } from 'fs'; import { clean as cleanPep440, satisfies as satisfiesPep440 } from '@renovatebot/pep440'; -import { Disposable, Event, EventEmitter, l10n, LogOutputChannel, MarkdownString, ThemeIcon, Uri } from 'vscode'; +import { Disposable, Event, EventEmitter, l10n, LogOutputChannel, MarkdownString, Memento, ThemeIcon, Uri } from 'vscode'; import { CreateEnvironmentOptions, CreateEnvironmentScope, @@ -49,7 +49,6 @@ import { } from '../../../common/inlineScript/routingRegistry'; import { CONDA_MANAGER_ID, - ENVS_EXTENSION_ID, INLINE_SCRIPT_MANAGER_ID, PYENV_MANAGER_ID, SYSTEM_MANAGER_ID, @@ -62,7 +61,7 @@ import { inspectFileLock, reclaimFileLock, } from '../../../common/lockfile.apis'; -import { getWorkspacePersistentState, PersistentState } from '../../../common/persistentState'; +import { InlineAssociationAccessor, InlineScriptAssociationStore } from './associationStore'; import { EventNames, InlineScriptEnvErrorCategory } from '../../../common/telemetry/constants'; import { sendTelemetryEvent } from '../../../common/telemetry/sender'; import { createDeferred, Deferred } from '../../../common/utils/deferred'; @@ -92,8 +91,6 @@ const CACHE_LOCK_TIMEOUT_MS = 5 * 60 * 1000; const CACHE_LOCK_RETRY_MS = 500; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; const DISCOVERY_RETRY_DELAYS_MS = [1_000, 5_000, 30_000] as const; -/** 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`; const PERSISTED_ASSOCIATION_SCHEMA_VERSION = 1 as const; interface SelectedBaseInterpreter { @@ -196,7 +193,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private discoveryRetryAttempt = 0; private discoveryRetryTimer: ReturnType | undefined; private readonly subscriptions: Disposable[] = []; - private persistenceQueue: Promise = Promise.resolve(); + private readonly associationStore: InlineScriptAssociationStore; private readonly persistedAssociationsLoaded: Promise; private selectionQueue: Promise = Promise.resolve(); private cacheMaintenanceQueue: Promise = Promise.resolve(); @@ -228,8 +225,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly baseManager: EnvironmentManager, private readonly globalStorageUri: Uri, public readonly log: LogOutputChannel, + workspaceState: Memento, private readonly routingRegistry: InlineScriptRoutingRegistry = new InlineScriptRoutingRegistry(), ) { + this.associationStore = new InlineScriptAssociationStore(workspaceState); this.subscriptions.push( this.routingRegistry.onDidChangeMetadata((event) => { void this.handleSavedMetadataChange(event).catch((error) => { @@ -1814,7 +1813,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private loadPersistedAssociations(): Promise { return this.enqueuePersistence(async (state) => { - const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + const rawAssociations = await state.get(); const parsed = this.parsePersistedAssociations(rawAssociations); this.applyPersistedAssociations(parsed?.records ?? {}); }); @@ -1840,9 +1839,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private async getPersistedAssociation(scriptPath: string): Promise { - await this.persistenceQueue; - const state = await getWorkspacePersistentState(); - const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + const rawAssociations = await this.associationStore.read(); if (rawAssociations === undefined) { this.applyPersistedAssociations({}); return undefined; @@ -1905,14 +1902,14 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private removeInvalidPersistedAssociation(scriptPath: string): Promise { return this.enqueuePersistence(async (state) => { - const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + const rawAssociations = await state.get(); if (rawAssociations === undefined) { this.applyPersistedAssociations({}); return; } const parsed = this.parsePersistedAssociations(rawAssociations); if (!parsed) { - await state.set(INLINE_SCRIPT_ENVS_KEY, {}); + await state.update({}); this.applyPersistedAssociations({}); return; } @@ -1920,7 +1917,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { delete parsed.rawEntries[scriptPath]; delete parsed.records[scriptPath]; parsed.invalidKeys.delete(scriptPath); - await state.set(INLINE_SCRIPT_ENVS_KEY, parsed.rawEntries); + await state.update(parsed.rawEntries); } this.applyPersistedAssociations(parsed.records); }); @@ -1928,7 +1925,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private updatePersistedAssociations(changes: readonly PersistedAssociationChange[]): Promise { return this.enqueuePersistence(async (state) => { - const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + const rawAssociations = await state.get(); const parsed = this.parsePersistedAssociations(rawAssociations); const rawEntries = { ...(parsed?.rawEntries ?? {}) }; const associations = { ...(parsed?.records ?? {}) }; @@ -1955,7 +1952,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { delete rawEntries[change.scriptPath]; } } - await state.set(INLINE_SCRIPT_ENVS_KEY, rawEntries); + await state.update(rawEntries); this.applyPersistedAssociations(associations); }); } @@ -2124,10 +2121,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }; } - private enqueuePersistence(operation: (state: PersistentState) => Promise): Promise { - const run = this.persistenceQueue.then(async () => operation(await getWorkspacePersistentState())); - this.persistenceQueue = run.catch(() => undefined); - return run; + private enqueuePersistence(operation: (state: InlineAssociationAccessor) => Promise): Promise { + return this.associationStore.runExclusive(operation); + } + + /** + * Deletes the entire inline-script association record through the inline-owned association + * store's failure-isolated queue. + * + * The store issues a direct key update to `undefined` on the inline-owned queue rather than a + * shared `PersistentState.clear`. A generic "Clear Cache" preserves this key and never mutates + * it, so the two operations are key-disjoint; and because this deletion is an ordinary queued + * write (never coalesced onto an in-flight shared clear) it cannot be silently dropped or + * resurrected. + */ + private clearPersistedAssociations(): Promise { + return this.associationStore.clear(); } private async waitForCacheMaintenance(operation: () => Promise): Promise { @@ -3198,7 +3207,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return undefined; } try { - await this.enqueuePersistence(async (state) => state.clear([INLINE_SCRIPT_ENVS_KEY])); + await this.clearPersistedAssociations(); return undefined; } catch (error) { this.log.error(`Failed to clear inline-script environment associations: ${getErrorMessage(error)}`); @@ -3212,7 +3221,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ); try { if (persistedPathsToClear.length === Object.keys(persistedAssociations).length) { - await this.enqueuePersistence(async (state) => state.clear([INLINE_SCRIPT_ENVS_KEY])); + await this.clearPersistedAssociations(); } else if (persistedPathsToClear.length > 0) { await this.updatePersistedAssociations( persistedPathsToClear.map((scriptPath) => ({ @@ -3248,9 +3257,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private async getPersistedAssociationSnapshot(): Promise { - await this.persistenceQueue; - const state = await getWorkspacePersistentState(); - return this.parsePersistedAssociations(await state.get(INLINE_SCRIPT_ENVS_KEY))?.records ?? {}; + return this.parsePersistedAssociations(await this.associationStore.read())?.records ?? {}; } private async removeCacheEntry(envDir: Uri): Promise { diff --git a/src/managers/builtin/inlineScript/main.ts b/src/managers/builtin/inlineScript/main.ts index daf9ad4d..44cee4d1 100644 --- a/src/managers/builtin/inlineScript/main.ts +++ b/src/managers/builtin/inlineScript/main.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { Disposable, LogOutputChannel, Uri } from 'vscode'; +import { Disposable, LogOutputChannel, Memento, Uri } from 'vscode'; import { EnvironmentManager, PythonEnvironmentApi } from '../../../api'; import { traceInfo, traceVerbose } from '../../../common/logging'; import { InlineScriptFeatureActivation } from '../../../features/inlineScript/activation'; @@ -20,6 +20,7 @@ export async function registerInlineScriptFeatures( baseManager: EnvironmentManager, globalStorageUri: Uri, activation: InlineScriptFeatureActivation, + workspaceState: Memento, ): Promise { if (!activation.enabled) { traceVerbose('Inline-script env manager: skipping registration (internal flag is off)'); @@ -31,7 +32,15 @@ export async function registerInlineScriptFeatures( } const api: PythonEnvironmentApi = await getPythonApi(); - const mgr = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log, routingRegistry); + const mgr = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + log, + workspaceState, + routingRegistry, + ); disposables.push(mgr, api.registerEnvironmentManager(mgr)); setImmediate(() => mgr.startActivationDiscovery()); traceInfo('Inline-script env manager: registered (internal flag is on)'); diff --git a/src/test/common/lockfile.apis.unit.test.ts b/src/test/common/lockfile.apis.unit.test.ts index a0230a23..5a1f1b1e 100644 --- a/src/test/common/lockfile.apis.unit.test.ts +++ b/src/test/common/lockfile.apis.unit.test.ts @@ -12,8 +12,10 @@ import { acquireFileLock, AcquireFileLockOptions, FILE_LOCK_OWNER_MARKER_PREFIX, + FILE_LOCK_RELEASE_MARKER_PREFIX, FILE_LOCK_RETAINED_MARKER, FILE_LOCK_RETAINED_MARKER_PREFIX, + FILE_LOCK_RETIRED_DIR_INFIX, getFileLockPath, inspectFileLock, reclaimFileLock, @@ -161,6 +163,147 @@ suite('lockfile APIs', () => { assert.strictEqual(await fs.pathExists(`${path.resolve(targetPath)}.lock`), false); }); + test('release keeps the canonical path reusable when retired-directory rmdir fails', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const rmdirStub = sinon + .stub(fsExtra, 'rmdir') + .rejects(Object.assign(new Error('rmdir failed'), { code: 'EACCES' })); + + await lock.release(); + + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + sinon.assert.called(rmdirStub); + assert.deepStrictEqual( + (await fs.readdir(tempRoot)).filter((entry) => entry.includes(FILE_LOCK_RETIRED_DIR_INFIX)), + [], + ); + rmdirStub.restore(); + const replacement = await acquireFileLock(targetPath, OPTIONS); + await replacement.release(); + }); + + test('release retries a transient canonical retirement failure while retaining ownership', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const lockPath = getFileLockPath(targetPath); + const originalRename = fsExtra.rename; + let retirementAttempts = 0; + sinon.stub(fsExtra, 'rename').callsFake(async (source, destination) => { + if (path.resolve(String(source)) === path.resolve(lockPath)) { + retirementAttempts += 1; + if (retirementAttempts === 1) { + throw Object.assign(new Error('sharing violation'), { code: 'EBUSY' }); + } + } + await originalRename(source, destination); + }); + + await lock.release(); + + assert.strictEqual(retirementAttempts, 2); + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + }); + + test('terminal retirement failure restores ownership and permits the same handle to retry later', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const lockPath = getFileLockPath(targetPath); + const originalRename = fsExtra.rename; + let blockRetirement = true; + let retirementAttempts = 0; + const renameStub = sinon.stub(fsExtra, 'rename').callsFake(async (source, destination) => { + if (path.resolve(String(source)) === path.resolve(lockPath)) { + retirementAttempts += 1; + if (blockRetirement) { + throw Object.assign(new Error('access denied'), { code: 'EACCES' }); + } + } + await originalRename(source, destination); + }); + + await assert.rejects( + lock.release(), + (error: NodeJS.ErrnoException) => error.code === 'ELOCKRELEASEFAILED', + ); + assert.strictEqual(retirementAttempts, 3); + assert.strictEqual(await inspectFileLock(targetPath), 'held'); + assert.strictEqual( + (await fs.readdir(lockPath)).filter((entry) => entry.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX)).length, + 1, + ); + + blockRetirement = false; + await lock.release(); + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + sinon.assert.called(renameStub); + }); + + test('release stays resumable when both retirement and ownership restoration fail', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const lockPath = getFileLockPath(targetPath); + const originalRename = fsExtra.rename; + let blockTransition = true; + sinon.stub(fsExtra, 'rename').callsFake(async (source, destination) => { + const resolvedSource = path.resolve(String(source)); + if (blockTransition) { + // Fail the canonical retirement (lockPath -> .retired-*)... + if (resolvedSource === path.resolve(lockPath)) { + throw Object.assign(new Error('access denied'), { code: 'EACCES' }); + } + // ...and fail the restoration rename (.release-* -> owner-*), while still + // allowing the initial owner-* -> .release-* transition to succeed. + if (path.basename(resolvedSource).startsWith(FILE_LOCK_RELEASE_MARKER_PREFIX)) { + throw Object.assign(new Error('access denied'), { code: 'EACCES' }); + } + } + await originalRename(source, destination); + }); + + await assert.rejects( + lock.release(), + (error: NodeJS.ErrnoException) => error.code === 'ELOCKRELEASEFAILED', + ); + // Ownership was NOT restored: a live .release-* marker remains, no owner marker. + const during = await fs.readdir(lockPath); + assert.strictEqual(during.filter((e) => e.startsWith(FILE_LOCK_OWNER_MARKER_PREFIX)).length, 0); + assert.strictEqual(during.filter((e) => e.startsWith(FILE_LOCK_RELEASE_MARKER_PREFIX)).length, 1); + assert.strictEqual(await inspectFileLock(targetPath), 'held'); + + // Retry on the SAME handle: it resumes retirement from the 'releasing' state. + blockTransition = false; + await lock.release(); + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + }); + + test('serializes concurrent release() calls through a shared in-flight promise', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const originalRename = fsExtra.rename; + let ownerToReleaseRenames = 0; + let signalStarted!: () => void; + const started = new Promise((resolve) => { + signalStarted = resolve; + }); + let openGate!: () => void; + const gate = new Promise((resolve) => { + openGate = resolve; + }); + sinon.stub(fsExtra, 'rename').callsFake(async (source, destination) => { + if (path.basename(String(destination)).startsWith(FILE_LOCK_RELEASE_MARKER_PREFIX)) { + ownerToReleaseRenames += 1; + signalStarted(); + await gate; + } + await originalRename(source, destination); + }); + + const first = lock.release(); + await started; + const second = lock.release(); + openGate(); + await Promise.all([first, second]); + + assert.strictEqual(ownerToReleaseRenames, 1); + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + }); + test('retained locks fail fast without waiting for the acquisition timeout', async () => { const lock = await acquireFileLock(targetPath, OPTIONS); await lock.retain(); @@ -246,6 +389,52 @@ suite('lockfile APIs', () => { await replacement.release(); }); + test('reclaims an interrupted release transition only after its claimant is dead', async () => { + const lockPath = getFileLockPath(targetPath); + const generationMarker = `${FILE_LOCK_OWNER_MARKER_PREFIX}424241-generation`; + const releaseMarker = + `${FILE_LOCK_RELEASE_MARKER_PREFIX}424242-${'c'.repeat(32)}-${generationMarker}`; + await fs.ensureDir(lockPath); + await fs.writeFile(path.join(lockPath, releaseMarker), ''); + const liveProbe = { checkProcessLiveness: sinon.stub().withArgs(424242).resolves('live') }; + + assert.strictEqual(await inspectFileLock(targetPath, liveProbe), 'held'); + assert.strictEqual(await reclaimFileLock(targetPath, liveProbe), false); + + const deadProbe = { checkProcessLiveness: sinon.stub().withArgs(424242).resolves('dead') }; + assert.strictEqual(await reclaimFileLock(targetPath, deadProbe), true); + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + }); + + test('an interrupted retired artifact neither blocks acquisition nor gets mistaken for a live canonical lock', async () => { + const lockPath = getFileLockPath(targetPath); + const lock = await acquireFileLock(targetPath, OPTIONS); + const rmdirStub = sinon + .stub(fsExtra, 'rmdir') + .rejects(Object.assign(new Error('cleanup interrupted'), { code: 'EACCES' })); + const removeStub = sinon + .stub(fsExtra, 'remove') + .rejects(Object.assign(new Error('cleanup interrupted'), { code: 'EACCES' })); + + await lock.release(); + rmdirStub.restore(); + removeStub.restore(); + + const retiredEntries = (await fs.readdir(tempRoot)).filter((entry) => + entry.startsWith(`${path.basename(lockPath)}${FILE_LOCK_RETIRED_DIR_INFIX}`), + ); + assert.strictEqual(retiredEntries.length, 1); + const retiredPath = path.join(tempRoot, retiredEntries[0]); + + assert.strictEqual(await inspectFileLock(targetPath), 'missing'); + const replacement = await acquireFileLock(targetPath, OPTIONS); + assert.strictEqual(await inspectFileLock(targetPath), 'held'); + assert.strictEqual(await fs.pathExists(retiredPath), true); + + await replacement.release(); + assert.strictEqual(await fs.pathExists(retiredPath), true); + }); + test('refuses to reclaim the ambiguous legacy retained marker', async () => { const lockPath = getFileLockPath(targetPath); await fs.ensureDir(lockPath); @@ -300,6 +489,34 @@ suite('lockfile APIs', () => { await replacement.release(); }); + test('treats retirement between lstat and readdir inspection as a missing lock', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const lockPath = getFileLockPath(targetPath); + const originalReaddir = fsExtra.readdir; + let releaseInspection: (() => void) | undefined; + let signalInspectionStarted: (() => void) | undefined; + const inspectionStarted = new Promise((resolve) => { + signalInspectionStarted = resolve; + }); + const inspectionGate = new Promise((resolve) => { + releaseInspection = resolve; + }); + sinon.stub(fsExtra, 'readdir').callsFake(async (candidatePath, options) => { + if (path.resolve(String(candidatePath)) === path.resolve(lockPath)) { + signalInspectionStarted!(); + await inspectionGate; + } + return originalReaddir(candidatePath, options as never); + }); + + const inspection = inspectFileLock(targetPath); + await inspectionStarted; + await lock.release(); + releaseInspection!(); + + assert.strictEqual(await inspection, 'missing'); + }); + 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/features/envCommands.unit.test.ts b/src/test/features/envCommands.unit.test.ts index 14655be6..d1159928 100644 --- a/src/test/features/envCommands.unit.test.ts +++ b/src/test/features/envCommands.unit.test.ts @@ -1,14 +1,16 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; import * as typeMoq from 'typemoq'; -import { Terminal, Uri } from 'vscode'; +import { Memento, Terminal, Uri } from 'vscode'; import { PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; import * as commandApi from '../../common/command.api'; -import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; +import { INLINE_SCRIPT_ENVS_KEY, INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; +import * as persistentState from '../../common/persistentState'; import * as managerApi from '../../common/pickers/managers'; import * as projectApi from '../../common/pickers/projects'; import * as windowApis from '../../common/window.apis'; import { + clearEnvironmentCachesCommand, clearScriptEnvironmentCacheCommand, createAnyEnvironmentCommand, removePythonProject, @@ -18,6 +20,8 @@ import { } from '../../features/envCommands'; import * as settingHelpers from '../../features/settings/settingHelpers'; import * as terminalRunner from '../../features/terminal/runInTerminal'; +import * as shellProviders from '../../features/terminal/shells/providers'; +import { ShellStartupScriptProvider } from '../../features/terminal/shells/startupProvider'; import { TerminalManager } from '../../features/terminal/terminalManager'; import { EnvManagerView } from '../../features/views/envManagersView'; import { ProjectEnvironment, ProjectItem } from '../../features/views/treeViewItems'; @@ -257,12 +261,16 @@ suite('Clear Script Environment Cache Command Tests', () => { test('clears cache before inline settings cleanup and unloads removed projects', async () => { const calls: string[] = []; + const selectionEvents: string[] = []; + let associationPresent = true; const inlineProject: PythonProject = { uri: Uri.file('/workspace/script.py'), name: 'script.py', }; const clearCache = sinon.stub().callsFake(async () => { calls.push('clearCache'); + associationPresent = false; + selectionEvents.push('cleared'); }); const envManagers = { getEnvironmentManager: sinon.stub().withArgs(INLINE_SCRIPT_MANAGER_ID).returns({ @@ -285,6 +293,8 @@ suite('Clear Script Environment Cache Command Tests', () => { .callsFake(async (projects) => { calls.push('removeInlineSettings'); assert.deepStrictEqual(projects, [inlineProject]); + assert.strictEqual(associationPresent, false); + assert.deepStrictEqual(selectionEvents, ['cleared']); return [inlineProject]; }); @@ -345,6 +355,119 @@ suite('Clear Script Environment Cache Command Tests', () => { }); }); +suite('Clear Environment Caches Command Tests', () => { + teardown(() => { + sinon.restore(); + }); + + function makeWorkspaceMemento(store: Map): Memento { + return { + get: (key: string) => store.get(key) as T | undefined, + update: async (key: string, value: unknown) => { + if (value === undefined) { + store.delete(key); + } else { + store.set(key, value); + } + }, + keys: () => [...store.keys()], + } as unknown as Memento; + } + + test('generic clear preserves the inline association key while clearing other workspace/global state and managers', async () => { + const inlineAssociations = { 'C:\\workspace\\script.py': 'C:\\cache\\python.exe' }; + const store = new Map([ + [INLINE_SCRIPT_ENVS_KEY, inlineAssociations], + ['other-workspace-state', { stale: true }], + ]); + const workspaceState = makeWorkspaceMemento(store); + + const calls: string[] = []; + let clearedWorkspaceKeys: readonly string[] | undefined; + let globalCleared = false; + const workspacePersistent = { + clear: sinon.stub().callsFake(async (keys?: string[]) => { + calls.push('workspace'); + clearedWorkspaceKeys = keys; + for (const key of keys ?? [...store.keys()]) { + store.delete(key); + } + }), + } as unknown as persistentState.PersistentState; + const globalPersistent = { + clear: sinon.stub().callsFake(async () => { + calls.push('global'); + globalCleared = true; + }), + } as unknown as persistentState.PersistentState; + sinon.stub(persistentState, 'getWorkspacePersistentState').resolves(workspacePersistent); + sinon.stub(persistentState, 'getGlobalPersistentState').resolves(globalPersistent); + + const envManagers = { + clearCache: sinon.stub().callsFake(async () => { + calls.push('managers'); + // The inline association key must still be present when non-inline managers run. + assert.deepStrictEqual(store.get(INLINE_SCRIPT_ENVS_KEY), inlineAssociations); + }), + } as unknown as EnvironmentManagers; + const startupProvider = { + clearCache: sinon.stub().callsFake(async () => { + calls.push('shells'); + }), + } as unknown as ShellStartupScriptProvider; + sinon.stub(shellProviders, 'clearShellProfileCache').callsFake(async (providers) => { + await Promise.all(providers.map((provider) => provider.clearCache())); + }); + + await clearEnvironmentCachesCommand(envManagers, [startupProvider], workspaceState); + + // Only the inline key is excluded from the explicit workspace key list handed to clear(). + assert.deepStrictEqual(clearedWorkspaceKeys, ['other-workspace-state']); + assert.strictEqual(globalCleared, true); + // Persistent state (workspace + global) is cleared before managers, which run before shells. + assert.ok(calls.indexOf('managers') > calls.indexOf('workspace'), 'managers run after workspace clear'); + assert.ok(calls.indexOf('managers') > calls.indexOf('global'), 'managers run after global clear'); + assert.ok(calls.indexOf('shells') > calls.indexOf('managers'), 'shells run after managers'); + assert.strictEqual(calls[calls.length - 1], 'shells'); + // Inline association key preserved; other workspace key cleared. + assert.deepStrictEqual(store.get(INLINE_SCRIPT_ENVS_KEY), inlineAssociations); + assert.strictEqual(store.has('other-workspace-state'), false); + sinon.assert.calledOnceWithExactly(envManagers.clearCache as sinon.SinonStub, undefined); + }); + + test('generic clear preserves a dormant inline association key when the inline manager is not registered', async () => { + const inlineAssociations = { 'C:\\workspace\\script.py': 'C:\\cache\\python.exe' }; + const store = new Map([[INLINE_SCRIPT_ENVS_KEY, inlineAssociations]]); + const workspaceState = makeWorkspaceMemento(store); + + let clearedWorkspaceKeys: readonly string[] | undefined; + const workspacePersistent = { + clear: sinon.stub().callsFake(async (keys?: string[]) => { + clearedWorkspaceKeys = keys; + for (const key of keys ?? [...store.keys()]) { + store.delete(key); + } + }), + } as unknown as persistentState.PersistentState; + const globalPersistent = { + clear: sinon.stub().resolves(), + } as unknown as persistentState.PersistentState; + sinon.stub(persistentState, 'getWorkspacePersistentState').resolves(workspacePersistent); + sinon.stub(persistentState, 'getGlobalPersistentState').resolves(globalPersistent); + + const envManagers = { + clearCache: sinon.stub().resolves(), + } as unknown as EnvironmentManagers; + sinon.stub(shellProviders, 'clearShellProfileCache').resolves(); + + await clearEnvironmentCachesCommand(envManagers, [], workspaceState); + + // The only workspace key is the inline key, so the explicit list is empty and it is preserved. + assert.deepStrictEqual(clearedWorkspaceKeys, []); + assert.deepStrictEqual(store.get(INLINE_SCRIPT_ENVS_KEY), inlineAssociations); + }); +}); + suite('Reveal Env In Manager View Command Tests', () => { let managerView: typeMoq.IMock; let executeCommandStub: sinon.SinonStub; diff --git a/src/test/managers/builtin/inlineScript/associationStore.unit.test.ts b/src/test/managers/builtin/inlineScript/associationStore.unit.test.ts new file mode 100644 index 00000000..8436f520 --- /dev/null +++ b/src/test/managers/builtin/inlineScript/associationStore.unit.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { Memento } from 'vscode'; +import { INLINE_SCRIPT_ENVS_KEY } from '../../../../common/constants'; +import * as logging from '../../../../common/logging'; +import { InlineScriptAssociationStore } from '../../../../managers/builtin/inlineScript/associationStore'; + +function createMemento(initial?: Record): { memento: Memento; store: Map } { + const store = new Map(initial ? Object.entries(initial) : []); + const memento = { + get: (key: string, defaultValue?: T) => (store.has(key) ? (store.get(key) as T) : defaultValue), + update: async (key: string, value: unknown) => { + if (value === undefined) { + store.delete(key); + } else { + store.set(key, value); + } + }, + keys: () => [...store.keys()], + } as unknown as Memento; + return { memento, store }; +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +suite('InlineScriptAssociationStore', () => { + teardown(() => { + sinon.restore(); + }); + + test('runs queued operations strictly in invocation order', async () => { + const { memento } = createMemento(); + const store = new InlineScriptAssociationStore(memento); + const order: number[] = []; + + const first = store.runExclusive(async () => { + await delay(10); + order.push(1); + }); + const second = store.runExclusive(async () => { + order.push(2); + }); + const third = store.runExclusive(async () => { + await delay(5); + order.push(3); + }); + + await Promise.all([first, second, third]); + assert.deepStrictEqual(order, [1, 2, 3]); + }); + + test('serializes a read-modify-write transaction as one atomic queue entry', async () => { + const { memento, store: backing } = createMemento(); + const store = new InlineScriptAssociationStore(memento); + + // Two interleaved read-modify-writes must not clobber each other. + const writeA = store.runExclusive(async (state) => { + const current = ((await state.get>()) ?? {}) as Record; + await delay(10); + await state.update({ ...current, 'a.py': 'env-a' }); + }); + const writeB = store.runExclusive(async (state) => { + const current = ((await state.get>()) ?? {}) as Record; + await state.update({ ...current, 'b.py': 'env-b' }); + }); + + await Promise.all([writeA, writeB]); + assert.deepStrictEqual(backing.get(INLINE_SCRIPT_ENVS_KEY), { 'a.py': 'env-a', 'b.py': 'env-b' }); + }); + + test('a dedicated clear queued behind an in-flight write wins and is not resurrected', async () => { + const { memento, store: backing } = createMemento(); + const store = new InlineScriptAssociationStore(memento); + + let releaseWrite!: () => void; + const writeGate = new Promise((resolve) => { + releaseWrite = resolve; + }); + + const writePromise = store.runExclusive(async (state) => { + await writeGate; + await state.update({ 'a.py': 'env-a' }); + }); + // Request the deletion while the write is still in flight. + const clearPromise = store.clear(); + releaseWrite(); + await Promise.all([writePromise, clearPromise]); + + assert.strictEqual(backing.has(INLINE_SCRIPT_ENVS_KEY), false); + assert.strictEqual(await store.read(), undefined); + }); + + test('a failed operation rejects its caller but the queue keeps running later operations', async () => { + const { memento, store: backing } = createMemento(); + const store = new InlineScriptAssociationStore(memento); + + const failing = store.runExclusive(async () => { + throw new Error('boom'); + }); + // Queue a follow-up behind the failing operation before awaiting the rejection. + const later = store.runExclusive((state) => state.update({ 'b.py': 'env-b' })); + + await assert.rejects(failing, /boom/); + await later; + assert.deepStrictEqual(backing.get(INLINE_SCRIPT_ENVS_KEY), { 'b.py': 'env-b' }); + }); + + test('read returns the latest value written through the queue', async () => { + const { memento } = createMemento(); + const store = new InlineScriptAssociationStore(memento); + + assert.strictEqual(await store.read(), undefined); + await store.runExclusive((state) => state.update({ 'c.py': 'env-c' })); + assert.deepStrictEqual(await store.read(), { 'c.py': 'env-c' }); + await store.clear(); + assert.strictEqual(await store.read(), undefined); + }); + + test('a verified write clears the key and logs when the read-back does not match', async () => { + const traceErrorStub = sinon.stub(logging, 'traceError'); + const backing = new Map(); + const memento = { + get: (key: string) => backing.get(key) as T | undefined, + update: async (key: string, value: unknown) => { + if (value === undefined) { + backing.delete(key); + } + // Silently drop non-undefined writes to force a read-back mismatch. + }, + keys: () => [...backing.keys()], + } as unknown as Memento; + const store = new InlineScriptAssociationStore(memento); + + await store.runExclusive((state) => state.update({ 'd.py': 'env-d' })); + + assert.strictEqual(backing.has(INLINE_SCRIPT_ENVS_KEY), false, 'a corrupt write must not be left persisted'); + sinon.assert.calledWithMatch(traceErrorStub, sinon.match.string, INLINE_SCRIPT_ENVS_KEY); + }); + + test('store operations resolve independently of a wedged external clear promise', async () => { + const { memento, store: backing } = createMemento(); + const store = new InlineScriptAssociationStore(memento); + + // Simulate a wedged shared PersistentState.clear() that never settles. The inline store + // holds no reference to it, so its own operations must still complete promptly. + const wedged = new Promise(() => undefined); + void wedged; + + const timeout = delay(1000).then(() => 'timeout' as const); + const write = (async () => { + await store.runExclusive((state) => state.update({ 'e.py': 'env-e' })); + return 'done' as const; + })(); + assert.strictEqual(await Promise.race([write, timeout]), 'done'); + assert.deepStrictEqual(backing.get(INLINE_SCRIPT_ENVS_KEY), { 'e.py': 'env-e' }); + + const clear = (async () => { + await store.clear(); + return 'done' as const; + })(); + assert.strictEqual(await Promise.race([clear, timeout]), 'done'); + assert.strictEqual(backing.has(INLINE_SCRIPT_ENVS_KEY), false); + }); +}); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 0d19a108..7c6c880c 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -7,7 +7,7 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; import * as sinon from 'sinon'; -import { Disposable, LogOutputChannel, TextDocument, Uri } from 'vscode'; +import { Disposable, LogOutputChannel, Memento, TextDocument, Uri } from 'vscode'; import { EnvironmentChangeKind, EnvironmentManager, @@ -19,17 +19,14 @@ import * as cacheLayout from '../../../../common/inlineScript/cacheLayout'; import * as metadataReader from '../../../../common/inlineScript/metadata'; import { InlineScriptRoutingRegistry } from '../../../../common/inlineScript/routingRegistry'; import * as lockfileApis from '../../../../common/lockfile.apis'; -import * as persistentState from '../../../../common/persistentState'; +import { INLINE_SCRIPT_ENVS_KEY } from '../../../../common/constants'; import { EventNames } from '../../../../common/telemetry/constants'; import * as telemetrySender from '../../../../common/telemetry/sender'; import { isWindows } from '../../../../common/utils/platformUtils'; import { normalizePath } from '../../../../common/utils/pathUtils'; import { getVenvPythonPath } from '../../../../common/utils/virtualEnvironment'; import * as workspaceApis from '../../../../common/workspace.apis'; -import { - InlineScriptEnvManager, - INLINE_SCRIPT_ENVS_KEY, -} from '../../../../managers/builtin/inlineScript/envManager'; +import { InlineScriptEnvManager } from '../../../../managers/builtin/inlineScript/envManager'; import * as builtinUtils from '../../../../managers/builtin/utils'; import * as uvPythonInstaller from '../../../../managers/builtin/uvPythonInstaller'; import * as venvUtils from '../../../../managers/builtin/venvUtils'; @@ -135,9 +132,10 @@ suite('InlineScriptEnvManager', () => { let renameFilesListener: ((e: { files: readonly { oldUri: Uri; newUri: Uri }[] }) => unknown) | undefined; let workspaceState: { get: sinon.SinonStub; - set: sinon.SinonStub; - clear: sinon.SinonStub; + update: sinon.SinonStub; + keys: sinon.SinonStub; }; + let workspaceMemento: Memento; let persistedAssociations: unknown; setup(async () => { @@ -166,18 +164,16 @@ suite('InlineScriptEnvManager', () => { get: sinon.stub().callsFake(async (key: string) => { return key === INLINE_SCRIPT_ENVS_KEY ? persistedAssociations : undefined; }), - set: sinon.stub().callsFake(async (key: string, value: unknown) => { + update: sinon.stub().callsFake(async (key: string, value: unknown) => { if (key === INLINE_SCRIPT_ENVS_KEY) { persistedAssociations = value; } }), - clear: sinon.stub().callsFake(async (keys?: string[]) => { - if (!keys || keys.includes(INLINE_SCRIPT_ENVS_KEY)) { - persistedAssociations = undefined; - } - }), + keys: sinon.stub().callsFake(() => + persistedAssociations === undefined ? [] : [INLINE_SCRIPT_ENVS_KEY], + ), }; - sinon.stub(persistentState, 'getWorkspacePersistentState').resolves(workspaceState); + workspaceMemento = workspaceState as unknown as Memento; readMetadataStub = sinon.stub(metadataReader, 'readInlineScriptMetadataFromFile').resolves(VALID_METADATA); computeCacheKeyStub = sinon.stub(cacheKey, 'computeCacheKey').callsFake((inputs) => { @@ -250,6 +246,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, routingRegistry, ); }); @@ -381,7 +378,7 @@ suite('InlineScriptEnvManager', () => { } function workspaceStateSetCalls(key: string): readonly sinon.SinonSpyCall[] { - return workspaceState.set.getCalls().filter((call) => call.args[0] === key); + return workspaceState.update.getCalls().filter((call) => call.args[0] === key); } function matchedAssociationRecord(environmentPath: string, metadataIdentity: string = VALID_METADATA_IDENTITY): unknown { @@ -1495,6 +1492,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -3410,7 +3408,7 @@ suite('InlineScriptEnvManager', () => { assert.deepStrictEqual(persistedAssociations, { [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), }); - assert.strictEqual(workspaceState.set.firstCall.args[0], INLINE_SCRIPT_ENVS_KEY); + assert.strictEqual(workspaceState.update.firstCall.args[0], INLINE_SCRIPT_ENVS_KEY); assert.strictEqual(listener.callCount, 1); assert.deepStrictEqual(listener.firstCall.args[0], { uri, old: undefined, new: environment }); @@ -3475,6 +3473,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -3500,6 +3499,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -3616,6 +3616,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -3650,6 +3651,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -3699,6 +3701,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -3752,11 +3755,12 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); - workspaceState.set.onFirstCall().rejects(new Error('Memento unavailable')); + workspaceState.update.onFirstCall().rejects(new Error('Memento unavailable')); await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); await fs.remove(environment.environmentPath.fsPath); clock.tick(5_000 - 1); @@ -3989,6 +3993,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4026,6 +4031,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4074,13 +4080,14 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); ((restarted as unknown as { subscriptions: Disposable[] }).subscriptions[0]).dispose(); - workspaceState.set.onFirstCall().rejects(new Error('Memento unavailable')); - workspaceState.set.onSecondCall().rejects(new Error('Memento unavailable')); + workspaceState.update.onFirstCall().rejects(new Error('Memento unavailable')); + workspaceState.update.onSecondCall().rejects(new Error('Memento unavailable')); await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); assert.deepStrictEqual(persistedAssociations, { @@ -4102,6 +4109,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4123,6 +4131,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4151,6 +4160,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4199,6 +4209,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4243,6 +4254,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4277,6 +4289,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4318,6 +4331,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4344,6 +4358,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4370,6 +4385,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4411,6 +4427,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4461,6 +4478,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4492,6 +4510,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4692,6 +4711,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4723,6 +4743,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -4749,6 +4770,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); @@ -4841,6 +4863,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); const listener = sinon.spy(); @@ -4871,6 +4894,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); const listener = sinon.spy(); @@ -4881,7 +4905,7 @@ suite('InlineScriptEnvManager', () => { assert.deepStrictEqual(persistedAssociations, { [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), }); - assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(workspaceState.update.callCount, 0); assert.strictEqual(listener.callCount, 0); assert.strictEqual(resolveVenvStub.callCount, 0); @@ -4960,7 +4984,7 @@ suite('InlineScriptEnvManager', () => { assert.deepStrictEqual(persistedAssociations, { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, }); - assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(workspaceState.update.callCount, 0); assert.strictEqual(resolveVenvStub.callCount, 0); }); @@ -4977,6 +5001,7 @@ suite('InlineScriptEnvManager', () => { baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, restartRoutingRegistry, ); await nextTurn(); @@ -5033,13 +5058,14 @@ suite('InlineScriptEnvManager', () => { } return persistedAssociations; }); - workspaceState.set.resetHistory(); + workspaceState.update.resetHistory(); manager = new InlineScriptEnvManager( nativeFinder, api, baseManager, globalStorageUri, makeFakeLog(), + workspaceMemento, routingRegistry, ); await initialReadStarted; @@ -5427,7 +5453,7 @@ suite('InlineScriptEnvManager', () => { assert.deepStrictEqual(persistedAssociations, { [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), }); - assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(workspaceState.update.callCount, 0); }); test('preserves an association when fallback resolution reports another manager', async () => { @@ -5443,7 +5469,7 @@ suite('InlineScriptEnvManager', () => { assert.deepStrictEqual(persistedAssociations, { [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, }); - assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(workspaceState.update.callCount, 0); }); test('rejects resolved and selected environments that are outside the owned cache', async () => { @@ -5465,11 +5491,11 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await manager.get(uri), undefined); assert.deepStrictEqual(persistedAssociations, {}); - workspaceState.set.resetHistory(); + workspaceState.update.resetHistory(); await assert.rejects(manager.set(uri, unowned), /not an owned cache entry/); assert.deepStrictEqual(persistedAssociations, {}); - assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(workspaceState.update.callCount, 0); assert.strictEqual(listener.callCount, 0); }); @@ -5503,7 +5529,7 @@ suite('InlineScriptEnvManager', () => { manager.onDidChangeEnvironment(listener); await manager.set(uri, first); - workspaceState.set.onSecondCall().rejects(new Error('Memento unavailable')); + workspaceState.update.onSecondCall().rejects(new Error('Memento unavailable')); await assert.rejects(manager.set(uri, second), /Memento unavailable/); assert.strictEqual(await manager.get(uri), first); @@ -5520,7 +5546,7 @@ suite('InlineScriptEnvManager', () => { manager.onDidChangeEnvironment(listener); await manager.set(uri, environment); - workspaceState.set.onSecondCall().rejects(new Error('Memento unavailable')); + workspaceState.update.onSecondCall().rejects(new Error('Memento unavailable')); await assert.rejects(manager.set(uri, undefined), /Memento unavailable/); assert.strictEqual(await manager.get(uri), environment); @@ -5630,7 +5656,7 @@ suite('InlineScriptEnvManager', () => { const pendingGet = manager.get(uri); await waitForStubCall(resolveVenvStub); - workspaceState.set.onFirstCall().rejects(new Error('Memento unavailable')); + workspaceState.update.onFirstCall().rejects(new Error('Memento unavailable')); await assert.rejects(manager.set(uri, newEnvironment), /Memento unavailable/); resolvePending!(oldEnvironment); @@ -5653,7 +5679,7 @@ suite('InlineScriptEnvManager', () => { /one or more local file URIs/, ); - assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(workspaceState.update.callCount, 0); assert.strictEqual(await manager.get(valid), undefined); assert.strictEqual(await manager.get(undefined), undefined); assert.strictEqual(await manager.get(Uri.parse('untitled:script.py')), undefined); @@ -5725,6 +5751,7 @@ suite('InlineScriptEnvManager', () => { baseManager, Uri.file(process.platform === 'win32' ? `${process.env.SystemDrive ?? 'C:'}\\` : '/'), makeFakeLog(), + workspaceMemento, ); await assert.rejects( @@ -5743,6 +5770,7 @@ suite('InlineScriptEnvManager', () => { baseManager, symlinkStorageUri, makeFakeLog(), + workspaceMemento, ); const realCacheRoot = cacheLayout.getScriptEnvCacheRoot(symlinkStorageUri).fsPath; const externalCacheRoot = path.join(tempRoot, 'external-cache-root'); @@ -5777,6 +5805,7 @@ suite('InlineScriptEnvManager', () => { baseManager, Uri.file(redirectedStoragePath), makeFakeLog(), + workspaceMemento, ); try { await fs.remove(redirectedStoragePath); @@ -5944,7 +5973,7 @@ suite('InlineScriptEnvManager', () => { await manager.set(uri, environment); const listener = sinon.spy(); manager.onDidChangeEnvironment(listener); - workspaceState.clear.onFirstCall().rejects(new Error('Memento unavailable')); + workspaceState.update.withArgs(INLINE_SCRIPT_ENVS_KEY, undefined).rejects(new Error('Memento unavailable')); await assert.rejects(manager.clearCache(), /Memento unavailable/); @@ -6056,14 +6085,12 @@ suite('InlineScriptEnvManager', () => { const clearStarted = new Promise((resolve) => { signalClearStarted = resolve; }); - workspaceState.clear.callsFake( - async (keys?: string[]) => + workspaceState.update.withArgs(INLINE_SCRIPT_ENVS_KEY, undefined).callsFake( + async () => new Promise((resolve) => { signalClearStarted!(); releaseClear = () => { - if (!keys || keys.includes(INLINE_SCRIPT_ENVS_KEY)) { - persistedAssociations = undefined; - } + persistedAssociations = undefined; resolve(); }; }), @@ -6080,5 +6107,60 @@ suite('InlineScriptEnvManager', () => { assert.ok(await createPromise); assert.ok(readMetadataStub.calledOnce); }); + + test('serializes a dedicated clear-cache deletion after an in-flight association write so the write cannot resurrect it', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + + let releaseWrite: (() => void) | undefined; + let signalWriteStarted: (() => void) | undefined; + const writeStarted = new Promise((resolve) => { + signalWriteStarted = resolve; + }); + workspaceState.update + .withArgs(INLINE_SCRIPT_ENVS_KEY, sinon.match((value: unknown) => value !== undefined)) + .callsFake( + (_key: string, value: unknown) => + new Promise((resolve) => { + persistedAssociations = value; + signalWriteStarted!(); + releaseWrite = resolve; + }), + ); + + const writePromise = manager.set(uri, environment); + await writeStarted; + + // Request the dedicated clear while the association write is still in flight. + const clearPromise = manager.clearCache(); + releaseWrite!(); + await Promise.all([writePromise, clearPromise]); + + assert.strictEqual(persistedAssociations, undefined); + assert.strictEqual(await manager.get(uri), undefined); + }); + + test('keeps the inline persistence queue usable after a failed dedicated deletion', async () => { + const firstUri = scriptUri('first.py'); + const environment = await createOwnedEnvironment(); + await manager.set(firstUri, environment); + workspaceState.update + .withArgs(INLINE_SCRIPT_ENVS_KEY, undefined) + .onFirstCall() + .rejects(new Error('Memento unavailable')); + + await assert.rejects(manager.clearCache(), /Memento unavailable/); + + // The inline-owned queue recovers: a later association write still persists. + const secondUri = scriptUri('second.py'); + const secondEnvironment = await createOwnedEnvironment('fedcba9876543210'); + await manager.set(secondUri, secondEnvironment); + + assert.strictEqual(await manager.get(secondUri), secondEnvironment); + assert.deepStrictEqual( + (persistedAssociations as Record | undefined)?.[normalizePath(secondUri.fsPath)], + matchedAssociationRecord(secondEnvironment.environmentPath.fsPath), + ); + }); }); }); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index 69f6d80b..d62def49 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -3,7 +3,7 @@ import assert from 'assert'; import * as sinon from 'sinon'; -import { Disposable, LogOutputChannel, Uri } from 'vscode'; +import { Disposable, LogOutputChannel, Memento, Uri } from 'vscode'; import { EnvironmentManager, PythonEnvironmentApi } from '../../../../api'; import * as cacheLayout from '../../../../common/inlineScript/cacheLayout'; import { InlineScriptRoutingRegistry } from '../../../../common/inlineScript/routingRegistry'; @@ -49,6 +49,11 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { const baseManager = {} as EnvironmentManager; const globalStorageUri = Uri.file('inline-script-global-storage'); const routingRegistry = new InlineScriptRoutingRegistry(); + const workspaceMemento = { + get: () => undefined, + update: async () => undefined, + keys: () => [], + } as unknown as Memento; setup(() => { isEnabledStub = sinon.stub(helpers, 'isInlineScriptsFeatureEnabled'); @@ -79,6 +84,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, { enabled: false, routingRegistry: undefined }, + workspaceMemento, ); assert.strictEqual(disposables.length, 0, 'no disposables should be added when flag is off'); @@ -97,6 +103,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, { enabled: true, routingRegistry: undefined }, + workspaceMemento, ), /routing registry/i, ); @@ -116,6 +123,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, { enabled: true, routingRegistry }, + workspaceMemento, ); assert.strictEqual(getPythonApiStub.callCount, 1); @@ -142,6 +150,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, { enabled: true, routingRegistry }, + workspaceMemento, ); assert.strictEqual( @@ -170,6 +179,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, activation, + workspaceMemento, ) : Promise.resolve()); @@ -203,6 +213,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, activation, + workspaceMemento, ) : Promise.resolve()); await nextTurn(); @@ -236,6 +247,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { baseManager, globalStorageUri, activation, + workspaceMemento, ) : Promise.resolve());