From 4e91f8f213258a480d9f310cbd40a92a4e0aa14a Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 23 Oct 2026 13:08:31 -0700 Subject: [PATCH 1/2] fix: persist remote system Python cache (Fixes #1581) Mirror the global system Python selection into workspace state so the same remote workspace can use its persisted fast path when global state is cold. Version mirrored entries and tombstones to preserve workspace-first behavior across partial storage failures, serialize mutations, and invalidate only definitively missing paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c45696e-17ec-4b65-9e06-21ea196234d6 --- docs/startup-flow.md | 9 +- src/common/persistentState.ts | 30 +- src/managers/builtin/cache.ts | 215 +++++++- src/managers/builtin/sysPythonManager.ts | 6 +- src/managers/common/fastPath.ts | 14 + src/test/common/persistentState.unit.test.ts | 109 ++++ .../cache.systemEnvGlobal.unit.test.ts | 515 ++++++++++++++++++ .../sysPythonManager.pymanager.unit.test.ts | 9 + .../managers/common/fastPath.unit.test.ts | 24 +- src/test/managers/fastPath.get.unit.test.ts | 23 + 10 files changed, 924 insertions(+), 30 deletions(-) create mode 100644 src/test/common/persistentState.unit.test.ts create mode 100644 src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts 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..9bad2f04a 100644 --- a/src/managers/builtin/cache.ts +++ b/src/managers/builtin/cache.ts @@ -1,14 +1,130 @@ +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 hasStoredGlobalSelection(value: unknown): boolean { + return (typeof value === 'string' && value.length > 0) || isGlobalSelectionCacheEntry(value); +} + +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 +164,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 (hasStoredGlobalSelection(workspace.selection)) { + return getStoredGlobalSelectionPath(workspace.selection); + } + 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..5ec3f70a4 --- /dev/null +++ b/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts @@ -0,0 +1,515 @@ +// 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('keeps the workspace value 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(), workspacePath); + }); + + 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('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'); From 4f8b9efb09ae3b9d7ab70fa27bca20cb4ce62c44 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 23 Oct 2026 13:31:31 -0700 Subject: [PATCH 2/2] fix: avoid stale cache resurrection (PR #1951) Treat a revision marker without a matching completed entry as a cache miss so an older workspace value cannot reappear after a partial clear and cold global state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c45696e-17ec-4b65-9e06-21ea196234d6 --- src/managers/builtin/cache.ts | 8 ++------ .../builtin/cache.systemEnvGlobal.unit.test.ts | 12 ++++++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/managers/builtin/cache.ts b/src/managers/builtin/cache.ts index 9bad2f04a..f5eba4745 100644 --- a/src/managers/builtin/cache.ts +++ b/src/managers/builtin/cache.ts @@ -48,10 +48,6 @@ function isGlobalSelectionCacheEntry(value: unknown): value is GlobalSelectionCa ); } -function hasStoredGlobalSelection(value: unknown): boolean { - return (typeof value === 'string' && value.length > 0) || isGlobalSelectionCacheEntry(value); -} - function getStoredGlobalSelectionPath(value: unknown): string | undefined { if (typeof value === 'string') { return value || undefined; @@ -187,8 +183,8 @@ export async function getSystemEnvForGlobal(): Promise { ) { return globalSelection.path; } - if (hasStoredGlobalSelection(workspace.selection)) { - return getStoredGlobalSelectionPath(workspace.selection); + if (workspace.revision) { + return undefined; } return getStoredGlobalSelectionPath(globalSelection); } diff --git a/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts b/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts index 5ec3f70a4..6ad94310c 100644 --- a/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts +++ b/src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts @@ -151,12 +151,12 @@ suite('System global environment cache persistence', () => { assert.strictEqual(await getSystemEnvForGlobal(), globalPath); }); - test('keeps the workspace value when neither layer completed the marked update', async () => { + 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(), workspacePath); + assert.strictEqual(await getSystemEnvForGlobal(), undefined); }); test('honors a workspace tombstone without resurrecting an older global value', async () => { @@ -176,6 +176,14 @@ suite('System global environment cache persistence', () => { 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);