diff --git a/docs/managing-python-projects.md b/docs/managing-python-projects.md index 487e8d80..e1acaa4f 100644 --- a/docs/managing-python-projects.md +++ b/docs/managing-python-projects.md @@ -113,6 +113,10 @@ Stop runs or debug sessions using the environment before deleting it. The extens On Windows, changing only the letter casing of a script's filename keeps its existing environment association. A rename does not validate unsaved dependency edits or install packages. +Unused cached script environments are cleaned up once per window, about two minutes after the extension activates, rather than being triggered by environment creation. Cleanup considers entries unused for more than 14 days and incomplete setups older than one day, removes at most three entries, and skips environments referenced by this workspace or entries it cannot safely inspect. There is no daily sweep. + +The background cache scan does not hold up interpreter lookups. If another window briefly locks an entry, or its last-used time cannot be updated safely, the extension retries the script association in the background with bounded delays. Saving without changing the inline requirements does not cancel, postpone, or reset those retries; changing the requirements or stored inline-environment association cancels outdated recovery work. A temporarily unavailable selected environment keeps its association instead of silently switching execution to another interpreter, and the setup action is available for an explicit retry. If automatic recovery does not succeed, use that action to retry. Cleanup never installs packages. + ## Assigning Environments to Projects Each project can have its own Python environment. This is the core benefit of project management. diff --git a/src/common/inlineScript/cacheLayout.ts b/src/common/inlineScript/cacheLayout.ts index 5906c646..0bfba00e 100644 --- a/src/common/inlineScript/cacheLayout.ts +++ b/src/common/inlineScript/cacheLayout.ts @@ -3,6 +3,7 @@ import * as crypto from 'crypto'; import * as fsapi from 'fs-extra'; +import { rename as renameWithoutRetry } from 'fs/promises'; import * as path from 'path'; import { Uri } from 'vscode'; import type { PythonEnvironment } from '../../api'; @@ -258,11 +259,22 @@ async function inspectMetaJsonFile(metaPath: string): Promise { +export interface WriteMetaJsonOptions { + /** + * Fail immediately instead of letting graceful-fs retry a Windows sharing violation on the + * rename for a full minute. Read-path bookkeeping must never hold the shared cache-entry lock + * that long: other windows cannot tell it apart from a build or a deletion. + */ + readonly failFast?: boolean; +} + +export function writeMetaJson(envDir: Uri, meta: InlineScriptEnvMeta, options?: WriteMetaJsonOptions): Promise { const finalPath = getMetaJsonPath(envDir).fsPath; const key = normalizePath(path.resolve(finalPath)); const previous = pendingMetaJsonWrites.get(key) ?? Promise.resolve(); - const operation = previous.catch(() => undefined).then(() => writeMetaJsonOnce(envDir, meta, finalPath)); + const operation = previous + .catch(() => undefined) + .then(() => writeMetaJsonOnce(envDir, meta, finalPath, options?.failFast === true)); let queued: Promise; queued = operation.finally(() => { if (pendingMetaJsonWrites.get(key) === queued) { @@ -273,7 +285,13 @@ export function writeMetaJson(envDir: Uri, meta: InlineScriptEnvMeta): Promise { +async function writeMetaJsonOnce( + envDir: Uri, + meta: InlineScriptEnvMeta, + finalPath: string, + failFast: boolean, +): Promise { + const rename = failFast ? renameWithoutRetry : fsapi.rename; await fsapi.ensureDir(envDir.fsPath); const tmpSuffix = crypto.randomBytes(6).toString('hex'); const tmpPath = `${finalPath}.tmp-${tmpSuffix}`; @@ -285,18 +303,20 @@ async function writeMetaJsonOnce(envDir: Uri, meta: InlineScriptEnvMeta, finalPa try { await fsapi.writeFile(tmpPath, payload, 'utf8'); try { - await fsapi.rename(tmpPath, finalPath); + await rename(tmpPath, finalPath); finalKnownToExist = true; return; } catch (err) { const code = (err as NodeJS.ErrnoException | undefined)?.code; - if (!['EPERM', 'EEXIST', 'EBUSY'].includes(code ?? '')) { + // Read-path bookkeeping must leave the old sidecar in place rather than enter + // the backup/restore path, whose recovery may itself need a retrying rename. + if (failFast || !['EPERM', 'EEXIST', 'EBUSY'].includes(code ?? '')) { throw err; } } try { - await fsapi.rename(finalPath, backupPath); + await rename(finalPath, backupPath); hasBackup = true; } catch (err) { if (!isFileNotFoundError(err)) { @@ -305,7 +325,7 @@ async function writeMetaJsonOnce(envDir: Uri, meta: InlineScriptEnvMeta, finalPa } try { - await fsapi.rename(tmpPath, finalPath); + await rename(tmpPath, finalPath); finalKnownToExist = true; } catch (replaceError) { if (hasBackup) { diff --git a/src/common/inlineScript/routingRegistry.ts b/src/common/inlineScript/routingRegistry.ts index de8ee940..0f778e2d 100644 --- a/src/common/inlineScript/routingRegistry.ts +++ b/src/common/inlineScript/routingRegistry.ts @@ -41,6 +41,7 @@ interface ScriptRoutingState { readonly metadataIdentity?: string; readonly metadataRevision: number; readonly validatedAssociation: boolean; + readonly environmentUnavailable?: boolean; } export class InlineScriptRoutingRegistry implements Disposable { @@ -49,6 +50,9 @@ export class InlineScriptRoutingRegistry implements Disposable { private readonly setupOutcomes = new Map(); private readonly _onDidChangeRouteability = new EventEmitter(); private readonly _onDidChangeMetadata = new EventEmitter(); + private readonly _onDidChangeAvailability = new EventEmitter(); + + public readonly onDidChangeAvailability: Event = this._onDidChangeAvailability.event; public readonly onDidChangeRouteability: Event = this._onDidChangeRouteability.event; @@ -73,6 +77,8 @@ export class InlineScriptRoutingRegistry implements Disposable { metadataRevision, validatedAssociation: state.metadataIdentity === metadataIdentity ? state.validatedAssociation : false, + environmentUnavailable: + state.metadataIdentity === metadataIdentity ? state.environmentUnavailable : false, }; }, true, @@ -129,9 +135,25 @@ export class InlineScriptRoutingRegistry implements Disposable { ...state, uri: script instanceof Uri ? script : state.uri, validatedAssociation, + environmentUnavailable: validatedAssociation ? state.environmentUnavailable : false, })); } + /** Keep temporary I/O failure separate from interpreter selection, while allowing setup to be retried. */ + public setEnvironmentUnavailable(uri: Uri, unavailable: boolean): void { + const scriptPath = getInlineScriptRoutingKey(uri); + if (scriptPath) { + this.update(scriptPath, (state) => ({ ...state, uri, environmentUnavailable: unavailable })); + } + } + + /** Whether a selected inline environment is temporarily withheld by a lookup. */ + public isEnvironmentUnavailable(uri: Uri): boolean { + const scriptPath = getInlineScriptRoutingKey(uri); + const state = scriptPath ? this.states.get(scriptPath) : undefined; + return this.isRouteable(state) && state?.environmentUnavailable === true; + } + public hasValidatedAssociation(script: Uri | string): boolean { const scriptPath = getInlineScriptRoutingKey(script); return scriptPath ? this.states.get(scriptPath)?.validatedAssociation === true : false; @@ -177,6 +199,7 @@ export class InlineScriptRoutingRegistry implements Disposable { this.setupOutcomes.clear(); this._onDidChangeMetadata.dispose(); this._onDidChangeRouteability.dispose(); + this._onDidChangeAvailability.dispose(); } private update( @@ -189,6 +212,7 @@ export class InlineScriptRoutingRegistry implements Disposable { validatedAssociation: false, }; const previousRouteable = this.isRouteable(previous); + const previouslyUnavailable = previousRouteable && previous.environmentUnavailable === true; const next = updater(previous); if (!next.metadata && !next.validatedAssociation) { @@ -214,6 +238,9 @@ export class InlineScriptRoutingRegistry implements Disposable { routeable, }); } + if (previouslyUnavailable !== (routeable && next.environmentUnavailable === true) && next.uri) { + this._onDidChangeAvailability.fire(next.uri); + } } private isRouteable(state: ScriptRoutingState | undefined): boolean { diff --git a/src/extensionApi.ts b/src/extensionApi.ts index 4bca3b6f..1643e40a 100644 --- a/src/extensionApi.ts +++ b/src/extensionApi.ts @@ -35,6 +35,7 @@ import type { SetEnvironmentScope, } from './types'; import { PackageVersionLookupNotSupportedError } from './publicErrors'; +import { INLINE_SCRIPT_MANAGER_ID } from './common/constants'; import { traceError, traceInfo } from './common/logging'; import { pickEnvironmentManager } from './common/pickers/managers'; import { timeout } from './common/utils/asyncUtils'; @@ -269,6 +270,13 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { // Keep the background resolution alive so the cache/last-known value gets populated and the // change event fires once it finishes. resolution.catch((ex) => traceError('Failed to resolve environment in background', ex)); + // Inline-script environments are reclaimed from a shared cache, and the manager withholds + // one it cannot prove is still safe. Serving the last-known value here would hand back the + // descriptor that decision just rejected, so only the timeout is skipped for them; every + // other manager keeps the fast fallback. + if (this.envManagers.getEnvironmentManager(currentScope)?.id === INLINE_SCRIPT_MANAGER_ID) { + return resolution; + } return this.envManagers.getLastKnownEnvironment(currentScope); } onDidChangeEnvironment: Event = this._onDidChangeEnvironment.event; diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index df8b7d62..4127aa54 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -233,6 +233,13 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { traceError('Failed to refresh inline-script routing:', error), ); }), + this.inlineScriptRouting.onDidChangeAvailability((uri) => { + if (this.getEnvironmentManager(uri)?.id === INLINE_SCRIPT_MANAGER_ID) { + void this.refreshEnvironment(uri, true).catch((error) => + traceError('Failed to refresh inline-script availability:', error), + ); + } + }), ); } } @@ -891,7 +898,11 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { return undefined; } - return manager.get(scope); + const environment = await manager.get(scope); + if (manager.id === INLINE_SCRIPT_MANAGER_ID && this.getEnvironmentManager(scope) !== manager) { + return this.getEnvironment(scope); + } + return environment; } /** @@ -902,8 +913,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { * * Unlike getEnvironment(), this IS a mutation — it updates internal state. * Unlike setEnvironment(), it does NOT call manager.set() or persist to settings. + * Availability recovery may republish an unchanged descriptor so consumers retry a failed lookup. */ - async refreshEnvironment(scope: GetEnvironmentScope): Promise { + async refreshEnvironment(scope: GetEnvironmentScope, notifyIfUnchanged = false): Promise { const manager = this.getEnvironmentManager(scope); if (!manager) { return; @@ -918,7 +930,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } const oldEnv = this._activeSelection.get(key); - if (this.isSameEnvironment(oldEnv, newEnv) || !this.commitSelectionOperation(key, operation)) { + if ( + (this.isSameEnvironment(oldEnv, newEnv) && !notifyIfUnchanged) || + !this.commitSelectionOperation(key, operation) + ) { return; } this._activeSelection.set(key, newEnv); diff --git a/src/features/inlineScript/codeLens.ts b/src/features/inlineScript/codeLens.ts index e0b374d9..460c4b8d 100644 --- a/src/features/inlineScript/codeLens.ts +++ b/src/features/inlineScript/codeLens.ts @@ -46,6 +46,7 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl ) { this.subscriptions.push( this.routing.onDidChangeRouteability(() => this._onDidChangeCodeLenses.fire()), + this.routing.onDidChangeAvailability(() => this._onDidChangeCodeLenses.fire()), // Only metadata arriving or changing can add/replace a lens; a scan that finds no metadata // (the common case for ordinary .py files) needs no refresh. Hiding a lens for an // edited/removed block is handled by VS Code re-querying on the document change itself. @@ -89,7 +90,7 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl const offset = metadata.sourceRange?.start ?? metadata.range.start; const position = document.positionAt(offset); const range = new Range(position, position); - if (this.routing.shouldRoute(uri)) { + if (this.routing.shouldRoute(uri) && !this.routing.isEnvironmentUnavailable(uri)) { // A validated inline-script environment matching the current metadata already exists. const key = getInlineScriptRoutingKey(uri); const confirmation = key ? this.readyConfirmations.get(key) : undefined; diff --git a/src/features/inlineScript/setupCodeAction.ts b/src/features/inlineScript/setupCodeAction.ts index 37275ffa..769d7b0f 100644 --- a/src/features/inlineScript/setupCodeAction.ts +++ b/src/features/inlineScript/setupCodeAction.ts @@ -92,7 +92,7 @@ export class InlineScriptSetupCodeActionProvider implements CodeActionProvider { if (!getInlineScriptRoutingKey(uri)) { return []; } - if (this.routing.shouldRoute(uri)) { + if (this.routing.shouldRoute(uri) && !this.routing.isEnvironmentUnavailable(uri)) { return []; } if (!readInlineScriptMetadata(sliceHeaderBytes(document.getText()), uri.fsPath)) { diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 956e1ec2..61eb7ec2 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -42,7 +42,6 @@ import { getErrorMessage } from '../../../common/errors/utils'; import { computeCacheKey, normalizeDependency } from '../../../common/inlineScript/cacheKey'; import { InlineScriptEnvironmentModifiedError } from '../../../common/inlineScript/errors'; import { - CacheEntrySummary, CacheEnvironmentInspection, compareInstalledPackages, getBaseInterpreterStatus, @@ -103,6 +102,38 @@ const CACHE_LOCK_RETRY_MS = 500; const CACHE_TTL_MS = 14 * 24 * 60 * 60 * 1000; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; const DISCOVERY_RETRY_DELAYS_MS = [1_000, 5_000, 30_000] as const; +const DISCOVERY_RETRY_WINDOW_MS = DISCOVERY_RETRY_DELAYS_MS.reduce((total, delay) => total + delay, 0); +/** + * Delay between activation and the opportunistic TTL sweep. Kept past + * {@link DISCOVERY_RETRY_WINDOW_MS} so it cannot exhaust the discovery retry ladder. + */ +const TTL_EVICTION_DELAY_MS = DISCOVERY_RETRY_WINDOW_MS + 60 * 1000; +/** + * Random spread added to {@link TTL_EVICTION_DELAY_MS}. Windows restored together would otherwise + * sweep the shared cache at the same offset and contend for the same entry locks. + */ +const TTL_EVICTION_JITTER_MS = 30 * 1000; +/** + * Age at which a retained entry's `lastUsedAt` is worth refreshing. Well short of the eviction-risk + * horizon so the refresh stays optional, and long enough that ordinary use rarely takes the entry + * lock at all. + */ +const LAST_USED_REFRESH_AFTER_MS = 7 * 24 * 60 * 60 * 1000; +/** Backoff when a touch could not be recorded; a contended stamp must not be skipped for a day. */ +const LAST_USED_TOUCH_RETRY_MS = 5 * 60 * 1000; +/** + * How long before {@link CACHE_TTL_MS} an unstamped entry is treated as at risk of another window's + * sweep. Being due a refresh is not the same as being evictable, so only entries inside this window + * have to prove protection before they can be handed out. + */ +const EVICTION_RISK_MARGIN_MS = 24 * 60 * 60 * 1000; +/** Entries deleted per sweep; a seeded venv is thousands of unlinks on a shared threadpool. */ +const MAX_EVICTIONS_PER_SWEEP = 3; +/** + * Grace before an entry with no usable sidecar is reclaimed. `buildCacheEntry` writes `.meta.json` + * last, so an interrupted install leaves a directory no `lastUsedAt` can ever age out. + */ +const INCOMPLETE_ENTRY_GRACE_MS = 24 * 60 * 60 * 1000; const PERSISTED_ASSOCIATION_SCHEMA_VERSION = 1 as const; /** Bounded retry for deleting a cache entry whose files may still be briefly held by a stopped installer. */ const CACHE_ENTRY_REMOVAL_ATTEMPTS = 4; @@ -175,6 +206,43 @@ interface CacheEntryRemovalOptions { readonly reclaimRetainedLock?: boolean; } +interface StaleCacheEvictionPlan { + readonly cacheRoot: Uri; + readonly physicalCacheRootPath: string; + readonly evictableStaleEntries: readonly string[]; + /** Planning clock, reused for the under-lock recheck so a reuse since then wins. */ + readonly now: Date; +} + +/** `unproven` means nothing currently shows the entry is protected, not that it is broken. */ +type UsageVerdict = 'usable' | 'unproven' | 'unusable'; + +interface LastUsedTouchState { + /** Last stamp observed on disk. Both the refresh and eviction-risk horizons derive from it. */ + readonly stampedAt?: number; + /** Earliest a new attempt may start after a failure; never permission to use the entry. */ + readonly retryNotBefore?: number; + /** Shared so overlapping resolves join one operation instead of starting their own. */ + readonly inFlight?: Promise; + /** Identifies the operation that owns this state so an older completion cannot overwrite it. */ + readonly generation?: number; + /** Bookkeeping-only refresh. Callers never await it; it exists to stop a second one starting. */ + readonly refreshInFlight?: Promise; + readonly refreshGeneration?: number; +} + +interface LastUsedStampOutcome { + readonly verdict: UsageVerdict; + readonly stampedAt?: number; + readonly retryNotBefore?: number; +} + +interface ObservedUsability { + readonly verdict: UsageVerdict; + readonly stampedAt?: number; + readonly stampDue: boolean; +} + type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; @@ -192,6 +260,13 @@ interface PendingMetadataRefresh { readonly promise: Promise; } +interface AssociationRetry { + readonly metadataIdentity: string; + readonly associationRevision: number; + attempt: number; + timer?: ReturnType; +} + interface ParsedPersistedAssociations { readonly rawEntries: Record; readonly records: PersistedInlineScriptEnvironments; @@ -212,6 +287,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private collection: PythonEnvironment[] = []; private readonly pendingRehydrations = new Map(); private readonly pendingMetadataRefreshes = new Map(); + private readonly associationRetries = new Map(); private readonly fsPathToEnv = new Map(); private readonly fsPathToPersistedAssociation = new Map(); private readonly cachedAssociationValidatedAt = new Map(); @@ -223,6 +299,12 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private activationDiscoveryActive = false; private discoveryRetryAttempt = 0; private discoveryRetryTimer: ReturnType | undefined; + private ttlEvictionTimer: ReturnType | undefined; + private readonly lastUsedTouchState = new Map(); + /** Entry dirs whose lock this manager holds purely for an optional last-used refresh. */ + private readonly bookkeepingLocks = new Set(); + private lastUsedTouchGeneration = 0; + private readonly pendingLastUsedTouches = new Set>(); private readonly subscriptions: Disposable[] = []; private readonly associationStore: InlineScriptAssociationStore; private readonly persistedAssociationsLoaded: Promise; @@ -295,7 +377,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ): Promise { this.activeCreateOperations += 1; try { - await this.runTtlEvictionOnce(); return await this.waitForCacheMaintenance(async () => { try { const scriptUri = this.getScriptUri(scope); @@ -568,7 +649,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } public startActivationDiscovery(): void { - if (this.disposed || this.activationDiscoveryActive) { + if (this.disposed) { + return; + } + this.scheduleTtlEviction(); + if (this.activationDiscoveryActive) { return; } this.activationDiscoveryActive = true; @@ -576,6 +661,34 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.runActivationDiscoveryPass(); } + /** + * Arm the once-per-window TTL sweep. Eviction is deliberately not tied to `create`: a user who + * stops creating environments would otherwise never reclaim orphaned cache entries. + */ + private scheduleTtlEviction(): void { + if (this.ttlEvictionTimer || this.ttlEviction) { + return; + } + this.ttlEvictionTimer = setTimeout(() => { + this.ttlEvictionTimer = undefined; + if (this.disposed) { + return; + } + void this.runTtlEvictionOnce(); + }, this.getTtlEvictionDelayMs()); + } + + private getTtlEvictionDelayMs(): number { + return TTL_EVICTION_DELAY_MS + Math.floor(Math.random() * TTL_EVICTION_JITTER_MS); + } + + private cancelTtlEviction(): void { + if (this.ttlEvictionTimer) { + clearTimeout(this.ttlEvictionTimer); + this.ttlEvictionTimer = undefined; + } + } + private getOrStartRefreshPass(checkForSnapshotChanges: boolean): Promise { const pending = this.pendingRefresh; if (pending) { @@ -1078,14 +1191,359 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return undefined; } - // An unreadable or invalid metadata block is indistinguishable from a transient - // read failure, so retain the association but do not return it. - const metadata = await readInlineScriptMetadataFromFile(scope); - if (!metadata) { - return undefined; + const scriptPath = normalizePath(scope.fsPath); + // A new selection or saved header may arrive while last-used protection is being written. + // Retry once with the current state, never hand out the superseded descriptor. + for (let attempt = 0; attempt < 2 && !this.disposed; attempt += 1) { + const revision = this.associationRevisions.get(scriptPath) ?? 0; + const metadataRevision = this.routingRegistry.getMetadataRevision(scope); + const isCurrent = () => + !this.disposed && + this.isCurrentAssociationRevision(scriptPath, revision) && + this.routingRegistry.getMetadataRevision(scope) === metadataRevision; + const metadata = await readInlineScriptMetadataFromFile(scope); + if (!metadata) { + return undefined; + } + + const environment = await this.getAssociationForMetadata(scriptPath, scope, metadata); + if (!isCurrent()) { + continue; + } + if (!environment) { + const busy = await this.isAssociatedEntryBusy(scriptPath); + if (!isCurrent()) { + continue; + } + if (busy) { + this.routingRegistry.setEnvironmentUnavailable(scope, true); + this.scheduleAssociationRetry(scope); + } else { + await this.reviewRoutingForMissingEnvironment(scope); + } + return undefined; + } + const usable = await this.noteEnvironmentUsed(environment); + if (!isCurrent()) { + continue; + } + if (!usable) { + // Offer retry without routing execution to an unrelated fallback interpreter. + this.routingRegistry.setEnvironmentUnavailable(scope, true); + this.scheduleAssociationRetry(scope); + return undefined; + } + this.routingRegistry.setEnvironmentUnavailable(scope, false); + return environment; + } + this.log.debug( + `Inline-script selection changed while resolving ${scope.fsPath}; withholding the old environment.`, + ); + return undefined; + } + + /** + * Routing is only re-evaluated on metadata changes, so an entry that becomes unusable on its own + * leaves the script with neither an environment nor the CodeLens to rebuild one. Only definitive + * verdicts act here; contention and unreadable metadata must not drop a working association. + */ + private async reviewRoutingForMissingEnvironment(scriptUri: Uri): Promise { + if (this.disposed || !this.routingRegistry.shouldRoute(scriptUri)) { + return; + } + const environmentPath = this.fsPathToPersistedAssociation.get(normalizePath(scriptUri.fsPath))?.environmentPath; + if (!environmentPath) { + return; + } + await this.invalidateUnusableEntry(path.dirname(path.dirname(environmentPath))); + } + + /** + * Publish an unusable verdict, but only after re-confirming it. A same-key rebuild can replace + * the entry while a lookup is pending, and a newer successful setup must win over the older + * lookup's result. + */ + private async invalidateUnusableEntry(envDirPath: string): Promise { + if (this.disposed || this.pendingCreations.has(path.basename(envDirPath))) { + return; + } + const envDir = Uri.file(envDirPath); + // Lock-free first, so a lookup that has nothing to publish never pays for the entry lock. + if (this.classifyUsability(await inspectMetaJson(envDir), Date.now()).verdict !== 'unusable') { + return; + } + try { + // Then under the lock so a rebuild cannot land between the reconfirmation and the + // invalidation. A newer successful setup must win over an older lookup's verdict. + await this.withCacheEntryLock( + envDir, + async () => { + if (this.disposed) { + return; + } + const confirmed = this.classifyUsability(await inspectMetaJson(envDir), Date.now()); + if (confirmed.verdict === 'unusable') { + this.unrouteScriptsUsingEnvironment(envDirPath); + } + }, + 0, + ); + } catch (error) { + if (!this.isLockContentionError(error)) { + this.log.warn(`Unable to confirm an unusable inline-script entry: ${getErrorMessage(error)}`); + } + // Someone owns the entry; let them finish rather than invalidating a moving target. + } + } + + /** + * Stamp the entry as used, and report whether it is still safe to hand out. Awaited, not fired + * and forgotten: another window's sweep cannot see this workspace's associations, so returning + * before the stamp lands lets it delete an environment that is about to be executed. + */ + private async noteEnvironmentUsed(environment: PythonEnvironment | undefined): Promise { + if (this.disposed || environment?.envId.managerId !== INLINE_SCRIPT_MANAGER_ID) { + return true; + } + const envDirPath = environment.sysPrefix; + if (this.pendingCreations.has(path.basename(envDirPath))) { + return true; + } + const verdict = await this.proveEnvironmentUsable(envDirPath); + if (verdict === 'unusable') { + // Give the script its setup action back rather than leaving it with neither an + // environment nor a CodeLens. + await this.invalidateUnusableEntry(envDirPath); + } + return verdict === 'usable'; + } + + /** + * Synchronous up to the point the shared operation is registered, so two overlapping lookups + * cannot both miss it and start their own. + */ + private proveEnvironmentUsable(envDirPath: string): Promise { + const key = normalizePath(envDirPath); + const existing = this.lastUsedTouchState.get(key); + if (existing?.inFlight) { + return existing.inFlight; + } + const now = Date.now(); + if (existing?.stampedAt !== undefined && now - existing.stampedAt < LAST_USED_REFRESH_AFTER_MS) { + return Promise.resolve('usable'); + } + + // Backoff suppresses repeated write attempts, never the recognition of protection another + // window may have established since. + const mayStamp = + existing?.refreshInFlight === undefined && + (existing?.retryNotBefore === undefined || + now >= existing.retryNotBefore || + this.verdictWithoutFreshStamp(existing.stampedAt, now) !== 'usable'); + const generation = (this.lastUsedTouchGeneration += 1); + const inFlight = this.proveAndStamp(key, Uri.file(envDirPath), generation, { + mayStamp, + retryNotBefore: mayStamp ? undefined : existing?.retryNotBefore, + }); + this.lastUsedTouchState.set(key, { ...existing, inFlight, generation }); + this.pendingLastUsedTouches.add(inFlight); + void inFlight.finally(() => this.pendingLastUsedTouches.delete(inFlight)); + return inFlight; + } + + /** Records a stamp this manager just wrote, so later lookups need no proof of their own. */ + private recordProvenStamp(envDirPath: string, stampedAt: number): void { + this.lastUsedTouchState.set(normalizePath(envDirPath), { stampedAt }); + } + + /** + * What a known stamp says when no fresh one could be taken. Being due a refresh is not the same + * as being evictable, so only entries close to the TTL are withheld. + */ + private verdictWithoutFreshStamp(stampedAt: number | undefined, now: number): UsageVerdict { + return stampedAt !== undefined && now - stampedAt < CACHE_TTL_MS - EVICTION_RISK_MARGIN_MS + ? 'usable' + : 'unproven'; + } + + /** The single reading of what a sidecar says about using the entry right now. */ + private classifyUsability(sidecar: InlineScriptMetaReadResult, now: number): ObservedUsability { + if (sidecar.kind === 'unsupported') { + // A newer extension owns this entry and manages its own lifecycle. We cannot stamp a + // schema we do not understand, and withholding would strand a downgraded window. + return { verdict: 'usable', stampDue: false }; + } + if (sidecar.kind === 'unavailable') { + // Transient I/O: no verdict on the entry, but no proof of protection either. + return { verdict: 'unproven', stampDue: false }; + } + if (sidecar.kind !== 'valid' || sidecar.metadata.manuallyModified) { + // Missing, invalid, or marked: the entry is being torn down or is already damaged. + return { verdict: 'unusable', stampDue: false }; + } + const stampedAt = new Date(sidecar.metadata.lastUsedAt).getTime(); + if (!Number.isFinite(stampedAt)) { + return { verdict: 'unproven', stampDue: true }; + } + return { + verdict: this.verdictWithoutFreshStamp(stampedAt, now), + stampedAt, + stampDue: now - stampedAt >= LAST_USED_REFRESH_AFTER_MS, + }; + } + + private async proveAndStamp( + key: string, + envDir: Uri, + generation: number, + options: { readonly mayStamp: boolean; readonly retryNotBefore?: number }, + ): Promise { + let observed: ObservedUsability | undefined; + let outcome: LastUsedStampOutcome; + try { + observed = this.classifyUsability(await inspectMetaJson(envDir), Date.now()); + if (!observed.stampDue || !options.mayStamp) { + outcome = { + verdict: observed.verdict, + stampedAt: observed.stampedAt, + retryNotBefore: options.retryNotBefore, + }; + } else if (observed.verdict === 'usable') { + // The recorded stamp already proves the entry is retained, so refreshing it is + // bookkeeping. Never make interpreter resolution wait for that write: `timeoutMs: 0` + // bounds only lock acquisition, and graceful-fs retries a Windows sharing violation + // on the rename for a full minute. + this.startBackgroundRefresh(key, envDir, observed); + outcome = { verdict: 'usable', stampedAt: observed.stampedAt }; + } else { + // At risk or unproven: the stamp is the answer, so it has to be awaited. + outcome = await this.writeLastUsedUnderLock(envDir, observed); + } + } catch (error) { + this.log.warn(`Failed to refresh the inline-script last-used time: ${getErrorMessage(error)}`); + const known = observed?.stampedAt ?? this.lastUsedTouchState.get(key)?.stampedAt; + const verdict = this.verdictWithoutFreshStamp(known, Date.now()); + outcome = { + verdict, + stampedAt: known, + retryNotBefore: verdict === 'usable' ? Date.now() + LAST_USED_TOUCH_RETRY_MS : undefined, + }; + } + this.recordTouchOutcome(key, generation, outcome); + return outcome.verdict; + } + + private recordTouchOutcome(key: string, generation: number, outcome: LastUsedStampOutcome): void { + const current = this.lastUsedTouchState.get(key); + if (current?.generation !== generation) { + // A newer operation owns this entry's state. + return; + } + this.lastUsedTouchState.set(key, { + stampedAt: outcome.stampedAt, + retryNotBefore: outcome.retryNotBefore, + refreshInFlight: current.refreshInFlight, + refreshGeneration: current.refreshGeneration, + }); + } + + /** Refreshes a stamp nobody is waiting on, while keeping its lock and lifecycle guarantees. */ + private startBackgroundRefresh(key: string, envDir: Uri, observed: ObservedUsability): void { + const refreshGeneration = (this.lastUsedTouchGeneration += 1); + const refresh = (async () => { + let outcome: LastUsedStampOutcome; + try { + outcome = await this.writeLastUsedUnderLock(envDir, observed, true); + } catch (error) { + this.log.warn(`Failed to refresh the inline-script last-used time: ${getErrorMessage(error)}`); + outcome = { verdict: 'usable', retryNotBefore: Date.now() + LAST_USED_TOUCH_RETRY_MS }; + } + const current = this.lastUsedTouchState.get(key); + if (current?.refreshGeneration !== refreshGeneration) { + return; + } + this.lastUsedTouchState.set(key, { + ...current, + refreshInFlight: undefined, + refreshGeneration: undefined, + stampedAt: outcome.stampedAt ?? current.stampedAt, + retryNotBefore: outcome.retryNotBefore, + }); + })(); + this.lastUsedTouchState.set(key, { + ...(this.lastUsedTouchState.get(key) ?? {}), + refreshInFlight: refresh, + refreshGeneration, + }); + this.pendingLastUsedTouches.add(refresh); + void refresh.finally(() => this.pendingLastUsedTouches.delete(refresh)); + } + + private async writeLastUsedUnderLock( + envDir: Uri, + observed: ObservedUsability, + markBookkeeping = false, + ): Promise { + const bookkeepingKey = normalizePath(envDir.fsPath); + try { + // Timeout 0: this runs off a read path and must never wait behind an installer. + return await this.withCacheEntryLock( + envDir, + async () => { + // Claim the exemption only while the lock is genuinely held: a pending or + // failed acquisition must never wave past another window's build or deletion. + if (markBookkeeping) { + this.bookkeepingLocks.add(bookkeepingKey); + } + try { + if (this.disposed) { + return { verdict: observed.verdict, stampedAt: observed.stampedAt }; + } + const sidecar = await inspectMetaJson(envDir); + const classified = this.classifyUsability(sidecar, Date.now()); + if (!classified.stampDue || sidecar.kind !== 'valid') { + return { verdict: classified.verdict, stampedAt: classified.stampedAt }; + } + const stampedAt = Date.now(); + await writeMetaJson( + envDir, + { + ...sidecar.metadata, + lastUsedAt: new Date(stampedAt).toISOString(), + }, + { failFast: true }, + ); + return { + verdict: 'usable' as const, + stampedAt, + }; + } finally { + if (markBookkeeping) { + this.bookkeepingLocks.delete(bookkeepingKey); + } + } + }, + 0, + ); + } catch (error) { + if (!this.isLockContentionError(error)) { + throw error; + } + // The holder may be building this entry (which stamps it) or deleting it. Re-read + // without the lock and let the recorded stamp decide; a brief collision must not take a + // safely retained environment out of service. + const classified = this.classifyUsability(await inspectMetaJson(envDir), Date.now()); + return { + verdict: classified.verdict, + stampedAt: classified.stampedAt, + retryNotBefore: + classified.verdict === 'usable' ? Date.now() + LAST_USED_TOUCH_RETRY_MS : undefined, + }; } + } - return this.getAssociationForMetadata(normalizePath(scope.fsPath), scope, metadata); + private forgetLastUsedTouch(envDirPath: string): void { + this.lastUsedTouchState.delete(normalizePath(envDirPath)); } private getScriptUris(scope: SetEnvironmentScope): ScriptReference[] { @@ -1511,18 +1969,116 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private async handleSavedMetadataChange(event: InlineScriptMetadataChangeEvent): Promise { + const scriptPath = normalizePath(event.uri.fsPath); if (event.metadata === undefined) { + this.cancelAssociationRetry(scriptPath); this.clearValidatedRouteableState(event.uri); return; } + const metadataIdentity = event.metadataIdentity ?? getInlineScriptMetadataRoutingIdentity(event.metadata)!; + // Body-only saves change the revision, not the requirements. Keep the retry's deadline and budget. + if (this.associationRetries.get(scriptPath)?.metadataIdentity !== metadataIdentity) { + this.cancelAssociationRetry(scriptPath); + } await this.refreshValidatedAssociationForMetadata( event.uri, event.metadata, - event.metadataIdentity ?? getInlineScriptMetadataRoutingIdentity(event.metadata)!, + metadataIdentity, event.metadataRevision, ); } + private scheduleAssociationRetry(uri: Uri): void { + const scriptPath = normalizePath(uri.fsPath); + const metadataIdentity = this.routingRegistry.getMetadataIdentity(uri); + const associationRevision = this.associationRevisions.get(scriptPath) ?? 0; + if ( + this.disposed || + metadataIdentity === undefined || + !this.fsPathToPersistedAssociation.has(scriptPath) + ) { + return; + } + let retry = this.associationRetries.get(scriptPath); + if ( + retry && + (retry.metadataIdentity !== metadataIdentity || retry.associationRevision !== associationRevision) + ) { + this.cancelAssociationRetry(scriptPath); + retry = undefined; + } + if (!retry) { + retry = { metadataIdentity, associationRevision, attempt: 0 }; + this.associationRetries.set(scriptPath, retry); + } + if (retry.timer) { + return; + } + const delay = DISCOVERY_RETRY_DELAYS_MS[retry.attempt]; + if (delay === undefined) { + this.routingRegistry.setEnvironmentUnavailable(uri, true); + if (retry.attempt === DISCOVERY_RETRY_DELAYS_MS.length) { + retry.attempt += 1; + this.log.warn( + `Inline-script environment for ${uri.fsPath} is still unavailable; use the script's setup action to retry.`, + ); + } + return; + } + retry.attempt += 1; + const scheduled = retry; + retry.timer = setTimeout(() => { + // Keep the timer recorded until the async work finishes so concurrent lookups coalesce. + void this.retryAssociation(uri, scheduled) + .catch((error) => this.log.warn(`Failed to retry inline-script environment: ${getErrorMessage(error)}`)) + .finally(() => { + if (this.associationRetries.get(scriptPath) === scheduled) { + scheduled.timer = undefined; + this.scheduleAssociationRetry(uri); + } + }); + }, delay); + } + + private async retryAssociation(uri: Uri, retry: AssociationRetry): Promise { + const scriptPath = normalizePath(uri.fsPath); + const currentUri = this.routingRegistry.getUri(scriptPath) ?? uri; + const isCurrent = () => + !this.disposed && + this.associationRetries.get(scriptPath) === retry && + this.isCurrentAssociationRevision(scriptPath, retry.associationRevision) && + this.routingRegistry.getMetadataIdentity(currentUri) === retry.metadataIdentity; + if (!isCurrent()) { + return; + } + const environment = await this.getInternal(currentUri); + const metadata = this.routingRegistry.getMetadata(currentUri); + if (!environment || !metadata || !isCurrent()) { + return; + } + await this.refreshValidatedAssociationForMetadata( + currentUri, + metadata, + retry.metadataIdentity, + this.routingRegistry.getMetadataRevision(currentUri), + ); + if ( + isCurrent() && + this.routingRegistry.shouldRoute(currentUri) && + !this.routingRegistry.isEnvironmentUnavailable(currentUri) + ) { + this.cancelAssociationRetry(scriptPath); + } + } + + private cancelAssociationRetry(scriptPath: string): void { + const retry = this.associationRetries.get(scriptPath); + if (retry?.timer) { + clearTimeout(retry.timer); + } + this.associationRetries.delete(scriptPath); + } + private async refreshValidatedAssociationForMetadata( uri: Uri, metadata: InlineScriptMetadata, @@ -1578,6 +2134,35 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return; } if (!environment) { + if (await this.isAssociatedEntryBusy(scriptPath)) { + // Transiently unavailable, not un-routeable. Clearing here would drop a good + // association and restore the setup CodeLens for an environment that still works, + // and nothing would restore it until the next save. + if ( + this.isCurrentMetadataRefreshTask( + uri, + metadataIdentity, + metadataRevision, + scriptPath, + associationRevision, + ) + ) { + this.scheduleAssociationRetry(uri); + } + return; + } + if ( + !this.isCurrentMetadataRefreshTask( + uri, + metadataIdentity, + metadataRevision, + scriptPath, + associationRevision, + ) + ) { + // A newer setup or selection landed while the busy check was in flight. + return; + } this.clearValidatedRouteableState(uri); return; } @@ -2003,6 +2588,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { associationRevision: number, ): boolean { return ( + !this.disposed && this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) && this.isCurrentAssociationRevision(scriptPath, associationRevision) ); @@ -2377,9 +2963,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private runTtlEvictionOnce(): Promise { if (!this.ttlEviction) { - this.ttlEviction = this.enqueueCacheMaintenance(() => - this.enqueueSelection(() => this.evictStaleCacheEntries()), - ).catch((error) => { + this.ttlEviction = this.evictStaleCacheEntries().catch((error) => { this.log.warn(`Unable to evict stale inline-script environments: ${getErrorMessage(error)}`); }); } @@ -2413,6 +2997,19 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } + /** + * Serialize against other cache maintenance without raising the barrier that `get`, `set`, and + * `create` wait on. Used for TTL deletion and publication, not explicit cache clearing. + */ + private enqueueBackgroundCacheMaintenance(operation: () => Promise): Promise { + const run = this.cacheMaintenanceQueue.then(operation); + this.cacheMaintenanceQueue = run.then( + () => undefined, + () => undefined, + ); + return run; + } + private enqueueSelection(operation: () => Promise): Promise { const run = this.selectionQueue.then(operation); this.selectionQueue = run.then( @@ -2598,10 +3195,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } + private async isAssociatedEntryBusy(scriptPath: string): Promise { + const environmentPath = this.fsPathToPersistedAssociation.get(scriptPath)?.environmentPath; + return environmentPath !== undefined + ? this.isCacheEntryBusy(path.dirname(path.dirname(environmentPath))) + : false; + } + private async isCacheEntryBusy(envDirPath: string): Promise { if (this.pendingCreations.has(path.basename(envDirPath))) { return true; } + if (this.bookkeepingLocks.has(normalizePath(envDirPath))) { + // Our own optional refresh holds the lock. It is not building or deleting anything, and + // the lock is exclusive, so nothing else can be either. + return false; + } try { await fs.lstat(getFileLockPath(envDirPath)); return true; @@ -2611,6 +3220,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private bumpAssociationRevision(scriptPath: string): void { + this.cancelAssociationRetry(scriptPath); this.associationRevisions.set(scriptPath, (this.associationRevisions.get(scriptPath) ?? 0) + 1); } @@ -2921,10 +3531,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { action: (lock: AcquiredFileLock) => Promise, timeoutMs = CACHE_LOCK_TIMEOUT_MS, ): Promise { - const lock = await acquireFileLock(envDir.fsPath, { - timeoutMs, - retryIntervalMs: CACHE_LOCK_RETRY_MS, - }); + const lock = await this.acquireCacheEntryLock(envDir.fsPath, timeoutMs); try { return await action(lock); } finally { @@ -2936,6 +3543,30 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } + /** + * Reclaims a lock whose owning process is gone. Doing this *before* waiting is the point: + * otherwise every later setup for the same cache key blocks for the whole timeout, then fails. + */ + private async acquireCacheEntryLock(envDirPath: string, timeoutMs: number): Promise { + try { + if ((await inspectFileLock(envDirPath)) === 'stale' && (await reclaimFileLock(envDirPath))) { + this.log.info( + `Reclaimed an inline-script cache lock left behind by a stopped process: ${getFileLockPath(envDirPath)}`, + ); + } + } catch (error) { + this.log.warn(`Unable to inspect an inline-script cache lock: ${getErrorMessage(error)}`); + } + if (this.disposed) { + // Disposal can land while the inspection above is in flight; creating a lock directory + // now would leave one behind with nobody left to release it. + throw Object.assign(new Error('Inline-script cache lock not acquired: the manager is disposed.'), { + code: 'ELOCKED', + }); + } + return acquireFileLock(envDirPath, { timeoutMs, retryIntervalMs: CACHE_LOCK_RETRY_MS }); + } + private mergePendingCreationSourceMetadataIdentityHashes( existing: readonly string[] | undefined, pendingCreation: PendingCreationContext, @@ -3147,6 +3778,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { ); return { kind: 'stale' }; } + const stampedAt = Date.now(); try { pendingCreation.hasStartedRecordingSourceMetadataIdentityHashes = true; const sourceMetadataIdentityHashes = this.mergePendingCreationSourceMetadataIdentityHashes( @@ -3159,13 +3791,29 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { packagesComparison === 'unknown' ? actualPackagesHash : sidecar.installedPackagesHash; await writeMetaJson(envDir, { ...sidecar, - lastUsedAt: new Date().toISOString(), + lastUsedAt: new Date(stampedAt).toISOString(), ...(installedPackagesHash ? { installedPackagesHash } : {}), ...(sourceMetadataIdentityHashes ? { sourceMetadataIdentityHashes } : {}), }); pendingCreation.recordedSourceMetadataIdentityHashes = sourceMetadataIdentityHashes; + this.recordProvenStamp(envDir.fsPath, stampedAt); } catch (error) { this.log.warn(`Failed to update inline-script cache metadata: ${getErrorMessage(error)}`); + this.forgetLastUsedTouch(envDir.fsPath); + const previousStamp = new Date(sidecar.lastUsedAt).getTime(); + if ( + this.verdictWithoutFreshStamp( + Number.isFinite(previousStamp) ? previousStamp : undefined, + stampedAt, + ) !== 'usable' + ) { + // Reporting success for an at-risk entry we could not stamp hands back an + // environment another window is still free to reclaim. + this.log.warn( + `Refusing to reuse an inline-script cache entry whose last-used time could not be refreshed: ${envDir.fsPath}`, + ); + return { kind: 'uncertain' }; + } } return { kind: 'reusable', environment }; } @@ -3240,15 +3888,17 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { // this write in the same locked section is what makes it impossible for the extension // to later mistake its own installation for an edit made outside setup. const installedPackagesHash = await readInstalledPackagesHash(envDir); + const stampedAt = Date.now(); await writeMetaJson(envDir, { schemaVersion: META_SCHEMA_VERSION, baseInterpreterPath: selectedBase.canonicalPath, baseInterpreterVersion: selectedBase.environment.version, - lastUsedAt: new Date().toISOString(), + lastUsedAt: new Date(stampedAt).toISOString(), ...(installedPackagesHash ? { installedPackagesHash } : {}), ...(sourceMetadataIdentityHashes ? { sourceMetadataIdentityHashes } : {}), }); pendingCreation.recordedSourceMetadataIdentityHashes = sourceMetadataIdentityHashes; + this.recordProvenStamp(envDir.fsPath, stampedAt); } catch (error) { this.log.error(`Failed to record inline-script cache metadata: ${getErrorMessage(error)}`); await this.removeCacheEntry(envDir); @@ -3258,11 +3908,33 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return { environment: result.environment }; } + /** + * Background cleanup never raises the barrier that lookups wait on. Candidate scans are + * advisory; references, ownership, and age are rechecked under the entry lock before deletion. + */ private async evictStaleCacheEntries(): Promise { + const plan = await this.planStaleCacheEviction(); + if (!plan || this.disposed) { + return; + } + + const removedCacheEntries = await this.enqueueBackgroundCacheMaintenance(() => + this.removeEvictableCacheEntries(plan), + ); + if (removedCacheEntries.size === 0 || this.disposed) { + return; + } + + await this.enqueueBackgroundCacheMaintenance(() => + this.enqueueSelection(() => this.publishCacheEvictionResults(removedCacheEntries)), + ); + } + + private async planStaleCacheEviction(): Promise { const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); const physicalCacheRootPath = await this.getPhysicalOwnedCacheRootPath(cacheRoot); if (!physicalCacheRootPath) { - return; + return undefined; } let entryNames: string[]; @@ -3270,30 +3942,32 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { entryNames = await fs.readdir(physicalCacheRootPath); } catch (error) { if (isFileNotFoundError(error)) { - return; + return undefined; } throw error; } const now = new Date(); - const entries: CacheEntrySummary[] = []; + const evictableEntries: string[] = []; for (const entryName of entryNames.sort()) { + if (this.disposed) { + return undefined; + } if (entryName.endsWith(FILE_LOCK_DIR_SUFFIX)) { continue; } const entryPath = path.join(physicalCacheRootPath, entryName); try { - const stat = await fs.lstat(entryPath); - if (!stat.isDirectory() || stat.isSymbolicLink()) { + const classification = await this.classifyCacheEntryForEviction(entryPath, now); + if (classification === 'keep') { continue; } - const sidecar = await inspectMetaJson(Uri.file(entryPath)); - if (sidecar.kind === 'valid') { - entries.push({ - envDirPath: entryPath, - lastUsedAt: new Date(sidecar.metadata.lastUsedAt), - }); + if (classification === 'incomplete') { + this.log.info( + `Inline-script cache: found an environment left incomplete by an interrupted setup: ${entryPath}`, + ); } + evictableEntries.push(entryPath); } catch (error) { if (!isFileNotFoundError(error)) { this.log.warn( @@ -3303,56 +3977,116 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } - const staleEntries = selectStaleEntries(entries, now, CACHE_TTL_MS); - if (staleEntries.length === 0) { - return; + if (evictableEntries.length === 0) { + return undefined; } const persistedAssociations = await this.getPersistedAssociationSnapshot(); const scriptPaths = this.getTrackedScriptPaths(persistedAssociations); - const priorSelections = this.getPriorSelections(scriptPaths); // Never evict an environment that a script association still points to. `lastUsedAt` is only - // refreshed when an environment is created or reused (never when it is resolved for run, debug, - // or Pylance), so an actively-used environment can look stale here. Reclaim only orphaned entries - // (e.g. superseded by a dependency change, or left behind by a deleted or deselected script). + // refreshed by windows that are actually running, so an entry belonging to a workspace nobody + // has opened lately can still look stale here. Reclaim only orphaned entries (e.g. superseded + // by a dependency change, or left behind by a deleted or deselected script). const referencedEnvDirs = this.getReferencedCacheEntryDirs(persistedAssociations, scriptPaths); - const evictableStaleEntries = staleEntries.filter( - (staleEntry) => !referencedEnvDirs.has(normalizePath(staleEntry)), + const evictableStaleEntries = evictableEntries.filter( + (entryPath) => !referencedEnvDirs.has(normalizePath(entryPath)), ); - if (evictableStaleEntries.length === 0) { - return; + return evictableStaleEntries.length === 0 + ? undefined + : { cacheRoot, physicalCacheRootPath, evictableStaleEntries, now }; + } + + /** + * `keep` covers everything uncertain. Used for both planning and the under-lock recheck so the + * decision has one definition. + */ + private async classifyCacheEntryForEviction( + entryPath: string, + now: Date, + ): Promise<'expired' | 'incomplete' | 'keep'> { + let stat: Stats; + try { + stat = await fs.lstat(entryPath); + } catch { + return 'keep'; + } + if (!stat.isDirectory() || stat.isSymbolicLink()) { + return 'keep'; } + + const sidecar = await inspectMetaJson(Uri.file(entryPath)); + if (sidecar.kind === 'valid') { + return selectStaleEntries( + [{ envDirPath: entryPath, lastUsedAt: new Date(sidecar.metadata.lastUsedAt) }], + now, + CACHE_TTL_MS, + ).length === 1 + ? 'expired' + : 'keep'; + } + // `unsupported` belongs to a newer extension and `unavailable` is a transient read failure; + // neither is evidence that the entry was abandoned. + if (sidecar.kind !== 'missing' && sidecar.kind !== 'invalid') { + return 'keep'; + } + + // `mtime` is when the directory's contents last changed, so for an abandoned build it is + // when the host died. Anything touching it later only delays reclamation. + return Number.isFinite(stat.mtimeMs) && + stat.mtimeMs > 0 && + now.getTime() - stat.mtimeMs > INCOMPLETE_ENTRY_GRACE_MS + ? 'incomplete' + : 'keep'; + } + + private async removeEvictableCacheEntries(plan: StaleCacheEvictionPlan): Promise> { const removedCacheEntries = new Set(); - for (const staleEntry of evictableStaleEntries) { + // Counts deletions performed, not entries considered: a contended or already-gone entry + // costs nothing, so it must not consume the budget. + let deletions = 0; + let deferred = 0; + for (const [index, staleEntry] of plan.evictableStaleEntries.entries()) { + if (this.disposed) { + break; + } + if (deletions >= MAX_EVICTIONS_PER_SWEEP) { + deferred = plan.evictableStaleEntries.length - index; + break; + } try { - const removed = await this.removeCacheEntryForClear( - cacheRoot, - physicalCacheRootPath, - path.basename(staleEntry), - { - reclaimRetainedLock: false, - afterRemove: () => { - this.cacheMutationRevision += 1; - }, - shouldRemove: async (entryPath) => { - const sidecar = await inspectMetaJson(Uri.file(entryPath)); - return ( - sidecar.kind === 'valid' && - selectStaleEntries( - [ - { - envDirPath: entryPath, - lastUsedAt: new Date(sidecar.metadata.lastUsedAt), - }, - ], - now, - CACHE_TTL_MS, - ).length === 1 - ); + // Per entry, so it cannot interleave with `setInternal` while leaving `get` — which + // never touches this queue — unblocked. + const removed = await this.enqueueSelection(async () => { + if (await this.isCacheEntryReferenced(staleEntry)) { + // Selecting beats reclaiming. + return undefined; + } + return this.removeCacheEntryForClear( + plan.cacheRoot, + plan.physicalCacheRootPath, + path.basename(staleEntry), + { + reclaimRetainedLock: false, + beforeRemove: async (entryPath) => { + // A partial removal must not leave survivors looking healthy. + const sidecar = await inspectMetaJson(Uri.file(entryPath)); + if (sidecar.kind === 'valid') { + await writeMetaJson(Uri.file(entryPath), { + ...sidecar.metadata, + manuallyModified: true, + }); + } + }, + afterRemove: () => { + this.cacheMutationRevision += 1; + }, + shouldRemove: async (entryPath) => + (await this.classifyCacheEntryForEviction(entryPath, plan.now)) !== 'keep', }, - }, - ); + ); + }); if (removed) { + deletions += 1; removedCacheEntries.add(normalizePath(removed)); } else if (await this.isCacheEntryDefinitelyMissing(staleEntry)) { this.cacheMutationRevision += 1; @@ -3368,14 +4102,31 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } } - - if (removedCacheEntries.size === 0) { - return; + if (removedCacheEntries.size > 0 || deferred > 0) { + this.log.info( + `Inline-script cache: reclaimed ${removedCacheEntries.size} unused environment(s)` + + (deferred > 0 ? `; ${deferred} left for a later session.` : '.'), + ); } + return removedCacheEntries; + } - this.replaceDiscoveredEnvironments( - this.collection.filter((environment) => !removedCacheEntries.has(normalizePath(environment.sysPrefix))), - ); + /** Call under the selection queue. */ + private async isCacheEntryReferenced(entryPath: string): Promise { + const persistedAssociations = await this.getPersistedAssociationSnapshot(); + const scriptPaths = this.getTrackedScriptPaths(persistedAssociations); + return this.getReferencedCacheEntryDirs(persistedAssociations, scriptPaths).has(normalizePath(entryPath)); + } + + /** + * Association state is re-read rather than carried over from planning: the deletions ran off the + * selection queue, so a selection made in the meantime must be observed here. + */ + private async publishCacheEvictionResults(removedCacheEntries: ReadonlySet): Promise { + await this.reconcileCollectionAfterRemoval(removedCacheEntries); + const persistedAssociations = await this.getPersistedAssociationSnapshot(); + const scriptPaths = this.getTrackedScriptPaths(persistedAssociations); + const priorSelections = this.getPriorSelections(scriptPaths); const invalidatedScriptPaths = await this.getInvalidatedAssociationPaths( scriptPaths, persistedAssociations, @@ -3384,6 +4135,39 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { await this.clearInvalidatedAssociations(invalidatedScriptPaths, persistedAssociations, priorSelections); } + /** + * A removed pathname is not proof on its own: creation can rebuild the same cache key while a + * sweep is still working, and that replacement must survive publication. Passing no candidates + * reconciles the whole catalog, which a full clear needs because another window may already + * have removed entries this invocation never touched. + */ + private async reconcileCollectionAfterRemoval(candidates?: ReadonlySet): Promise { + const stillRemoved = new Set(); + for (const environment of this.collection) { + const key = normalizePath(environment.sysPrefix); + if (candidates && !candidates.has(key)) { + continue; + } + try { + if (!(await fs.pathExists(environment.environmentPath.fsPath))) { + stillRemoved.add(key); + } + } catch (error) { + // Uncertain, so keep it listed. + this.log.warn( + `Unable to verify a removed inline-script cache entry ${environment.sysPrefix}: ${getErrorMessage(error)}`, + ); + } + } + if (stillRemoved.size === 0) { + return false; + } + this.replaceDiscoveredEnvironments( + this.collection.filter((environment) => !stillRemoved.has(normalizePath(environment.sysPrefix))), + ); + return true; + } + private async isCacheEntryDefinitelyMissing(entryPath: string): Promise { try { await fs.lstat(entryPath); @@ -3525,13 +4309,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } - if (selectedEntryPath !== undefined && - (removedCacheEntries.size > 0 || await this.isCacheEntryDefinitelyMissing(selectedEntryPath))) { - this.cacheMutationRevision += 1; - selectedEntryKeys.forEach((entryPath) => removedCacheEntries.add(entryPath)); - this.replaceDiscoveredEnvironments( - this.collection.filter((entry) => !selectedEntryKeys.has(normalizePath(entry.sysPrefix))), - ); + if (selectedEntryPath !== undefined) { + if (removedCacheEntries.size > 0 || (await this.isCacheEntryDefinitelyMissing(selectedEntryPath))) { + this.cacheMutationRevision += 1; + selectedEntryKeys.forEach((entryPath) => removedCacheEntries.add(entryPath)); + this.replaceDiscoveredEnvironments( + this.collection.filter((entry) => !selectedEntryKeys.has(normalizePath(entry.sysPrefix))), + ); + } + } else { + // Full clears reconcile the whole catalog, including entries another window already + // removed, so a clear that deleted nothing still drops dead rows. Entries whose removal + // failed or whose state is uncertain stay listed. + const reconciled = await this.reconcileCollectionAfterRemoval(); + if (removedCacheEntries.size > 0 || reconciled) { + this.cacheMutationRevision += 1; + } } const invalidatedScriptPaths = await this.getInvalidatedAssociationPaths( @@ -3793,6 +4586,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private deleteCacheEntryForClear(entryPath: string): Promise { + this.forgetLastUsedTouch(entryPath); return fs.remove(entryPath); } @@ -3844,17 +4638,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { environmentPath: string, removedCacheEntries: ReadonlySet, ): Promise { - const envDirPath = path.dirname(path.dirname(environmentPath)); - if (removedCacheEntries.has(normalizePath(envDirPath))) { - return true; - } try { + // Existence is authoritative: a rebuild can reoccupy a path this sweep removed, so + // membership in `removedCacheEntries` is not on its own evidence the association is dead. return !(await fs.pathExists(environmentPath)); } catch (error) { this.log.warn( `Unable to verify inline-script environment association ${environmentPath}: ${getErrorMessage(error)}`, ); - return false; + return removedCacheEntries.has(normalizePath(path.dirname(path.dirname(environmentPath)))); } } @@ -3979,6 +4771,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private async removeCacheEntry(envDir: Uri): Promise { + this.forgetLastUsedTouch(envDir.fsPath); let lastError: unknown; for (let attempt = 0; attempt < CACHE_ENTRY_REMOVAL_ATTEMPTS; attempt += 1) { try { @@ -4047,6 +4840,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { dispose(): void { this.disposed = true; this.stopActivationDiscovery(); + this.cancelTtlEviction(); + for (const scriptPath of this.associationRetries.keys()) { + this.cancelAssociationRetry(scriptPath); + } this.pendingMetadataRefreshes.clear(); this.subscriptions.forEach((subscription) => subscription.dispose()); this._onDidChangeEnvironments.dispose(); diff --git a/src/test/common/inlineScript/cacheLayout.unit.test.ts b/src/test/common/inlineScript/cacheLayout.unit.test.ts index 53815837..5bb3c5de 100644 --- a/src/test/common/inlineScript/cacheLayout.unit.test.ts +++ b/src/test/common/inlineScript/cacheLayout.unit.test.ts @@ -37,6 +37,10 @@ import { createDeferred } from '../../../common/utils/deferred'; import * as platformUtils from '../../../common/utils/platformUtils'; import { getVenvPythonPath } from '../../../common/utils/virtualEnvironment'; +// `import * as` would copy the module, leaving the spy unable to observe the real rename that +// cacheLayout calls. +import nodeFsPromises = require('fs/promises'); + function makeMeta(overrides: Partial = {}): InlineScriptEnvMeta { return { schemaVersion: META_SCHEMA_VERSION, @@ -117,6 +121,63 @@ suite('inlineScriptCacheLayout', () => { assert.deepStrictEqual(tmpFiles, []); }); + // Optional bookkeeping fails fast rather than holding the shared cache-entry lock through + // graceful-fs's minute-long rename retry, which other windows read as a build or deletion. + test('a failFast write still produces a readable sidecar', async () => { + await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2031-02-03T04:05:06.000Z' }), { failFast: true }); + + const read = await readMetaJson(envDir); + assert.strictEqual(read?.lastUsedAt, '2031-02-03T04:05:06.000Z'); + const entries = await fs.readdir(envDir.fsPath); + assert.deepStrictEqual( + entries.filter((name) => name.includes('.tmp-') || name.includes('.backup-')), + [], + ); + }); + + test('a failFast write replaces an existing sidecar', async () => { + await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2020-01-01T00:00:00.000Z' })); + + await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2032-01-01T00:00:00.000Z' }), { failFast: true }); + + const read = await readMetaJson(envDir); + assert.strictEqual(read?.lastUsedAt, '2032-01-01T00:00:00.000Z'); + }); + + test('failFast bypasses the retrying rename implementation', async () => { + const immediate = sinon.spy(nodeFsPromises, 'rename'); + + await writeMetaJson(envDir, makeMeta(), { failFast: true }); + + sinon.assert.called(immediate); + }); + + test('a required write keeps the retrying rename implementation', async () => { + const immediate = sinon.spy(nodeFsPromises, 'rename'); + + await writeMetaJson(envDir, makeMeta()); + + sinon.assert.notCalled(immediate); + }); + + test('a failed fast write leaves the existing sidecar untouched without backup or restore', async () => { + const existing = makeMeta(); + await writeMetaJson(envDir, existing); + const error = Object.assign(new Error('sharing violation'), { code: 'EPERM' }); + const immediate = sinon.stub(nodeFsPromises, 'rename').rejects(error); + const retrying = sinon.spy(fsExtra, 'rename'); + + await assert.rejects( + writeMetaJson(envDir, makeMeta({ lastUsedAt: '2030-01-01T00:00:00.000Z' }), { failFast: true }), + (actual) => actual === error, + ); + + sinon.assert.calledOnce(immediate); + sinon.assert.notCalled(retrying); + assert.deepStrictEqual(await readMetaJson(envDir), existing); + assert.deepStrictEqual(await fs.readdir(envDir.fsPath), [META_JSON_FILENAME]); + }); + test('writeMetaJson overwrites an existing sidecar (last write wins)', async () => { await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2020-01-01T00:00:00.000Z' })); await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2030-01-01T00:00:00.000Z' })); diff --git a/src/test/common/inlineScript/routingRegistry.unit.test.ts b/src/test/common/inlineScript/routingRegistry.unit.test.ts index db29fb58..c05f907b 100644 --- a/src/test/common/inlineScript/routingRegistry.unit.test.ts +++ b/src/test/common/inlineScript/routingRegistry.unit.test.ts @@ -15,6 +15,40 @@ const METADATA = { }; suite('InlineScriptRoutingRegistry', () => { + test('temporary unavailability preserves routing and notifies only on availability changes', () => { + const registry = new InlineScriptRoutingRegistry(); + const uri = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); + registry.setMetadata(uri, METADATA); + registry.setValidatedAssociation(uri, true); + const availability: boolean[] = []; + const routeability: boolean[] = []; + registry.onDidChangeAvailability((changed) => availability.push(registry.isEnvironmentUnavailable(changed))); + registry.onDidChangeRouteability((event) => routeability.push(event.routeable)); + + registry.setEnvironmentUnavailable(uri, true); + registry.setEnvironmentUnavailable(uri, true); + assert.strictEqual(registry.shouldRoute(uri), true); + registry.setEnvironmentUnavailable(uri, false); + + assert.deepStrictEqual(availability, [true, false]); + assert.deepStrictEqual(routeability, []); + registry.dispose(); + }); + + test('a changed metadata identity drops the previous availability state', () => { + const registry = new InlineScriptRoutingRegistry(); + const uri = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); + registry.setMetadata(uri, METADATA); + registry.setValidatedAssociation(uri, true); + registry.setEnvironmentUnavailable(uri, true); + + registry.setMetadata(uri, { ...METADATA, dependencies: ['rich'] }); + registry.setValidatedAssociation(uri, true); + + assert.strictEqual(registry.isEnvironmentUnavailable(uri), false); + registry.dispose(); + }); + test('invalidates a validated association synchronously when metadata identity changes', () => { const registry = new InlineScriptRoutingRegistry(); const uri = Uri.file('/workspace/script.py'); diff --git a/src/test/extensionApi.unit.test.ts b/src/test/extensionApi.unit.test.ts index bc717cec..d49bc79c 100644 --- a/src/test/extensionApi.unit.test.ts +++ b/src/test/extensionApi.unit.test.ts @@ -97,6 +97,7 @@ suite('PythonEnvironmentApiImpl - getEnvironment timeout fallback', () => { }), ), getLastKnownEnvironment: sinon.stub().withArgs(scope).returns(lastKnown), + getEnvironmentManager: sinon.stub().returns({ id: 'ms-python.python:venv' }), } as unknown as ApiArgs[0]; const mockProjectCreators = {} as unknown as ApiArgs[2]; const mockTerminalManager = {} as unknown as ApiArgs[3]; @@ -116,4 +117,46 @@ suite('PythonEnvironmentApiImpl - getEnvironment timeout fallback', () => { assert.strictEqual(await pending, lastKnown); resolveEnvironment?.(undefined); }); + + test('waits for the real resolution for inline-script scopes instead of serving last-known', async () => { + const scope = Uri.file('/w/script.py'); + const lastKnown = { + name: 'stale', + displayName: 'stale', + displayPath: '/env/stale', + version: '3.12.0', + environmentPath: Uri.file('/env/stale'), + execInfo: { run: { executable: '/env/stale/python', args: [] } }, + sysPrefix: '/env/stale', + } as unknown as PythonEnvironment; + let resolveEnvironment: ((value: PythonEnvironment | undefined) => void) | undefined; + + type ApiArgs = ConstructorParameters; + const mockEnvManagers = { + onDidChangeActiveEnvironment: new EventEmitter().event, + getEnvironment: sinon.stub().returns( + new Promise((resolve) => { + resolveEnvironment = resolve; + }), + ), + getLastKnownEnvironment: sinon.stub().returns(lastKnown), + getEnvironmentManager: sinon.stub().returns({ id: 'ms-python.python:inline-script' }), + } as unknown as ApiArgs[0]; + + const api = new PythonEnvironmentApiImpl( + mockEnvManagers, + { getProjects: () => [], onDidChangeProjects: new EventEmitter().event } as unknown as ApiArgs[1], + {} as unknown as ApiArgs[2], + {} as unknown as ApiArgs[3], + { onDidChangeEnvironmentVariables: new EventEmitter().event } as unknown as ApiArgs[4], + ); + + const pending = api.getEnvironment(scope); + await clock.tickAsync(2_000); + // The manager withheld the environment; serving last-known would hand back exactly the + // descriptor that decision rejected. + resolveEnvironment?.(undefined); + + assert.strictEqual(await pending, undefined); + }); }); diff --git a/src/test/features/envManagers.lastKnown.unit.test.ts b/src/test/features/envManagers.lastKnown.unit.test.ts index 003f347e..8fb2115d 100644 --- a/src/test/features/envManagers.lastKnown.unit.test.ts +++ b/src/test/features/envManagers.lastKnown.unit.test.ts @@ -142,6 +142,50 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => { assert.strictEqual(envManagers.getLastKnownEnvironment(undefined), undefined); }); + test('re-resolves the manager when an inline lookup finishes after a selection change', async () => { + const scope = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); + let finishInlineLookup: ((environment: PythonEnvironment) => void) | undefined; + const inlineLookup = new Promise((resolve) => { + finishInlineLookup = resolve; + }); + const inlineId = registerManager(() => inlineLookup, undefined, 'inline-script'); + const selected = makeEnv('selected'); + const selectedId = registerManager(async () => selected, undefined, 'system'); + defaultManagerId = inlineId; + + const lookup = envManagers.getEnvironment(scope); + defaultManagerId = selectedId; + finishInlineLookup!(makeEnv('superseded')); + + assert.strictEqual(await lookup, selected); + }); + + test('availability recovery republishes the inline environment even when the last-known descriptor matches', async () => { + const scope = Uri.joinPath(Uri.file(process.cwd()), 'script.py'); + const environment = makeEnv('inline'); + registerManager(async () => environment, undefined, 'inline-script'); + markInlineScript(scope); + await envManagers.refreshEnvironment(scope); + await new Promise((resolve) => setImmediate(resolve)); + const events: DidChangeEnvironmentEventArgs[] = []; + const recovered = new Promise((resolve) => { + envManagers.onDidChangeActiveEnvironment((event) => { + events.push(event); + if (!routingRegistry.isEnvironmentUnavailable(scope)) { + resolve(); + } + }); + }); + + routingRegistry.setEnvironmentUnavailable(scope, true); + await new Promise((resolve) => setImmediate(resolve)); + routingRegistry.setEnvironmentUnavailable(scope, false); + await recovered; + + assert.ok(events.some((event) => event.uri === scope && event.new === environment)); + assert.strictEqual(envManagers.getEnvironmentManager(scope)?.id, 'ms-python.python:inline-script'); + }); + test('returns the active environment after it has been resolved', async () => { const env = makeEnv('env1'); registerManager(async () => env); diff --git a/src/test/features/inlineScript/codeLens.unit.test.ts b/src/test/features/inlineScript/codeLens.unit.test.ts index 46fb1bdc..acae445f 100644 --- a/src/test/features/inlineScript/codeLens.unit.test.ts +++ b/src/test/features/inlineScript/codeLens.unit.test.ts @@ -74,6 +74,26 @@ suite('Inline script CodeLens provider', () => { assert.strictEqual(lenses.length, 0); }); + test('offers setup while a selected environment is temporarily unavailable and hides it on recovery', () => { + routing.setMetadata(scriptUri, makeMetadata()); + routing.setValidatedAssociation(scriptUri, true); + let refreshes = 0; + const subscription = provider.onDidChangeCodeLenses(() => { + refreshes += 1; + }); + + routing.setEnvironmentUnavailable(scriptUri, true); + const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never); + assert.strictEqual(lenses.length, 1); + assert.strictEqual(lenses[0].command?.command, SETUP_COMMAND); + assert.strictEqual(routing.shouldRoute(scriptUri), true); + routing.setEnvironmentUnavailable(scriptUri, false); + + assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri), {} as never).length, 0); + assert.strictEqual(refreshes, 2); + subscription.dispose(); + }); + test('refreshes CodeLenses when routing state changes', () => { let fireCount = 0; const sub = provider.onDidChangeCodeLenses(() => (fireCount += 1)); diff --git a/src/test/features/inlineScript/setupCodeAction.unit.test.ts b/src/test/features/inlineScript/setupCodeAction.unit.test.ts index 37d2a129..fdbe9448 100644 --- a/src/test/features/inlineScript/setupCodeAction.unit.test.ts +++ b/src/test/features/inlineScript/setupCodeAction.unit.test.ts @@ -192,6 +192,20 @@ suite('Inline script setup code action', () => { assert.strictEqual(actions.length, 0); }); + test('offers an explicit retry without losing a temporarily unavailable selection', () => { + routing.setMetadata(scriptUri, makeMetadata()); + routing.setValidatedAssociation(scriptUri, true); + routing.setEnvironmentUnavailable(scriptUri, true); + + const actions = provide(makeDocument(scriptUri), [makeDiagnostic('reportMissingImports')]); + + assert.strictEqual(actions.length, 1); + assert.strictEqual(actions[0].command?.command, SETUP_COMMAND); + assert.strictEqual(routing.shouldRoute(scriptUri), true); + routing.setEnvironmentUnavailable(scriptUri, false); + assert.strictEqual(provide(makeDocument(scriptUri), [makeDiagnostic('reportMissingImports')]).length, 0); + }); + test('offers nothing when the inline-scripts feature flag is off', () => { featureEnabledStub.returns(false); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index fcb4ee42..6eb3954a 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -269,6 +269,10 @@ suite('InlineScriptEnvManager', () => { teardown(async () => { manager.dispose(); + // Let any in-flight last-used stamp settle before the temp tree is removed underneath it. + await Promise.all([ + ...(manager as unknown as { pendingLastUsedTouches: Set> }).pendingLastUsedTouches, + ]); sinon.restore(); await fs.remove(tempRoot); }); @@ -1334,6 +1338,26 @@ suite('InlineScriptEnvManager', () => { assert.ok(options.retryIntervalMs > 0); }); + test('reclaims a lock left by a stopped process before waiting on it', async () => { + const inspect = sinon.stub(lockfileApis, 'inspectFileLock').resolves('stale'); + const reclaim = sinon.stub(lockfileApis, 'reclaimFileLock').resolves(true); + + assert.ok(await manager.create(scriptUri())); + + sinon.assert.calledWith(inspect, envDir().fsPath); + sinon.assert.calledWith(reclaim, envDir().fsPath); + assert.ok(reclaim.calledBefore(lockStub), 'the dead generation must be cleared before waiting'); + }); + + test('leaves a lock held by a live process alone', async () => { + sinon.stub(lockfileApis, 'inspectFileLock').resolves('held'); + const reclaim = sinon.stub(lockfileApis, 'reclaimFileLock').resolves(true); + + assert.ok(await manager.create(scriptUri())); + + sinon.assert.notCalled(reclaim); + }); + test('reuses a restart cache entry from an older backup matching the selected base', async () => { const directory = envDir(); const executable = venvPythonPath(directory.fsPath); @@ -2516,6 +2540,34 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(routingRegistry.shouldRoute(uri), true); }); + // Reporting success for an at-risk entry whose stamp failed hands back an environment + // another window is still free to reclaim. + test('refuses to reuse an at-risk entry whose last-used time cannot be refreshed', async () => { + const first = scriptUri(); + const environment = await manager.create(first); + assert.ok(environment); + setSidecar( + await makeSidecar({ lastUsedAt: new Date(NOW.getTime() - 20 * 24 * 60 * 60 * 1000).toISOString() }), + Uri.file(environment.sysPrefix), + ); + writeMetaStub.rejects(Object.assign(new Error('sidecar is busy'), { code: 'EBUSY' })); + + assert.strictEqual(await manager.create(scriptUri('second.py')), undefined); + }); + + test('still reuses an entry that is nowhere near eviction when its stamp fails', async () => { + const first = scriptUri(); + const environment = await manager.create(first); + assert.ok(environment); + setSidecar( + await makeSidecar({ lastUsedAt: new Date(NOW.getTime() - 2 * 24 * 60 * 60 * 1000).toISOString() }), + Uri.file(environment.sysPrefix), + ); + writeMetaStub.rejects(Object.assign(new Error('sidecar is busy'), { code: 'EBUSY' })); + + assert.ok(await manager.create(scriptUri('second.py'))); + }); + test('unavailable inventory does not invalidate an otherwise usable environment', async () => { const uri = scriptUri(); const environment = await manager.create(uri); @@ -6324,7 +6376,17 @@ suite('InlineScriptEnvManager', () => { ); } - test('evicts only entries older than 14 days before the first create', async () => { + /** Runs the sweep directly; when it runs is covered by the `eviction scheduling` suite. */ + function runTtlEviction(): Promise { + return (manager as unknown as { runTtlEvictionOnce(): Promise }).runTtlEvictionOnce(); + } + + /** Bypasses the once-per-window latch. */ + function runLaterSweep(): Promise { + return (manager as unknown as { evictStaleCacheEntries(): Promise }).evictStaleCacheEntries(); + } + + test('evicts only entries older than 14 days', async () => { const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); const recent = await createOwnedEnvironment('bbbbbbbbbbbbbbbb'); const exactCutoff = await createOwnedEnvironment('cccccccccccccccc'); @@ -6332,15 +6394,143 @@ suite('InlineScriptEnvManager', () => { await setLastUsedAt(recent, new Date(NOW.getTime() - TTL_MS + 1)); await setLastUsedAt(exactCutoff, new Date(NOW.getTime() - TTL_MS)); - const created = await manager.create(scriptUri()); + await runTtlEviction(); - assert.ok(created); assert.strictEqual(await fs.pathExists(stale.sysPrefix), false); assert.strictEqual(await fs.pathExists(recent.sysPrefix), true); assert.strictEqual(await fs.pathExists(exactCutoff.sysPrefix), true); }); - test('attempts the eviction sweep once and does not block creation when it fails', async () => { + test('deletes at most three entries per sweep and drains the rest later', async () => { + const orphans = []; + for (const key of ['aaaaaaaaaaaaaaaa', 'bbbbbbbbbbbbbbbb', 'cccccccccccccccc', 'dddddddddddddddd']) { + const orphan = await createOwnedEnvironment(key); + await setLastUsedAt(orphan, new Date(NOW.getTime() - TTL_MS - 1)); + orphans.push(orphan); + } + + await runTtlEviction(); + + const afterFirst = await Promise.all(orphans.map((orphan) => fs.pathExists(orphan.sysPrefix))); + assert.strictEqual(afterFirst.filter((exists) => !exists).length, 3); + + await runLaterSweep(); + + const afterSecond = await Promise.all(orphans.map((orphan) => fs.pathExists(orphan.sysPrefix))); + assert.deepStrictEqual(afterSecond, [false, false, false, false]); + }); + + /** A build interrupted before `buildCacheEntry` could write `.meta.json`. */ + async function createInterruptedEntry(cacheKey: string, abandonedAt: Date): Promise { + const location = cacheLayout.getScriptEnvDir(globalStorageUri, cacheKey).fsPath; + await fs.outputFile(getVenvPythonPath(location), ''); + await fs.utimes(location, abandonedAt, abandonedAt); + return location; + } + + test('reclaims an entry left incomplete by an interrupted setup', async () => { + const incomplete = await createInterruptedEntry( + 'eeeeeeeeeeeeeeee', + new Date(NOW.getTime() - 25 * 60 * 60 * 1000), + ); + + await runTtlEviction(); + + assert.strictEqual(await fs.pathExists(incomplete), false); + }); + + test('keeps a recently interrupted entry until the grace period passes', async () => { + const incomplete = await createInterruptedEntry( + 'eeeeeeeeeeeeeeee', + new Date(NOW.getTime() - 60 * 60 * 1000), + ); + + await runTtlEviction(); + + assert.strictEqual(await fs.pathExists(incomplete), true); + }); + + test('never reclaims an entry whose sidecar a newer extension wrote', async () => { + const future = await createInterruptedEntry( + 'ffffffffffffffff', + new Date(NOW.getTime() - 25 * 60 * 60 * 1000), + ); + setSidecarResults({ ffffffffffffffff: { kind: 'unsupported' } }); + + await runTtlEviction(); + + assert.strictEqual(await fs.pathExists(future), true); + }); + + test('never reclaims an entry whose sidecar cannot be read', async () => { + const unreadable = await createInterruptedEntry( + 'ffffffffffffffff', + new Date(NOW.getTime() - 25 * 60 * 60 * 1000), + ); + setSidecarResults({ ffffffffffffffff: { kind: 'unavailable' } }); + + await runTtlEviction(); + + assert.strictEqual(await fs.pathExists(unreadable), true); + }); + + test('a selection made after planning wins over reclaiming', async () => { + type EvictionPlan = { readonly evictableStaleEntries: readonly string[] }; + const uri = scriptUri('claimed.py'); + const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1)); + const internalManager = manager as unknown as { + planStaleCacheEviction(): Promise; + removeEvictableCacheEntries(plan: EvictionPlan): Promise>; + }; + + const plan = await internalManager.planStaleCacheEviction(); + assert.ok(plan, 'the orphan should be evictable at planning time'); + + await manager.set(uri, stale); + const removed = await internalManager.removeEvictableCacheEntries(plan); + + assert.strictEqual(removed.size, 0); + assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); + assert.strictEqual(await manager.get(uri), stale); + }); + + test('marks an entry unusable before deleting it', async () => { + const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1)); + writeMetaStub.resetHistory(); + + await runTtlEviction(); + + assert.ok( + writeMetaStub.getCalls().some((call) => call.args[1]?.manuallyModified === true), + 'a partially failed deletion must not leave the survivors looking healthy', + ); + }); + + test('keeps an association whose cache path was rebuilt after removal', async () => { + const uri = scriptUri('rebuilt.py'); + const rebuilt = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await manager.set(uri, rebuilt); + (manager as unknown as { collection: PythonEnvironment[] }).collection = [rebuilt]; + const listener = sinon.spy(); + const collectionListener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + manager.onDidChangeEnvironments(collectionListener); + + await ( + manager as unknown as { + publishCacheEvictionResults(removed: ReadonlySet): Promise; + } + ).publishCacheEvictionResults(new Set([normalizePath(rebuilt.sysPrefix)])); + + assert.strictEqual(await manager.get(uri), rebuilt); + assert.ok((await manager.getEnvironments('all')).includes(rebuilt)); + sinon.assert.notCalled(listener); + sinon.assert.notCalled(collectionListener); + }); + + test('sweeps at most once per window even when the sweep fails', async () => { const internalManager = manager as unknown as { evictStaleCacheEntries(): Promise; }; @@ -6348,10 +6538,133 @@ suite('InlineScriptEnvManager', () => { .stub(internalManager, 'evictStaleCacheEntries') .rejects(new Error('cache scan unavailable')); + await runTtlEviction(); + await runTtlEviction(); + + sinon.assert.calledOnce(eviction); + }); + + test('does not sweep as a side effect of creating an environment', async () => { const internalManager = manager as unknown as { + evictStaleCacheEntries(): Promise; + }; + const eviction = sinon.stub(internalManager, 'evictStaleCacheEntries').resolves(); + assert.ok(await manager.create(scriptUri('first.py'))); assert.ok(await manager.create(scriptUri('second.py'))); - sinon.assert.calledOnce(eviction); + sinon.assert.notCalled(eviction); + }); + + test('lets a create started during an in-flight sweep complete', async () => { + const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1)); + + const eviction = runTtlEviction(); + const created = await manager.create(scriptUri('trigger.py')); + await eviction; + + assert.ok(created); + assert.strictEqual(await fs.pathExists(stale.sysPrefix), false); + }); + + // `get` sits on the language-server configuration path and must not wait for a sweep. + test('resolves environments while the sweep is deleting', async () => { + const uri = scriptUri('associated.py'); + const referenced = await createOwnedEnvironment('bbbbbbbbbbbbbbbb'); + await manager.set(uri, referenced); + const orphan = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await setLastUsedAt(orphan, new Date(NOW.getTime() - TTL_MS - 1)); + + const internalManager = manager as unknown as { + deleteCacheEntryForClear(entryPath: string): Promise; + }; + const originalDelete = internalManager.deleteCacheEntryForClear.bind(manager); + const enteredDeletion = createDeferred(); + const finishDeletion = createDeferred(); + sinon.stub(internalManager, 'deleteCacheEntryForClear').callsFake(async (entryPath) => { + enteredDeletion.resolve(); + await finishDeletion.promise; + return originalDelete(entryPath); + }); + + const eviction = runTtlEviction(); + await enteredDeletion.promise; + const resolved = await Promise.race([ + manager.get(uri), + new Promise((resolve) => setTimeout(() => resolve('blocked by the sweep'), 2000)), + ]); + finishDeletion.resolve(); + await eviction; + + assert.strictEqual(resolved, referenced); + assert.strictEqual(await fs.pathExists(orphan.sysPrefix), false); + assert.strictEqual(await fs.pathExists(referenced.sysPrefix), true); + }); + + test('lookup and setup are not blocked by an unrelated slow cache scan', async () => { + const uri = scriptUri('associated.py'); + const referenced = await createOwnedEnvironment(); + await manager.set(uri, referenced); + const scanning = createDeferred(); + const finishScan = createDeferred(); + const unrelatedKey = 'eeeeeeeeeeeeeeee'; + await createInterruptedEntry(unrelatedKey, NOW); + inspectMetaStub.callsFake(async (entry: Uri) => { + if (path.basename(entry.fsPath) === unrelatedKey) { + scanning.resolve(); + await finishScan.promise; + return { kind: 'missing' }; + } + return { kind: 'valid', metadata: await makeSidecar() }; + }); + + const sweep = runTtlEviction(); + await scanning.promise; + try { + const results = await Promise.race([ + Promise.all([manager.get(uri), manager.create(scriptUri('new.py'))]), + new Promise<'blocked'>((resolve) => setTimeout(() => resolve('blocked'), 500)), + ]); + assert.notStrictEqual(results, 'blocked'); + assert.ok(Array.isArray(results)); + assert.strictEqual(results[0], referenced); + assert.ok(results[1]); + } finally { + finishScan.resolve(); + await sweep; + } + }); + + test('lookup is not blocked while eviction results are reconciled', async () => { + const uri = scriptUri('associated.py'); + const referenced = await createOwnedEnvironment('bbbbbbbbbbbbbbbb'); + await manager.set(uri, referenced); + const orphan = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await setLastUsedAt(orphan, new Date(NOW.getTime() - TTL_MS - 1)); + const publishing = createDeferred(); + const finishPublication = createDeferred(); + const internalManager = manager as unknown as { + reconcileCollectionAfterRemoval(candidates?: ReadonlySet): Promise; + }; + const reconcile = internalManager.reconcileCollectionAfterRemoval.bind(manager); + sinon.stub(internalManager, 'reconcileCollectionAfterRemoval').callsFake(async (candidates) => { + publishing.resolve(); + await finishPublication.promise; + return reconcile(candidates); + }); + + const sweep = runTtlEviction(); + await publishing.promise; + try { + const result = await Promise.race([ + manager.get(uri), + new Promise<'blocked'>((resolve) => setTimeout(() => resolve('blocked'), 500)), + ]); + assert.strictEqual(result, referenced); + } finally { + finishPublication.resolve(); + await sweep; + } }); test('rechecks lastUsedAt under the entry lock before deleting', async () => { @@ -6364,12 +6677,12 @@ suite('InlineScriptEnvManager', () => { return { release: releaseLockStub, retain: retainLockStub }; }); - assert.ok(await manager.create(scriptUri())); + await runTtlEviction(); assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); }); - test('preserves a locked stale entry without failing the triggering create', async () => { + test('preserves a locked stale entry', async () => { const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1)); lockStub.callsFake(async (entryPath: string) => { @@ -6379,9 +6692,8 @@ suite('InlineScriptEnvManager', () => { return { release: releaseLockStub, retain: retainLockStub }; }); - const created = await manager.create(scriptUri()); + await runTtlEviction(); - assert.ok(created); assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); }); @@ -6395,13 +6707,13 @@ suite('InlineScriptEnvManager', () => { }); await lock.retain(); - assert.ok(await manager.create(scriptUri())); + await runTtlEviction(); assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); assert.strictEqual(await fs.pathExists(lockfileApis.getFileLockPath(stale.sysPrefix)), true); }); - test('preserves a stale entry when deletion fails without failing creation', async () => { + test('preserves a stale entry when deletion fails', async () => { const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1)); const internalManager = manager as unknown as { @@ -6415,9 +6727,8 @@ suite('InlineScriptEnvManager', () => { return originalDelete(entryPath); }); - const created = await manager.create(scriptUri()); + await runTtlEviction(); - assert.ok(created); assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); }); @@ -6428,7 +6739,7 @@ suite('InlineScriptEnvManager', () => { const collectionListener = sinon.spy(); manager.onDidChangeEnvironments(collectionListener); - assert.ok(await manager.create(scriptUri('trigger.py'))); + await runTtlEviction(); assert.strictEqual(await fs.pathExists(stale.sysPrefix), false); assert.deepStrictEqual(await manager.getEnvironments('all'), []); @@ -6446,7 +6757,7 @@ suite('InlineScriptEnvManager', () => { const selectionListener = sinon.spy(); manager.onDidChangeEnvironment(selectionListener); - assert.ok(await manager.create(scriptUri('trigger.py'))); + await runTtlEviction(); assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); assert.strictEqual(await manager.get(uri), stale); @@ -6473,7 +6784,7 @@ suite('InlineScriptEnvManager', () => { }; const removeSpy = sinon.spy(internalManager, 'removeCacheEntryForClear'); - assert.ok(await manager.create(scriptUri('trigger.py'))); + await runTtlEviction(); sinon.assert.notCalled(removeSpy); assert.strictEqual(await fs.pathExists(stale.sysPrefix), true); @@ -6481,6 +6792,22 @@ suite('InlineScriptEnvManager', () => { assert.notStrictEqual(persistedAssociations, undefined); }); + test('does not evict an orphaned entry whose last-used time was refreshed on use', async () => { + const uri = scriptUri('used.py'); + const environment = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + await manager.set(uri, environment); + await setLastUsedAt(environment, new Date(NOW.getTime() - TTL_MS - 1)); + writeMetaStub.resetHistory(); + + assert.strictEqual(await manager.get(uri), environment); + await waitForStubCallCount(writeMetaStub, 1); + await manager.set(uri, undefined); + + await runTtlEviction(); + + assert.strictEqual(await fs.pathExists(environment.sysPrefix), true); + }); + test('does not let an in-flight refresh re-add an evicted environment', async () => { const stale = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); await setLastUsedAt(stale, new Date(NOW.getTime() - TTL_MS - 1)); @@ -6510,13 +6837,13 @@ suite('InlineScriptEnvManager', () => { const refresh = manager.refresh(undefined); await refreshStarted; - const create = manager.create(scriptUri('trigger.py')); + const eviction = runTtlEviction(); await waitForCondition( async () => !(await fs.pathExists(stale.sysPrefix)), 'Expected TTL eviction to delete the stale entry', ); releaseRefresh!(); - await Promise.all([refresh, create]); + await Promise.all([refresh, eviction]); assert.strictEqual(await fs.pathExists(stale.sysPrefix), false); assert.deepStrictEqual(await manager.getEnvironments('all'), []); @@ -6535,8 +6862,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(await internalManager.isCacheEntryDefinitelyMissing(entryPath), false); }); - test('does not fail creation when association cleanup cannot be persisted', async () => { - const orphan = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); + test('does not fail the sweep when association cleanup cannot be persisted', async () => { const orphan = await createOwnedEnvironment('aaaaaaaaaaaaaaaa'); await setLastUsedAt(orphan, new Date(NOW.getTime() - TTL_MS - 1)); // A separate association whose environment was deleted out from under us. Evicting the // orphaned entry above drives the association cleanup pass, and persisting that cleanup is @@ -6547,7 +6873,8 @@ suite('InlineScriptEnvManager', () => { await fs.remove(missing.sysPrefix); workspaceState.update.onSecondCall().rejects(new Error('Memento unavailable')); - assert.ok(await manager.create(scriptUri('trigger.py'))); + await runTtlEviction(); + assert.strictEqual(await fs.pathExists(orphan.sysPrefix), false); }); }); @@ -6855,7 +7182,867 @@ suite('InlineScriptEnvManager', () => { }); }); - suite('clear cache', () => { + suite('last-used refresh on use', () => { + const DAY_MS = 24 * 60 * 60 * 1000; + + async function associateWithLastUsedAt( + lastUsedAt: Date, + ): Promise<{ uri: Uri; environment: PythonEnvironment }> { + const uri = scriptUri('used.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + setSidecar(await makeSidecar({ lastUsedAt: lastUsedAt.toISOString() }), Uri.file(environment.sysPrefix)); + writeMetaStub.resetHistory(); + return { uri, environment }; + } + + test('refreshes a stale last-used time when the environment is resolved', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + + assert.strictEqual(await manager.get(uri), environment); + + await waitForStubCallCount(writeMetaStub, 1); + assert.strictEqual(writeMetaStub.firstCall.args[1].lastUsedAt, NOW.toISOString()); + }); + + test('stamps an at-risk entry before handing it out', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + + await manager.get(uri); + + sinon.assert.calledOnce(writeMetaStub); + }); + + // The recorded stamp already proves the entry is retained, so refreshing it is bookkeeping. + // `timeoutMs: 0` bounds only lock acquisition, and graceful-fs retries a Windows sharing + // violation on the rename for a full minute. + test('does not wait for the refresh of a safely retained entry', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + const blockedWrite = createDeferred(); + writeMetaStub.callsFake(() => blockedWrite.promise); + + const resolved = await Promise.race([ + manager.get(uri), + nextTurn().then(() => 'blocked' as const), + ]); + await waitForStubCall(writeMetaStub); + blockedWrite.resolve(); + + assert.strictEqual(resolved, environment); + }); + + test('does not take the entry lock when the stamp is still fresh', async () => { + const { uri } = await associateWithLastUsedAt(NOW); + lockStub.resetHistory(); + + await manager.get(uri); + + sinon.assert.notCalled(lockStub); + }); + + test('leaves a recently used entry untouched', async () => { + const { uri } = await associateWithLastUsedAt(NOW); + + await manager.get(uri); + await new Promise((resolve) => setTimeout(resolve, 25)); + + sinon.assert.notCalled(writeMetaStub); + }); + + test('refreshes at most once per interval across repeated resolves', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + + await manager.get(uri); + await waitForStubCallCount(writeMetaStub, 1); + await manager.get(uri); + await manager.get(uri); + await new Promise((resolve) => setTimeout(resolve, 25)); + + sinon.assert.calledOnce(writeMetaStub); + }); + + test('skips the refresh when the entry lock is held elsewhere', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + lockStub.rejects(Object.assign(new Error('cache entry is locked'), { code: 'ELOCKED' })); + + await manager.get(uri); + await new Promise((resolve) => setTimeout(resolve, 25)); + + sinon.assert.notCalled(writeMetaStub); + }); + + // Backoff must suppress repeated write attempts, not recognition of protection another + // window has since established. + test('recognizes protection established elsewhere during backoff', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + lockStub.rejects(Object.assign(new Error('cache entry is locked'), { code: 'ELOCKED' })); + + assert.strictEqual(await manager.get(uri), undefined); + + setSidecar(await makeSidecar({ lastUsedAt: NOW.toISOString() }), Uri.file(environment.sysPrefix)); + + assert.strictEqual(await manager.get(uri), environment); + }); + + // The refresh holds the same lock construction and deletion use. Readers must not mistake it + // for a rebuild, or a working environment disappears once the validation cache expires. + test('a background refresh does not make the entry look busy', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + // Stand in for a refresh holding the real entry lock while its write is in flight. + await fs.ensureDir(lockfileApis.getFileLockPath(environment.sysPrefix)); + (manager as unknown as { bookkeepingLocks: Set }).bookkeepingLocks.add( + normalizePath(environment.sysPrefix), + ); + clock.tick(6_000); + + assert.strictEqual(await manager.get(uri), environment); + }); + + // A busy entry is transiently unavailable, not un-routeable; clearing here would strand the + // script with the setup CodeLens until its next save. + test('a busy entry does not clear validated routing on save', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + await fs.ensureDir(lockfileApis.getFileLockPath(environment.sysPrefix)); + clock.tick(6_000); + + await triggerSavedMetadataChange(routingRegistry, manager, uri); + + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + }); + + // The exemption must track real ownership. While acquisition is still pending another + // window may be mid-rebuild, and calling the entry idle deletes a good association. + test('a refresh awaiting its lock does not exempt the entry', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + await fs.ensureDir(lockfileApis.getFileLockPath(environment.sysPrefix)); + sinon.stub(lockfileApis, 'inspectFileLock').resolves('held'); + let allowAcquire: () => void = () => undefined; + const acquired = new Promise((resolve) => { + allowAcquire = resolve; + }); + lockStub.callsFake(async () => { + await acquired; + return { release: releaseLockStub, retain: retainLockStub }; + }); + + await manager.get(uri); + await nextTurn(); + const busy = await ( + manager as unknown as { isCacheEntryBusy(envDirPath: string): Promise } + ).isCacheEntryBusy(environment.sysPrefix); + allowAcquire(); + + assert.strictEqual(busy, true); + }); + + // An older save-time validation parked on the busy inspection must not undo a setup that + // completed while it was waiting. + test('a newer setup wins over a pending save-time busy check', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + const scriptPath = normalizePath(uri.fsPath); + const internals = manager as unknown as { + refreshValidatedAssociationForMetadataInternal( + scriptPath: string, + uri: Uri, + metadata: metadataReader.InlineScriptMetadata, + metadataIdentity: string, + metadataRevision: number, + associationRevision: number, + ): Promise; + getAssociationForMetadata(...args: unknown[]): Promise; + isAssociatedEntryBusy(scriptPath: string): Promise; + bumpAssociationRevision(scriptPath: string): void; + associationRevisions: Map; + }; + const staleRevision = internals.associationRevisions.get(scriptPath) ?? 0; + sinon.stub(internals, 'getAssociationForMetadata').resolves(undefined); + let finishBusyCheck: (busy: boolean) => void = () => undefined; + sinon.stub(internals, 'isAssociatedEntryBusy').returns( + new Promise((resolve) => { + finishBusyCheck = resolve; + }), + ); + + const task = internals.refreshValidatedAssociationForMetadataInternal( + scriptPath, + uri, + VALID_METADATA, + routingRegistry.getMetadataIdentity(uri)!, + routingRegistry.getMetadataRevision(uri)!, + staleRevision, + ); + await nextTurn(); + internals.bumpAssociationRevision(scriptPath); + finishBusyCheck(false); + await task; + + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + }); + + // Optional bookkeeping must fail fast rather than hold the shared entry lock through a + // minute-long rename retry, which another window cannot distinguish from a rebuild. + test('a background refresh writes without the retrying rename', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + + await manager.get(uri); + await waitForStubCallCount(writeMetaStub, 1); + + assert.deepStrictEqual(writeMetaStub.firstCall.args[2], { failFast: true }); + }); + + test('a required read-path stamp also avoids the retrying rename', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + + await manager.get(uri); + await waitForStubCallCount(writeMetaStub, 1); + + assert.deepStrictEqual(writeMetaStub.firstCall.args[2], { failFast: true }); + }); + + test('withholds an at-risk entry when a contended stamp cannot prove it', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + lockStub.rejects(Object.assign(new Error('cache entry is locked'), { code: 'ELOCKED' })); + + assert.strictEqual(await manager.get(uri), undefined); + + const state = touchState().get(normalizePath(environment.sysPrefix)); + assert.strictEqual( + state?.retryNotBefore, undefined, 'required protection must not inherit optional backoff', + ); + }); + + // Being due a refresh is not the same as being evictable, so a brief collision must not take + // a safely retained environment out of service for five minutes. + test('still serves a refresh-due entry that is nowhere near eviction', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + lockStub.rejects(Object.assign(new Error('cache entry is locked'), { code: 'ELOCKED' })); + + assert.strictEqual(await manager.get(uri), environment); + }); + + test('withholds an at-risk entry when the stamp itself fails', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + writeMetaStub.rejects(Object.assign(new Error('sidecar is busy'), { code: 'EBUSY' })); + + assert.strictEqual(await manager.get(uri), undefined); + }); + + test('does not let a pending stamp grant use to a concurrent lookup', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + const blockedWrite = createDeferred(); + writeMetaStub.callsFake(() => blockedWrite.promise); + + const first = manager.get(uri); + await waitForStubCall(writeMetaStub); + const second = manager.get(uri); + const raced = await Promise.race([ + second.then(() => 'answered' as const), + nextTurn().then(() => 'waiting' as const), + ]); + blockedWrite.resolve(); + + assert.strictEqual(raced, 'waiting', 'a concurrent lookup must join the stamp, not skip it'); + assert.ok(await first); + assert.ok(await second); + sinon.assert.calledOnce(writeMetaStub); + }); + + // Two lookups can reach the stamp before either has finished its first read, so the shared + // operation has to be registered before that read, not after it. + test('joins a stamp started by an overlapping lookup', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + const blockedWrite = createDeferred(); + writeMetaStub.callsFake(() => blockedWrite.promise); + + const both = [manager.get(uri), manager.get(uri)]; + await waitForStubCall(writeMetaStub); + blockedWrite.resolve(); + const [firstResult, secondResult] = await Promise.all(both); + + assert.ok(firstResult); + assert.strictEqual(secondResult, firstResult); + sinon.assert.calledOnce(writeMetaStub); + }); + + test('does not return an environment unset during its required stamp', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + const finishWrite = createDeferred(); + writeMetaStub.callsFake(async (entry: Uri, sidecar: cacheLayout.InlineScriptEnvMeta) => { + await finishWrite.promise; + setSidecar(sidecar, entry); + }); + const lookup = manager.get(uri); + try { + await waitForStubCall(writeMetaStub); + await manager.set(uri, undefined); + } finally { + finishWrite.resolve(); + } + + assert.strictEqual(await lookup, undefined); + assert.strictEqual(await manager.get(uri), undefined); + }); + + test('returns the replacement when selection changes during a required stamp', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + const replacement = await createOwnedEnvironment('replacement'); + const finishWrite = createDeferred(); + writeMetaStub.callsFake(async (entry: Uri, sidecar: cacheLayout.InlineScriptEnvMeta) => { + await finishWrite.promise; + setSidecar(sidecar, entry); + }); + const lookup = manager.get(uri); + try { + await waitForStubCall(writeMetaStub); + await manager.set(uri, replacement); + } finally { + finishWrite.resolve(); + } + + assert.strictEqual(await lookup, replacement); + }); + + test('does not return a descriptor for metadata changed during its required stamp', async () => { + const { uri } = await associateWithLastUsedAt(new Date(NOW.getTime() - 20 * DAY_MS)); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + const finishWrite = createDeferred(); + writeMetaStub.callsFake(async (entry: Uri, sidecar: cacheLayout.InlineScriptEnvMeta) => { + await finishWrite.promise; + setSidecar(sidecar, entry); + }); + const lookup = manager.get(uri); + try { + await waitForStubCall(writeMetaStub); + const changed = { ...VALID_METADATA, dependencies: ['rich'] }; + readMetadataStub.resolves(changed); + routingRegistry.setMetadata(uri, changed); + } finally { + finishWrite.resolve(); + } + + assert.strictEqual(await lookup, undefined); + }); + + test('forgets touch bookkeeping for a removed entry', async () => { + const { uri, environment } = await associateWithLastUsedAt(new Date(NOW.getTime() - 8 * DAY_MS)); + + await manager.get(uri); + await waitForStubCallCount(writeMetaStub, 1); + assert.ok(touchState().has(normalizePath(environment.sysPrefix))); + + await manager.remove(environment); + + assert.strictEqual(touchState().has(normalizePath(environment.sysPrefix)), false); + }); + + test('restores the setup action when the entry becomes unusable', async () => { + const uri = scriptUri('unusable.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + setSidecar(await makeSidecar({ manuallyModified: true }), Uri.file(environment.sysPrefix)); + + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + sinon.assert.calledOnceWithExactly(listener, { uri, old: environment, new: undefined }); + }); + + // A same-key rebuild can replace the entry while a lookup is pending; the newer setup wins. + test('does not publish an unusable verdict the entry no longer deserves', async () => { + const uri = scriptUri('repaired.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + + await ( + manager as unknown as { invalidateUnusableEntry(envDirPath: string): Promise } + ).invalidateUnusableEntry(environment.sysPrefix); + + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + sinon.assert.notCalled(listener); + }); + + // The extra read covers repairs finished before reconfirmation; the entry lock covers ones + // finishing while it is pending. + test('does not invalidate while a rebuild owns the entry', async () => { + const uri = scriptUri('rebuilding.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + inspectMetaStub.resolves({ kind: 'missing' }); + lockStub.rejects(Object.assign(new Error('Entry is being rebuilt'), { code: 'ELOCKED' })); + + await ( + manager as unknown as { invalidateUnusableEntry(envDirPath: string): Promise } + ).invalidateUnusableEntry(environment.sysPrefix); + + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + sinon.assert.notCalled(listener); + }); + + function touchState(): Map { + return ( + manager as unknown as { + lastUsedTouchState: Map; + } + ).lastUsedTouchState; + } + + function proveUsable(environment: PythonEnvironment): Promise { + return ( + manager as unknown as { proveEnvironmentUsable(envDirPath: string): Promise } + ).proveEnvironmentUsable(environment.sysPrefix); + } + + test('treats unreadable metadata as unproven rather than permission', async () => { + const environment = await createOwnedEnvironment(); + inspectMetaStub.resolves({ kind: 'unavailable' }); + + assert.strictEqual(await proveUsable(environment), 'unproven'); + }); + + test('keeps serving an entry whose sidecar a newer extension owns', async () => { + const environment = await createOwnedEnvironment(); + inspectMetaStub.resolves({ kind: 'unsupported' }); + + assert.strictEqual(await proveUsable(environment), 'usable'); + }); + + test('treats missing or invalid metadata as unusable', async () => { + const environment = await createOwnedEnvironment(); + + inspectMetaStub.resolves({ kind: 'missing' }); + assert.strictEqual(await proveUsable(environment), 'unusable'); + + touchState().clear(); + inspectMetaStub.resolves({ kind: 'invalid' }); + assert.strictEqual(await proveUsable(environment), 'unusable'); + }); + }); + + suite('temporary association recovery', () => { + setup(() => { + clock.restore(); + clock = sinon.useFakeTimers({ now: NOW, toFake: ['Date', 'setTimeout', 'clearTimeout'] }); + }); + + function whenRouteable(uri: Uri): Promise { + const ready = createDeferred(); + const listener = routingRegistry.onDidChangeRouteability((event) => { + if (normalizePath(event.uri.fsPath) === normalizePath(uri.fsPath) && event.routeable) { + listener.dispose(); + ready.resolve(); + } + }); + return ready.promise; + } + + function whenAvailable(uri: Uri): Promise { + const ready = createDeferred(); + const listener = routingRegistry.onDidChangeAvailability((changedUri) => { + if ( + normalizePath(changedUri.fsPath) === normalizePath(uri.fsPath) && + !routingRegistry.isEnvironmentUnavailable(changedUri) + ) { + listener.dispose(); + ready.resolve(); + } + }); + return ready.promise; + } + + async function failFirstUsageStamp(uri: Uri): Promise { + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + setSidecar( + await makeSidecar({ + lastUsedAt: new Date(NOW.getTime() - 20 * 24 * 60 * 60 * 1000).toISOString(), + }), + Uri.file(environment.sysPrefix), + ); + writeMetaStub.resetHistory(); + writeMetaStub.onFirstCall().rejects(Object.assign(new Error('sidecar busy'), { code: 'EBUSY' })); + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(routingRegistry.isEnvironmentUnavailable(uri), true); + return environment; + } + + test('restores a startup association after another window releases its entry lock', async () => { + const uri = scriptUri('restored.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + manager.dispose(); + routingRegistry.dispose(); + routingRegistry = new InlineScriptRoutingRegistry(); + lockStub.resetBehavior(); + lockStub.callThrough(); + const foreignLock = await lockfileApis.acquireFileLock(environment.sysPrefix, { + timeoutMs: 0, + retryIntervalMs: 1, + }); + try { + manager = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + workspaceMemento, + routingRegistry, + ); + await (manager as unknown as { initializePersistedAssociations(): Promise }) + .initializePersistedAssociations(); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + } finally { + await foreignLock.release(); + } + const recovered = whenRouteable(uri); + await clock.tickAsync(1_000); + await recovered; + + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + assert.ok(await manager.get(uri)); + sinon.assert.notCalled(createWithProgressStub); + }); + + test('recovers an old usable environment after a single stamp failure without a save', async () => { + const uri = scriptUri('recover.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + setSidecar( + await makeSidecar({ + lastUsedAt: new Date(NOW.getTime() - 20 * 24 * 60 * 60 * 1000).toISOString(), + }), + Uri.file(environment.sysPrefix), + ); + writeMetaStub.resetHistory(); + writeMetaStub.onFirstCall().rejects(Object.assign(new Error('sidecar busy'), { code: 'EBUSY' })); + + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(routingRegistry.shouldRoute(uri), true); + assert.strictEqual(routingRegistry.isEnvironmentUnavailable(uri), true); + const provider = new InlineScriptCodeLensProvider(routingRegistry, 'setup'); + try { + const document = new MockDocument( + '# /// script\n# dependencies = ["requests"]\n# ///\n', + uri.fsPath, + async () => true, + ); + assert.strictEqual(provider.provideCodeLenses(document, {} as never).length, 1); + const ready = createDeferred(); + const listener = routingRegistry.onDidChangeAvailability((changedUri) => { + if (!routingRegistry.isEnvironmentUnavailable(changedUri)) { + listener.dispose(); + ready.resolve(); + } + }); + await clock.tickAsync(1_000); + await ready.promise; + + assert.strictEqual(await manager.get(uri), environment); + assert.strictEqual(provider.provideCodeLenses(document, {} as never).length, 0); + assert.strictEqual(writeMetaStub.callCount, 2); + sinon.assert.notCalled(createWithProgressStub); + } finally { + provider.dispose(); + } + }); + + test('same-requirement saves preserve the original recovery deadline', async () => { + const uri = scriptUri('autosaved.py'); + const environment = await failFirstUsageStamp(uri); + const ready = whenAvailable(uri); + + for (let save = 0; save < 4; save += 1) { + await clock.tickAsync(200); + await triggerSavedMetadataChange(routingRegistry, manager, uri, { + ...VALID_METADATA, + dependencies: ['Requests'], + }); + } + assert.strictEqual(writeMetaStub.callCount, 1); + await clock.tickAsync(200); + await ready; + + assert.strictEqual(await manager.get(uri), environment); + assert.strictEqual(writeMetaStub.callCount, 2); + sinon.assert.notCalled(createWithProgressStub); + }); + + test('a body-only save after a real entry lock is released preserves automatic recovery', async () => { + const uri = scriptUri('saved-after-lock.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + clock.tick(6_000); + lockStub.resetBehavior(); + lockStub.callThrough(); + const foreignLock = await lockfileApis.acquireFileLock(environment.sysPrefix, { + timeoutMs: 0, + retryIntervalMs: 1, + }); + try { + assert.strictEqual(await manager.get(uri), undefined); + assert.strictEqual(routingRegistry.isEnvironmentUnavailable(uri), true); + } finally { + await foreignLock.release(); + } + const ready = whenAvailable(uri); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + await clock.tickAsync(1_000); + await ready; + + assert.strictEqual(await manager.get(uri), environment); + sinon.assert.notCalled(createWithProgressStub); + }); + + test('same-requirement saves do not restart an exhausted recovery budget', async () => { + const uri = scriptUri('persistent-lock.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + const internals = manager as unknown as { + getAssociationForMetadata(...args: unknown[]): Promise; + isAssociatedEntryBusy(scriptPath: string): Promise; + retryAssociation(...args: unknown[]): Promise; + }; + sinon.stub(internals, 'getAssociationForMetadata').resolves(undefined); + sinon.stub(internals, 'isAssociatedEntryBusy').resolves(true); + const retries = sinon.spy(internals, 'retryAssociation'); + + await manager.get(uri); + await clock.tickAsync(36_000); + assert.strictEqual(retries.callCount, 3); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + await clock.tickAsync(36_000); + + assert.strictEqual(retries.callCount, 3); + assert.strictEqual(routingRegistry.isEnvironmentUnavailable(uri), true); + }); + + for (const changed of [undefined, { ...VALID_METADATA, dependencies: ['rich'] }]) { + test(`${changed ? 'changed' : 'removed'} requirements cancel a queued recovery`, async () => { + const uri = scriptUri('changed-requirements.py'); + await failFirstUsageStamp(uri); + const internals = manager as unknown as { + retryAssociation(...args: unknown[]): Promise; + }; + const retries = sinon.spy(internals, 'retryAssociation'); + readMetadataStub.resolves(changed); + if (changed) { + await triggerSavedMetadataChange(routingRegistry, manager, uri, changed); + } else { + routingRegistry.clearMetadata(uri); + } + await clock.tickAsync(36_000); + + sinon.assert.notCalled(retries); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + assert.strictEqual(writeMetaStub.callCount, 1); + sinon.assert.notCalled(createWithProgressStub); + }); + } + + for (const requirementsChanged of [false, true]) { + const change = requirementsChanged ? 'changed' : 'unchanged'; + test(`a save with ${change} requirements during in-flight recovery respects current metadata`, async () => { + const uri = scriptUri('inflight-save.py'); + const environment = await failFirstUsageStamp(uri); + const writing = createDeferred(); + const finishWrite = createDeferred(); + writeMetaStub.onSecondCall().callsFake( + async (entry: Uri, sidecar: cacheLayout.InlineScriptEnvMeta) => { + writing.resolve(); + await finishWrite.promise; + setSidecar(sidecar, entry); + }, + ); + const internals = manager as unknown as { + retryAssociation(...args: unknown[]): Promise; + }; + const retries = sinon.spy(internals, 'retryAssociation'); + await clock.tickAsync(1_000); + await writing.promise; + try { + const saved = requirementsChanged + ? { ...VALID_METADATA, dependencies: ['rich'] } + : { ...VALID_METADATA }; + readMetadataStub.resolves(saved); + await triggerSavedMetadataChange(routingRegistry, manager, uri, saved); + } finally { + finishWrite.resolve(); + } + await retries.firstCall.returnValue; + await nextTurn(); + + assert.strictEqual(await manager.get(uri), requirementsChanged ? undefined : environment); + assert.strictEqual(routingRegistry.shouldRoute(uri), !requirementsChanged); + await clock.tickAsync(36_000); + sinon.assert.calledOnce(retries); + sinon.assert.notCalled(createWithProgressStub); + }); + } + + test('limits recovery retries and cancels them after an unset', async () => { + const uri = scriptUri('busy.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + const internalManager = manager as unknown as { + getAssociationForMetadata(...args: unknown[]): Promise; + isAssociatedEntryBusy(scriptPath: string): Promise; + }; + const resolution = sinon.stub(internalManager, 'getAssociationForMetadata').resolves(undefined); + sinon.stub(internalManager, 'isAssociatedEntryBusy').resolves(true); + + await manager.get(uri); + await clock.tickAsync(36_000); + assert.strictEqual(resolution.callCount, 4, 'one lookup plus three background attempts'); + await clock.tickAsync(60_000); + assert.strictEqual(resolution.callCount, 4); + + routingRegistry.setMetadata(uri, VALID_METADATA); + await clock.tickAsync(0); + await manager.set(uri, undefined); + const afterUnset = resolution.callCount; + await clock.tickAsync(60_000); + + assert.strictEqual(resolution.callCount, afterUnset); + assert.strictEqual(routingRegistry.shouldRoute(uri), false); + }); + + test('disposal cancels a queued association retry', async () => { + const uri = scriptUri('disposed.py'); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + await triggerSavedMetadataChange(routingRegistry, manager, uri); + const internalManager = manager as unknown as { + getAssociationForMetadata(...args: unknown[]): Promise; + isAssociatedEntryBusy(scriptPath: string): Promise; + }; + const resolution = sinon.stub(internalManager, 'getAssociationForMetadata').resolves(undefined); + sinon.stub(internalManager, 'isAssociatedEntryBusy').resolves(true); + + await manager.get(uri); + manager.dispose(); + await clock.tickAsync(60_000); + + sinon.assert.calledOnce(resolution); + }); + }); + + suite('eviction scheduling', () => { + function stubEvictionDelay(delayMs: number): void { + sinon + .stub(manager as unknown as { getTtlEvictionDelayMs(): number }, 'getTtlEvictionDelayMs') + .returns(delayMs); + } + + function stubSweep(): sinon.SinonStub { + return sinon + .stub(manager as unknown as { evictStaleCacheEntries(): Promise }, 'evictStaleCacheEntries') + .resolves(); + } + + test('sweeps after activation without any environment being created', async () => { + const sweep = stubSweep(); + stubEvictionDelay(0); + + manager.startActivationDiscovery(); + + await waitForStubCallCount(sweep, 1); + }); + + // DISCOVERY_RETRY_DELAYS_MS totals 36s. + test('delays the sweep past the activation discovery retry window', () => { + const getDelay = (manager as unknown as { getTtlEvictionDelayMs(): number }).getTtlEvictionDelayMs; + const delays = Array.from({ length: 50 }, () => getDelay.call(manager)); + + assert.ok( + Math.min(...delays) > 36_000, + `expected every sweep delay past the 36s discovery window, got ${Math.min(...delays)}ms`, + ); + }); + + test('arms the sweep only once when activation discovery is requested repeatedly', async () => { + const sweep = stubSweep(); + stubEvictionDelay(0); + + manager.startActivationDiscovery(); + manager.startActivationDiscovery(); + await waitForStubCallCount(sweep, 1); + await new Promise((resolve) => setTimeout(resolve, 25)); + + sinon.assert.calledOnce(sweep); + }); + + test('does not sweep after dispose cancels the pending timer', async () => { + const sweep = stubSweep(); + stubEvictionDelay(20); + + manager.startActivationDiscovery(); + manager.dispose(); + await new Promise((resolve) => setTimeout(resolve, 60)); + + sinon.assert.notCalled(sweep); + }); + + test('keeps the pending sweep armed when a refresh stops activation discovery', async () => { + const sweep = stubSweep(); + stubEvictionDelay(20); + + manager.startActivationDiscovery(); + await manager.refresh(undefined); + + await waitForStubCallCount(sweep, 1); + }); + }); + + suite('clear cache', () => { + test('removes cleared environments from the catalog', async () => { + const environment = await createOwnedEnvironment(); + (manager as unknown as { collection: PythonEnvironment[] }).collection = [environment]; + const collectionListener = sinon.spy(); + manager.onDidChangeEnvironments(collectionListener); + + await manager.clearCache(); + + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + sinon.assert.calledOnceWithExactly(collectionListener, [ + { kind: EnvironmentChangeKind.remove, environment }, + ]); + }); + + // A clear that deletes nothing still has to drop rows another window already removed. + test('drops catalog entries another window already removed', async () => { + const environment = await createOwnedEnvironment(); + (manager as unknown as { collection: PythonEnvironment[] }).collection = [environment]; + await fs.remove(environment.sysPrefix); + const collectionListener = sinon.spy(); + manager.onDidChangeEnvironments(collectionListener); + + await manager.clearCache(); + + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + sinon.assert.calledOnceWithExactly(collectionListener, [ + { kind: EnvironmentChangeKind.remove, environment }, + ]); + }); + test('clears cached environments, persisted associations, and in-memory selections', async () => { const first = scriptUri('first.py'); const second = scriptUri('second.py');