From 7b1df897194b0f692d2fcc1d0a0d939bdc821ca9 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Tue, 25 Aug 2026 12:20:07 -0700 Subject: [PATCH 1/3] fix: harden inline script routing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/common/inlineScript/cacheLayout.ts | 5 +- src/features/envManagers.ts | 53 +- .../builtin/inlineScript/envManager.ts | 593 ++++++++++++++---- .../inlineScript/cacheLayout.unit.test.ts | 15 +- .../envManagers.lastKnown.unit.test.ts | 89 +++ .../inlineScript/envManager.unit.test.ts | 473 ++++++++++++++ .../builtin/inlineScript/main.unit.test.ts | 5 + 7 files changed, 1101 insertions(+), 132 deletions(-) diff --git a/src/common/inlineScript/cacheLayout.ts b/src/common/inlineScript/cacheLayout.ts index 71f125d4..7554ed08 100644 --- a/src/common/inlineScript/cacheLayout.ts +++ b/src/common/inlineScript/cacheLayout.ts @@ -151,10 +151,13 @@ export async function restoreMetaJsonBackupUnderLock( } const validBackups: Array<{ readonly path: string; readonly metadata: InlineScriptEnvMeta }> = []; + let hasUnsupportedBackup = false; for (const entry of entries.filter((name) => META_JSON_BACKUP_FILENAME_RE.test(name))) { const result = await inspectMetaJsonFile(path.join(envDir.fsPath, entry)); if (result.kind === 'valid' && isCompatible(result.metadata)) { validBackups.push({ path: path.join(envDir.fsPath, entry), metadata: result.metadata }); + } else if (result.kind === 'unsupported') { + hasUnsupportedBackup = true; } else if (result.kind === 'unavailable' || result.kind === 'missing') { // A listed candidate changing or becoming unreadable is an // uncertain scan; preserve the entry rather than rebuilding it. @@ -163,7 +166,7 @@ export async function restoreMetaJsonBackupUnderLock( } if (validBackups.length === 0) { - return { kind: 'missing' }; + return { kind: hasUnsupportedBackup ? 'unsupported' : 'missing' }; } // `lastUsedAt` is schema-validated canonical ISO text. Prefer the newest diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index a99198e4..b7d76421 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -420,8 +420,22 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } if (scope instanceof Uri) { + const inlineOperation = + manager.id === INLINE_SCRIPT_MANAGER_ID + ? operation + : (inlineOverrideHandoffOperation ?? inlineClearOperation); + if ( + !this.commitSelectionOperations([ + { key, operation }, + ...(inlineOperation === undefined + ? [] + : [{ key: this.getInlineScriptSelectionKey(scope), operation: inlineOperation }]), + ]) + ) { + return; + } this.updateInlineRoutingOverride(scope, manager, environment); - this.clearInlineActiveSelection(scope, manager, inlineClearOperation); + this.clearInlineActiveSelection(scope, manager, inlineOperation); if ( clearingInlineRoutingOverride && (await this.publishEffectiveEnvironmentAfterOverrideClear( @@ -511,6 +525,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { await setAllManagerSettings(settings); } selections.forEach((selection) => { + if (!this.commitPendingSelection(selection, manager)) { + return; + } this.updateInlineRoutingOverride(selection.scope, manager, environment); this.clearInlineActiveSelection(selection.scope, manager, selection.inlineClearOperation); if (!selection.publishInlineSelection) { @@ -932,6 +949,20 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } + private commitPendingSelection( + selection: PendingEnvironmentSelection, + manager: InternalEnvironmentManager, + ): boolean { + const inlineOperation = + manager.id === INLINE_SCRIPT_MANAGER_ID ? selection.operation : selection.inlineClearOperation; + return this.commitSelectionOperations([ + { key: selection.key, operation: selection.operation }, + ...(inlineOperation === undefined + ? [] + : [{ key: this.getInlineScriptSelectionKey(selection.scope), operation: inlineOperation }]), + ]); + } + private canPersistManagerSettingForScope( scope: Uri, manager: InternalEnvironmentManager, @@ -951,10 +982,24 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } private commitSelectionOperation(key: string, operation: number): boolean { - if ((this._selectionRevisions.get(key) ?? 0) > operation) { - return false; + return this.commitSelectionOperations([{ key, operation }]); + } + + private commitSelectionOperations( + operations: readonly { readonly key: string; readonly operation: number }[], + ): boolean { + const latestByKey = new Map(); + for (const { key, operation } of operations) { + latestByKey.set(key, Math.max(latestByKey.get(key) ?? operation, operation)); + } + for (const [key, operation] of latestByKey) { + if ((this._selectionRevisions.get(key) ?? 0) > operation) { + return false; + } + } + for (const [key, operation] of latestByKey) { + this._selectionRevisions.set(key, operation); } - this._selectionRevisions.set(key, operation); return true; } diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 34a847c3..b507c9a4 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -151,19 +151,41 @@ type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; +type AssociationValidationResult = + | { readonly kind: 'resolved'; readonly environment: PythonEnvironment } + | { readonly kind: 'missing' } + | { readonly kind: 'busy' }; + +type AssociationValidationOrigin = 'ordinary' | 'retry'; + interface PendingAssociationValidation { + readonly origin: AssociationValidationOrigin; + readonly retryGeneration?: number; readonly metadataIdentity: string; readonly associationRevision: number; - readonly promise: Promise; + readonly promise: Promise; } interface PendingMetadataRefresh { + readonly origin: AssociationValidationOrigin; + readonly retryGeneration?: number; readonly metadataIdentity: string; readonly metadataRevision: number; readonly associationRevision: number; readonly promise: Promise; } +interface AssociationValidationRetry { + readonly uri: Uri; + readonly metadata: InlineScriptMetadata; + readonly metadataIdentity: string; + readonly metadataRevision: number; + readonly associationRevision: number; + readonly retryGeneration: number; + attempt: number; + timer?: ReturnType; +} + interface ParsedPersistedAssociations { readonly rawEntries: Record; readonly records: PersistedInlineScriptEnvironments; @@ -182,8 +204,9 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly directlyResolvedBaseInterpreters = new Map(); private baseInterpreterInstallationQueue: Promise = Promise.resolve(); private collection: PythonEnvironment[] = []; - private readonly pendingRehydrations = new Map(); - private readonly pendingMetadataRefreshes = new Map(); + private readonly pendingRehydrations = new Map>(); + private readonly pendingMetadataRefreshes = new Map>(); + private readonly associationValidationRetries = new Map(); private readonly fsPathToEnv = new Map(); private readonly fsPathToPersistedAssociation = new Map(); private readonly cachedAssociationValidatedAt = new Map(); @@ -203,6 +226,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private cacheMaintenanceBarrier: Deferred | undefined; private pendingCacheMaintenances = 0; private activeCreateOperations = 0; + private associationValidationRetryGeneration = 0; private disposed = false; private readonly _onDidChangeEnvironments = new EventEmitter(); @@ -228,7 +252,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly baseManager: EnvironmentManager, private readonly globalStorageUri: Uri, public readonly log: LogOutputChannel, - private readonly routingRegistry: InlineScriptRoutingRegistry = new InlineScriptRoutingRegistry(), + private readonly routingRegistry: InlineScriptRoutingRegistry, ) { this.subscriptions.push( this.routingRegistry.onDidChangeMetadata((event) => { @@ -985,11 +1009,12 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return undefined; } - return this.getAssociationForMetadata( + const association = await this.getAssociationForMetadata( normalizePath(scope.fsPath), scope, metadata, ); + return association.kind === 'resolved' ? association.environment : undefined; } private getScriptUris(scope: SetEnvironmentScope): ScriptReference[] { @@ -1018,8 +1043,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { scriptPath: string, scriptUri: Uri, metadata: InlineScriptMetadata, - ): Promise { - const pending = this.pendingRehydrations.get(scriptPath); + retryGeneration?: number, + ): Promise { + const operationKey = this.getAssociationValidationOperationKey(retryGeneration); + const pendingForScript = this.pendingRehydrations.get(scriptPath); + const pending = pendingForScript?.get(operationKey); const cached = this.fsPathToEnv.get(scriptPath); const revision = this.associationRevisions.get(scriptPath) ?? 0; const metadataIdentity = getInlineScriptMetadataRoutingIdentity(metadata)!; @@ -1040,7 +1068,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.lastValidatedMetadataIdentities.get(scriptPath) === metadataIdentity && Date.now() - validatedAt < CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS ) { - return cached; + return { kind: 'resolved', environment: cached }; } const validation = this.validateCachedAssociation( scriptPath, @@ -1049,17 +1077,25 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, metadataIdentity, metadata, + retryGeneration, ); - this.pendingRehydrations.set(scriptPath, { + const operations = pendingForScript ?? new Map(); + operations.set(operationKey, { + origin: this.getAssociationValidationOrigin(retryGeneration), + retryGeneration, metadataIdentity, associationRevision: revision, promise: validation, }); + this.pendingRehydrations.set(scriptPath, operations); try { return await validation; } finally { - if (this.pendingRehydrations.get(scriptPath)?.promise === validation) { - this.pendingRehydrations.delete(scriptPath); + if (operations.get(operationKey)?.promise === validation) { + operations.delete(operationKey); + if (operations.size === 0 && this.pendingRehydrations.get(scriptPath) === operations) { + this.pendingRehydrations.delete(scriptPath); + } } } } @@ -1070,17 +1106,25 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, metadataIdentity, metadata, + retryGeneration, ); - this.pendingRehydrations.set(scriptPath, { + const operations = pendingForScript ?? new Map(); + operations.set(operationKey, { + origin: this.getAssociationValidationOrigin(retryGeneration), + retryGeneration, metadataIdentity, associationRevision: revision, promise: rehydration, }); + this.pendingRehydrations.set(scriptPath, operations); try { return await rehydration; } finally { - if (this.pendingRehydrations.get(scriptPath)?.promise === rehydration) { - this.pendingRehydrations.delete(scriptPath); + if (operations.get(operationKey)?.promise === rehydration) { + operations.delete(operationKey); + if (operations.size === 0 && this.pendingRehydrations.get(scriptPath) === operations) { + this.pendingRehydrations.delete(scriptPath); + } } } } @@ -1101,21 +1145,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision: number, metadataIdentity: string, metadata: InlineScriptMetadata, - ): Promise { + retryGeneration?: number, + ): Promise { const environmentPath = cached.environmentPath.fsPath; const expectedPersistedAssociation = this.fsPathToPersistedAssociation.get(scriptPath); const envDirPath = path.dirname(path.dirname(environmentPath)); const busy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (busy) { - return undefined; + return { kind: 'busy' }; } try { const stat = await fs.stat(environmentPath); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (stat.isFile()) { const resolved = await resolveVenvPythonEnvironmentPath( @@ -1125,15 +1170,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this, this.baseManager, ); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (!resolved) { - return undefined; + return { kind: 'missing' }; } const ownership = await this.inspectAssociationOwnership(resolved); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (ownership === 'stale') { await this.removeStalePersistedAssociation( @@ -1142,80 +1187,88 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, scriptUri, expectedPersistedAssociation, + retryGeneration, ); - return undefined; + return { kind: 'missing' }; } if (ownership !== 'expected') { - return undefined; + return { kind: 'missing' }; } const metadataMatch = this.inspectAssociationMetadata(scriptPath, metadataIdentity, true); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (metadataMatch === 'mismatched') { - return undefined; + return { kind: 'missing' }; } const sidecar = await this.readCurrentCacheEntrySidecar(resolved); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); + } if (sidecar && !this.cacheEntryMatchesRuntimeAndMetadata(sidecar, resolved, metadata)) { - return undefined; + return { kind: 'missing' }; } const metadataIdentityProven = !!sidecar && this.cacheEntryProvesSourceMetadataIdentity(sidecar, resolved, metadataIdentity, metadata); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } const current = this.fsPathToEnv.get(scriptPath); this.cachedAssociationValidatedAt.set(scriptPath, Date.now()); this.lastValidatedMetadataIdentities.set(scriptPath, metadataIdentity); this.lastValidatedMetadataIdentityProofs.set(scriptPath, metadataIdentityProven); if (current && this.isSameEnvironment(current, resolved)) { - return current; + return { kind: 'resolved', environment: current }; } if (cached.version === resolved.version) { - return cached; + return { kind: 'resolved', environment: cached }; } this.fsPathToEnv.set(scriptPath, resolved); this._onDidChangeEnvironment.fire({ uri: scriptUri, old: cached, new: resolved }); - return resolved; + return { kind: 'resolved', environment: resolved }; } const becameBusy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } - if (!becameBusy) { - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - expectedPersistedAssociation, - ); + if (becameBusy) { + return { kind: 'busy' }; } + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + expectedPersistedAssociation, + retryGeneration, + ); } catch (error) { - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (this.isDefinitivelyStalePathError(error)) { const becameBusy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } - if (!becameBusy) { - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - expectedPersistedAssociation, - ); + if (becameBusy) { + return { kind: 'busy' }; } + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + expectedPersistedAssociation, + retryGeneration, + ); } else { this.log.warn( `Unable to inspect cached inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); } } - return undefined; + return { kind: 'missing' }; } private async rehydrateAssociation( @@ -1224,20 +1277,21 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision: number, metadataIdentity: string, metadata: InlineScriptMetadata, - ): Promise { + retryGeneration?: number, + ): Promise { let persistedAssociation: PersistedAssociationRecord | undefined; try { - persistedAssociation = await this.getPersistedAssociation(scriptPath); + persistedAssociation = await this.getPersistedAssociation(scriptPath, retryGeneration); } catch (error) { this.log.warn(`Failed to read inline-script environment association: ${getErrorMessage(error)}`); - return undefined; + return { kind: 'missing' }; + } + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } const environmentPath = persistedAssociation?.environmentPath; if (!environmentPath) { - return undefined; - } - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + return { kind: 'missing' }; } if (!path.isAbsolute(environmentPath)) { await this.removeStalePersistedAssociation( @@ -1246,45 +1300,68 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, scriptUri, persistedAssociation, + retryGeneration, ); - return undefined; + return { kind: 'missing' }; } const envDirPath = path.dirname(path.dirname(environmentPath)); - if (await this.isCacheEntryBusy(envDirPath)) { - return undefined; + const busy = await this.isCacheEntryBusy(envDirPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); + } + if (busy) { + return { kind: 'busy' }; } try { const stat = await fs.stat(environmentPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); + } if (!stat.isFile()) { - if (!(await this.isCacheEntryBusy(envDirPath))) { - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - persistedAssociation, - ); + const becameBusy = await this.isCacheEntryBusy(envDirPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } - return undefined; + if (becameBusy) { + return { kind: 'busy' }; + } + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + persistedAssociation, + retryGeneration, + ); + return { kind: 'missing' }; } } catch (error) { + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); + } if (this.isDefinitivelyStalePathError(error)) { - if (!(await this.isCacheEntryBusy(envDirPath))) { - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - persistedAssociation, - ); + const becameBusy = await this.isCacheEntryBusy(envDirPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); + } + if (becameBusy) { + return { kind: 'busy' }; } + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + persistedAssociation, + retryGeneration, + ); } else { this.log.warn( `Unable to inspect persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); } - return undefined; + return { kind: 'missing' }; } let resolved: PythonEnvironment | undefined; @@ -1300,15 +1377,14 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.log.warn( `Unable to resolve persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); - return undefined; + return { kind: 'missing' }; + } + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (!resolved) { // PET/API resolution can fail transiently. Keep the association for a later retry. - return undefined; - } - - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + return { kind: 'missing' }; } let ownership: CacheEnvironmentInspection; try { @@ -1317,7 +1393,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.log.warn( `Unable to inspect persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); - return undefined; + return { kind: 'missing' }; + } + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } if (ownership === 'stale') { await this.removeStalePersistedAssociation( @@ -1326,24 +1405,28 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, scriptUri, persistedAssociation, + retryGeneration, ); - return undefined; + return { kind: 'missing' }; } if (ownership !== 'expected') { - return undefined; + return { kind: 'missing' }; } const metadataMatch = this.inspectAssociationMetadata(scriptPath, metadataIdentity, true); if (metadataMatch === 'mismatched') { - return undefined; + return { kind: 'missing' }; } const sidecar = await this.readCurrentCacheEntrySidecar(resolved); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); + } if (sidecar && !this.cacheEntryMatchesRuntimeAndMetadata(sidecar, resolved, metadata)) { - return undefined; + return { kind: 'missing' }; } const metadataIdentityProven = !!sidecar && this.cacheEntryProvesSourceMetadataIdentity(sidecar, resolved, metadataIdentity, metadata); - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { - return this.fsPathToEnv.get(scriptPath); + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + return this.getCurrentAssociationValidationResult(scriptPath); } const current = this.fsPathToEnv.get(scriptPath); @@ -1351,14 +1434,30 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.lastValidatedMetadataIdentities.set(scriptPath, metadataIdentity); this.lastValidatedMetadataIdentityProofs.set(scriptPath, metadataIdentityProven); if (current && this.isSameEnvironment(current, resolved)) { - return current; + return { kind: 'resolved', environment: current }; } - if (!this.isCurrentAssociationRevision(scriptPath, revision) || this.fsPathToEnv.has(scriptPath)) { - return this.fsPathToEnv.get(scriptPath); + if ( + !this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration) || + this.fsPathToEnv.has(scriptPath) + ) { + return this.getCurrentAssociationValidationResult(scriptPath); } this.fsPathToEnv.set(scriptPath, resolved); this._onDidChangeEnvironment.fire({ uri: scriptUri, old: undefined, new: resolved }); - return resolved; + return { kind: 'resolved', environment: resolved }; + } + + private getCurrentAssociationValidationResult(scriptPath: string): AssociationValidationResult { + const environment = this.fsPathToEnv.get(scriptPath); + return environment ? { kind: 'resolved', environment } : { kind: 'missing' }; + } + + private getAssociationValidationOperationKey(retryGeneration?: number): string { + return retryGeneration === undefined ? 'ordinary' : `retry:${retryGeneration}`; + } + + private getAssociationValidationOrigin(retryGeneration?: number): AssociationValidationOrigin { + return retryGeneration === undefined ? 'ordinary' : 'retry'; } private inspectAssociationMetadata( @@ -1403,6 +1502,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private async handleSavedMetadataChange(event: InlineScriptMetadataChangeEvent): Promise { if (event.metadata === undefined) { + this.cancelAssociationValidationRetry(normalizePath(event.uri.fsPath)); this.clearValidatedRouteableState(event.uri); return; } @@ -1419,10 +1519,19 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadata: InlineScriptMetadata, metadataIdentity: string, metadataRevision: number, + retryGeneration?: number, ): Promise { const scriptPath = normalizePath(uri.fsPath); const associationRevision = this.associationRevisions.get(scriptPath) ?? 0; - const pendingRefresh = this.pendingMetadataRefreshes.get(scriptPath); + this.cancelStaleAssociationValidationRetry( + scriptPath, + metadataIdentity, + metadataRevision, + associationRevision, + ); + const operationKey = this.getAssociationValidationOperationKey(retryGeneration); + const pendingForScript = this.pendingMetadataRefreshes.get(scriptPath); + const pendingRefresh = pendingForScript?.get(operationKey); if ( pendingRefresh && pendingRefresh.metadataIdentity === metadataIdentity && @@ -1438,18 +1547,26 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataIdentity, metadataRevision, associationRevision, + retryGeneration, ); - this.pendingMetadataRefreshes.set(scriptPath, { + const operations = pendingForScript ?? new Map(); + operations.set(operationKey, { + origin: this.getAssociationValidationOrigin(retryGeneration), + retryGeneration, metadataIdentity, metadataRevision, associationRevision, promise: refresh, }); + this.pendingMetadataRefreshes.set(scriptPath, operations); try { await refresh; } finally { - if (this.pendingMetadataRefreshes.get(scriptPath)?.promise === refresh) { - this.pendingMetadataRefreshes.delete(scriptPath); + if (operations.get(operationKey)?.promise === refresh) { + operations.delete(operationKey); + if (operations.size === 0 && this.pendingMetadataRefreshes.get(scriptPath) === operations) { + this.pendingMetadataRefreshes.delete(scriptPath); + } } } } @@ -1461,15 +1578,40 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataIdentity: string, metadataRevision: number, associationRevision: number, + retryGeneration?: number, ): Promise { - const environment = await this.getAssociationForMetadata(scriptPath, uri, metadata); - if (!this.isCurrentMetadataRefreshTask(uri, metadataIdentity, metadataRevision, scriptPath, associationRevision)) { + const association = await this.getAssociationForMetadata(scriptPath, uri, metadata, retryGeneration); + if ( + !this.isCurrentMetadataRefreshTask( + uri, + metadataIdentity, + metadataRevision, + scriptPath, + associationRevision, + retryGeneration, + ) + ) { return; } - if (!environment) { + if (association.kind === 'busy') { + this.clearValidatedRouteableState(uri); + this.scheduleAssociationValidationRetry( + scriptPath, + uri, + metadata, + metadataIdentity, + metadataRevision, + associationRevision, + retryGeneration, + ); + return; + } + this.cancelAssociationValidationRetry(scriptPath); + if (association.kind === 'missing') { this.clearValidatedRouteableState(uri); return; } + const environment = association.environment; let metadataIdentityProven = this.lastValidatedMetadataIdentityProofs.get(scriptPath); if ( this.lastValidatedMetadataIdentities.get(scriptPath) !== metadataIdentity || @@ -1487,6 +1629,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, scriptPath, associationRevision, + retryGeneration, ) ) { return; @@ -1508,8 +1651,18 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, associationRevision, uri, + retryGeneration, ); - if (!this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision)) { + if ( + !this.isCurrentMetadataRefreshTask( + uri, + metadataIdentity, + metadataRevision, + scriptPath, + associationRevision, + retryGeneration, + ) + ) { return; } if ( @@ -1531,6 +1684,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, currentAssociationRevision, uri, + retryGeneration, ); if ( !this.isCurrentMetadataRefreshTask( @@ -1539,12 +1693,19 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, scriptPath, currentAssociationRevision, + retryGeneration, ) ) { return; } } - } else if (!this.isCurrentAssociationRevision(scriptPath, associationRevision)) { + } else if ( + !this.isCurrentAssociationValidationTask( + scriptPath, + associationRevision, + retryGeneration, + ) + ) { return; } if (bindResult !== 'bound') { @@ -1566,6 +1727,114 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.routingRegistry.setValidatedAssociation(uri, true); } + private scheduleAssociationValidationRetry( + scriptPath: string, + uri: Uri, + metadata: InlineScriptMetadata, + metadataIdentity: string, + metadataRevision: number, + associationRevision: number, + retryGeneration?: number, + ): void { + if (this.disposed) { + return; + } + this.cancelStaleAssociationValidationRetry( + scriptPath, + metadataIdentity, + metadataRevision, + associationRevision, + ); + const existingRetry = this.associationValidationRetries.get(scriptPath); + const retry = + existingRetry ?? + { + uri, + metadata, + metadataIdentity, + metadataRevision, + associationRevision, + retryGeneration: retryGeneration ?? this.associationValidationRetryGeneration, + attempt: 0, + }; + if (!existingRetry) { + this.associationValidationRetries.set(scriptPath, retry); + } + if (retry.timer) { + return; + } + + const delayMs = this.getAssociationValidationRetryDelayMs(retry.attempt); + if (delayMs === undefined) { + return; + } + retry.attempt += 1; + retry.timer = setTimeout(() => { + if (this.associationValidationRetries.get(scriptPath) !== retry) { + return; + } + retry.timer = undefined; + if ( + this.disposed || + !this.isCurrentMetadataRefreshTask( + retry.uri, + retry.metadataIdentity, + retry.metadataRevision, + scriptPath, + retry.associationRevision, + retry.retryGeneration, + ) + ) { + this.cancelAssociationValidationRetry(scriptPath); + return; + } + void this.refreshValidatedAssociationForMetadata( + retry.uri, + retry.metadata, + retry.metadataIdentity, + retry.metadataRevision, + retry.retryGeneration, + ).catch((error) => { + this.log.warn(`Failed to retry inline-script association validation: ${getErrorMessage(error)}`); + }); + }, delayMs); + } + + private getAssociationValidationRetryDelayMs(attempt: number): number | undefined { + return DISCOVERY_RETRY_DELAYS_MS[attempt]; + } + + private cancelStaleAssociationValidationRetry( + scriptPath: string, + metadataIdentity: string, + metadataRevision: number, + associationRevision: number, + ): void { + const retry = this.associationValidationRetries.get(scriptPath); + if ( + retry && + (retry.metadataIdentity !== metadataIdentity || + retry.metadataRevision !== metadataRevision || + retry.associationRevision !== associationRevision) + ) { + this.cancelAssociationValidationRetry(scriptPath); + } + } + + private cancelAssociationValidationRetry(scriptPath: string): void { + const retry = this.associationValidationRetries.get(scriptPath); + if (retry?.timer) { + clearTimeout(retry.timer); + } + this.associationValidationRetries.delete(scriptPath); + } + + private cancelAllAssociationValidationRetries(): void { + for (const scriptPath of this.associationValidationRetries.keys()) { + this.cancelAssociationValidationRetry(scriptPath); + } + } + private async updateValidatedStateForSelection(script: ScriptReference): Promise { const savedMetadata = await this.getSavedMetadataForPersistence(script.uri); if (!savedMetadata.identity) { @@ -1737,10 +2006,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision: number, associationRevision: number, uri: Uri, + retryGeneration?: number, ): Promise<'bound' | 'stale' | 'failed'> { return this.enqueueSelection(async () => { if ( - !this.isCurrentAssociationRevision(scriptPath, associationRevision) || + !this.isCurrentAssociationValidationTask(scriptPath, associationRevision, retryGeneration) || !this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) ) { return 'stale'; @@ -1763,13 +2033,13 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { persistedAssociation: matchedAssociation, expectedPersistedAssociation: expectedAssociation, }, - ]); + ], retryGeneration); } catch (error) { this.log.warn(`Failed to bind inline-script metadata identity: ${getErrorMessage(error)}`); return 'failed'; } if ( - !this.isCurrentAssociationRevision(scriptPath, associationRevision) || + !this.isCurrentAssociationValidationTask(scriptPath, associationRevision, retryGeneration) || !this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) ) { return 'stale'; @@ -1786,10 +2056,30 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision: number, scriptPath: string, associationRevision: number, + retryGeneration?: number, ): boolean { return ( + !this.disposed && this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) && - this.isCurrentAssociationRevision(scriptPath, associationRevision) + this.isCurrentAssociationValidationTask(scriptPath, associationRevision, retryGeneration) + ); + } + + private isCurrentAssociationValidationTask( + scriptPath: string, + associationRevision: number, + retryGeneration?: number, + ): boolean { + return ( + this.isCurrentAssociationRevision(scriptPath, associationRevision) && + this.isCurrentAssociationValidationRetry(retryGeneration) + ); + } + + private isCurrentAssociationValidationRetry(retryGeneration?: number): boolean { + return ( + retryGeneration === undefined || + (!this.disposed && retryGeneration === this.associationValidationRetryGeneration) ); } @@ -1839,22 +2129,37 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } - private async getPersistedAssociation(scriptPath: string): Promise { + private async getPersistedAssociation( + scriptPath: string, + retryGeneration?: number, + ): Promise { await this.persistenceQueue; + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return this.getPersistedAssociationFromMemory(scriptPath); + } const state = await getWorkspacePersistentState(); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return this.getPersistedAssociationFromMemory(scriptPath); + } const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return this.getPersistedAssociationFromMemory(scriptPath); + } if (rawAssociations === undefined) { this.applyPersistedAssociations({}); return undefined; } const parsed = this.parsePersistedAssociations(rawAssociations); if (!parsed) { - await this.removeInvalidPersistedAssociation(scriptPath); + await this.removeInvalidPersistedAssociation(scriptPath, retryGeneration); return this.getPersistedAssociationFromMemory(scriptPath); } const rawValue = (rawAssociations as Record)[scriptPath]; if (rawValue !== undefined && this.parsePersistedAssociationValue(rawValue).kind === 'invalid') { - await this.removeInvalidPersistedAssociation(scriptPath); + await this.removeInvalidPersistedAssociation(scriptPath, retryGeneration); + return this.getPersistedAssociationFromMemory(scriptPath); + } + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { return this.getPersistedAssociationFromMemory(scriptPath); } this.applyPersistedAssociations(parsed.records); @@ -1867,9 +2172,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision: number, scriptUri?: Uri, expectedPersistedAssociation?: PersistedAssociationRecord, + retryGeneration?: number, ): Promise { await this.enqueueSelection(async () => { - if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { return; } try { @@ -1880,11 +2186,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { expectedEnvironmentPath, expectedPersistedAssociation, }, - ]); + ], retryGeneration); if ( normalizePath(persistedPathBeforeUpdate ?? '') === normalizePath(expectedEnvironmentPath) && !this.fsPathToPersistedAssociation.has(scriptPath) && - this.isCurrentAssociationRevision(scriptPath, revision) + this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration) ) { const old = this.fsPathToEnv.get(scriptPath); this.bumpAssociationRevision(scriptPath); @@ -1903,16 +2209,28 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } - private removeInvalidPersistedAssociation(scriptPath: string): Promise { + private removeInvalidPersistedAssociation(scriptPath: string, retryGeneration?: number): Promise { return this.enqueuePersistence(async (state) => { + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } if (rawAssociations === undefined) { this.applyPersistedAssociations({}); return; } const parsed = this.parsePersistedAssociations(rawAssociations); if (!parsed) { + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } await state.set(INLINE_SCRIPT_ENVS_KEY, {}); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } this.applyPersistedAssociations({}); return; } @@ -1920,15 +2238,30 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { delete parsed.rawEntries[scriptPath]; delete parsed.records[scriptPath]; parsed.invalidKeys.delete(scriptPath); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } await state.set(INLINE_SCRIPT_ENVS_KEY, parsed.rawEntries); } + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } this.applyPersistedAssociations(parsed.records); }); } - private updatePersistedAssociations(changes: readonly PersistedAssociationChange[]): Promise { + private updatePersistedAssociations( + changes: readonly PersistedAssociationChange[], + retryGeneration?: number, + ): Promise { return this.enqueuePersistence(async (state) => { + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } const parsed = this.parsePersistedAssociations(rawAssociations); const rawEntries = { ...(parsed?.rawEntries ?? {}) }; const associations = { ...(parsed?.records ?? {}) }; @@ -1955,7 +2288,13 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { delete rawEntries[change.scriptPath]; } } + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } await state.set(INLINE_SCRIPT_ENVS_KEY, rawEntries); + if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + return; + } this.applyPersistedAssociations(associations); }); } @@ -2211,6 +2550,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private bumpAssociationRevision(scriptPath: string): void { + this.cancelAssociationValidationRetry(scriptPath); this.associationRevisions.set(scriptPath, (this.associationRevisions.get(scriptPath) ?? 0) + 1); } @@ -3306,8 +3646,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } dispose(): void { + this.associationValidationRetryGeneration += 1; this.disposed = true; this.stopActivationDiscovery(); + this.cancelAllAssociationValidationRetries(); this.pendingMetadataRefreshes.clear(); this.subscriptions.forEach((subscription) => subscription.dispose()); this._onDidChangeEnvironments.dispose(); @@ -3318,6 +3660,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { const nextPaths = new Set(Object.keys(associations)); for (const scriptPath of this.fsPathToPersistedAssociation.keys()) { if (!nextPaths.has(scriptPath)) { + this.cancelAssociationValidationRetry(scriptPath); this.fsPathToPersistedAssociation.delete(scriptPath); this.clearValidatedRouteableState(scriptPath); } diff --git a/src/test/common/inlineScript/cacheLayout.unit.test.ts b/src/test/common/inlineScript/cacheLayout.unit.test.ts index 57c93084..53815837 100644 --- a/src/test/common/inlineScript/cacheLayout.unit.test.ts +++ b/src/test/common/inlineScript/cacheLayout.unit.test.ts @@ -330,18 +330,29 @@ suite('inlineScriptCacheLayout', () => { assert.strictEqual(await fs.pathExists(getMetaJsonPath(envDir).fsPath), false); }); - test('rejects temp, malformed, unsupported, and oversized artifacts without restoring them', async () => { + test('rejects temp, malformed, and oversized artifacts without restoring them', async () => { const finalPath = getMetaJsonPath(envDir).fsPath; await fs.writeFile(`${finalPath}.tmp-abcdef123456`, JSON.stringify(makeMeta())); await fs.writeFile(`${finalPath}.backup-ABCDEF123456`, JSON.stringify(makeMeta())); await writeBackup('111111111111', 'not json'); - await writeBackup('222222222222', JSON.stringify({ ...makeMeta(), schemaVersion: 99 })); await writeBackup('333333333333', Buffer.alloc(1024 * 1024 + 1, 0x20)); assert.deepStrictEqual(await restoreMetaJsonBackupUnderLock(envDir), { kind: 'missing' }); assert.strictEqual(await fs.pathExists(finalPath), false); }); + test('preserves a future-schema backup as unsupported when the primary is absent', async () => { + const finalPath = getMetaJsonPath(envDir).fsPath; + const backup = backupPath('222222222222'); + await writeBackup('111111111111', 'not json'); + await writeBackup('222222222222', JSON.stringify({ ...makeMeta(), schemaVersion: 99 })); + await writeBackup('333333333333', Buffer.alloc(1024 * 1024 + 1, 0x20)); + + assert.deepStrictEqual(await restoreMetaJsonBackupUnderLock(envDir), { kind: 'unsupported' }); + assert.strictEqual(await fs.pathExists(finalPath), false); + assert.strictEqual(await fs.pathExists(backup), true); + }); + test('rejects a symlink backup without restoring it', async function () { const externalPath = path.join(tmpDir, 'external-meta.json'); await fs.writeFile(externalPath, JSON.stringify(makeMeta())); diff --git a/src/test/features/envManagers.lastKnown.unit.test.ts b/src/test/features/envManagers.lastKnown.unit.test.ts index 38f416c8..e6e64d43 100644 --- a/src/test/features/envManagers.lastKnown.unit.test.ts +++ b/src/test/features/envManagers.lastKnown.unit.test.ts @@ -299,6 +299,95 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => { assert.deepStrictEqual(events.map((event) => event.new), [second]); }); + test('does not let an older non-inline selection install an override after a newer inline selection', async () => { + const script = Uri.file('/workspace/project/script.py'); + projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') }); + let releaseOlderSelection: (() => void) | undefined; + let signalOlderSelection: (() => void) | undefined; + const olderSelectionStarted = new Promise((resolve) => { + signalOlderSelection = resolve; + }); + const olderSelectionGate = new Promise((resolve) => { + releaseOlderSelection = resolve; + }); + const selectedSet = sinon.stub().callsFake(async () => { + signalOlderSelection!(); + await olderSelectionGate; + }); + let selectedEnvironment: PythonEnvironment; + const selectedId = registerManager(async () => selectedEnvironment, selectedSet, 'venv'); + let inlineEnvironment: PythonEnvironment; + const inlineId = registerManager(async () => inlineEnvironment, async () => undefined, 'inline-script'); + selectedEnvironment = { + ...makeEnv('selected'), + envId: { id: 'selected', managerId: selectedId }, + }; + inlineEnvironment = { + ...makeEnv('inline'), + envId: { id: 'inline', managerId: inlineId }, + }; + defaultManagerId = selectedId; + markInlineScript(script); + const events: DidChangeEnvironmentEventArgs[] = []; + envManagers.onDidChangeActiveEnvironment((event) => events.push(event)); + + const olderSelection = envManagers.setEnvironment(script, selectedEnvironment, false); + await olderSelectionStarted; + await envManagers.setEnvironment(script, inlineEnvironment, false); + const eventsAfterNewerSelection = [...events]; + releaseOlderSelection!(); + await olderSelection; + + assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, inlineId); + assert.strictEqual(envManagers.getLastKnownEnvironment(script), inlineEnvironment); + assert.deepStrictEqual(events, eventsAfterNewerSelection); + assert.ok(events.every((event) => event.new !== selectedEnvironment)); + }); + + test('does not let an older batch inline selection clear a newer non-inline override', async () => { + const script = Uri.file('/workspace/project/script.py'); + projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') }); + let releaseOlderBatch: (() => void) | undefined; + let signalOlderBatch: (() => void) | undefined; + const olderBatchStarted = new Promise((resolve) => { + signalOlderBatch = resolve; + }); + const olderBatchGate = new Promise((resolve) => { + releaseOlderBatch = resolve; + }); + let selectedEnvironment: PythonEnvironment; + const selectedId = registerManager(async () => selectedEnvironment, async () => undefined, 'venv'); + const inlineSet = sinon.stub().callsFake(async () => { + signalOlderBatch!(); + await olderBatchGate; + }); + let inlineEnvironment: PythonEnvironment; + const inlineId = registerManager(async () => inlineEnvironment, inlineSet, 'inline-script'); + selectedEnvironment = { + ...makeEnv('selected'), + envId: { id: 'selected', managerId: selectedId }, + }; + inlineEnvironment = { + ...makeEnv('inline'), + envId: { id: 'inline', managerId: inlineId }, + }; + defaultManagerId = selectedId; + markInlineScript(script); + const events: DidChangeEnvironmentEventArgs[] = []; + envManagers.onDidChangeActiveEnvironment((event) => events.push(event)); + + const olderBatch = envManagers.setEnvironments([script], inlineEnvironment, false); + await olderBatchStarted; + await envManagers.setEnvironment(script, selectedEnvironment, false); + const eventsAfterNewerSelection = [...events]; + releaseOlderBatch!(); + await olderBatch; + + assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, selectedId); + assert.strictEqual(envManagers.getLastKnownEnvironment(script), selectedEnvironment); + assert.deepStrictEqual(events, eventsAfterNewerSelection); + }); + test('publishes inline environments with the same ID at different paths', async () => { const scope = Uri.file('/workspace/script.py'); const managerId = registerManager(async () => undefined, async () => undefined, 'inline-script'); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index c95bfc78..bbe8fe7c 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -1293,6 +1293,23 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(createWithProgressStub.callCount, 0); }); + test('preserves a restart cache entry whose only usable backup has a future schema', async () => { + const directory = envDir(); + const markerPath = path.join(directory.fsPath, 'keep.txt'); + const backupPath = `${cacheLayout.getMetaJsonPath(directory).fsPath}.backup-abcdef123456`; + await fs.outputFile(markerPath, 'keep'); + await fs.writeFile( + backupPath, + JSON.stringify({ ...(await makeSidecar()), schemaVersion: cacheLayout.META_SCHEMA_VERSION + 1 }), + ); + inspectMetaStub.restore(); + + assert.strictEqual(await manager.create(scriptUri()), undefined); + assert.strictEqual(await fs.readFile(markerPath, 'utf8'), 'keep'); + assert.strictEqual(await fs.pathExists(backupPath), true); + assert.strictEqual(createWithProgressStub.callCount, 0); + }); + test('coalesces simultaneous same-key creation within one extension host', async () => { let continueCreation: (() => void) | undefined; let creationStarted: (() => void) | undefined; @@ -4905,6 +4922,459 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(resolveVenvStub.callCount, 0); }); + test('autonomously validates a persisted association after its cache lock is released', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; + await fs.ensureDir(lockPath); + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + }; + const retryDelayStub = sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); + await nextTurn(); + + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + + assert.strictEqual(retryDelayStub.callCount, 1, 'duplicate validation must share one retry'); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + assert.strictEqual(resolveVenvStub.callCount, 0); + + await fs.remove(lockPath); + await waitForCondition( + () => restartRoutingRegistry.hasValidatedAssociation(uri), + 'Expected lock release to be followed by autonomous association validation', + ); + + assert.strictEqual(resolveVenvStub.callCount, 1); + restarted.dispose(); + }); + + test('bounds repeated busy association validation retries', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + await fs.ensureDir(`${path.resolve(environment.sysPrefix)}.lock`); + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + }; + const retryDelayStub = sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt < 3 ? 0 : undefined)); + await nextTurn(); + + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + await waitForCondition( + () => retryDelayStub.callCount === 4, + 'Expected three bounded follow-up validation attempts', + ); + await new Promise((resolve) => setTimeout(resolve, 25)); + + assert.strictEqual(retryDelayStub.callCount, 4, 'busy validation must stop after the retry budget'); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + restarted.dispose(); + }); + + test('cancels a busy validation retry when the association revision changes', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; + await fs.ensureDir(lockPath); + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + }; + const retryDelayStub = sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); + await nextTurn(); + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + sinon.assert.calledOnce(retryDelayStub); + + await restarted.set(uri, undefined); + await fs.remove(lockPath); + await new Promise((resolve) => setTimeout(resolve, 40)); + + assert.deepStrictEqual(persistedAssociations, {}); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + restarted.dispose(); + }); + + test('dispose cancels a pending busy association validation retry', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; + await fs.ensureDir(lockPath); + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + }; + const retryDelayStub = sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); + await nextTurn(); + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + sinon.assert.calledOnce(retryDelayStub); + + restarted.dispose(); + await fs.remove(lockPath); + await new Promise((resolve) => setTimeout(resolve, 40)); + + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + }); + + test('dispose invalidates an already-fired busy association validation retry', async () => { + const uri = scriptUri(); + const scriptPath = normalizePath(uri.fsPath); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + let signalRetryBusyCheck: (() => void) | undefined; + let releaseRetryBusyCheck: ((busy: boolean) => void) | undefined; + const retryBusyCheckStarted = new Promise((resolve) => { + signalRetryBusyCheck = resolve; + }); + const retryBusyCheckGate = new Promise((resolve) => { + releaseRetryBusyCheck = resolve; + }); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + isCacheEntryBusy(envDirPath: string): Promise; + pendingMetadataRefreshes: Map< + string, + Map }> + >; + fsPathToEnv: Map; + fsPathToPersistedAssociation: Map; + _onDidChangeEnvironment: { fire(event: unknown): void }; + }; + sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt === 0 ? 0 : undefined)); + const busyCheckStub = sinon.stub(retryManager, 'isCacheEntryBusy'); + busyCheckStub.onFirstCall().resolves(true); + busyCheckStub.onSecondCall().callsFake(async () => { + signalRetryBusyCheck!(); + return retryBusyCheckGate; + }); + const environmentEventFire = sinon.spy(retryManager._onDidChangeEnvironment, 'fire'); + const routePublication = sinon.spy(restartRoutingRegistry, 'setValidatedAssociation'); + await nextTurn(); + + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + await retryBusyCheckStarted; + const inFlightRetry = Array.from( + retryManager.pendingMetadataRefreshes.get(scriptPath)?.values() ?? [], + ).find((operation) => operation.origin === 'retry')?.promise; + assert.ok(inFlightRetry, 'retry should be in flight before disposal'); + assert.strictEqual(busyCheckStub.callCount, 2); + assert.strictEqual(retryManager.fsPathToEnv.has(scriptPath), false); + workspaceState.set.resetHistory(); + environmentEventFire.resetHistory(); + routePublication.resetHistory(); + + restarted.dispose(); + releaseRetryBusyCheck!(false); + await inFlightRetry; + + assert.deepStrictEqual(persistedAssociations, { + [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), + }); + assert.strictEqual(workspaceState.set.callCount, 0, 'disposed retry must not remove persistence'); + assert.strictEqual(retryManager.fsPathToPersistedAssociation.has(scriptPath), true); + assert.strictEqual(retryManager.fsPathToEnv.has(scriptPath), false, 'disposed retry must not repopulate cache'); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(environmentEventFire.callCount, 0, 'disposed retry must not fire manager events'); + assert.strictEqual(routePublication.callCount, 0, 'disposed retry must not publish routeability'); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + }); + + test('keeps an ordinary get independent when it arrives during a fired retry', async () => { + const uri = scriptUri(); + const scriptPath = normalizePath(uri.fsPath); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + let signalRetryBusyCheck: (() => void) | undefined; + let releaseRetryBusyCheck: ((busy: boolean) => void) | undefined; + const retryBusyCheckStarted = new Promise((resolve) => { + signalRetryBusyCheck = resolve; + }); + const retryBusyCheckGate = new Promise((resolve) => { + releaseRetryBusyCheck = resolve; + }); + let releaseOrdinaryResolution: (() => void) | undefined; + const ordinaryResolutionGate = new Promise((resolve) => { + releaseOrdinaryResolution = resolve; + }); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + isCacheEntryBusy(envDirPath: string): Promise; + refreshValidatedAssociationForMetadata( + candidate: Uri, + metadata: metadataReader.InlineScriptMetadata, + metadataIdentity: string, + metadataRevision: number, + retryGeneration?: number, + ): Promise; + pendingRehydrations: Map< + string, + Map }> + >; + pendingMetadataRefreshes: Map< + string, + Map< + string, + { + readonly origin: 'ordinary' | 'retry'; + readonly retryGeneration?: number; + readonly promise: Promise; + } + > + >; + fsPathToEnv: Map; + fsPathToPersistedAssociation: Map; + _onDidChangeEnvironment: { fire(event: unknown): void }; + }; + sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt === 0 ? 0 : undefined)); + const busyCheckStub = sinon.stub(retryManager, 'isCacheEntryBusy'); + busyCheckStub.onFirstCall().resolves(true); + busyCheckStub.onSecondCall().callsFake(async () => { + signalRetryBusyCheck!(); + return retryBusyCheckGate; + }); + busyCheckStub.onThirdCall().resolves(false); + resolveVenvStub.callsFake(async () => { + await ordinaryResolutionGate; + return environment; + }); + const environmentEventFire = sinon.spy(retryManager._onDidChangeEnvironment, 'fire'); + const routePublication = sinon.spy(restartRoutingRegistry, 'setValidatedAssociation'); + await nextTurn(); + + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + await retryBusyCheckStarted; + const retryRefresh = Array.from( + retryManager.pendingMetadataRefreshes.get(scriptPath)?.values() ?? [], + ).find((operation) => operation.origin === 'retry'); + assert.ok(retryRefresh?.retryGeneration !== undefined); + const equivalentRetry = retryManager.refreshValidatedAssociationForMetadata( + uri, + VALID_METADATA, + restartRoutingRegistry.getMetadataIdentity(uri)!, + restartRoutingRegistry.getMetadataRevision(uri), + retryRefresh!.retryGeneration, + ); + await nextTurn(); + assert.strictEqual(busyCheckStub.callCount, 2, 'same-generation retries must coalesce'); + + const ordinaryGet = restarted.get(uri); + await waitForStubCall(resolveVenvStub); + assert.deepStrictEqual( + Array.from(retryManager.pendingRehydrations.get(scriptPath)?.values() ?? []) + .map((operation) => operation.origin) + .sort(), + ['ordinary', 'retry'], + ); + workspaceState.set.resetHistory(); + environmentEventFire.resetHistory(); + routePublication.resetHistory(); + + restarted.dispose(); + releaseRetryBusyCheck!(false); + releaseOrdinaryResolution!(); + const ordinaryResult = await ordinaryGet; + await Promise.all([retryRefresh!.promise, equivalentRetry]); + + assert.strictEqual(ordinaryResult, environment, 'ordinary get must survive retry disposal'); + assert.deepStrictEqual(persistedAssociations, { + [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), + }); + assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(retryManager.fsPathToPersistedAssociation.has(scriptPath), true); + assert.strictEqual(retryManager.fsPathToEnv.get(scriptPath), environment); + assert.strictEqual(environmentEventFire.callCount, 1, 'only ordinary work may publish a manager event'); + assert.strictEqual(routePublication.callCount, 0, 'disposed retry must not publish routeability'); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + }); + + test('keeps a fired retry independent from an existing ordinary validation', async () => { + const uri = scriptUri(); + const scriptPath = normalizePath(uri.fsPath); + const environment = await createOwnedEnvironment(); + persistedAssociations = { + [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), + }; + const restartRoutingRegistry = new InlineScriptRoutingRegistry(); + const restarted = new InlineScriptEnvManager( + nativeFinder, + api, + baseManager, + globalStorageUri, + makeFakeLog(), + restartRoutingRegistry, + ); + let releaseOrdinaryResolution: (() => void) | undefined; + const ordinaryResolutionGate = new Promise((resolve) => { + releaseOrdinaryResolution = resolve; + }); + let signalRetryBusyCheck: (() => void) | undefined; + let releaseRetryBusyCheck: ((busy: boolean) => void) | undefined; + const retryBusyCheckStarted = new Promise((resolve) => { + signalRetryBusyCheck = resolve; + }); + const retryBusyCheckGate = new Promise((resolve) => { + releaseRetryBusyCheck = resolve; + }); + const retryManager = restarted as unknown as { + getAssociationValidationRetryDelayMs(attempt: number): number | undefined; + isCacheEntryBusy(envDirPath: string): Promise; + pendingRehydrations: Map< + string, + Map }> + >; + pendingMetadataRefreshes: Map< + string, + Map }> + >; + fsPathToEnv: Map; + fsPathToPersistedAssociation: Map; + _onDidChangeEnvironment: { fire(event: unknown): void }; + }; + sinon + .stub(retryManager, 'getAssociationValidationRetryDelayMs') + .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); + const busyCheckStub = sinon.stub(retryManager, 'isCacheEntryBusy'); + busyCheckStub.onFirstCall().resolves(true); + busyCheckStub.onSecondCall().resolves(false); + busyCheckStub.onThirdCall().callsFake(async () => { + signalRetryBusyCheck!(); + return retryBusyCheckGate; + }); + resolveVenvStub.callsFake(async () => { + await ordinaryResolutionGate; + return environment; + }); + const environmentEventFire = sinon.spy(retryManager._onDidChangeEnvironment, 'fire'); + const routePublication = sinon.spy(restartRoutingRegistry, 'setValidatedAssociation'); + await nextTurn(); + + await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); + const ordinaryGet = restarted.get(uri); + await waitForStubCall(resolveVenvStub); + const equivalentOrdinaryGet = restarted.get(uri); + await nextTurn(); + assert.strictEqual(resolveVenvStub.callCount, 1, 'equivalent ordinary gets must coalesce'); + + await waitForStubCallCount(busyCheckStub, 3); + await retryBusyCheckStarted; + assert.deepStrictEqual( + Array.from(retryManager.pendingRehydrations.get(scriptPath)?.values() ?? []) + .map((operation) => operation.origin) + .sort(), + ['ordinary', 'retry'], + ); + const retryPromise = Array.from( + retryManager.pendingMetadataRefreshes.get(scriptPath)?.values() ?? [], + ).find((operation) => operation.origin === 'retry')?.promise; + assert.ok(retryPromise); + workspaceState.set.resetHistory(); + environmentEventFire.resetHistory(); + routePublication.resetHistory(); + + restarted.dispose(); + releaseRetryBusyCheck!(false); + releaseOrdinaryResolution!(); + const [ordinaryResult, equivalentOrdinaryResult] = await Promise.all([ + ordinaryGet, + equivalentOrdinaryGet, + ]); + await retryPromise; + + assert.strictEqual(ordinaryResult, environment); + assert.strictEqual(equivalentOrdinaryResult, environment); + assert.deepStrictEqual(persistedAssociations, { + [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), + }); + assert.strictEqual(workspaceState.set.callCount, 0); + assert.strictEqual(retryManager.fsPathToPersistedAssociation.has(scriptPath), true); + assert.strictEqual(retryManager.fsPathToEnv.get(scriptPath), environment); + assert.strictEqual(environmentEventFire.callCount, 1, 'coalesced ordinary work publishes once'); + assert.strictEqual(routePublication.callCount, 0, 'disposed retry must not publish routeability'); + assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); + }); + test('clears the routing registry when a stale persisted association is removed', async () => { const uri = scriptUri(); const environment = await createOwnedEnvironment(); @@ -5666,6 +6136,7 @@ suite('InlineScriptEnvManager', () => { baseManager, Uri.file(process.platform === 'win32' ? `${process.env.SystemDrive ?? 'C:'}\\` : '/'), makeFakeLog(), + routingRegistry, ); await assert.rejects( @@ -5684,6 +6155,7 @@ suite('InlineScriptEnvManager', () => { baseManager, symlinkStorageUri, makeFakeLog(), + routingRegistry, ); const realCacheRoot = cacheLayout.getScriptEnvCacheRoot(symlinkStorageUri).fsPath; const externalCacheRoot = path.join(tempRoot, 'external-cache-root'); @@ -5718,6 +6190,7 @@ suite('InlineScriptEnvManager', () => { baseManager, Uri.file(redirectedStoragePath), makeFakeLog(), + routingRegistry, ); try { await fs.remove(redirectedStoragePath); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index 69f6d80b..a21c2d1f 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -127,6 +127,11 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { disposables.includes(registerEnvironmentManagerStub.firstCall.returnValue), 'registration disposable should be disposed', ); + assert.strictEqual( + (manager as unknown as { routingRegistry: InlineScriptRoutingRegistry }).routingRegistry, + routingRegistry, + 'the registered manager must share the activation routing registry', + ); assert.strictEqual(typeof manager.create, 'function'); await nextTurn(); disposables.forEach((disposable) => disposable.dispose()); From f20011f737e5c8db6de22a1d3e05150a5b7ffef3 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Tue, 25 Aug 2026 15:32:32 -0700 Subject: [PATCH 2/3] Scope down inline-script routing hardening to settings ordering + backup classification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trim this PR to the minimal set of fixes for the default-off PEP 723 inline-script feature: - Batch settings persistence now obeys the selection revision-ordering guarantee. In the Uri[] branch of setEnvironments, the winning (non-superseded) selections are committed via the revision-commit guard BEFORE settings are persisted, and settings/routing/events are applied only for those winners. This fixes the settings/routing divergence where an out-of-order older batch could persist a stale manager to settings.json even though its routing override was rejected. - Future-schema backup sidecars are classified as `unsupported` (not `missing`) so a newer-schema cache is not rebuilt by an older client. Reverts the association-validation retry/generation subsystem in the inline-script env manager (AssociationValidationResult, retry generations, nested pending-op maps, conditional Memento commit guards, required routing-registry injection) — deferred to a follow-up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f881304e-3c5f-4144-9f0d-d80c68768d18 --- src/features/envManagers.ts | 16 +- .../builtin/inlineScript/envManager.ts | 593 ++++-------------- .../envManagers.lastKnown.unit.test.ts | 43 ++ .../inlineScript/envManager.unit.test.ts | 473 -------------- .../builtin/inlineScript/main.unit.test.ts | 5 - 5 files changed, 179 insertions(+), 951 deletions(-) diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index b7d76421..e0362d65 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -507,7 +507,13 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (Array.isArray(scope) && scope.every((s) => s instanceof Uri)) { const selections = scope.map((uri) => this.beginPendingSelection(uri, manager)); await manager.set(scope, environment); - selections.forEach((selection) => { + // Commit the winning (non-superseded) selections BEFORE persisting settings so an + // out-of-order older batch cannot write a manager setting to settings.json while its + // matching routing/selection update is rejected (settings/routing divergence). + const committedSelections = selections.filter((selection) => + this.commitPendingSelection(selection, manager), + ); + committedSelections.forEach((selection) => { const m = this.getEnvironmentManager(selection.scope); // Always add settings when persisting, OR when manager differs if ( @@ -524,7 +530,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (shouldPersistSettings) { await setAllManagerSettings(settings); } - selections.forEach((selection) => { + committedSelections.forEach((selection) => { + // Re-validate across the (awaited) settings write: a newer selection may have + // superseded this one while settings were being persisted, in which case its + // routing override and active-selection publish must be skipped. if (!this.commitPendingSelection(selection, manager)) { return; } @@ -533,9 +542,6 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (!selection.publishInlineSelection) { return; } - if (!this.commitSelectionOperation(selection.key, selection.operation)) { - return; - } const oldEnv = this._activeSelection.get(selection.key); if (!this.isSameEnvironment(oldEnv, environment)) { this._activeSelection.set(selection.key, environment); diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index b507c9a4..34a847c3 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -151,41 +151,19 @@ type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; -type AssociationValidationResult = - | { readonly kind: 'resolved'; readonly environment: PythonEnvironment } - | { readonly kind: 'missing' } - | { readonly kind: 'busy' }; - -type AssociationValidationOrigin = 'ordinary' | 'retry'; - interface PendingAssociationValidation { - readonly origin: AssociationValidationOrigin; - readonly retryGeneration?: number; readonly metadataIdentity: string; readonly associationRevision: number; - readonly promise: Promise; + readonly promise: Promise; } interface PendingMetadataRefresh { - readonly origin: AssociationValidationOrigin; - readonly retryGeneration?: number; readonly metadataIdentity: string; readonly metadataRevision: number; readonly associationRevision: number; readonly promise: Promise; } -interface AssociationValidationRetry { - readonly uri: Uri; - readonly metadata: InlineScriptMetadata; - readonly metadataIdentity: string; - readonly metadataRevision: number; - readonly associationRevision: number; - readonly retryGeneration: number; - attempt: number; - timer?: ReturnType; -} - interface ParsedPersistedAssociations { readonly rawEntries: Record; readonly records: PersistedInlineScriptEnvironments; @@ -204,9 +182,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly directlyResolvedBaseInterpreters = new Map(); private baseInterpreterInstallationQueue: Promise = Promise.resolve(); private collection: PythonEnvironment[] = []; - private readonly pendingRehydrations = new Map>(); - private readonly pendingMetadataRefreshes = new Map>(); - private readonly associationValidationRetries = new Map(); + private readonly pendingRehydrations = new Map(); + private readonly pendingMetadataRefreshes = new Map(); private readonly fsPathToEnv = new Map(); private readonly fsPathToPersistedAssociation = new Map(); private readonly cachedAssociationValidatedAt = new Map(); @@ -226,7 +203,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private cacheMaintenanceBarrier: Deferred | undefined; private pendingCacheMaintenances = 0; private activeCreateOperations = 0; - private associationValidationRetryGeneration = 0; private disposed = false; private readonly _onDidChangeEnvironments = new EventEmitter(); @@ -252,7 +228,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly baseManager: EnvironmentManager, private readonly globalStorageUri: Uri, public readonly log: LogOutputChannel, - private readonly routingRegistry: InlineScriptRoutingRegistry, + private readonly routingRegistry: InlineScriptRoutingRegistry = new InlineScriptRoutingRegistry(), ) { this.subscriptions.push( this.routingRegistry.onDidChangeMetadata((event) => { @@ -1009,12 +985,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return undefined; } - const association = await this.getAssociationForMetadata( + return this.getAssociationForMetadata( normalizePath(scope.fsPath), scope, metadata, ); - return association.kind === 'resolved' ? association.environment : undefined; } private getScriptUris(scope: SetEnvironmentScope): ScriptReference[] { @@ -1043,11 +1018,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { scriptPath: string, scriptUri: Uri, metadata: InlineScriptMetadata, - retryGeneration?: number, - ): Promise { - const operationKey = this.getAssociationValidationOperationKey(retryGeneration); - const pendingForScript = this.pendingRehydrations.get(scriptPath); - const pending = pendingForScript?.get(operationKey); + ): Promise { + const pending = this.pendingRehydrations.get(scriptPath); const cached = this.fsPathToEnv.get(scriptPath); const revision = this.associationRevisions.get(scriptPath) ?? 0; const metadataIdentity = getInlineScriptMetadataRoutingIdentity(metadata)!; @@ -1068,7 +1040,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.lastValidatedMetadataIdentities.get(scriptPath) === metadataIdentity && Date.now() - validatedAt < CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS ) { - return { kind: 'resolved', environment: cached }; + return cached; } const validation = this.validateCachedAssociation( scriptPath, @@ -1077,25 +1049,17 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, metadataIdentity, metadata, - retryGeneration, ); - const operations = pendingForScript ?? new Map(); - operations.set(operationKey, { - origin: this.getAssociationValidationOrigin(retryGeneration), - retryGeneration, + this.pendingRehydrations.set(scriptPath, { metadataIdentity, associationRevision: revision, promise: validation, }); - this.pendingRehydrations.set(scriptPath, operations); try { return await validation; } finally { - if (operations.get(operationKey)?.promise === validation) { - operations.delete(operationKey); - if (operations.size === 0 && this.pendingRehydrations.get(scriptPath) === operations) { - this.pendingRehydrations.delete(scriptPath); - } + if (this.pendingRehydrations.get(scriptPath)?.promise === validation) { + this.pendingRehydrations.delete(scriptPath); } } } @@ -1106,25 +1070,17 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, metadataIdentity, metadata, - retryGeneration, ); - const operations = pendingForScript ?? new Map(); - operations.set(operationKey, { - origin: this.getAssociationValidationOrigin(retryGeneration), - retryGeneration, + this.pendingRehydrations.set(scriptPath, { metadataIdentity, associationRevision: revision, promise: rehydration, }); - this.pendingRehydrations.set(scriptPath, operations); try { return await rehydration; } finally { - if (operations.get(operationKey)?.promise === rehydration) { - operations.delete(operationKey); - if (operations.size === 0 && this.pendingRehydrations.get(scriptPath) === operations) { - this.pendingRehydrations.delete(scriptPath); - } + if (this.pendingRehydrations.get(scriptPath)?.promise === rehydration) { + this.pendingRehydrations.delete(scriptPath); } } } @@ -1145,22 +1101,21 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision: number, metadataIdentity: string, metadata: InlineScriptMetadata, - retryGeneration?: number, - ): Promise { + ): Promise { const environmentPath = cached.environmentPath.fsPath; const expectedPersistedAssociation = this.fsPathToPersistedAssociation.get(scriptPath); const envDirPath = path.dirname(path.dirname(environmentPath)); const busy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } if (busy) { - return { kind: 'busy' }; + return undefined; } try { const stat = await fs.stat(environmentPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } if (stat.isFile()) { const resolved = await resolveVenvPythonEnvironmentPath( @@ -1170,15 +1125,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this, this.baseManager, ); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } if (!resolved) { - return { kind: 'missing' }; + return undefined; } const ownership = await this.inspectAssociationOwnership(resolved); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } if (ownership === 'stale') { await this.removeStalePersistedAssociation( @@ -1187,88 +1142,80 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, scriptUri, expectedPersistedAssociation, - retryGeneration, ); - return { kind: 'missing' }; + return undefined; } if (ownership !== 'expected') { - return { kind: 'missing' }; + return undefined; } const metadataMatch = this.inspectAssociationMetadata(scriptPath, metadataIdentity, true); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } if (metadataMatch === 'mismatched') { - return { kind: 'missing' }; + return undefined; } const sidecar = await this.readCurrentCacheEntrySidecar(resolved); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } if (sidecar && !this.cacheEntryMatchesRuntimeAndMetadata(sidecar, resolved, metadata)) { - return { kind: 'missing' }; + return undefined; } const metadataIdentityProven = !!sidecar && this.cacheEntryProvesSourceMetadataIdentity(sidecar, resolved, metadataIdentity, metadata); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } const current = this.fsPathToEnv.get(scriptPath); this.cachedAssociationValidatedAt.set(scriptPath, Date.now()); this.lastValidatedMetadataIdentities.set(scriptPath, metadataIdentity); this.lastValidatedMetadataIdentityProofs.set(scriptPath, metadataIdentityProven); if (current && this.isSameEnvironment(current, resolved)) { - return { kind: 'resolved', environment: current }; + return current; } if (cached.version === resolved.version) { - return { kind: 'resolved', environment: cached }; + return cached; } this.fsPathToEnv.set(scriptPath, resolved); this._onDidChangeEnvironment.fire({ uri: scriptUri, old: cached, new: resolved }); - return { kind: 'resolved', environment: resolved }; + return resolved; } const becameBusy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } - if (becameBusy) { - return { kind: 'busy' }; - } - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - expectedPersistedAssociation, - retryGeneration, - ); - } catch (error) { - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } - if (this.isDefinitivelyStalePathError(error)) { - const becameBusy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } - if (becameBusy) { - return { kind: 'busy' }; - } + if (!becameBusy) { await this.removeStalePersistedAssociation( scriptPath, environmentPath, revision, scriptUri, expectedPersistedAssociation, - retryGeneration, ); + } + } catch (error) { + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); + } + if (this.isDefinitivelyStalePathError(error)) { + const becameBusy = await this.isCacheEntryBusy(envDirPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); + } + if (!becameBusy) { + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + expectedPersistedAssociation, + ); + } } else { this.log.warn( `Unable to inspect cached inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); } } - return { kind: 'missing' }; + return undefined; } private async rehydrateAssociation( @@ -1277,21 +1224,20 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision: number, metadataIdentity: string, metadata: InlineScriptMetadata, - retryGeneration?: number, - ): Promise { + ): Promise { let persistedAssociation: PersistedAssociationRecord | undefined; try { - persistedAssociation = await this.getPersistedAssociation(scriptPath, retryGeneration); + persistedAssociation = await this.getPersistedAssociation(scriptPath); } catch (error) { this.log.warn(`Failed to read inline-script environment association: ${getErrorMessage(error)}`); - return { kind: 'missing' }; - } - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + return undefined; } const environmentPath = persistedAssociation?.environmentPath; if (!environmentPath) { - return { kind: 'missing' }; + return undefined; + } + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } if (!path.isAbsolute(environmentPath)) { await this.removeStalePersistedAssociation( @@ -1300,68 +1246,45 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, scriptUri, persistedAssociation, - retryGeneration, ); - return { kind: 'missing' }; + return undefined; } const envDirPath = path.dirname(path.dirname(environmentPath)); - const busy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } - if (busy) { - return { kind: 'busy' }; + if (await this.isCacheEntryBusy(envDirPath)) { + return undefined; } try { const stat = await fs.stat(environmentPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } if (!stat.isFile()) { - const becameBusy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } - if (becameBusy) { - return { kind: 'busy' }; + if (!(await this.isCacheEntryBusy(envDirPath))) { + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + persistedAssociation, + ); } - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - persistedAssociation, - retryGeneration, - ); - return { kind: 'missing' }; + return undefined; } } catch (error) { - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } if (this.isDefinitivelyStalePathError(error)) { - const becameBusy = await this.isCacheEntryBusy(envDirPath); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } - if (becameBusy) { - return { kind: 'busy' }; + if (!(await this.isCacheEntryBusy(envDirPath))) { + await this.removeStalePersistedAssociation( + scriptPath, + environmentPath, + revision, + scriptUri, + persistedAssociation, + ); } - await this.removeStalePersistedAssociation( - scriptPath, - environmentPath, - revision, - scriptUri, - persistedAssociation, - retryGeneration, - ); } else { this.log.warn( `Unable to inspect persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); } - return { kind: 'missing' }; + return undefined; } let resolved: PythonEnvironment | undefined; @@ -1377,14 +1300,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.log.warn( `Unable to resolve persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); - return { kind: 'missing' }; - } - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + return undefined; } if (!resolved) { // PET/API resolution can fail transiently. Keep the association for a later retry. - return { kind: 'missing' }; + return undefined; + } + + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } let ownership: CacheEnvironmentInspection; try { @@ -1393,10 +1317,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.log.warn( `Unable to inspect persisted inline-script environment ${environmentPath}: ${getErrorMessage(error)}`, ); - return { kind: 'missing' }; - } - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + return undefined; } if (ownership === 'stale') { await this.removeStalePersistedAssociation( @@ -1405,28 +1326,24 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision, scriptUri, persistedAssociation, - retryGeneration, ); - return { kind: 'missing' }; + return undefined; } if (ownership !== 'expected') { - return { kind: 'missing' }; + return undefined; } const metadataMatch = this.inspectAssociationMetadata(scriptPath, metadataIdentity, true); if (metadataMatch === 'mismatched') { - return { kind: 'missing' }; + return undefined; } const sidecar = await this.readCurrentCacheEntrySidecar(resolved); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); - } if (sidecar && !this.cacheEntryMatchesRuntimeAndMetadata(sidecar, resolved, metadata)) { - return { kind: 'missing' }; + return undefined; } const metadataIdentityProven = !!sidecar && this.cacheEntryProvesSourceMetadataIdentity(sidecar, resolved, metadataIdentity, metadata); - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { + return this.fsPathToEnv.get(scriptPath); } const current = this.fsPathToEnv.get(scriptPath); @@ -1434,30 +1351,14 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.lastValidatedMetadataIdentities.set(scriptPath, metadataIdentity); this.lastValidatedMetadataIdentityProofs.set(scriptPath, metadataIdentityProven); if (current && this.isSameEnvironment(current, resolved)) { - return { kind: 'resolved', environment: current }; + return current; } - if ( - !this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration) || - this.fsPathToEnv.has(scriptPath) - ) { - return this.getCurrentAssociationValidationResult(scriptPath); + if (!this.isCurrentAssociationRevision(scriptPath, revision) || this.fsPathToEnv.has(scriptPath)) { + return this.fsPathToEnv.get(scriptPath); } this.fsPathToEnv.set(scriptPath, resolved); this._onDidChangeEnvironment.fire({ uri: scriptUri, old: undefined, new: resolved }); - return { kind: 'resolved', environment: resolved }; - } - - private getCurrentAssociationValidationResult(scriptPath: string): AssociationValidationResult { - const environment = this.fsPathToEnv.get(scriptPath); - return environment ? { kind: 'resolved', environment } : { kind: 'missing' }; - } - - private getAssociationValidationOperationKey(retryGeneration?: number): string { - return retryGeneration === undefined ? 'ordinary' : `retry:${retryGeneration}`; - } - - private getAssociationValidationOrigin(retryGeneration?: number): AssociationValidationOrigin { - return retryGeneration === undefined ? 'ordinary' : 'retry'; + return resolved; } private inspectAssociationMetadata( @@ -1502,7 +1403,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private async handleSavedMetadataChange(event: InlineScriptMetadataChangeEvent): Promise { if (event.metadata === undefined) { - this.cancelAssociationValidationRetry(normalizePath(event.uri.fsPath)); this.clearValidatedRouteableState(event.uri); return; } @@ -1519,19 +1419,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadata: InlineScriptMetadata, metadataIdentity: string, metadataRevision: number, - retryGeneration?: number, ): Promise { const scriptPath = normalizePath(uri.fsPath); const associationRevision = this.associationRevisions.get(scriptPath) ?? 0; - this.cancelStaleAssociationValidationRetry( - scriptPath, - metadataIdentity, - metadataRevision, - associationRevision, - ); - const operationKey = this.getAssociationValidationOperationKey(retryGeneration); - const pendingForScript = this.pendingMetadataRefreshes.get(scriptPath); - const pendingRefresh = pendingForScript?.get(operationKey); + const pendingRefresh = this.pendingMetadataRefreshes.get(scriptPath); if ( pendingRefresh && pendingRefresh.metadataIdentity === metadataIdentity && @@ -1547,26 +1438,18 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataIdentity, metadataRevision, associationRevision, - retryGeneration, ); - const operations = pendingForScript ?? new Map(); - operations.set(operationKey, { - origin: this.getAssociationValidationOrigin(retryGeneration), - retryGeneration, + this.pendingMetadataRefreshes.set(scriptPath, { metadataIdentity, metadataRevision, associationRevision, promise: refresh, }); - this.pendingMetadataRefreshes.set(scriptPath, operations); try { await refresh; } finally { - if (operations.get(operationKey)?.promise === refresh) { - operations.delete(operationKey); - if (operations.size === 0 && this.pendingMetadataRefreshes.get(scriptPath) === operations) { - this.pendingMetadataRefreshes.delete(scriptPath); - } + if (this.pendingMetadataRefreshes.get(scriptPath)?.promise === refresh) { + this.pendingMetadataRefreshes.delete(scriptPath); } } } @@ -1578,40 +1461,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataIdentity: string, metadataRevision: number, associationRevision: number, - retryGeneration?: number, ): Promise { - const association = await this.getAssociationForMetadata(scriptPath, uri, metadata, retryGeneration); - if ( - !this.isCurrentMetadataRefreshTask( - uri, - metadataIdentity, - metadataRevision, - scriptPath, - associationRevision, - retryGeneration, - ) - ) { + const environment = await this.getAssociationForMetadata(scriptPath, uri, metadata); + if (!this.isCurrentMetadataRefreshTask(uri, metadataIdentity, metadataRevision, scriptPath, associationRevision)) { return; } - if (association.kind === 'busy') { - this.clearValidatedRouteableState(uri); - this.scheduleAssociationValidationRetry( - scriptPath, - uri, - metadata, - metadataIdentity, - metadataRevision, - associationRevision, - retryGeneration, - ); - return; - } - this.cancelAssociationValidationRetry(scriptPath); - if (association.kind === 'missing') { + if (!environment) { this.clearValidatedRouteableState(uri); return; } - const environment = association.environment; let metadataIdentityProven = this.lastValidatedMetadataIdentityProofs.get(scriptPath); if ( this.lastValidatedMetadataIdentities.get(scriptPath) !== metadataIdentity || @@ -1629,7 +1487,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, scriptPath, associationRevision, - retryGeneration, ) ) { return; @@ -1651,18 +1508,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, associationRevision, uri, - retryGeneration, ); - if ( - !this.isCurrentMetadataRefreshTask( - uri, - metadataIdentity, - metadataRevision, - scriptPath, - associationRevision, - retryGeneration, - ) - ) { + if (!this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision)) { return; } if ( @@ -1684,7 +1531,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, currentAssociationRevision, uri, - retryGeneration, ); if ( !this.isCurrentMetadataRefreshTask( @@ -1693,19 +1539,12 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision, scriptPath, currentAssociationRevision, - retryGeneration, ) ) { return; } } - } else if ( - !this.isCurrentAssociationValidationTask( - scriptPath, - associationRevision, - retryGeneration, - ) - ) { + } else if (!this.isCurrentAssociationRevision(scriptPath, associationRevision)) { return; } if (bindResult !== 'bound') { @@ -1727,114 +1566,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.routingRegistry.setValidatedAssociation(uri, true); } - private scheduleAssociationValidationRetry( - scriptPath: string, - uri: Uri, - metadata: InlineScriptMetadata, - metadataIdentity: string, - metadataRevision: number, - associationRevision: number, - retryGeneration?: number, - ): void { - if (this.disposed) { - return; - } - this.cancelStaleAssociationValidationRetry( - scriptPath, - metadataIdentity, - metadataRevision, - associationRevision, - ); - const existingRetry = this.associationValidationRetries.get(scriptPath); - const retry = - existingRetry ?? - { - uri, - metadata, - metadataIdentity, - metadataRevision, - associationRevision, - retryGeneration: retryGeneration ?? this.associationValidationRetryGeneration, - attempt: 0, - }; - if (!existingRetry) { - this.associationValidationRetries.set(scriptPath, retry); - } - if (retry.timer) { - return; - } - - const delayMs = this.getAssociationValidationRetryDelayMs(retry.attempt); - if (delayMs === undefined) { - return; - } - retry.attempt += 1; - retry.timer = setTimeout(() => { - if (this.associationValidationRetries.get(scriptPath) !== retry) { - return; - } - retry.timer = undefined; - if ( - this.disposed || - !this.isCurrentMetadataRefreshTask( - retry.uri, - retry.metadataIdentity, - retry.metadataRevision, - scriptPath, - retry.associationRevision, - retry.retryGeneration, - ) - ) { - this.cancelAssociationValidationRetry(scriptPath); - return; - } - void this.refreshValidatedAssociationForMetadata( - retry.uri, - retry.metadata, - retry.metadataIdentity, - retry.metadataRevision, - retry.retryGeneration, - ).catch((error) => { - this.log.warn(`Failed to retry inline-script association validation: ${getErrorMessage(error)}`); - }); - }, delayMs); - } - - private getAssociationValidationRetryDelayMs(attempt: number): number | undefined { - return DISCOVERY_RETRY_DELAYS_MS[attempt]; - } - - private cancelStaleAssociationValidationRetry( - scriptPath: string, - metadataIdentity: string, - metadataRevision: number, - associationRevision: number, - ): void { - const retry = this.associationValidationRetries.get(scriptPath); - if ( - retry && - (retry.metadataIdentity !== metadataIdentity || - retry.metadataRevision !== metadataRevision || - retry.associationRevision !== associationRevision) - ) { - this.cancelAssociationValidationRetry(scriptPath); - } - } - - private cancelAssociationValidationRetry(scriptPath: string): void { - const retry = this.associationValidationRetries.get(scriptPath); - if (retry?.timer) { - clearTimeout(retry.timer); - } - this.associationValidationRetries.delete(scriptPath); - } - - private cancelAllAssociationValidationRetries(): void { - for (const scriptPath of this.associationValidationRetries.keys()) { - this.cancelAssociationValidationRetry(scriptPath); - } - } - private async updateValidatedStateForSelection(script: ScriptReference): Promise { const savedMetadata = await this.getSavedMetadataForPersistence(script.uri); if (!savedMetadata.identity) { @@ -2006,11 +1737,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision: number, associationRevision: number, uri: Uri, - retryGeneration?: number, ): Promise<'bound' | 'stale' | 'failed'> { return this.enqueueSelection(async () => { if ( - !this.isCurrentAssociationValidationTask(scriptPath, associationRevision, retryGeneration) || + !this.isCurrentAssociationRevision(scriptPath, associationRevision) || !this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) ) { return 'stale'; @@ -2033,13 +1763,13 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { persistedAssociation: matchedAssociation, expectedPersistedAssociation: expectedAssociation, }, - ], retryGeneration); + ]); } catch (error) { this.log.warn(`Failed to bind inline-script metadata identity: ${getErrorMessage(error)}`); return 'failed'; } if ( - !this.isCurrentAssociationValidationTask(scriptPath, associationRevision, retryGeneration) || + !this.isCurrentAssociationRevision(scriptPath, associationRevision) || !this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) ) { return 'stale'; @@ -2056,30 +1786,10 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { metadataRevision: number, scriptPath: string, associationRevision: number, - retryGeneration?: number, ): boolean { return ( - !this.disposed && this.isCurrentRoutingMetadata(uri, metadataIdentity, metadataRevision) && - this.isCurrentAssociationValidationTask(scriptPath, associationRevision, retryGeneration) - ); - } - - private isCurrentAssociationValidationTask( - scriptPath: string, - associationRevision: number, - retryGeneration?: number, - ): boolean { - return ( - this.isCurrentAssociationRevision(scriptPath, associationRevision) && - this.isCurrentAssociationValidationRetry(retryGeneration) - ); - } - - private isCurrentAssociationValidationRetry(retryGeneration?: number): boolean { - return ( - retryGeneration === undefined || - (!this.disposed && retryGeneration === this.associationValidationRetryGeneration) + this.isCurrentAssociationRevision(scriptPath, associationRevision) ); } @@ -2129,37 +1839,22 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } - private async getPersistedAssociation( - scriptPath: string, - retryGeneration?: number, - ): Promise { + private async getPersistedAssociation(scriptPath: string): Promise { await this.persistenceQueue; - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return this.getPersistedAssociationFromMemory(scriptPath); - } const state = await getWorkspacePersistentState(); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return this.getPersistedAssociationFromMemory(scriptPath); - } const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return this.getPersistedAssociationFromMemory(scriptPath); - } if (rawAssociations === undefined) { this.applyPersistedAssociations({}); return undefined; } const parsed = this.parsePersistedAssociations(rawAssociations); if (!parsed) { - await this.removeInvalidPersistedAssociation(scriptPath, retryGeneration); + await this.removeInvalidPersistedAssociation(scriptPath); return this.getPersistedAssociationFromMemory(scriptPath); } const rawValue = (rawAssociations as Record)[scriptPath]; if (rawValue !== undefined && this.parsePersistedAssociationValue(rawValue).kind === 'invalid') { - await this.removeInvalidPersistedAssociation(scriptPath, retryGeneration); - return this.getPersistedAssociationFromMemory(scriptPath); - } - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { + await this.removeInvalidPersistedAssociation(scriptPath); return this.getPersistedAssociationFromMemory(scriptPath); } this.applyPersistedAssociations(parsed.records); @@ -2172,10 +1867,9 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { revision: number, scriptUri?: Uri, expectedPersistedAssociation?: PersistedAssociationRecord, - retryGeneration?: number, ): Promise { await this.enqueueSelection(async () => { - if (!this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration)) { + if (!this.isCurrentAssociationRevision(scriptPath, revision)) { return; } try { @@ -2186,11 +1880,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { expectedEnvironmentPath, expectedPersistedAssociation, }, - ], retryGeneration); + ]); if ( normalizePath(persistedPathBeforeUpdate ?? '') === normalizePath(expectedEnvironmentPath) && !this.fsPathToPersistedAssociation.has(scriptPath) && - this.isCurrentAssociationValidationTask(scriptPath, revision, retryGeneration) + this.isCurrentAssociationRevision(scriptPath, revision) ) { const old = this.fsPathToEnv.get(scriptPath); this.bumpAssociationRevision(scriptPath); @@ -2209,28 +1903,16 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } - private removeInvalidPersistedAssociation(scriptPath: string, retryGeneration?: number): Promise { + private removeInvalidPersistedAssociation(scriptPath: string): Promise { return this.enqueuePersistence(async (state) => { - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } if (rawAssociations === undefined) { this.applyPersistedAssociations({}); return; } const parsed = this.parsePersistedAssociations(rawAssociations); if (!parsed) { - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } await state.set(INLINE_SCRIPT_ENVS_KEY, {}); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } this.applyPersistedAssociations({}); return; } @@ -2238,30 +1920,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { delete parsed.rawEntries[scriptPath]; delete parsed.records[scriptPath]; parsed.invalidKeys.delete(scriptPath); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } await state.set(INLINE_SCRIPT_ENVS_KEY, parsed.rawEntries); } - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } this.applyPersistedAssociations(parsed.records); }); } - private updatePersistedAssociations( - changes: readonly PersistedAssociationChange[], - retryGeneration?: number, - ): Promise { + private updatePersistedAssociations(changes: readonly PersistedAssociationChange[]): Promise { return this.enqueuePersistence(async (state) => { - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } const rawAssociations = await state.get(INLINE_SCRIPT_ENVS_KEY); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } const parsed = this.parsePersistedAssociations(rawAssociations); const rawEntries = { ...(parsed?.rawEntries ?? {}) }; const associations = { ...(parsed?.records ?? {}) }; @@ -2288,13 +1955,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { delete rawEntries[change.scriptPath]; } } - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } await state.set(INLINE_SCRIPT_ENVS_KEY, rawEntries); - if (!this.isCurrentAssociationValidationRetry(retryGeneration)) { - return; - } this.applyPersistedAssociations(associations); }); } @@ -2550,7 +2211,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private bumpAssociationRevision(scriptPath: string): void { - this.cancelAssociationValidationRetry(scriptPath); this.associationRevisions.set(scriptPath, (this.associationRevisions.get(scriptPath) ?? 0) + 1); } @@ -3646,10 +3306,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } dispose(): void { - this.associationValidationRetryGeneration += 1; this.disposed = true; this.stopActivationDiscovery(); - this.cancelAllAssociationValidationRetries(); this.pendingMetadataRefreshes.clear(); this.subscriptions.forEach((subscription) => subscription.dispose()); this._onDidChangeEnvironments.dispose(); @@ -3660,7 +3318,6 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { const nextPaths = new Set(Object.keys(associations)); for (const scriptPath of this.fsPathToPersistedAssociation.keys()) { if (!nextPaths.has(scriptPath)) { - this.cancelAssociationValidationRetry(scriptPath); this.fsPathToPersistedAssociation.delete(scriptPath); this.clearValidatedRouteableState(scriptPath); } diff --git a/src/test/features/envManagers.lastKnown.unit.test.ts b/src/test/features/envManagers.lastKnown.unit.test.ts index e6e64d43..20610a6b 100644 --- a/src/test/features/envManagers.lastKnown.unit.test.ts +++ b/src/test/features/envManagers.lastKnown.unit.test.ts @@ -967,6 +967,49 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => { ]); }); + test('does not persist manager settings for a batch superseded before its settings write', async () => { + const script = Uri.file('/workspace/script.py'); + const scriptProject = { name: 'script.py', uri: script }; + projectsByUri.set(script.toString(), scriptProject); + let releaseStaleBatch: (() => void) | undefined; + let signalStaleBatch: (() => void) | undefined; + const staleBatchStarted = new Promise((resolve) => { + signalStaleBatch = resolve; + }); + const staleBatchGate = new Promise((resolve) => { + releaseStaleBatch = resolve; + }); + const managerSet = sinon.stub(); + managerSet.onFirstCall().callsFake(async () => { + signalStaleBatch!(); + await staleBatchGate; + }); + managerSet.onSecondCall().resolves(); + const managerId = registerManager(async () => undefined, managerSet, 'inline-script'); + const staleEnv = { ...makeEnv('stale'), envId: { id: 'stale', managerId } }; + const newerEnv = { ...makeEnv('newer'), envId: { id: 'newer', managerId } }; + stubPackageManager(); + const settings = sinon.stub(settingHelpers, 'setAllManagerSettings').resolves(); + + const staleBatch = envManagers.setEnvironments([script], staleEnv); + await staleBatchStarted; + await envManagers.setEnvironment(script, newerEnv); + releaseStaleBatch!(); + await staleBatch; + + // The newer selection persists the manager setting exactly once. + assert.deepStrictEqual(settings.firstCall.args[0], [ + { + project: scriptProject, + envManager: managerId, + packageManager: 'ms-python.python:pip', + }, + ]); + // The superseded batch must not persist a stale selection to settings.json; it writes nothing. + assert.strictEqual(settings.callCount, 2); + assert.deepStrictEqual(settings.secondCall.args[0], []); + }); + test('retains an earlier successful refresh when a later refresh fails', async () => { const refreshed = makeEnv('refreshed'); let resolveFirst: ((environment: PythonEnvironment) => void) | undefined; diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index bbe8fe7c..c95bfc78 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -1293,23 +1293,6 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(createWithProgressStub.callCount, 0); }); - test('preserves a restart cache entry whose only usable backup has a future schema', async () => { - const directory = envDir(); - const markerPath = path.join(directory.fsPath, 'keep.txt'); - const backupPath = `${cacheLayout.getMetaJsonPath(directory).fsPath}.backup-abcdef123456`; - await fs.outputFile(markerPath, 'keep'); - await fs.writeFile( - backupPath, - JSON.stringify({ ...(await makeSidecar()), schemaVersion: cacheLayout.META_SCHEMA_VERSION + 1 }), - ); - inspectMetaStub.restore(); - - assert.strictEqual(await manager.create(scriptUri()), undefined); - assert.strictEqual(await fs.readFile(markerPath, 'utf8'), 'keep'); - assert.strictEqual(await fs.pathExists(backupPath), true); - assert.strictEqual(createWithProgressStub.callCount, 0); - }); - test('coalesces simultaneous same-key creation within one extension host', async () => { let continueCreation: (() => void) | undefined; let creationStarted: (() => void) | undefined; @@ -4922,459 +4905,6 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(resolveVenvStub.callCount, 0); }); - test('autonomously validates a persisted association after its cache lock is released', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; - await fs.ensureDir(lockPath); - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - }; - const retryDelayStub = sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); - await nextTurn(); - - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - - assert.strictEqual(retryDelayStub.callCount, 1, 'duplicate validation must share one retry'); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - assert.strictEqual(resolveVenvStub.callCount, 0); - - await fs.remove(lockPath); - await waitForCondition( - () => restartRoutingRegistry.hasValidatedAssociation(uri), - 'Expected lock release to be followed by autonomous association validation', - ); - - assert.strictEqual(resolveVenvStub.callCount, 1); - restarted.dispose(); - }); - - test('bounds repeated busy association validation retries', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - await fs.ensureDir(`${path.resolve(environment.sysPrefix)}.lock`); - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - }; - const retryDelayStub = sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt < 3 ? 0 : undefined)); - await nextTurn(); - - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - await waitForCondition( - () => retryDelayStub.callCount === 4, - 'Expected three bounded follow-up validation attempts', - ); - await new Promise((resolve) => setTimeout(resolve, 25)); - - assert.strictEqual(retryDelayStub.callCount, 4, 'busy validation must stop after the retry budget'); - assert.strictEqual(resolveVenvStub.callCount, 0); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - restarted.dispose(); - }); - - test('cancels a busy validation retry when the association revision changes', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; - await fs.ensureDir(lockPath); - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - }; - const retryDelayStub = sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); - await nextTurn(); - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - sinon.assert.calledOnce(retryDelayStub); - - await restarted.set(uri, undefined); - await fs.remove(lockPath); - await new Promise((resolve) => setTimeout(resolve, 40)); - - assert.deepStrictEqual(persistedAssociations, {}); - assert.strictEqual(resolveVenvStub.callCount, 0); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - restarted.dispose(); - }); - - test('dispose cancels a pending busy association validation retry', async () => { - const uri = scriptUri(); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [normalizePath(uri.fsPath)]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; - await fs.ensureDir(lockPath); - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - }; - const retryDelayStub = sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); - await nextTurn(); - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - sinon.assert.calledOnce(retryDelayStub); - - restarted.dispose(); - await fs.remove(lockPath); - await new Promise((resolve) => setTimeout(resolve, 40)); - - assert.strictEqual(resolveVenvStub.callCount, 0); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - }); - - test('dispose invalidates an already-fired busy association validation retry', async () => { - const uri = scriptUri(); - const scriptPath = normalizePath(uri.fsPath); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - let signalRetryBusyCheck: (() => void) | undefined; - let releaseRetryBusyCheck: ((busy: boolean) => void) | undefined; - const retryBusyCheckStarted = new Promise((resolve) => { - signalRetryBusyCheck = resolve; - }); - const retryBusyCheckGate = new Promise((resolve) => { - releaseRetryBusyCheck = resolve; - }); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - isCacheEntryBusy(envDirPath: string): Promise; - pendingMetadataRefreshes: Map< - string, - Map }> - >; - fsPathToEnv: Map; - fsPathToPersistedAssociation: Map; - _onDidChangeEnvironment: { fire(event: unknown): void }; - }; - sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt === 0 ? 0 : undefined)); - const busyCheckStub = sinon.stub(retryManager, 'isCacheEntryBusy'); - busyCheckStub.onFirstCall().resolves(true); - busyCheckStub.onSecondCall().callsFake(async () => { - signalRetryBusyCheck!(); - return retryBusyCheckGate; - }); - const environmentEventFire = sinon.spy(retryManager._onDidChangeEnvironment, 'fire'); - const routePublication = sinon.spy(restartRoutingRegistry, 'setValidatedAssociation'); - await nextTurn(); - - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - await retryBusyCheckStarted; - const inFlightRetry = Array.from( - retryManager.pendingMetadataRefreshes.get(scriptPath)?.values() ?? [], - ).find((operation) => operation.origin === 'retry')?.promise; - assert.ok(inFlightRetry, 'retry should be in flight before disposal'); - assert.strictEqual(busyCheckStub.callCount, 2); - assert.strictEqual(retryManager.fsPathToEnv.has(scriptPath), false); - workspaceState.set.resetHistory(); - environmentEventFire.resetHistory(); - routePublication.resetHistory(); - - restarted.dispose(); - releaseRetryBusyCheck!(false); - await inFlightRetry; - - assert.deepStrictEqual(persistedAssociations, { - [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), - }); - assert.strictEqual(workspaceState.set.callCount, 0, 'disposed retry must not remove persistence'); - assert.strictEqual(retryManager.fsPathToPersistedAssociation.has(scriptPath), true); - assert.strictEqual(retryManager.fsPathToEnv.has(scriptPath), false, 'disposed retry must not repopulate cache'); - assert.strictEqual(resolveVenvStub.callCount, 0); - assert.strictEqual(environmentEventFire.callCount, 0, 'disposed retry must not fire manager events'); - assert.strictEqual(routePublication.callCount, 0, 'disposed retry must not publish routeability'); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - }); - - test('keeps an ordinary get independent when it arrives during a fired retry', async () => { - const uri = scriptUri(); - const scriptPath = normalizePath(uri.fsPath); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - let signalRetryBusyCheck: (() => void) | undefined; - let releaseRetryBusyCheck: ((busy: boolean) => void) | undefined; - const retryBusyCheckStarted = new Promise((resolve) => { - signalRetryBusyCheck = resolve; - }); - const retryBusyCheckGate = new Promise((resolve) => { - releaseRetryBusyCheck = resolve; - }); - let releaseOrdinaryResolution: (() => void) | undefined; - const ordinaryResolutionGate = new Promise((resolve) => { - releaseOrdinaryResolution = resolve; - }); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - isCacheEntryBusy(envDirPath: string): Promise; - refreshValidatedAssociationForMetadata( - candidate: Uri, - metadata: metadataReader.InlineScriptMetadata, - metadataIdentity: string, - metadataRevision: number, - retryGeneration?: number, - ): Promise; - pendingRehydrations: Map< - string, - Map }> - >; - pendingMetadataRefreshes: Map< - string, - Map< - string, - { - readonly origin: 'ordinary' | 'retry'; - readonly retryGeneration?: number; - readonly promise: Promise; - } - > - >; - fsPathToEnv: Map; - fsPathToPersistedAssociation: Map; - _onDidChangeEnvironment: { fire(event: unknown): void }; - }; - sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt === 0 ? 0 : undefined)); - const busyCheckStub = sinon.stub(retryManager, 'isCacheEntryBusy'); - busyCheckStub.onFirstCall().resolves(true); - busyCheckStub.onSecondCall().callsFake(async () => { - signalRetryBusyCheck!(); - return retryBusyCheckGate; - }); - busyCheckStub.onThirdCall().resolves(false); - resolveVenvStub.callsFake(async () => { - await ordinaryResolutionGate; - return environment; - }); - const environmentEventFire = sinon.spy(retryManager._onDidChangeEnvironment, 'fire'); - const routePublication = sinon.spy(restartRoutingRegistry, 'setValidatedAssociation'); - await nextTurn(); - - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - await retryBusyCheckStarted; - const retryRefresh = Array.from( - retryManager.pendingMetadataRefreshes.get(scriptPath)?.values() ?? [], - ).find((operation) => operation.origin === 'retry'); - assert.ok(retryRefresh?.retryGeneration !== undefined); - const equivalentRetry = retryManager.refreshValidatedAssociationForMetadata( - uri, - VALID_METADATA, - restartRoutingRegistry.getMetadataIdentity(uri)!, - restartRoutingRegistry.getMetadataRevision(uri), - retryRefresh!.retryGeneration, - ); - await nextTurn(); - assert.strictEqual(busyCheckStub.callCount, 2, 'same-generation retries must coalesce'); - - const ordinaryGet = restarted.get(uri); - await waitForStubCall(resolveVenvStub); - assert.deepStrictEqual( - Array.from(retryManager.pendingRehydrations.get(scriptPath)?.values() ?? []) - .map((operation) => operation.origin) - .sort(), - ['ordinary', 'retry'], - ); - workspaceState.set.resetHistory(); - environmentEventFire.resetHistory(); - routePublication.resetHistory(); - - restarted.dispose(); - releaseRetryBusyCheck!(false); - releaseOrdinaryResolution!(); - const ordinaryResult = await ordinaryGet; - await Promise.all([retryRefresh!.promise, equivalentRetry]); - - assert.strictEqual(ordinaryResult, environment, 'ordinary get must survive retry disposal'); - assert.deepStrictEqual(persistedAssociations, { - [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), - }); - assert.strictEqual(workspaceState.set.callCount, 0); - assert.strictEqual(retryManager.fsPathToPersistedAssociation.has(scriptPath), true); - assert.strictEqual(retryManager.fsPathToEnv.get(scriptPath), environment); - assert.strictEqual(environmentEventFire.callCount, 1, 'only ordinary work may publish a manager event'); - assert.strictEqual(routePublication.callCount, 0, 'disposed retry must not publish routeability'); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - }); - - test('keeps a fired retry independent from an existing ordinary validation', async () => { - const uri = scriptUri(); - const scriptPath = normalizePath(uri.fsPath); - const environment = await createOwnedEnvironment(); - persistedAssociations = { - [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), - }; - const restartRoutingRegistry = new InlineScriptRoutingRegistry(); - const restarted = new InlineScriptEnvManager( - nativeFinder, - api, - baseManager, - globalStorageUri, - makeFakeLog(), - restartRoutingRegistry, - ); - let releaseOrdinaryResolution: (() => void) | undefined; - const ordinaryResolutionGate = new Promise((resolve) => { - releaseOrdinaryResolution = resolve; - }); - let signalRetryBusyCheck: (() => void) | undefined; - let releaseRetryBusyCheck: ((busy: boolean) => void) | undefined; - const retryBusyCheckStarted = new Promise((resolve) => { - signalRetryBusyCheck = resolve; - }); - const retryBusyCheckGate = new Promise((resolve) => { - releaseRetryBusyCheck = resolve; - }); - const retryManager = restarted as unknown as { - getAssociationValidationRetryDelayMs(attempt: number): number | undefined; - isCacheEntryBusy(envDirPath: string): Promise; - pendingRehydrations: Map< - string, - Map }> - >; - pendingMetadataRefreshes: Map< - string, - Map }> - >; - fsPathToEnv: Map; - fsPathToPersistedAssociation: Map; - _onDidChangeEnvironment: { fire(event: unknown): void }; - }; - sinon - .stub(retryManager, 'getAssociationValidationRetryDelayMs') - .callsFake((attempt) => (attempt === 0 ? 25 : undefined)); - const busyCheckStub = sinon.stub(retryManager, 'isCacheEntryBusy'); - busyCheckStub.onFirstCall().resolves(true); - busyCheckStub.onSecondCall().resolves(false); - busyCheckStub.onThirdCall().callsFake(async () => { - signalRetryBusyCheck!(); - return retryBusyCheckGate; - }); - resolveVenvStub.callsFake(async () => { - await ordinaryResolutionGate; - return environment; - }); - const environmentEventFire = sinon.spy(retryManager._onDidChangeEnvironment, 'fire'); - const routePublication = sinon.spy(restartRoutingRegistry, 'setValidatedAssociation'); - await nextTurn(); - - await triggerSavedMetadataChange(restartRoutingRegistry, restarted, uri); - const ordinaryGet = restarted.get(uri); - await waitForStubCall(resolveVenvStub); - const equivalentOrdinaryGet = restarted.get(uri); - await nextTurn(); - assert.strictEqual(resolveVenvStub.callCount, 1, 'equivalent ordinary gets must coalesce'); - - await waitForStubCallCount(busyCheckStub, 3); - await retryBusyCheckStarted; - assert.deepStrictEqual( - Array.from(retryManager.pendingRehydrations.get(scriptPath)?.values() ?? []) - .map((operation) => operation.origin) - .sort(), - ['ordinary', 'retry'], - ); - const retryPromise = Array.from( - retryManager.pendingMetadataRefreshes.get(scriptPath)?.values() ?? [], - ).find((operation) => operation.origin === 'retry')?.promise; - assert.ok(retryPromise); - workspaceState.set.resetHistory(); - environmentEventFire.resetHistory(); - routePublication.resetHistory(); - - restarted.dispose(); - releaseRetryBusyCheck!(false); - releaseOrdinaryResolution!(); - const [ordinaryResult, equivalentOrdinaryResult] = await Promise.all([ - ordinaryGet, - equivalentOrdinaryGet, - ]); - await retryPromise; - - assert.strictEqual(ordinaryResult, environment); - assert.strictEqual(equivalentOrdinaryResult, environment); - assert.deepStrictEqual(persistedAssociations, { - [scriptPath]: matchedAssociationRecord(environment.environmentPath.fsPath), - }); - assert.strictEqual(workspaceState.set.callCount, 0); - assert.strictEqual(retryManager.fsPathToPersistedAssociation.has(scriptPath), true); - assert.strictEqual(retryManager.fsPathToEnv.get(scriptPath), environment); - assert.strictEqual(environmentEventFire.callCount, 1, 'coalesced ordinary work publishes once'); - assert.strictEqual(routePublication.callCount, 0, 'disposed retry must not publish routeability'); - assert.strictEqual(restartRoutingRegistry.hasValidatedAssociation(uri), false); - }); - test('clears the routing registry when a stale persisted association is removed', async () => { const uri = scriptUri(); const environment = await createOwnedEnvironment(); @@ -6136,7 +5666,6 @@ suite('InlineScriptEnvManager', () => { baseManager, Uri.file(process.platform === 'win32' ? `${process.env.SystemDrive ?? 'C:'}\\` : '/'), makeFakeLog(), - routingRegistry, ); await assert.rejects( @@ -6155,7 +5684,6 @@ suite('InlineScriptEnvManager', () => { baseManager, symlinkStorageUri, makeFakeLog(), - routingRegistry, ); const realCacheRoot = cacheLayout.getScriptEnvCacheRoot(symlinkStorageUri).fsPath; const externalCacheRoot = path.join(tempRoot, 'external-cache-root'); @@ -6190,7 +5718,6 @@ suite('InlineScriptEnvManager', () => { baseManager, Uri.file(redirectedStoragePath), makeFakeLog(), - routingRegistry, ); try { await fs.remove(redirectedStoragePath); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index a21c2d1f..69f6d80b 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -127,11 +127,6 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { disposables.includes(registerEnvironmentManagerStub.firstCall.returnValue), 'registration disposable should be disposed', ); - assert.strictEqual( - (manager as unknown as { routingRegistry: InlineScriptRoutingRegistry }).routingRegistry, - routingRegistry, - 'the registered manager must share the activation routing registry', - ); assert.strictEqual(typeof manager.create, 'function'); await nextTurn(); disposables.forEach((disposable) => disposable.dispose()); From 089aebac5bef0987a676942c9b806e2632a71272 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Tue, 25 Aug 2026 16:55:05 -0700 Subject: [PATCH 3/3] Scope inline-script routing hardening to a PEP 723 per-script guard Replace the generic cross-manager selection-finalization machinery with a narrow PEP 723-only routing-mutation guard. Only a current (non-superseded) inline-routing operation may mutate a script's per-script routing override; the ordinary project/global active-selection and settings lanes are committed independently and are no longer suppressed by a stale inline operation. - Restore the origin/main batch settings construction/write order (no committedSelections, no pre-settings revision filter, no post-settings recheck) and the one-key commitSelectionOperation. - Add commitInlineRoutingOperation, gated to inlineScriptRouting + file .py scopes via getInlineScriptRoutingKey; feature-off and non-script scopes return true immediately and preserve the old flow. - Guard only the single-URI and batch inline routing blocks. Each script commits on its own per-file inline key, so two scripts under the same containing project no longer contend on a shared project revision (the first script now installs its override instead of skipping it). - The generic cross-manager settings write race is explicitly deferred. Tests: adapt the stale-non-inline-vs-newer-inline regression to assert the script still routes to inline without asserting containing-project suppression; add a same-project batch non-inline regression and a feature-off distinct-projects batch regression; remove the batch settings-ordering test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f881304e-3c5f-4144-9f0d-d80c68768d18 --- src/features/envManagers.ts | 120 +++++++++--------- .../envManagers.lastKnown.unit.test.ts | 99 ++++++++------- 2 files changed, 110 insertions(+), 109 deletions(-) diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index e0362d65..b8639ada 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -18,6 +18,7 @@ import { import { InlineScriptRouteabilityChangeEvent, InlineScriptRoutingRegistry, + getInlineScriptRoutingKey, } from '../common/inlineScript/routingRegistry'; import { traceError, traceVerbose } from '../common/logging'; import { StopWatch } from '../common/stopWatch'; @@ -420,33 +421,29 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } if (scope instanceof Uri) { - const inlineOperation = - manager.id === INLINE_SCRIPT_MANAGER_ID - ? operation - : (inlineOverrideHandoffOperation ?? inlineClearOperation); if ( - !this.commitSelectionOperations([ - { key, operation }, - ...(inlineOperation === undefined - ? [] - : [{ key: this.getInlineScriptSelectionKey(scope), operation: inlineOperation }]), - ]) - ) { - return; - } - this.updateInlineRoutingOverride(scope, manager, environment); - this.clearInlineActiveSelection(scope, manager, inlineOperation); - if ( - clearingInlineRoutingOverride && - (await this.publishEffectiveEnvironmentAfterOverrideClear( + this.commitInlineRoutingOperation( scope, manager, - key, operation, + inlineClearOperation, inlineOverrideHandoffOperation, - )) + ) ) { - return; + this.updateInlineRoutingOverride(scope, manager, environment); + this.clearInlineActiveSelection(scope, manager, inlineOverrideHandoffOperation ?? inlineClearOperation); + if ( + clearingInlineRoutingOverride && + (await this.publishEffectiveEnvironmentAfterOverrideClear( + scope, + manager, + key, + operation, + inlineOverrideHandoffOperation, + )) + ) { + return; + } } } if (!publishInlineSelection) { @@ -507,13 +504,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (Array.isArray(scope) && scope.every((s) => s instanceof Uri)) { const selections = scope.map((uri) => this.beginPendingSelection(uri, manager)); await manager.set(scope, environment); - // Commit the winning (non-superseded) selections BEFORE persisting settings so an - // out-of-order older batch cannot write a manager setting to settings.json while its - // matching routing/selection update is rejected (settings/routing divergence). - const committedSelections = selections.filter((selection) => - this.commitPendingSelection(selection, manager), - ); - committedSelections.forEach((selection) => { + selections.forEach((selection) => { const m = this.getEnvironmentManager(selection.scope); // Always add settings when persisting, OR when manager differs if ( @@ -530,18 +521,27 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { if (shouldPersistSettings) { await setAllManagerSettings(settings); } - committedSelections.forEach((selection) => { - // Re-validate across the (awaited) settings write: a newer selection may have - // superseded this one while settings were being persisted, in which case its - // routing override and active-selection publish must be skipped. - if (!this.commitPendingSelection(selection, manager)) { - return; + selections.forEach((selection) => { + // Only a current (non-superseded) PEP 723 routing operation may mutate the + // per-script override; the ordinary project/global selection lane below is + // committed independently so a stale inline op cannot suppress it. + if ( + this.commitInlineRoutingOperation( + selection.scope, + manager, + selection.operation, + selection.inlineClearOperation, + ) + ) { + this.updateInlineRoutingOverride(selection.scope, manager, environment); + this.clearInlineActiveSelection(selection.scope, manager, selection.inlineClearOperation); } - this.updateInlineRoutingOverride(selection.scope, manager, environment); - this.clearInlineActiveSelection(selection.scope, manager, selection.inlineClearOperation); if (!selection.publishInlineSelection) { return; } + if (!this.commitSelectionOperation(selection.key, selection.operation)) { + return; + } const oldEnv = this._activeSelection.get(selection.key); if (!this.isSameEnvironment(oldEnv, environment)) { this._activeSelection.set(selection.key, environment); @@ -955,18 +955,26 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } - private commitPendingSelection( - selection: PendingEnvironmentSelection, + private commitInlineRoutingOperation( + scope: Uri, manager: InternalEnvironmentManager, + selectionOperation: number, + inlineClearOperation?: number, + inlineOverrideHandoffOperation?: number, ): boolean { - const inlineOperation = - manager.id === INLINE_SCRIPT_MANAGER_ID ? selection.operation : selection.inlineClearOperation; - return this.commitSelectionOperations([ - { key: selection.key, operation: selection.operation }, - ...(inlineOperation === undefined - ? [] - : [{ key: this.getInlineScriptSelectionKey(selection.scope), operation: inlineOperation }]), - ]); + // Gate strictly to the manually enabled PEP 723 routing feature and to file .py scopes. + // For the feature-off or non-script case, proceed exactly as before. + if (!this.inlineScriptRouting || getInlineScriptRoutingKey(scope) === undefined) { + return true; + } + const operation = + manager.id === INLINE_SCRIPT_MANAGER_ID + ? selectionOperation + : (inlineOverrideHandoffOperation ?? inlineClearOperation); + return ( + operation === undefined || + this.commitSelectionOperation(this.getInlineScriptSelectionKey(scope), operation) + ); } private canPersistManagerSettingForScope( @@ -988,24 +996,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } private commitSelectionOperation(key: string, operation: number): boolean { - return this.commitSelectionOperations([{ key, operation }]); - } - - private commitSelectionOperations( - operations: readonly { readonly key: string; readonly operation: number }[], - ): boolean { - const latestByKey = new Map(); - for (const { key, operation } of operations) { - latestByKey.set(key, Math.max(latestByKey.get(key) ?? operation, operation)); - } - for (const [key, operation] of latestByKey) { - if ((this._selectionRevisions.get(key) ?? 0) > operation) { - return false; - } - } - for (const [key, operation] of latestByKey) { - this._selectionRevisions.set(key, operation); + if ((this._selectionRevisions.get(key) ?? 0) > operation) { + return false; } + this._selectionRevisions.set(key, operation); return true; } diff --git a/src/test/features/envManagers.lastKnown.unit.test.ts b/src/test/features/envManagers.lastKnown.unit.test.ts index 20610a6b..a91d8e43 100644 --- a/src/test/features/envManagers.lastKnown.unit.test.ts +++ b/src/test/features/envManagers.lastKnown.unit.test.ts @@ -334,14 +334,15 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => { const olderSelection = envManagers.setEnvironment(script, selectedEnvironment, false); await olderSelectionStarted; await envManagers.setEnvironment(script, inlineEnvironment, false); - const eventsAfterNewerSelection = [...events]; releaseOlderSelection!(); await olderSelection; + // The stale non-inline selection must not hijack the script's PEP 723 routing… assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, inlineId); assert.strictEqual(envManagers.getLastKnownEnvironment(script), inlineEnvironment); - assert.deepStrictEqual(events, eventsAfterNewerSelection); - assert.ok(events.every((event) => event.new !== selectedEnvironment)); + assert.ok(events.some((event) => event.new === inlineEnvironment)); + // …but its ordinary containing-project selection lane is independent and is not suppressed. + assert.ok(events.some((event) => event.new === selectedEnvironment)); }); test('does not let an older batch inline selection clear a newer non-inline override', async () => { @@ -388,6 +389,55 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => { assert.deepStrictEqual(events, eventsAfterNewerSelection); }); + test('routes each same-project script to its own override in a batch non-inline selection', async () => { + const project = { name: 'project', uri: Uri.file('/workspace/project') }; + const firstScript = Uri.file('/workspace/project/first.py'); + const secondScript = Uri.file('/workspace/project/second.py'); + projectsByUri.set(firstScript.toString(), project); + projectsByUri.set(secondScript.toString(), project); + let selectedEnvironment: PythonEnvironment; + const selectedId = registerManager(async () => selectedEnvironment, async () => undefined, 'venv'); + registerManager(async () => undefined, async () => undefined, 'inline-script'); + selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId: selectedId } }; + markInlineScript(firstScript); + markInlineScript(secondScript); + + await envManagers.setEnvironments([firstScript, secondScript], selectedEnvironment, false); + + // Each script commits on its own per-file inline key, so the shared containing-project + // revision cannot make the first script skip installing its routing override. + assert.strictEqual(envManagers.getEnvironmentManager(firstScript)?.id, selectedId); + assert.strictEqual(envManagers.getEnvironmentManager(secondScript)?.id, selectedId); + }); + + test('applies a normal non-inline batch across distinct projects without a routing registry', async () => { + recreateEnvManagersWithoutRouting(); + const projectOne = { name: 'one', uri: Uri.file('/workspace/one') }; + const projectTwo = { name: 'two', uri: Uri.file('/workspace/two') }; + projectsByUri.set(projectOne.uri.toString(), projectOne); + projectsByUri.set(projectTwo.uri.toString(), projectTwo); + const managerSet = sinon.stub().resolves(); + let selectedEnvironment: PythonEnvironment; + const managerId = registerManager(async () => selectedEnvironment, managerSet, 'venv'); + selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId } }; + const settings = sinon.stub(settingHelpers, 'setAllManagerSettings').resolves(); + const events: DidChangeEnvironmentEventArgs[] = []; + envManagers.onDidChangeActiveEnvironment((event) => events.push(event)); + + await envManagers.setEnvironments([projectOne.uri, projectTwo.uri], selectedEnvironment); + + assert.strictEqual(managerSet.callCount, 1); + assert.deepStrictEqual(managerSet.firstCall.args[0], [projectOne.uri, projectTwo.uri]); + assert.strictEqual(envManagers.getLastKnownEnvironment(projectOne.uri), selectedEnvironment); + assert.strictEqual(envManagers.getLastKnownEnvironment(projectTwo.uri), selectedEnvironment); + assert.deepStrictEqual( + events.map((event) => event.new), + [selectedEnvironment, selectedEnvironment], + ); + assert.strictEqual(settings.callCount, 1); + assert.strictEqual(settings.firstCall.args[0].length, 2); + }); + test('publishes inline environments with the same ID at different paths', async () => { const scope = Uri.file('/workspace/script.py'); const managerId = registerManager(async () => undefined, async () => undefined, 'inline-script'); @@ -967,49 +1017,6 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => { ]); }); - test('does not persist manager settings for a batch superseded before its settings write', async () => { - const script = Uri.file('/workspace/script.py'); - const scriptProject = { name: 'script.py', uri: script }; - projectsByUri.set(script.toString(), scriptProject); - let releaseStaleBatch: (() => void) | undefined; - let signalStaleBatch: (() => void) | undefined; - const staleBatchStarted = new Promise((resolve) => { - signalStaleBatch = resolve; - }); - const staleBatchGate = new Promise((resolve) => { - releaseStaleBatch = resolve; - }); - const managerSet = sinon.stub(); - managerSet.onFirstCall().callsFake(async () => { - signalStaleBatch!(); - await staleBatchGate; - }); - managerSet.onSecondCall().resolves(); - const managerId = registerManager(async () => undefined, managerSet, 'inline-script'); - const staleEnv = { ...makeEnv('stale'), envId: { id: 'stale', managerId } }; - const newerEnv = { ...makeEnv('newer'), envId: { id: 'newer', managerId } }; - stubPackageManager(); - const settings = sinon.stub(settingHelpers, 'setAllManagerSettings').resolves(); - - const staleBatch = envManagers.setEnvironments([script], staleEnv); - await staleBatchStarted; - await envManagers.setEnvironment(script, newerEnv); - releaseStaleBatch!(); - await staleBatch; - - // The newer selection persists the manager setting exactly once. - assert.deepStrictEqual(settings.firstCall.args[0], [ - { - project: scriptProject, - envManager: managerId, - packageManager: 'ms-python.python:pip', - }, - ]); - // The superseded batch must not persist a stale selection to settings.json; it writes nothing. - assert.strictEqual(settings.callCount, 2); - assert.deepStrictEqual(settings.secondCall.args[0], []); - }); - test('retains an earlier successful refresh when a later refresh fails', async () => { const refreshed = makeEnv('refreshed'); let resolveFirst: ((environment: PythonEnvironment) => void) | undefined;