diff --git a/docs/startup-flow.md b/docs/startup-flow.md index 3a1f7c262..7b670659a 100644 --- a/docs/startup-flow.md +++ b/docs/startup-flow.md @@ -46,11 +46,12 @@ python environments extension begins activation - **If P3 won:** environment is already resolved → done, no `get()` call needed. - **Otherwise:** calls `manager.get(scope)`, which has two internal paths: - **Fast path** (`tryFastPathGet` in `fastPath.ts`) — entered when `_initialized` hasn't completed and scope is a `Uri`: + **Fast path** (`tryFastPathGet` in `fastPath.ts`) — entered when `_initialized` hasn't completed and the manager has a persisted path for the requested scope: 1. Synchronously create `_initialized` deferred + kick off `startBackgroundInit()` (fire-and-forget full PET discovery) - 2. Read persisted env path from workspace state - 3. If persisted path exists → `resolve(path)` → return immediately (background init continues in parallel) - 4. If no persisted path or resolve fails → fall through to slow path + 2. Read the workspace-scoped path, or for the global system Python read the workspace-state mirror first and global state as a compatibility/recovery fallback + 3. If the persisted path exists → `resolve(path)` → return immediately (background init continues in parallel) + 4. If no persisted path or resolve fails → fall through to slow path; definitively missing global paths are invalidated in both stores + - Global mirror writes carry a revision and tombstone so partial Memento failures cannot resurrect or overwrite a newer selection - *On background init failure:* clears `_initialized` so next `get()` retries **Slow path** — fast path skipped or failed: diff --git a/src/common/persistentState.ts b/src/common/persistentState.ts index c6b0f2632..f838e2e68 100644 --- a/src/common/persistentState.ts +++ b/src/common/persistentState.ts @@ -10,10 +10,12 @@ export interface PersistentState { class PersistentStateImpl implements PersistentState { private clearing: Deferred; + constructor(private readonly momento: Memento) { this.clearing = createDeferred(); this.clearing.resolve(); } + async get(key: string, defaultValue?: T): Promise { await this.clearing.promise; if (defaultValue === undefined) { @@ -21,6 +23,7 @@ class PersistentStateImpl implements PersistentState { } return this.momento.get(key, defaultValue); } + async set(key: string, value: T): Promise { await this.clearing.promise; await this.momento.update(key, value); @@ -32,12 +35,22 @@ class PersistentStateImpl implements PersistentState { traceError('Error while updating state for key:', key); } } + async clear(keys?: string[]): Promise { if (this.clearing.completed) { - this.clearing = createDeferred(); - const _keys = keys ?? this.momento.keys(); - await Promise.all(_keys.map((key) => this.momento.update(key, undefined))); - this.clearing.resolve(); + const clearing = (this.clearing = createDeferred()); + try { + const _keys = keys ?? this.momento.keys(); + const results = await Promise.allSettled( + _keys.map((key) => this.momento.update(key, undefined)), + ); + const failure = results.find((result) => result.status === 'rejected'); + if (failure?.status === 'rejected') { + throw failure.reason; + } + } finally { + clearing.resolve(); + } } return this.clearing.promise; } @@ -46,9 +59,14 @@ class PersistentStateImpl implements PersistentState { const _workspace = createDeferred(); const _global = createDeferred(); +/** Creates a persistent-state wrapper for the provided VS Code Memento. */ +export function createPersistentState(memento: Memento): PersistentState { + return new PersistentStateImpl(memento); +} + export function setPersistentState(context: ExtensionContext): void { - _workspace.resolve(new PersistentStateImpl(context.workspaceState)); - _global.resolve(new PersistentStateImpl(context.globalState)); + _workspace.resolve(createPersistentState(context.workspaceState)); + _global.resolve(createPersistentState(context.globalState)); } export function getWorkspacePersistentState(): Promise { diff --git a/src/managers/builtin/cache.ts b/src/managers/builtin/cache.ts index f6bf5ba0d..f5eba4745 100644 --- a/src/managers/builtin/cache.ts +++ b/src/managers/builtin/cache.ts @@ -1,14 +1,126 @@ +import { randomUUID } from 'crypto'; import { ENVS_EXTENSION_ID } from '../../common/constants'; -import { getGlobalPersistentState, getWorkspacePersistentState } from '../../common/persistentState'; +import { traceError } from '../../common/logging'; +import { + getGlobalPersistentState, + getWorkspacePersistentState, + type PersistentState, +} from '../../common/persistentState'; export const SYSTEM_WORKSPACE_KEY = `${ENVS_EXTENSION_ID}:system:WORKSPACE_SELECTED`; export const SYSTEM_GLOBAL_KEY = `${ENVS_EXTENSION_ID}:system:GLOBAL_SELECTED`; +export const SYSTEM_GLOBAL_REVISION_KEY = `${SYSTEM_GLOBAL_KEY}:REVISION`; -export async function clearSystemEnvCache(): Promise { - const workspaceState = await getWorkspacePersistentState(); - await workspaceState.clear([SYSTEM_WORKSPACE_KEY]); - const globalState = await getGlobalPersistentState(); - await globalState.clear([SYSTEM_GLOBAL_KEY]); +type CacheLayer = 'globalState' | 'workspaceState'; +type CacheOperationResult = { succeeded: true } | { succeeded: false; error: unknown }; + +interface GlobalSelectionCacheEntry { + readonly path?: string; + readonly revision: string; +} + +interface WorkspaceGlobalSelection { + readonly selection?: unknown; + readonly revision?: string; + readonly failed: boolean; +} + +let globalCacheMutation: Promise = Promise.resolve(); + +function enqueueGlobalCacheMutation(operation: () => Promise): Promise { + const result = globalCacheMutation.then(operation, operation); + globalCacheMutation = result.then( + () => undefined, + () => undefined, + ); + return result; +} + +function isGlobalSelectionCacheEntry(value: unknown): value is GlobalSelectionCacheEntry { + if (!value || typeof value !== 'object') { + return false; + } + const entry = value as { path?: unknown; revision?: unknown }; + return ( + typeof entry.revision === 'string' && + entry.revision.length > 0 && + (entry.path === undefined || typeof entry.path === 'string') + ); +} + +function getStoredGlobalSelectionPath(value: unknown): string | undefined { + if (typeof value === 'string') { + return value || undefined; + } + return isGlobalSelectionCacheEntry(value) ? value.path : undefined; +} + +function isCurrentWorkspaceSelection(selection: unknown, revision: string | undefined): boolean { + if (typeof selection === 'string') { + return selection.length > 0 && revision === undefined; + } + return isGlobalSelectionCacheEntry(selection) && (revision === undefined || selection.revision === revision); +} + +async function readWorkspaceGlobalSelection(): Promise { + try { + const state = await getWorkspacePersistentState(); + const [selection, revision] = await Promise.all([ + state.get(SYSTEM_GLOBAL_KEY), + state.get(SYSTEM_GLOBAL_REVISION_KEY), + ]); + return { selection, revision, failed: false }; + } catch (error) { + traceError('Failed to read workspaceState system Python cache:', error); + return { failed: true }; + } +} + +async function readGlobalSelection(): Promise { + try { + const state = await getGlobalPersistentState(); + return await state.get(SYSTEM_GLOBAL_KEY); + } catch (error) { + traceError('Failed to read globalState system Python cache:', error); + return undefined; + } +} + +async function runCacheOperation( + operation: 'clear' | 'invalidate' | 'update', + layer: CacheLayer, + action: () => Promise, +): Promise { + try { + await action(); + return { succeeded: true }; + } catch (error) { + traceError(`Failed to ${operation} ${layer} system Python cache:`, error); + return { succeeded: false, error }; + } +} + +/** + * Clears system Python selections from workspace and global persistent state. + * Both layers are attempted before the first failure is rethrown. + */ +export function clearSystemEnvCache(): Promise { + return enqueueGlobalCacheMutation(async () => { + const results = await Promise.all([ + runCacheOperation('clear', 'workspaceState', async () => { + const state = await getWorkspacePersistentState(); + await state.clear([SYSTEM_WORKSPACE_KEY, SYSTEM_GLOBAL_KEY, SYSTEM_GLOBAL_REVISION_KEY]); + }), + runCacheOperation('clear', 'globalState', async () => { + const state = await getGlobalPersistentState(); + await state.clear([SYSTEM_GLOBAL_KEY]); + }), + ]); + const failure = results.find((result) => !result.succeeded); + if (failure && !failure.succeeded) { + throw failure.error; + } + }); } export async function getSystemEnvForWorkspace(fsPath: string): Promise { @@ -48,12 +160,89 @@ export async function setSystemEnvForWorkspaces(fsPath: string[], envPath: strin await state.set(SYSTEM_WORKSPACE_KEY, data); } +/** + * Returns the persisted global system Python path. + * + * A complete `workspaceState` entry is primary so the last system Python + * resolved for this workspace survives a cold remote `globalState`. The global + * entry is read only for legacy cold caches or to recover an incomplete mirrored + * write identified by the workspace revision marker. + */ export async function getSystemEnvForGlobal(): Promise { - const state = await getGlobalPersistentState(); - return await state.get(SYSTEM_GLOBAL_KEY); + await globalCacheMutation; + const workspace = await readWorkspaceGlobalSelection(); + if (!workspace.failed && isCurrentWorkspaceSelection(workspace.selection, workspace.revision)) { + return getStoredGlobalSelectionPath(workspace.selection); + } + + const globalSelection = await readGlobalSelection(); + if ( + workspace.revision && + isGlobalSelectionCacheEntry(globalSelection) && + globalSelection.revision === workspace.revision + ) { + return globalSelection.path; + } + if (workspace.revision) { + return undefined; + } + return getStoredGlobalSelectionPath(globalSelection); +} + +/** + * Removes a stale global system Python path without overwriting a newer selection. + * Each layer is compared and updated independently inside the mutation queue. + */ +export function invalidateSystemEnvForGlobal(stalePath: string): Promise { + return enqueueGlobalCacheMutation(async () => { + await Promise.all([ + runCacheOperation('invalidate', 'workspaceState', async () => { + const state = await getWorkspacePersistentState(); + const selection = await state.get(SYSTEM_GLOBAL_KEY); + if (getStoredGlobalSelectionPath(selection) === stalePath) { + await state.set(SYSTEM_GLOBAL_KEY, undefined); + } + }), + runCacheOperation('invalidate', 'globalState', async () => { + const state = await getGlobalPersistentState(); + const selection = await state.get(SYSTEM_GLOBAL_KEY); + if (getStoredGlobalSelectionPath(selection) === stalePath) { + await state.set(SYSTEM_GLOBAL_KEY, undefined); + } + }), + ]); + }); } -export async function setSystemEnvForGlobal(envPath: string | undefined): Promise { - const state = await getGlobalPersistentState(); - await state.set(SYSTEM_GLOBAL_KEY, envPath); +/** + * Persists the global system Python path in both global and workspace state. + * + * Each write is best-effort and independent so a failure in one storage layer + * cannot block discovery or prevent the other layer from being updated. An + * `undefined` path is persisted as a tombstone so a failed mirror deletion cannot + * resurrect an older fallback value. + */ +export function setSystemEnvForGlobal(envPath: string | undefined): Promise { + return enqueueGlobalCacheMutation(async () => { + const revision = randomUUID(); + const selection: GlobalSelectionCacheEntry = { path: envPath, revision }; + let workspaceState: PersistentState | undefined; + try { + workspaceState = await getWorkspacePersistentState(); + await workspaceState.set(SYSTEM_GLOBAL_REVISION_KEY, revision); + } catch (error) { + traceError('Failed to prepare workspaceState system Python cache update:', error); + } + + await Promise.all([ + runCacheOperation('update', 'workspaceState', async () => { + const state = workspaceState ?? (await getWorkspacePersistentState()); + await state.set(SYSTEM_GLOBAL_KEY, selection); + }), + runCacheOperation('update', 'globalState', async () => { + const state = await getGlobalPersistentState(); + await state.set(SYSTEM_GLOBAL_KEY, selection); + }), + ]); + }); } diff --git a/src/managers/builtin/sysPythonManager.ts b/src/managers/builtin/sysPythonManager.ts index bede37b24..c40f8a5a9 100644 --- a/src/managers/builtin/sysPythonManager.ts +++ b/src/managers/builtin/sysPythonManager.ts @@ -33,6 +33,7 @@ import { clearSystemEnvCache, getSystemEnvForGlobal, getSystemEnvForWorkspace, + invalidateSystemEnvForGlobal, setSystemEnvForGlobal, setSystemEnvForWorkspace, setSystemEnvForWorkspaces, @@ -321,6 +322,7 @@ export class SysPythonManager implements EnvironmentManager { getProjectFsPath: (s) => getProjectFsPathForScope(this.api, s), getPersistedPath: (fsPath) => getSystemEnvForWorkspace(fsPath), getGlobalPersistedPath: () => getSystemEnvForGlobal(), + invalidateGlobalPersistedPath: (stalePath) => invalidateSystemEnvForGlobal(stalePath), resolve: (p) => resolveSystemPythonEnvironmentPath(p, this.nativeFinder, this.api, this), startBackgroundInit: () => this.initializeDiscovery(), }); @@ -340,9 +342,7 @@ export class SysPythonManager implements EnvironmentManager { async set(scope: SetEnvironmentScope, environment?: PythonEnvironment): Promise { if (scope === undefined) { this.globalEnv = environment ?? getDefaultGlobalPython(this.collection); - if (environment) { - await setSystemEnvForGlobal(environment.environmentPath.fsPath); - } + await setSystemEnvForGlobal(environment?.environmentPath.fsPath); } if (scope instanceof Uri) { diff --git a/src/managers/common/fastPath.ts b/src/managers/common/fastPath.ts index 813702103..f93012e7e 100644 --- a/src/managers/common/fastPath.ts +++ b/src/managers/common/fastPath.ts @@ -32,6 +32,8 @@ export interface FastPathOptions { startBackgroundInit: () => Promise | Thenable; /** Optional: reads the persisted env path for global scope (when scope is undefined). */ getGlobalPersistedPath?: () => Promise; + /** Optional: removes a definitively stale persisted path for global scope. */ + invalidateGlobalPersistedPath?: (persistedPath: string) => Promise; } /** @@ -71,6 +73,17 @@ async function persistedPathExists(persistedPath: string): Promise { } } +async function invalidateGlobalPersistedPath(opts: FastPathOptions, persistedPath: string): Promise { + if (!opts.invalidateGlobalPersistedPath) { + return; + } + try { + await opts.invalidateGlobalPersistedPath(persistedPath); + } catch (error) { + traceError(`[${opts.label}] Failed to invalidate stale global path '${persistedPath}':`, error); + } +} + /** * Attempts fast-path resolution for manager.get(): if full initialization hasn't completed yet * and there's a persisted environment for the workspace, resolve it directly via nativeFinder @@ -133,6 +146,7 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise(operation: Promise): Promise { + let timer: NodeJS.Timeout | undefined; + const timeout = new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error('Persistent state operation remained blocked')), 250); + }); + try { + return await Promise.race([operation, timeout]); + } finally { + if (timer) { + clearTimeout(timer); + } + } +} + +suite('Persistent state', () => { + test('releases later reads and writes after a clear fails', async () => { + const store = new Map([['stale', 'value']]); + const failure = new Error('storage update failed'); + let failClear = true; + const memento = { + get: (key: string, defaultValue?: T) => (store.has(key) ? (store.get(key) as T) : defaultValue), + update: async (key: string, value: unknown) => { + if (key === 'stale' && value === undefined && failClear) { + failClear = false; + throw failure; + } + if (value === undefined) { + store.delete(key); + } else { + store.set(key, value); + } + }, + keys: () => [...store.keys()], + } as Memento; + const state = createPersistentState(memento); + + await assert.rejects(state.clear(['stale']), (error: unknown) => error === failure); + + assert.strictEqual(await withTimeout(state.get('stale')), 'value'); + await withTimeout(state.set('fresh', 'saved')); + assert.strictEqual(store.get('fresh'), 'saved'); + + await withTimeout(state.clear(['stale'])); + assert.strictEqual(store.has('stale'), false); + }); + + test('waits for every deletion to settle before releasing operations after a failure', async () => { + const store = new Map([ + ['fails', 'value'], + ['delayed', 'value'], + ]); + const failure = new Error('first deletion failed'); + let releaseDelayedDeletion: (() => void) | undefined; + let delayedDeletionStarted: (() => void) | undefined; + const delayedStarted = new Promise((resolve) => { + delayedDeletionStarted = resolve; + }); + const delayedRelease = new Promise((resolve) => { + releaseDelayedDeletion = resolve; + }); + const memento = { + get: (key: string, defaultValue?: T) => (store.has(key) ? (store.get(key) as T) : defaultValue), + update: async (key: string, value: unknown) => { + if (key === 'fails' && value === undefined) { + throw failure; + } + if (key === 'delayed' && value === undefined) { + delayedDeletionStarted!(); + await delayedRelease; + } + if (value === undefined) { + store.delete(key); + } else { + store.set(key, value); + } + }, + keys: () => [...store.keys()], + } as Memento; + const state = createPersistentState(memento); + + const clearing = state.clear(['fails', 'delayed']); + await delayedStarted; + let readCompleted = false; + const readDuringClear = state.get('delayed').then((value) => { + readCompleted = true; + return value; + }); + let writeCompleted = false; + const writeDuringClear = state.set('delayed', 'new selection').then(() => { + writeCompleted = true; + }); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(readCompleted, false, 'reads must remain blocked while a sibling deletion is pending'); + assert.strictEqual(writeCompleted, false, 'writes must remain blocked while a sibling deletion is pending'); + + releaseDelayedDeletion!(); + await assert.rejects(clearing, (error: unknown) => error === failure); + assert.strictEqual(await withTimeout(readDuringClear), undefined); + await withTimeout(writeDuringClear); + assert.strictEqual(store.get('delayed'), 'new selection'); + }); +}); \ No newline at end of file diff --git a/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts b/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts new file mode 100644 index 000000000..6ad94310c --- /dev/null +++ b/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts @@ -0,0 +1,523 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as logging from '../../../common/logging'; +import * as persistentState from '../../../common/persistentState'; +import { createDeferred } from '../../../common/utils/deferred'; +import { + clearSystemEnvCache, + getSystemEnvForGlobal, + invalidateSystemEnvForGlobal, + setSystemEnvForGlobal, + SYSTEM_GLOBAL_KEY, + SYSTEM_GLOBAL_REVISION_KEY, + SYSTEM_WORKSPACE_KEY, +} from '../../../managers/builtin/cache'; + +interface MockPersistentState { + get: sinon.SinonStub; + set: sinon.SinonStub; + clear: sinon.SinonStub; +} + +interface CacheEntry { + readonly path?: string; + readonly revision: string; +} + +function entry(pathValue: string | undefined, revision: string): CacheEntry { + return { path: pathValue, revision }; +} + +function createMemoryState(backing: Map, sandbox: sinon.SinonSandbox): MockPersistentState { + return { + get: sandbox.stub().callsFake(async (key: string) => backing.get(key)), + set: sandbox.stub().callsFake(async (key: string, value: unknown) => { + if (value === undefined) { + backing.delete(key); + } else { + backing.set(key, value); + } + }), + clear: sandbox.stub().callsFake(async (keys?: string[]) => { + (keys ?? [...backing.keys()]).forEach((key) => backing.delete(key)); + }), + }; +} + +function getSetValue(state: MockPersistentState, key: string): unknown { + const call = state.set.getCalls().find((candidate) => candidate.args[0] === key); + assert.ok(call, `Expected ${key} to be written`); + return call.args[1]; +} + +suite('System global environment cache persistence', () => { + let sandbox: sinon.SinonSandbox; + let workspaceState: MockPersistentState; + let globalState: MockPersistentState; + let getWorkspaceState: sinon.SinonStub; + let getGlobalState: sinon.SinonStub; + + const workspacePath = path.resolve('cache-test', 'workspace-python'); + const globalPath = path.resolve('cache-test', 'global-python'); + + setup(() => { + sandbox = sinon.createSandbox(); + workspaceState = { + get: sandbox.stub().resolves(undefined), + set: sandbox.stub().resolves(), + clear: sandbox.stub().resolves(), + }; + globalState = { + get: sandbox.stub().resolves(undefined), + set: sandbox.stub().resolves(), + clear: sandbox.stub().resolves(), + }; + getWorkspaceState = sandbox + .stub(persistentState, 'getWorkspacePersistentState') + .resolves(workspaceState as unknown as persistentState.PersistentState); + getGlobalState = sandbox + .stub(persistentState, 'getGlobalPersistentState') + .resolves(globalState as unknown as persistentState.PersistentState); + }); + + teardown(() => { + sandbox.restore(); + }); + + suite('getSystemEnvForGlobal', () => { + test('prefers the workspace mirror over a different globalState value', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(workspacePath); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(globalPath); + + assert.strictEqual(await getSystemEnvForGlobal(), workspacePath); + sinon.assert.calledWithExactly(workspaceState.get, SYSTEM_GLOBAL_KEY); + sinon.assert.notCalled(getGlobalState); + }); + + test('falls back to a legacy globalState value when the workspace mirror is cold', async () => { + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(globalPath); + + assert.strictEqual(await getSystemEnvForGlobal(), globalPath); + sinon.assert.calledWithExactly(workspaceState.get, SYSTEM_GLOBAL_KEY); + sinon.assert.calledOnceWithExactly(globalState.get, SYSTEM_GLOBAL_KEY); + }); + + test('falls back to globalState when the workspaceState read fails', async () => { + const failure = new Error('workspace read failed'); + const logError = sandbox.stub(logging, 'traceError'); + workspaceState.get.rejects(failure); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(globalPath); + + assert.strictEqual(await getSystemEnvForGlobal(), globalPath); + sinon.assert.calledOnceWithExactly( + logError, + 'Failed to read workspaceState system Python cache:', + failure, + ); + }); + + test('falls back to globalState when workspaceState is unavailable', async () => { + const failure = new Error('workspace state unavailable'); + const logError = sandbox.stub(logging, 'traceError'); + getWorkspaceState.rejects(failure); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(globalPath); + + assert.strictEqual(await getSystemEnvForGlobal(), globalPath); + sinon.assert.calledOnceWithExactly( + logError, + 'Failed to read workspaceState system Python cache:', + failure, + ); + }); + + test('returns a cache miss and logs when the globalState fallback read fails', async () => { + const failure = new Error('global read failed'); + const logError = sandbox.stub(logging, 'traceError'); + globalState.get.rejects(failure); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + sinon.assert.calledOnceWithExactly(logError, 'Failed to read globalState system Python cache:', failure); + }); + + test('uses a matching global revision to recover a failed workspace mirror update', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(workspacePath, 'old')); + workspaceState.get.withArgs(SYSTEM_GLOBAL_REVISION_KEY).resolves('new'); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(globalPath, 'new')); + + assert.strictEqual(await getSystemEnvForGlobal(), globalPath); + }); + + test('misses when neither layer completed the marked update', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(workspacePath, 'old')); + workspaceState.get.withArgs(SYSTEM_GLOBAL_REVISION_KEY).resolves('new'); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(workspacePath, 'old')); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + }); + + test('honors a workspace tombstone without resurrecting an older global value', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(undefined, 'clear')); + workspaceState.get.withArgs(SYSTEM_GLOBAL_REVISION_KEY).resolves('clear'); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(workspacePath, 'old')); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + sinon.assert.notCalled(getGlobalState); + }); + + test('uses a matching global tombstone when the workspace clear failed', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(workspacePath, 'old')); + workspaceState.get.withArgs(SYSTEM_GLOBAL_REVISION_KEY).resolves('clear'); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(undefined, 'clear')); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + }); + + test('does not resurrect a cleared workspace entry after globalState becomes cold', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(entry(workspacePath, 'old')); + workspaceState.get.withArgs(SYSTEM_GLOBAL_REVISION_KEY).resolves('clear'); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(undefined); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + }); + + test('returns undefined without mutating storage when both layers are cold', async () => { + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + sinon.assert.notCalled(workspaceState.set); + sinon.assert.notCalled(globalState.set); + }); + + test('survives a cold globalState only for the same workspace', async () => { + const globalBacking = new Map(); + const workspaceBacking = new Map(); + getGlobalState.resolves( + createMemoryState(globalBacking, sandbox) as unknown as persistentState.PersistentState, + ); + getWorkspaceState.resolves( + createMemoryState(workspaceBacking, sandbox) as unknown as persistentState.PersistentState, + ); + + await setSystemEnvForGlobal(globalPath); + globalBacking.delete(SYSTEM_GLOBAL_KEY); + + assert.strictEqual(await getSystemEnvForGlobal(), globalPath, 'same workspace should use its mirror'); + + getWorkspaceState.resolves( + createMemoryState(new Map(), sandbox) as unknown as persistentState.PersistentState, + ); + + assert.strictEqual( + await getSystemEnvForGlobal(), + undefined, + 'a new workspace with cold globalState must still miss', + ); + }); + }); + + suite('invalidateSystemEnvForGlobal', () => { + test('removes the matching stale path from both storage layers', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(workspacePath); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(workspacePath); + + await invalidateSystemEnvForGlobal(workspacePath); + + sinon.assert.calledOnceWithExactly(workspaceState.set, SYSTEM_GLOBAL_KEY, undefined); + sinon.assert.calledOnceWithExactly(globalState.set, SYSTEM_GLOBAL_KEY, undefined); + }); + + test('removes only layers that still contain the stale path', async () => { + workspaceState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(globalPath); + globalState.get.withArgs(SYSTEM_GLOBAL_KEY).resolves(workspacePath); + + await invalidateSystemEnvForGlobal(workspacePath); + + sinon.assert.notCalled(workspaceState.set); + sinon.assert.calledOnceWithExactly(globalState.set, SYSTEM_GLOBAL_KEY, undefined); + }); + + test('does not erase a newer selection queued before stale invalidation', async () => { + const workspaceBacking = new Map([[SYSTEM_GLOBAL_KEY, workspacePath]]); + const globalBacking = new Map([[SYSTEM_GLOBAL_KEY, workspacePath]]); + getWorkspaceState.resolves( + createMemoryState(workspaceBacking, sandbox) as unknown as persistentState.PersistentState, + ); + getGlobalState.resolves( + createMemoryState(globalBacking, sandbox) as unknown as persistentState.PersistentState, + ); + + const newerSelection = setSystemEnvForGlobal(globalPath); + const staleInvalidation = invalidateSystemEnvForGlobal(workspacePath); + await Promise.all([newerSelection, staleInvalidation]); + + assert.strictEqual( + (workspaceBacking.get(SYSTEM_GLOBAL_KEY) as CacheEntry).path, + globalPath, + ); + assert.strictEqual((globalBacking.get(SYSTEM_GLOBAL_KEY) as CacheEntry).path, globalPath); + }); + }); + + suite('setSystemEnvForGlobal', () => { + test('writes the same revisioned path to workspaceState and globalState', async () => { + await setSystemEnvForGlobal(globalPath); + + const workspaceEntry = getSetValue(workspaceState, SYSTEM_GLOBAL_KEY) as CacheEntry; + const globalEntry = getSetValue(globalState, SYSTEM_GLOBAL_KEY) as CacheEntry; + const revision = getSetValue(workspaceState, SYSTEM_GLOBAL_REVISION_KEY); + assert.strictEqual(workspaceEntry.path, globalPath); + assert.strictEqual(globalEntry.path, globalPath); + assert.strictEqual(workspaceEntry.revision, revision); + assert.strictEqual(globalEntry.revision, revision); + }); + + test('writes matching tombstones to both layers when the selection is cleared', async () => { + await setSystemEnvForGlobal(undefined); + + const workspaceEntry = getSetValue(workspaceState, SYSTEM_GLOBAL_KEY) as CacheEntry; + const globalEntry = getSetValue(globalState, SYSTEM_GLOBAL_KEY) as CacheEntry; + assert.strictEqual(workspaceEntry.path, undefined); + assert.strictEqual(globalEntry.path, undefined); + assert.strictEqual(workspaceEntry.revision, globalEntry.revision); + }); + + test('updates globalState when the workspace selection write fails', async () => { + const failure = new Error('workspace write failed'); + const logError = sandbox.stub(logging, 'traceError'); + workspaceState.set.callsFake(async (key: string) => { + if (key === SYSTEM_GLOBAL_KEY) { + throw failure; + } + }); + + await setSystemEnvForGlobal(globalPath); + + assert.strictEqual((getSetValue(globalState, SYSTEM_GLOBAL_KEY) as CacheEntry).path, globalPath); + sinon.assert.calledWithExactly( + logError, + 'Failed to update workspaceState system Python cache:', + failure, + ); + }); + + test('updates globalState when workspaceState is unavailable', async () => { + const failure = new Error('workspace state unavailable'); + const logError = sandbox.stub(logging, 'traceError'); + getWorkspaceState.rejects(failure); + + await setSystemEnvForGlobal(globalPath); + + assert.strictEqual((getSetValue(globalState, SYSTEM_GLOBAL_KEY) as CacheEntry).path, globalPath); + sinon.assert.calledWithExactly( + logError, + 'Failed to update workspaceState system Python cache:', + failure, + ); + }); + + test('updates workspaceState when the globalState write fails', async () => { + const failure = new Error('global write failed'); + const logError = sandbox.stub(logging, 'traceError'); + globalState.set.rejects(failure); + + await setSystemEnvForGlobal(globalPath); + + assert.strictEqual((getSetValue(workspaceState, SYSTEM_GLOBAL_KEY) as CacheEntry).path, globalPath); + sinon.assert.calledOnceWithExactly( + logError, + 'Failed to update globalState system Python cache:', + failure, + ); + }); + + test('reads the successful global update when the workspace mirror write fails', async () => { + const workspaceBacking = new Map([ + [SYSTEM_GLOBAL_KEY, entry(workspacePath, 'old')], + [SYSTEM_GLOBAL_REVISION_KEY, 'old'], + ]); + const globalBacking = new Map([[SYSTEM_GLOBAL_KEY, entry(workspacePath, 'old')]]); + const failure = new Error('workspace selection write failed'); + workspaceState = createMemoryState(workspaceBacking, sandbox); + globalState = createMemoryState(globalBacking, sandbox); + workspaceState.set.callsFake(async (key: string, value: unknown) => { + if (key === SYSTEM_GLOBAL_KEY) { + throw failure; + } + workspaceBacking.set(key, value); + }); + getWorkspaceState.resolves(workspaceState as unknown as persistentState.PersistentState); + getGlobalState.resolves(globalState as unknown as persistentState.PersistentState); + + await setSystemEnvForGlobal(globalPath); + + assert.strictEqual(await getSystemEnvForGlobal(), globalPath); + }); + + test('does not resurrect globalState when its tombstone write fails', async () => { + const workspaceBacking = new Map([ + [SYSTEM_GLOBAL_KEY, entry(workspacePath, 'old')], + [SYSTEM_GLOBAL_REVISION_KEY, 'old'], + ]); + const globalBacking = new Map([[SYSTEM_GLOBAL_KEY, entry(workspacePath, 'old')]]); + workspaceState = createMemoryState(workspaceBacking, sandbox); + globalState = createMemoryState(globalBacking, sandbox); + globalState.set.rejects(new Error('global tombstone write failed')); + getWorkspaceState.resolves(workspaceState as unknown as persistentState.PersistentState); + getGlobalState.resolves(globalState as unknown as persistentState.PersistentState); + + await setSystemEnvForGlobal(undefined); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + }); + + test('uses the global tombstone when the workspace tombstone write fails', async () => { + const workspaceBacking = new Map([ + [SYSTEM_GLOBAL_KEY, entry(workspacePath, 'old')], + [SYSTEM_GLOBAL_REVISION_KEY, 'old'], + ]); + const globalBacking = new Map([[SYSTEM_GLOBAL_KEY, entry(workspacePath, 'old')]]); + workspaceState = createMemoryState(workspaceBacking, sandbox); + globalState = createMemoryState(globalBacking, sandbox); + workspaceState.set.callsFake(async (key: string, value: unknown) => { + if (key === SYSTEM_GLOBAL_KEY) { + throw new Error('workspace tombstone write failed'); + } + workspaceBacking.set(key, value); + }); + getWorkspaceState.resolves(workspaceState as unknown as persistentState.PersistentState); + getGlobalState.resolves(globalState as unknown as persistentState.PersistentState); + + await setSystemEnvForGlobal(undefined); + + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + }); + test('logs both selection failures without rejecting discovery', async () => { + const workspaceFailure = new Error('workspace write failed'); + const globalFailure = new Error('global write failed'); + const logError = sandbox.stub(logging, 'traceError'); + workspaceState.set.callsFake(async (key: string) => { + if (key === SYSTEM_GLOBAL_KEY) { + throw workspaceFailure; + } + }); + globalState.set.rejects(globalFailure); + + await setSystemEnvForGlobal(globalPath); + + sinon.assert.calledWithExactly( + logError, + 'Failed to update workspaceState system Python cache:', + workspaceFailure, + ); + sinon.assert.calledWithExactly( + logError, + 'Failed to update globalState system Python cache:', + globalFailure, + ); + }); + + test('serializes overlapping updates so the latest selection wins in both layers', async () => { + const workspaceBacking = new Map(); + const globalBacking = new Map(); + workspaceState = createMemoryState(workspaceBacking, sandbox); + globalState = createMemoryState(globalBacking, sandbox); + getWorkspaceState.resolves(workspaceState as unknown as persistentState.PersistentState); + getGlobalState.resolves(globalState as unknown as persistentState.PersistentState); + + const firstWriteStarted = createDeferred(); + const releaseFirstWrite = createDeferred(); + workspaceState.set.callsFake(async (key: string, value: unknown) => { + if (key === SYSTEM_GLOBAL_KEY && (value as CacheEntry).path === workspacePath) { + firstWriteStarted.resolve(); + await releaseFirstWrite.promise; + } + if (value === undefined) { + workspaceBacking.delete(key); + } else { + workspaceBacking.set(key, value); + } + }); + + const first = setSystemEnvForGlobal(workspacePath); + await firstWriteStarted.promise; + const second = setSystemEnvForGlobal(undefined); + await new Promise((resolve) => setImmediate(resolve)); + + try { + assert.strictEqual(workspaceState.set.callCount, 2, 'second workspace write must remain queued'); + assert.strictEqual(globalState.set.callCount, 1, 'second global write must remain queued'); + } finally { + releaseFirstWrite.resolve(); + } + + await Promise.all([first, second]); + assert.strictEqual(await getSystemEnvForGlobal(), undefined); + assert.strictEqual((workspaceBacking.get(SYSTEM_GLOBAL_KEY) as CacheEntry).path, undefined); + assert.strictEqual((globalBacking.get(SYSTEM_GLOBAL_KEY) as CacheEntry).path, undefined); + }); + }); + + suite('clearSystemEnvCache', () => { + test('clears workspace selections, mirror metadata, and the global cache', async () => { + await clearSystemEnvCache(); + + sinon.assert.calledOnceWithExactly(workspaceState.clear, [ + SYSTEM_WORKSPACE_KEY, + SYSTEM_GLOBAL_KEY, + SYSTEM_GLOBAL_REVISION_KEY, + ]); + sinon.assert.calledOnceWithExactly(globalState.clear, [SYSTEM_GLOBAL_KEY]); + }); + + test('clears globalState when the workspaceState clear fails', async () => { + const failure = new Error('workspace clear failed'); + const logError = sandbox.stub(logging, 'traceError'); + workspaceState.clear.rejects(failure); + + await assert.rejects(clearSystemEnvCache, (error: unknown) => error === failure); + + sinon.assert.calledOnceWithExactly(globalState.clear, [SYSTEM_GLOBAL_KEY]); + sinon.assert.calledOnceWithExactly( + logError, + 'Failed to clear workspaceState system Python cache:', + failure, + ); + }); + + test('clears globalState when workspaceState is unavailable', async () => { + const failure = new Error('workspace state unavailable'); + const logError = sandbox.stub(logging, 'traceError'); + getWorkspaceState.rejects(failure); + + await assert.rejects(clearSystemEnvCache, (error: unknown) => error === failure); + + sinon.assert.calledOnceWithExactly(globalState.clear, [SYSTEM_GLOBAL_KEY]); + sinon.assert.calledOnceWithExactly( + logError, + 'Failed to clear workspaceState system Python cache:', + failure, + ); + }); + + test('clears workspaceState when the globalState clear fails', async () => { + const failure = new Error('global clear failed'); + const logError = sandbox.stub(logging, 'traceError'); + globalState.clear.rejects(failure); + + await assert.rejects(clearSystemEnvCache, (error: unknown) => error === failure); + + sinon.assert.calledOnceWithExactly(workspaceState.clear, [ + SYSTEM_WORKSPACE_KEY, + SYSTEM_GLOBAL_KEY, + SYSTEM_GLOBAL_REVISION_KEY, + ]); + sinon.assert.calledOnceWithExactly( + logError, + 'Failed to clear globalState system Python cache:', + failure, + ); + }); + }); +}); diff --git a/src/test/managers/builtin/sysPythonManager.pymanager.unit.test.ts b/src/test/managers/builtin/sysPythonManager.pymanager.unit.test.ts index e0593a853..acea04dfc 100644 --- a/src/test/managers/builtin/sysPythonManager.pymanager.unit.test.ts +++ b/src/test/managers/builtin/sysPythonManager.pymanager.unit.test.ts @@ -112,6 +112,15 @@ suite('System Python PyManager integration', () => { sinon.assert.notCalled(list); }); + test('clears the persisted selection when global system Python is reset', async () => { + await manager.getEnvironments('global'); + + await manager.set(undefined); + + sinon.assert.calledOnceWithExactly(setGlobal, undefined); + assert.strictEqual(await manager.get(undefined), native); + }); + test('delegates missing-Python acquisition without re-entering the environment API', async () => { nativeRefresh.resolves([]); const installed = environment(path.join(root, 'installed', 'python.exe'), '3.14.6'); diff --git a/src/test/managers/common/fastPath.unit.test.ts b/src/test/managers/common/fastPath.unit.test.ts index 4c6bb17ae..8cc731c10 100644 --- a/src/test/managers/common/fastPath.unit.test.ts +++ b/src/test/managers/common/fastPath.unit.test.ts @@ -115,32 +115,38 @@ suite('tryFastPathGet', () => { assert.strictEqual(props.result, 'miss'); }); - test('reports stale when global cached path resolves to undefined', async () => { + test('reports stale without invalidating when global cached path resolves to undefined', async () => { const globalPath = __filename; + const invalidateGlobalPersistedPath = sinon.stub().resolves(); const { opts } = createOpts({ scope: undefined, getGlobalPersistedPath: sinon.stub().resolves(globalPath), + invalidateGlobalPersistedPath, resolve: sinon.stub().resolves(undefined), }); const result = await tryFastPathGet(opts); assert.strictEqual(result, undefined, 'Should fall through when cached env resolves to undefined'); + sinon.assert.notCalled(invalidateGlobalPersistedPath); assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale cache'); const [eventName, , props] = sendTelemetryStub.firstCall.args; assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE); assert.strictEqual(props.result, 'stale'); }); - test('returns undefined for global scope when cached path resolve fails', async () => { + test('returns undefined without invalidating when cached path resolve throws', async () => { const globalPath = __filename; + const invalidateGlobalPersistedPath = sinon.stub().resolves(); const { opts } = createOpts({ scope: undefined, getGlobalPersistedPath: sinon.stub().resolves(globalPath), - resolve: sinon.stub().rejects(new Error('python was uninstalled')), + invalidateGlobalPersistedPath, + resolve: sinon.stub().rejects(new Error('transient resolve failure')), }); const result = await tryFastPathGet(opts); - assert.strictEqual(result, undefined, 'Should fall through when cached global env is stale'); + assert.strictEqual(result, undefined, 'Should fall through when cached global env cannot be resolved'); + sinon.assert.notCalled(invalidateGlobalPersistedPath); // Verify cache stale telemetry assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale global cache'); @@ -201,31 +207,37 @@ suite('tryFastPathGet', () => { assert.strictEqual(result, undefined); }); - test('skips resolve and falls through when workspace persisted path no longer exists on disk', async () => { + test('skips resolve without invalidating global state when a workspace path is missing', async () => { const missingPath = path.resolve('does', 'not', 'exist', 'python-missing'); const resolve = sinon.stub().resolves(createMockEnv(missingPath)); + const invalidateGlobalPersistedPath = sinon.stub().resolves(); const { opts } = createOpts({ getPersistedPath: sinon.stub().resolves(missingPath), + invalidateGlobalPersistedPath, resolve, }); const result = await tryFastPathGet(opts); assert.strictEqual(result, undefined, 'Should fall through when cached path is missing'); assert.ok(resolve.notCalled, 'Should not invoke resolve (and thus PET) when path is missing'); + sinon.assert.notCalled(invalidateGlobalPersistedPath); }); - test('skips resolve and reports stale when global persisted path no longer exists on disk', async () => { + test('skips resolve, reports stale, and invalidates when global persisted path is missing', async () => { const missingPath = path.resolve('does', 'not', 'exist', 'python-missing'); const resolve = sinon.stub().resolves(createMockEnv(missingPath)); + const invalidateGlobalPersistedPath = sinon.stub().resolves(); const { opts } = createOpts({ scope: undefined, getGlobalPersistedPath: sinon.stub().resolves(missingPath), + invalidateGlobalPersistedPath, resolve, }); const result = await tryFastPathGet(opts); assert.strictEqual(result, undefined, 'Should fall through when cached global path is missing'); assert.ok(resolve.notCalled, 'Should not invoke resolve (and thus PET) when path is missing'); + sinon.assert.calledOnceWithExactly(invalidateGlobalPersistedPath, missingPath); assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale global cache'); const [eventName, , props] = sendTelemetryStub.firstCall.args; assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE); diff --git a/src/test/managers/fastPath.get.unit.test.ts b/src/test/managers/fastPath.get.unit.test.ts index c7142ed05..e0c728ec7 100644 --- a/src/test/managers/fastPath.get.unit.test.ts +++ b/src/test/managers/fastPath.get.unit.test.ts @@ -302,6 +302,29 @@ suite('Manager get() fast path', () => { }); }); + suite('SysPythonManager global cache', () => { + test('invalidates a definitively missing global selection through both cache layers', async () => { + const persistedPath = path.resolve('missing', 'global-python'); + sandbox.stub(sysCache, 'getSystemEnvForGlobal').resolves(persistedPath); + const invalidate = sandbox.stub(sysCache, 'invalidateSystemEnvForGlobal').resolves(); + const resolve = sandbox.stub(sysUtils, 'resolveSystemPythonEnvironmentPath').resolves(undefined); + (fs.promises.access as sinon.SinonStub).withArgs(persistedPath).rejects( + Object.assign(new Error('missing'), { code: 'ENOENT' }), + ); + sandbox.stub(sysUtils, 'refreshPythons').resolves([]); + const manager = new SysPythonManager(createMockNativeFinder(), createMockApi(), createMockLog()); + sandbox + .stub(manager as unknown as { initializeDiscovery: () => Promise }, 'initializeDiscovery') + .resolves(); + sandbox.stub(manager, 'initialize').resolves(); + + assert.strictEqual(await manager.get(undefined), undefined); + + sinon.assert.notCalled(resolve); + sinon.assert.calledOnceWithExactly(invalidate, persistedPath); + }); + }); + suite('VenvManager specific', () => { test('fast path: background init failure resets _initialized for retry', async () => { const persistedPath = path.resolve('test-workspace', '.venv');