diff --git a/src/common/inlineScript/cacheLayout.ts b/src/common/inlineScript/cacheLayout.ts index 7554ed08..45854223 100644 --- a/src/common/inlineScript/cacheLayout.ts +++ b/src/common/inlineScript/cacheLayout.ts @@ -42,6 +42,8 @@ export interface InlineScriptEnvMeta { readonly baseInterpreterVersion: string; /** Last successful use as a canonical UTC string produced by `Date.toISOString()`. */ readonly lastUsedAt: string; + /** Set when packages changed outside setup; the entry no longer matches its declared dependencies. */ + readonly manuallyModified?: boolean; /** Bounded SHA-256 hashes of metadata identities proven for this cache entry. */ readonly sourceMetadataIdentityHashes?: readonly string[]; } @@ -447,11 +449,7 @@ function validateMeta(value: unknown): InlineScriptEnvMeta | 'unsupported' | und return undefined; } const obj = value as Record; - if ( - typeof obj.schemaVersion !== 'number' || - !Number.isSafeInteger(obj.schemaVersion) || - obj.schemaVersion <= 0 - ) { + if (typeof obj.schemaVersion !== 'number' || !Number.isSafeInteger(obj.schemaVersion) || obj.schemaVersion <= 0) { return undefined; } if (obj.schemaVersion > META_SCHEMA_VERSION) { @@ -469,6 +467,9 @@ function validateMeta(value: unknown): InlineScriptEnvMeta | 'unsupported' | und if (!isCanonicalIsoTimestamp(obj.lastUsedAt)) { return undefined; } + if (obj.manuallyModified !== undefined && typeof obj.manuallyModified !== 'boolean') { + return undefined; + } const sourceMetadataIdentityHashes = validateSourceMetadataIdentityHashes(obj.sourceMetadataIdentityHashes); if (obj.sourceMetadataIdentityHashes !== undefined && sourceMetadataIdentityHashes === undefined) { return undefined; @@ -479,6 +480,7 @@ function validateMeta(value: unknown): InlineScriptEnvMeta | 'unsupported' | und baseInterpreterPath: obj.baseInterpreterPath, baseInterpreterVersion: obj.baseInterpreterVersion, lastUsedAt: obj.lastUsedAt, + ...(obj.manuallyModified === true ? { manuallyModified: true } : {}), ...(sourceMetadataIdentityHashes ? { sourceMetadataIdentityHashes } : {}), }; } diff --git a/src/common/inlineScript/routingRegistry.ts b/src/common/inlineScript/routingRegistry.ts index 6387bf8f..de8ee940 100644 --- a/src/common/inlineScript/routingRegistry.ts +++ b/src/common/inlineScript/routingRegistry.ts @@ -3,10 +3,10 @@ import * as path from 'path'; import { Disposable, Event, EventEmitter, Uri } from 'vscode'; +import type { InlineScriptEnvErrorCategory } from '../telemetry/constants'; +import { normalizePath } from '../utils/pathUtils'; import { normalizeDependency } from './cacheKey'; import { InlineScriptMetadata } from './metadata'; -import { normalizePath } from '../utils/pathUtils'; -import type { InlineScriptEnvErrorCategory } from '../telemetry/constants'; export interface InlineScriptRouteabilityChangeEvent { readonly uri: Uri; @@ -32,6 +32,7 @@ export type InlineScriptSetupOutcome = readonly category: InlineScriptEnvErrorCategory; readonly requiresPython?: string; } + | { readonly kind: 'cancelled' } | { readonly kind: 'skipped' }; interface ScriptRoutingState { @@ -150,6 +151,11 @@ export class InlineScriptRoutingRegistry implements Disposable { } } + public getSetupOutcome(script: Uri | string): InlineScriptSetupOutcome | undefined { + const scriptPath = getInlineScriptRoutingKey(script); + return scriptPath ? this.setupOutcomes.get(scriptPath) : undefined; + } + public takeSetupOutcome(script: Uri | string): InlineScriptSetupOutcome | undefined { const scriptPath = getInlineScriptRoutingKey(script); if (!scriptPath) { diff --git a/src/features/inlineScript/setupEnvironment.ts b/src/features/inlineScript/setupEnvironment.ts index 0088864d..6a8a0cb0 100644 --- a/src/features/inlineScript/setupEnvironment.ts +++ b/src/features/inlineScript/setupEnvironment.ts @@ -75,6 +75,9 @@ export async function setUpInlineScriptEnvironment( return undefined; } await em.setEnvironment(scriptUri, environment); + // Outcomes are read non-consumingly, so a success must retire the previous attempt's outcome + // rather than relying on the next `create` to clear it on entry. + routing.clearSetupOutcome(scriptUri); return environment; } @@ -124,12 +127,16 @@ function setupInlineScriptEnvironmentHandler( }; } -function notifyInlineScriptSetupOutcome(uri: Uri, routing: InlineScriptRoutingRegistry): void { - const outcome = routing.takeSetupOutcome(uri); +export function notifyInlineScriptSetupOutcome(uri: Uri, routing: InlineScriptRoutingRegistry): void { + const outcome = routing.getSetupOutcome(uri); if (outcome?.kind === 'skipped') { // Env built but intentionally not associated (metadata changed mid-setup); stay silent. return; } + if (outcome?.kind === 'cancelled') { + showInformationMessage(l10n.t('Environment setup was canceled.')); + return; + } if (outcome?.kind === 'failed') { if (outcome.category === 'compatible-python-declined') { // User declined the install prompt; don't nag. @@ -223,16 +230,57 @@ export async function setUpInlineScriptEnvironmentsInWorkspace( return; } let succeeded = 0; + let attempted = 0; + let failed = 0; + let cancelled = false; for (const pick of picks) { + attempted += 1; try { if (await setUpInlineScriptEnvironment(pick.uri, em, routing)) { succeeded += 1; + continue; + } + const outcome = routing.getSetupOutcome(pick.uri); + if (outcome?.kind === 'cancelled') { + // Cancelling one script's installer stops the whole run rather than immediately + // starting the next script's install. + cancelled = true; + break; + } + if (outcome?.kind !== 'skipped') { + failed += 1; } } catch (error) { + failed += 1; traceError(`Failed to set up the inline-script environment for ${pick.uri.fsPath}:`, error); } } - traceInfo(`Inline-script bulk setup: created or reused ${succeeded} of ${picks.length} environment(s).`); + traceInfo( + `Inline-script bulk setup: created or reused ${succeeded} of ${picks.length} environment(s)` + + `${cancelled ? ' (canceled)' : ''}.`, + ); + if (cancelled) { + showWarningMessage( + l10n.t( + 'Environment setup was canceled. Set up {0} of {1} selected inline script environment(s); the remaining {2} were not started.', + succeeded, + picks.length, + picks.length - attempted, + ), + ); + return; + } + if (failed > 0) { + showWarningMessage( + l10n.t( + 'Set up {0} of {1} selected inline script environment(s). {2} failed — see the Python Environments output for details.', + succeeded, + picks.length, + failed, + ), + ); + return; + } showInformationMessage(l10n.t('Set up {0} of {1} selected inline script environment(s).', succeeded, picks.length)); } diff --git a/src/features/terminal/shellStartupActivationVariablesManager.ts b/src/features/terminal/shellStartupActivationVariablesManager.ts index 3ea63a09..317b9ff7 100644 --- a/src/features/terminal/shellStartupActivationVariablesManager.ts +++ b/src/features/terminal/shellStartupActivationVariablesManager.ts @@ -48,21 +48,25 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA private async handleEnvironmentChange(e: DidChangeEnvironmentEventArgs) { const autoActType = getAutoActivationType(); - if (autoActType === ACT_TYPE_SHELL && e.uri) { - const wf = getWorkspaceFolder(e.uri); - if (wf) { - const envVars = this.envCollection.getScoped({ workspaceFolder: wf }); - if (envVars) { - this.shellEnvsProviders.forEach((provider) => { - if (e.new) { - provider.updateEnvVariables(envVars, e.new); - } else { - provider.removeEnvVariables(envVars); - } - }); - } - } + if (autoActType !== ACT_TYPE_SHELL || !e.uri) { + return; } + const wf = getWorkspaceFolder(e.uri); + if (!wf) { + return; + } + const envVars = this.envCollection.getScoped({ workspaceFolder: wf }); + if (!envVars) { + return; + } + const folderEnvironment = await this.api.getEnvironment(wf.uri); + this.shellEnvsProviders.forEach((provider) => { + if (folderEnvironment) { + provider.updateEnvVariables(envVars, folderEnvironment); + } else { + provider.removeEnvVariables(envVars); + } + }); } private async initializeInternal(): Promise { diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index 35a51d03..e15906a0 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -20,6 +20,7 @@ import { CreateEnvironmentScope, DidChangeEnvironmentEventArgs, DidChangeEnvironmentsEventArgs, + DidChangePackagesEventArgs, EnvironmentChangeKind, EnvironmentManager, GetEnvironmentScope, @@ -51,6 +52,7 @@ import { inspectMetaJson, inspectOwnedCacheEntry, mergeSourceMetadataIdentityHashes, + META_JSON_FILENAME, META_SCHEMA_VERSION, resolveCacheEntryPath, restoreMetaJsonBackupUnderLock, @@ -61,6 +63,7 @@ import { extractLowerBoundVersion, pickCompatibleInterpreter } from '../../../co import { InlineScriptMetadata, readInlineScriptMetadataFromFile } from '../../../common/inlineScript/metadata'; import { getInlineScriptMetadataRoutingIdentity, + getInlineScriptRoutingKey, InlineScriptMetadataChangeEvent, InlineScriptRoutingRegistry, } from '../../../common/inlineScript/routingRegistry'; @@ -74,9 +77,10 @@ import { } from '../../../common/lockfile.apis'; import { EventNames, InlineScriptEnvErrorCategory } from '../../../common/telemetry/constants'; import { sendTelemetryEvent } from '../../../common/telemetry/sender'; +import { timeout } from '../../../common/utils/asyncUtils'; import { createDeferred, Deferred } from '../../../common/utils/deferred'; import { isFileNotFoundError } from '../../../common/utils/filesystem'; -import { normalizePath } from '../../../common/utils/pathUtils'; +import { isSameOrParentPath, normalizePath } from '../../../common/utils/pathUtils'; import { PythonVersion } from '../../../common/pythonVersion'; import { PythonVersionSpecifier, splitClause } from '../../../common/pythonVersionSpecifier'; import { getVenvPythonPath } from '../../../common/utils/virtualEnvironment'; @@ -96,6 +100,9 @@ const CACHE_TTL_MS = 14 * 24 * 60 * 60 * 1000; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; const DISCOVERY_RETRY_DELAYS_MS = [1_000, 5_000, 30_000] as const; const PERSISTED_ASSOCIATION_SCHEMA_VERSION = 1 as const; +/** Bounded retry for deleting a cache entry whose files may still be briefly held by a stopped installer. */ +const CACHE_ENTRY_REMOVAL_ATTEMPTS = 4; +const CACHE_ENTRY_REMOVAL_RETRY_MS = 150; interface SelectedBaseInterpreter { readonly environment: PythonEnvironment; @@ -113,7 +120,6 @@ interface CreateOrReuseEnvironmentOptions { interface BuildCacheEntryResult { readonly environment?: PythonEnvironment; - readonly retainLock?: boolean; readonly errorCategory?: InlineScriptEnvErrorCategory; } @@ -142,6 +148,15 @@ interface PendingCreationContext { sourceMetadataIdentityHashes?: readonly string[]; hasStartedRecordingSourceMetadataIdentityHashes: boolean; recordedSourceMetadataIdentityHashes?: readonly string[]; + /** + * Outcome of the shared build, recorded once and read by every caller joined to it. Cancelling + * a build that several scripts joined must report cancellation to all of them, not just to the + * script that happened to start it. + */ + failure?: { + readonly category: InlineScriptEnvErrorCategory; + readonly cancelled?: boolean; + }; } interface MergeCacheEntrySourceMetadataIdentityHashResult { @@ -256,9 +271,16 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); }), onDidRenameFiles((event) => { - void this.clearAssociationsForScripts(event.files.map((file) => file.oldUri)).catch((error) => { + void this.handleRenamedScripts(event.files).catch((error) => { this.log.warn( - `Failed to clear inline-script associations for renamed files: ${getErrorMessage(error)}`, + `Failed to update inline-script associations for renamed files: ${getErrorMessage(error)}`, + ); + }); + }), + this.api.onDidChangePackages((event) => { + void this.handlePackagesChanged(event).catch((error) => { + this.log.warn( + `Failed to record an inline-script package change: ${getErrorMessage(error)}`, ); }); }), @@ -362,6 +384,9 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { pending.hasStartedRecordingSourceMetadataIdentityHashes; this.addPendingCreationSourceMetadataIdentityHash(pending, sourceMetadataIdentityHash); const environment = await pending.promise; + if (!environment) { + this.notePendingCreationFailure(scriptUri, metadata, pending); + } return await this.finalizeCreateForScript( cacheKey, environment, @@ -387,6 +412,9 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.pendingCreations.set(cacheKey, pendingCreation); try { const environment = await creation; + if (!environment) { + this.notePendingCreationFailure(scriptUri, metadata, pendingCreation); + } return await this.finalizeCreateForScript( cacheKey, environment, @@ -401,6 +429,31 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } + /** + * Translate the shared build's recorded failure into a routing outcome for one caller. Called + * for every script joined to the build so a cancellation is reported as a cancellation to all + * of them rather than surfacing as a generic failure for the joiners. + */ + private notePendingCreationFailure( + scriptUri: Uri, + metadata: InlineScriptMetadata, + pendingCreation: PendingCreationContext, + ): void { + const failure = pendingCreation.failure; + if (!failure) { + return; + } + if (failure.cancelled) { + this.routingRegistry.noteSetupOutcome(scriptUri, { kind: 'cancelled' }); + return; + } + this.routingRegistry.noteSetupOutcome(scriptUri, { + kind: 'failed', + category: failure.category, + requiresPython: metadata.requiresPython, + }); + } + private getPendingSetupKey( scriptUri: Uri, metadata: InlineScriptMetadata, @@ -2228,6 +2281,194 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return run; } + /** + * Editing packages outside setup silently affects every script sharing the entry, so mark the + * entry non-reusable and un-route each of them; the next setup rebuilds from declared metadata. + */ + private async handlePackagesChanged(event: DidChangePackagesEventArgs): Promise { + if (this.disposed || event.environment.envId.managerId !== INLINE_SCRIPT_MANAGER_ID) { + return; + } + if (event.changes.length === 0) { + return; + } + const envDirPath = event.environment.sysPrefix; + // Setup installs through `managePackages`, which fires this event too. The emitter is + // synchronous, so a build still registered here owns this change and must not self-mark. + if (this.isBuildInFlightFor(envDirPath)) { + return; + } + const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); + const envDir = Uri.file(envDirPath); + if ((await inspectOwnedCacheEntry(event.environment, cacheRoot, envDir)) !== 'expected') { + return; + } + if (!(await this.markCacheEntryManuallyModified(envDir))) { + return; + } + this.unrouteScriptsUsingEnvironment(envDirPath); + } + + private isBuildInFlightFor(envDirPath: string): boolean { + return this.pendingCreations.has(path.basename(envDirPath)); + } + + /** Mark under the entry lock so it cannot race a concurrent build. */ + private async markCacheEntryManuallyModified(envDir: Uri): Promise { + try { + return await this.withCacheEntryLock(envDir, async () => { + // Re-check now that the build, if any, has released the lock. + if (this.isBuildInFlightFor(envDir.fsPath)) { + return false; + } + const sidecar = await inspectMetaJson(envDir); + if (sidecar.kind !== 'valid' || sidecar.metadata.manuallyModified) { + return false; + } + await writeMetaJson(envDir, { ...sidecar.metadata, manuallyModified: true }); + this.cacheMutationRevision += 1; + this.log.info( + `Inline-script environment packages were modified outside setup; it will be rebuilt on next setup: ${envDir.fsPath}`, + ); + return true; + }); + } catch (error) { + // A build holding the lock rewrites the sidecar anyway. + this.log.warn( + `Failed to record an inline-script package modification for ${envDir.fsPath}: ${getErrorMessage(error)}`, + ); + return false; + } + } + + /** Un-route every script associated with the given cache entry. */ + private unrouteScriptsUsingEnvironment(envDirPath: string): void { + for (const [scriptPath, association] of this.fsPathToPersistedAssociation.entries()) { + if (!isSameOrParentPath(envDirPath, association.environmentPath)) { + continue; + } + this.invalidateCachedAssociationValidation(scriptPath); + const uri = this.routingRegistry.getUri(scriptPath) ?? Uri.file(scriptPath); + this.routingRegistry.setValidatedAssociation(uri, false); + this.log.info(`Inline-script association for ${scriptPath} needs setup again after a package change.`); + } + } + + /** + * Follow a rename instead of dropping the association. + * + * A cache entry is keyed by the script's dependencies and base interpreter, never by its path, + * so renaming a script does not invalidate its environment. `PythonProjectManagerImpl` already + * rewrites the matching `python-envs.pythonProjects` entry to the new path, so clearing the + * association here would leave a managed inline-script project entry with no environment behind + * it. + * + * The moved record is re-validated afterwards rather than trusted: its metadata binding is + * content-derived, so if the file at the new path no longer matches, ordinary validation clears + * the association and the setup CodeLens returns. + */ + private async handleRenamedScripts( + files: readonly { readonly oldUri: Uri; readonly newUri: Uri }[], + ): Promise { + const transfers = await this.enqueueSelection(async () => { + await this.persistedAssociationsLoaded; + + const moves: RenamedScriptTransfer[] = []; + const clears: ScriptReference[] = []; + const seen = new Set(); + for (const { oldUri, newUri } of files) { + if (oldUri.scheme !== 'file') { + continue; + } + const oldPath = normalizePath(oldUri.fsPath); + if (seen.has(oldPath)) { + continue; + } + seen.add(oldPath); + if (!this.fsPathToEnv.has(oldPath) && !this.fsPathToPersistedAssociation.has(oldPath)) { + continue; + } + const record = this.fsPathToPersistedAssociation.get(oldPath); + // Only follow the rename when the destination is still a routable local `.py` file; + // renaming to another extension (or off the local filesystem) drops the association. + const newPath = getInlineScriptRoutingKey(newUri); + if (!record || newPath === undefined || newPath === oldPath) { + clears.push({ uri: oldUri, scriptPath: oldPath }); + continue; + } + moves.push({ + oldUri, + oldPath, + newUri, + newPath, + record, + environment: this.fsPathToEnv.get(oldPath), + }); + } + + if (moves.length === 0 && clears.length === 0) { + return []; + } + + await this.updatePersistedAssociations([ + ...clears.map(({ scriptPath }) => ({ scriptPath })), + // Remove the old key and write the new one in the same transaction. Writing the new + // key unconditionally means a rename onto an already-associated script replaces it. + ...moves.flatMap((move) => [ + { scriptPath: move.oldPath, expectedPersistedAssociation: move.record }, + { scriptPath: move.newPath, persistedAssociation: move.record }, + ]), + ]); + + for (const clear of clears) { + this.forgetScriptAssociationState(clear.scriptPath); + this.clearValidatedRouteableState(clear.uri); + } + for (const move of moves) { + this.forgetScriptAssociationState(move.oldPath); + this.clearValidatedRouteableState(move.oldUri); + // Reset validation bookkeeping for the destination without discarding the record + // `updatePersistedAssociations` just wrote for it. + this.bumpAssociationRevision(move.newPath); + this.pendingRehydrations.delete(move.newPath); + this.pendingMetadataRefreshes.delete(move.newPath); + if (move.environment) { + this.fsPathToEnv.set(move.newPath, move.environment); + } else { + this.fsPathToEnv.delete(move.newPath); + } + this.clearValidatedRouteableState(move.newUri); + this.log.info(`Moved inline-script association from ${move.oldPath} to ${move.newPath}.`); + } + return moves; + }); + + // Re-validate outside the selection queue, mirroring how activation primes associations. + for (const move of transfers) { + await this.seedRoutingMetadataFromSavedFile(move.newUri, move.newPath); + const metadata = this.routingRegistry.getMetadata(move.newPath); + const metadataIdentity = metadata ? getInlineScriptMetadataRoutingIdentity(metadata) : undefined; + if (!metadata || !metadataIdentity) { + continue; + } + const uri = this.routingRegistry.getUri(move.newPath) ?? move.newUri; + await this.refreshValidatedAssociationForMetadata( + uri, + metadata, + metadataIdentity, + this.routingRegistry.getMetadataRevision(uri), + ); + } + } + + private forgetScriptAssociationState(scriptPath: string): void { + this.bumpAssociationRevision(scriptPath); + this.pendingRehydrations.delete(scriptPath); + this.pendingMetadataRefreshes.delete(scriptPath); + this.fsPathToEnv.delete(scriptPath); + this.fsPathToPersistedAssociation.delete(scriptPath); + } + private clearAssociationsForScripts(scripts: readonly Uri[]): Promise { return this.enqueueSelection(async () => { await this.persistedAssociationsLoaded; @@ -2658,7 +2899,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { try { await fs.ensureDir(cacheRoot.fsPath); - return await this.withCacheEntryLock(envDir, async (lock) => { + return await this.withCacheEntryLock(envDir, async () => { const cached = await this.inspectCacheEntry(cacheRoot, envDir, metadata, selectedBase, pendingCreation); if (cached.kind === 'reusable') { this.sendInlineScriptEnvReuseHitTelemetry(dependencyCount); @@ -2668,11 +2909,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.log.warn( `Preserving an inline-script cache entry that could not be safely inspected: ${envDir.fsPath}`, ); + pendingCreation.failure = { category: 'setup-failure' }; this.sendInlineScriptEnvErrorTelemetry('setup-failure'); return undefined; } if (cached.kind === 'stale') { - if (!(await this.removeCacheEntry(envDir))) { + if (!(await this.discardCacheEntry(envDir))) { + // The sidecar is gone, so the entry is inert; a later attempt retries the + // deletion once the files are released. + pendingCreation.failure = { category: 'setup-failure' }; this.sendInlineScriptEnvErrorTelemetry('setup-failure'); return undefined; } @@ -2687,26 +2932,23 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { pendingCreation, scriptUri, ); - if (build.retainLock) { - try { - await lock.retain(); - } catch (error) { - this.log.error( - `Failed to mark the inline-script cache lock as retained: ${getErrorMessage(error)}`, - ); - } - } if (build.environment) { this.sendInlineScriptEnvCreatedTelemetry(buildStartAtMs, dependencyCount); return build.environment; } if (build.errorCategory) { + pendingCreation.failure = { + category: build.errorCategory, + ...(build.errorCategory === 'package-install-cancelled' ? { cancelled: true } : {}), + }; this.sendInlineScriptEnvErrorTelemetry(build.errorCategory); } return undefined; }); } catch (error) { - this.sendInlineScriptEnvErrorTelemetry(this.getCreateOrReuseErrorCategory(error)); + const category = this.getCreateOrReuseErrorCategory(error); + pendingCreation.failure = { category }; + this.sendInlineScriptEnvErrorTelemetry(category); this.log.error(`Failed to create or reuse inline-script cache entry: ${getErrorMessage(error)}`); return undefined; } @@ -2759,6 +3001,13 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }; } const sidecar = sidecarResult.metadata; + if (sidecar.manuallyModified) { + // Packages changed outside setup, so the entry no longer matches its dependencies. + this.log.info( + `Rebuilding an inline-script cache entry whose packages were modified outside setup: ${envDir.fsPath}`, + ); + return { kind: 'stale' }; + } if (!this.matchesSelectedBase(sidecar, selectedBase)) { return { kind: 'stale' }; } @@ -2847,10 +3096,11 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } if (result?.pkgInstallationCancelled) { - this.log.warn( - 'Inline-script package installation was cancelled; retaining the cache lock until explicit cleanup.', + this.log.info( + 'Inline-script package installation was cancelled; discarding the incomplete environment.', ); - return { retainLock: true, errorCategory: 'package-install-cancelled' }; + await this.discardCacheEntry(envDir); + return { errorCategory: 'package-install-cancelled' }; } if (!result?.environment || result.envCreationErr || result.pkgInstallationErr) { const error = @@ -3484,14 +3734,58 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return new Map(Array.from(scriptPaths, (scriptPath) => [scriptPath, this.fsPathToEnv.get(scriptPath)])); } - private async removeCacheEntry(envDir: Uri): Promise { + /** + * Make a cache entry permanently unreusable, then best-effort delete it. + * + * The sidecar is removed first and is the correctness guarantee: `inspectCacheEntry` treats a + * missing/invalid `.meta.json` as `stale`, so an entry whose directory survives is inert and + * gets rebuilt (or swept by TTL eviction) rather than reused. Directory removal is retried + * briefly because a just-stopped installer can hold file handles for a short while, + * especially on Windows. + */ + private async discardCacheEntry(envDir: Uri): Promise { + await this.removeCacheEntrySidecars(envDir); + return this.removeCacheEntry(envDir); + } + + /** Delete `.meta.json` and any backup sidecars so a surviving directory cannot be revalidated. */ + private async removeCacheEntrySidecars(envDir: Uri): Promise { + let entries: string[]; try { - await fs.remove(envDir.fsPath); - return true; + entries = await fs.readdir(envDir.fsPath); } catch (error) { - this.log.error(`Failed to remove incomplete inline-script environment: ${getErrorMessage(error)}`); - return false; + if (!isFileNotFoundError(error)) { + this.log.warn(`Failed to scan inline-script cache sidecars: ${getErrorMessage(error)}`); + } + return; } + const sidecars = entries.filter( + (entry) => entry === META_JSON_FILENAME || entry.startsWith(`${META_JSON_FILENAME}.backup-`), + ); + for (const sidecar of sidecars) { + try { + await fs.remove(path.join(envDir.fsPath, sidecar)); + } catch (error) { + this.log.warn(`Failed to remove inline-script cache sidecar ${sidecar}: ${getErrorMessage(error)}`); + } + } + } + + private async removeCacheEntry(envDir: Uri): Promise { + let lastError: unknown; + for (let attempt = 0; attempt < CACHE_ENTRY_REMOVAL_ATTEMPTS; attempt += 1) { + try { + await fs.remove(envDir.fsPath); + return true; + } catch (error) { + lastError = error; + if (attempt < CACHE_ENTRY_REMOVAL_ATTEMPTS - 1) { + await timeout(CACHE_ENTRY_REMOVAL_RETRY_MS * (attempt + 1)); + } + } + } + this.log.error(`Failed to remove incomplete inline-script environment: ${getErrorMessage(lastError)}`); + return false; } private isDefinitivelyStalePathError(error: unknown): boolean { @@ -3597,6 +3891,15 @@ interface ScriptReference { readonly scriptPath: string; } +interface RenamedScriptTransfer { + readonly oldUri: Uri; + readonly oldPath: string; + readonly newUri: Uri; + readonly newPath: string; + readonly record: PersistedAssociationRecord; + readonly environment: PythonEnvironment | undefined; +} + interface PendingScriptUpdate extends ScriptReference { readonly before: PythonEnvironment | undefined; readonly persistedAssociation?: PersistedAssociationRecord; diff --git a/src/test/features/inlineScript/setupEnvironment.unit.test.ts b/src/test/features/inlineScript/setupEnvironment.unit.test.ts index 4a16c5c5..9f3ff1a8 100644 --- a/src/test/features/inlineScript/setupEnvironment.unit.test.ts +++ b/src/test/features/inlineScript/setupEnvironment.unit.test.ts @@ -13,6 +13,7 @@ import { InlineScriptRoutingRegistry } from '../../../common/inlineScript/routin import * as winapi from '../../../common/window.apis'; import * as wapi from '../../../common/workspace.apis'; import { + notifyInlineScriptSetupOutcome, setUpInlineScriptEnvironment, setUpInlineScriptEnvironmentsInWorkspace, } from '../../../features/inlineScript/setupEnvironment'; @@ -91,6 +92,20 @@ suite('setUpInlineScriptEnvironment', () => { em.verify((m) => m.setEnvironment(scriptUri, env), typemoq.Times.once()); }); + test('retires a previous cancellation once setup succeeds', async () => { + // Outcomes are read non-consumingly, so a stale cancellation must not survive a later + // successful setup and be reported against it. + const env = makeEnv(); + routing.noteSetupOutcome(scriptUri, { kind: 'cancelled' }); + manager.setup((m) => m.create(scriptUri, undefined)).returns(() => Promise.resolve(env)); + em.setup((m) => m.setEnvironment(scriptUri, env)).returns(() => Promise.resolve()); + + const result = await setUpInlineScriptEnvironment(scriptUri, em.object, routing); + + assert.strictEqual(result, env); + assert.strictEqual(routing.getSetupOutcome(scriptUri), undefined); + }); + test('publishes saved metadata for a closed script so its project can route', async () => { // The lazy detector only observes open documents, so bulk setup of a closed script would // otherwise leave the registry with no metadata and the script permanently non-routeable. @@ -209,4 +224,92 @@ suite('setUpInlineScriptEnvironmentsInWorkspace', () => { manager.verify((m) => m.create(withoutMeta, undefined), typemoq.Times.never()); em.verify((m) => m.setEnvironment(withMeta, env), typemoq.Times.once()); }); + + test('stops the whole run and reports when a script setup is cancelled', async () => { + const warningStub = sinon.stub(winapi, 'showWarningMessage').resolves(undefined); + // The picker sorts by label, so `a_` runs before `z_`. + const first = Uri.file('/workspace/a_first.py'); + const second = Uri.file('/workspace/z_second.py'); + findFilesStub.resolves([first, second]); + readMetadataStub.resolves(makeMetadata(['requests'])); + quickPickStub.callsFake((items) => items); + manager + .setup((m) => m.create(first, undefined)) + .returns(async () => { + routing.noteSetupOutcome(first, { kind: 'cancelled' }); + return undefined; + }); + + await setUpInlineScriptEnvironmentsInWorkspace(em.object, routing); + + manager.verify((m) => m.create(first, undefined), typemoq.Times.once()); + manager.verify((m) => m.create(second, undefined), typemoq.Times.never()); + assert.strictEqual( + warningStub.firstCall.args[0], + 'Environment setup was canceled. Set up 0 of 2 selected inline script environment(s); the remaining 1 were not started.', + ); + sinon.assert.notCalled(infoStub); + }); + + test('reports failures instead of silently counting them as successes', async () => { + const warningStub = sinon.stub(winapi, 'showWarningMessage').resolves(undefined); + findFilesStub.resolves([withMeta]); + readMetadataStub.resolves(makeMetadata(['requests'])); + quickPickStub.callsFake((items) => items); + manager + .setup((m) => m.create(withMeta, undefined)) + .returns(async () => { + routing.noteSetupOutcome(withMeta, { kind: 'failed', category: 'install-failure' }); + return undefined; + }); + + await setUpInlineScriptEnvironmentsInWorkspace(em.object, routing); + + assert.strictEqual( + warningStub.firstCall.args[0], + 'Set up 0 of 1 selected inline script environment(s). 1 failed — see the Python Environments output for details.', + ); + sinon.assert.notCalled(infoStub); + }); +}); + +suite('notifyInlineScriptSetupOutcome', () => { + const scriptUri = Uri.file('/workspace/script.py'); + let routing: InlineScriptRoutingRegistry; + + setup(() => { + routing = new InlineScriptRoutingRegistry(); + }); + + teardown(() => { + routing.dispose(); + sinon.restore(); + }); + + test('reports cancellation as an information message with nothing to clean up', () => { + const infoStub = sinon.stub(winapi, 'showInformationMessage').resolves(undefined); + const errorStub = sinon.stub(winapi, 'showErrorMessage').resolves(undefined); + const warningStub = sinon.stub(winapi, 'showWarningMessage').resolves(undefined); + routing.noteSetupOutcome(scriptUri, { kind: 'cancelled' }); + + notifyInlineScriptSetupOutcome(scriptUri, routing); + + // The exact message matters: cancelling must not ask the user to clean up, retry, or + // mention quarantined state, because the incomplete environment is already discarded. + assert.strictEqual(infoStub.firstCall.args[0], 'Environment setup was canceled.'); + sinon.assert.notCalled(errorStub); + sinon.assert.notCalled(warningStub); + }); + + test('lets coalesced setup callers observe the same outcome', () => { + const infoStub = sinon.stub(winapi, 'showInformationMessage').resolves(undefined); + const errorStub = sinon.stub(winapi, 'showErrorMessage').resolves(undefined); + routing.noteSetupOutcome(scriptUri, { kind: 'cancelled' }); + + notifyInlineScriptSetupOutcome(scriptUri, routing); + notifyInlineScriptSetupOutcome(scriptUri, routing); + + sinon.assert.calledTwice(infoStub); + sinon.assert.notCalled(errorStub); + }); }); diff --git a/src/test/features/terminal/shellStartupActivationVariablesManager.unit.test.ts b/src/test/features/terminal/shellStartupActivationVariablesManager.unit.test.ts new file mode 100644 index 00000000..9ff6b1cd --- /dev/null +++ b/src/test/features/terminal/shellStartupActivationVariablesManager.unit.test.ts @@ -0,0 +1,142 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { Disposable, GlobalEnvironmentVariableCollection, Uri, WorkspaceFolder } from 'vscode'; + +import { DidChangeEnvironmentEventArgs, PythonEnvironment, PythonProjectEnvironmentApi } from '../../../api'; +import * as workspaceApis from '../../../common/workspace.apis'; +import { ShellStartupActivationVariablesManagerImpl } from '../../../features/terminal/shellStartupActivationVariablesManager'; +import { ShellEnvsProvider } from '../../../features/terminal/shells/startupProvider'; +import * as terminalUtils from '../../../features/terminal/utils'; + +function makeEnvironment(id: string, managerId: string): PythonEnvironment { + return { + envId: { id, managerId }, + name: id, + displayName: id, + displayPath: `/envs/${id}`, + version: '3.12.4', + environmentPath: Uri.file(`/envs/${id}/bin/python`), + sysPrefix: `/envs/${id}`, + execInfo: { run: { executable: `/envs/${id}/bin/python` } }, + } as unknown as PythonEnvironment; +} + +class RecordingEnvsProvider implements ShellEnvsProvider { + public readonly shellType = 'pwsh'; + public readonly updated: PythonEnvironment[] = []; + public removeCalls = 0; + + updateEnvVariables(_collection: unknown, env: PythonEnvironment): void { + this.updated.push(env); + } + + removeEnvVariables(): void { + this.removeCalls += 1; + } + + getEnvVariables(): Map | undefined { + return undefined; + } +} + +suite('ShellStartupActivationVariablesManager', () => { + const folderUri = Uri.file('/workspace'); + const workspaceFolder = { uri: folderUri, name: 'workspace', index: 0 } as WorkspaceFolder; + const folderEnvironment = makeEnvironment('folder-venv', 'ms-python.python:venv'); + const scriptEnvironment = makeEnvironment('script-env', 'ms-python.python:inline-script'); + + let provider: RecordingEnvsProvider; + let scopedCollection: object; + let envCollection: GlobalEnvironmentVariableCollection; + let getEnvironmentStub: sinon.SinonStub; + let changeListener: ((e: DidChangeEnvironmentEventArgs) => Promise) | undefined; + let manager: ShellStartupActivationVariablesManagerImpl; + + setup(() => { + provider = new RecordingEnvsProvider(); + scopedCollection = {}; + envCollection = { + description: undefined, + getScoped: () => scopedCollection, + } as unknown as GlobalEnvironmentVariableCollection; + + sinon.stub(terminalUtils, 'getAutoActivationType').returns(terminalUtils.ACT_TYPE_SHELL); + sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(workspaceFolder); + sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns(new Disposable(() => undefined)); + + getEnvironmentStub = sinon.stub().resolves(folderEnvironment); + const api = { + getEnvironment: getEnvironmentStub, + setEnvironment: sinon.stub().resolves(), + onDidChangeEnvironment: (listener: (e: DidChangeEnvironmentEventArgs) => Promise) => { + changeListener = listener; + return new Disposable(() => undefined); + }, + } as unknown as PythonProjectEnvironmentApi; + + manager = new ShellStartupActivationVariablesManagerImpl(envCollection, [provider], api); + }); + + teardown(() => { + manager.dispose(); + sinon.restore(); + }); + + test('does not write a file-scoped inline-script environment into folder startup variables', async () => { + assert.ok(changeListener, 'expected the manager to subscribe to environment changes'); + + // A PEP 723 script selection fires with the `.py` file uri and the script's own environment. + await changeListener!({ + uri: Uri.file('/workspace/script.py'), + new: scriptEnvironment, + old: undefined, + }); + + assert.deepStrictEqual( + provider.updated.map((env) => env.envId.id), + ['folder-venv'], + 'the folder default should be written, never the script environment', + ); + sinon.assert.calledOnceWithExactly(getEnvironmentStub, folderUri); + }); + + test('writes the folder environment resolved at folder scope, not the event payload', async () => { + await changeListener!({ + uri: folderUri, + new: makeEnvironment('stale-payload', 'ms-python.python:venv'), + old: undefined, + }); + + assert.deepStrictEqual( + provider.updated.map((env) => env.envId.id), + ['folder-venv'], + ); + }); + + test('removes startup variables when the folder has no environment', async () => { + getEnvironmentStub.resolves(undefined); + + await changeListener!({ + uri: Uri.file('/workspace/script.py'), + new: scriptEnvironment, + old: undefined, + }); + + assert.strictEqual(provider.updated.length, 0); + assert.strictEqual(provider.removeCalls, 1); + }); + + test('ignores environment changes when shell startup activation is off', async () => { + (terminalUtils.getAutoActivationType as sinon.SinonStub).returns('command'); + + await changeListener!({ + uri: Uri.file('/workspace/script.py'), + new: scriptEnvironment, + old: undefined, + }); + + assert.strictEqual(provider.updated.length, 0); + assert.strictEqual(provider.removeCalls, 0); + sinon.assert.notCalled(getEnvironmentStub); + }); +}); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 7d8c71e2..815bf95a 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -9,6 +9,7 @@ import * as path from 'path'; import * as sinon from 'sinon'; import { Disposable, LogOutputChannel, Memento, TextDocument, Uri } from 'vscode'; import { + DidChangePackagesEventArgs, EnvironmentChangeKind, EnvironmentManager, PythonEnvironment, @@ -101,6 +102,7 @@ suite('InlineScriptEnvManager', () => { let api: PythonEnvironmentApi; let apiGetEnvironmentsStub: sinon.SinonStub; let apiRefreshEnvironmentsStub: sinon.SinonStub; + let packagesChangedListener: ((e: DidChangePackagesEventArgs) => unknown) | undefined; let baseEnvironment: PythonEnvironment; let baseExecutable: string; let baseManager: EnvironmentManager; @@ -150,6 +152,10 @@ suite('InlineScriptEnvManager', () => { api = { getEnvironments: apiGetEnvironmentsStub, refreshEnvironments: apiRefreshEnvironmentsStub, + onDidChangePackages: (listener: (e: DidChangePackagesEventArgs) => unknown) => { + packagesChangedListener = listener; + return new Disposable(() => undefined); + }, } as unknown as PythonEnvironmentApi; nativeFinder = {} as NativePythonFinder; routingRegistry = new InlineScriptRoutingRegistry(); @@ -2300,8 +2306,115 @@ suite('InlineScriptEnvManager', () => { }); }); + suite('package drift', () => { + // The listener is fire-and-forget, so wait for its async work rather than the call. + function firePackagesChanged(environment: PythonEnvironment): void { + assert.ok(packagesChangedListener, 'expected the manager to subscribe to package changes'); + packagesChangedListener!({ + environment, + manager: {} as never, + changes: [{ kind: 0 as never, pkg: {} as never }], + }); + } + + function sidecarFor(environment: PythonEnvironment): cacheLayout.InlineScriptEnvMeta | undefined { + const entry = sidecarsByEnvDir.get(normalizePath(environment.sysPrefix)); + return typeof entry === 'string' ? undefined : entry; + } + + test('marks an owned entry manually modified and un-routes every script using it', async () => { + const first = scriptUri('shared_one.py'); + const second = scriptUri('shared_two.py'); + const environment = await createOwnedEnvironment(); + await manager.set(first, environment); + await manager.set(second, environment); + routingRegistry.setValidatedAssociation(first, true); + routingRegistry.setValidatedAssociation(second, true); + + firePackagesChanged(environment); + await waitForCondition( + () => sidecarFor(environment)?.manuallyModified === true, + 'the modified cache entry should be marked in its sidecar', + ); + + // Both scripts sharing the entry lose routing, not just the edited one. + assert.strictEqual(routingRegistry.hasValidatedAssociation(first), false); + assert.strictEqual(routingRegistry.hasValidatedAssociation(second), false); + }); + + test('rebuilds instead of reusing an entry whose packages were modified', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + firePackagesChanged(environment); + await waitForCondition( + () => sidecarFor(environment)?.manuallyModified === true, + 'the modified cache entry should be marked in its sidecar', + ); + createWithProgressStub.resetHistory(); + + await manager.create(uri); + + assert.strictEqual( + createWithProgressStub.callCount, + 1, + 'a drifted entry must be rebuilt, not reused', + ); + }); + + test('ignores package changes for environments it does not own', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + routingRegistry.setValidatedAssociation(uri, true); + + firePackagesChanged({ + ...environment, + envId: { managerId: 'ms-python.python:venv', id: 'some-venv' }, + }); + await nextTurn(); + await nextTurn(); + await nextTurn(); + + assert.strictEqual(sidecarFor(environment)?.manuallyModified, undefined); + assert.strictEqual(routingRegistry.hasValidatedAssociation(uri), true); + }); + test('does not mark an entry while its own build is in flight', async () => { + // The real createWithProgress installs through managePackages, which fires this same + // event while the build still holds the cache-entry lock, so setup must not mark the + // entry it is building. Two entries are used so the outcomes are distinguishable: the + // unguarded entry settling proves the guarded one had at least as long to settle. + const building = await createOwnedEnvironment(); + const edited = await createOwnedEnvironment('bbbbbbbbbbbbbbbb'); + const buildingScript = scriptUri('building.py'); + await manager.set(buildingScript, building); + routingRegistry.setValidatedAssociation(buildingScript, true); + const pendingCreations = ( + manager as unknown as { pendingCreations: Map } + ).pendingCreations; + pendingCreations.set(CACHE_KEY, {}); + + firePackagesChanged(building); + firePackagesChanged(edited); + await waitForCondition( + () => sidecarFor(edited)?.manuallyModified === true, + 'a package change outside a build should be recorded', + ); + await nextTurn(); + await nextTurn(); + + assert.strictEqual( + sidecarFor(building)?.manuallyModified, + undefined, + 'setup must not mark the entry it is building', + ); + assert.strictEqual(routingRegistry.hasValidatedAssociation(buildingScript), true); + }); + }); + suite('transaction rollback', () => { - test('retains the partial environment and lock when package installation is cancelled', async () => { + test('discards the partial environment when package installation is cancelled', async () => { + const uri = scriptUri(); createWithProgressStub.callsFake(async (...args: unknown[]) => { const target = args[6] as string; await fs.outputFile(venvPythonPath(target), ''); @@ -2317,29 +2430,94 @@ suite('InlineScriptEnvManager', () => { }; }); - assert.strictEqual(await manager.create(scriptUri()), undefined); - assert.strictEqual(await fs.pathExists(envDir().fsPath), true); + assert.strictEqual(await manager.create(uri), undefined); + assert.strictEqual(await fs.pathExists(envDir().fsPath), false); assert.strictEqual(writeMetaStub.callCount, 0); - assert.ok(retainLockStub.calledOnce); + sinon.assert.notCalled(retainLockStub); assert.ok(releaseLockStub.calledOnce); + assert.deepStrictEqual(routingRegistry.takeSetupOutcome(uri), { kind: 'cancelled' }); }); - test('keeps a failed lock-retain transition fail-closed', async () => { - createWithProgressStub.resolves({ - environment: makeEnvironment( - 'ms-python.python:inline-script', - '3.12.4', - venvPythonPath(envDir().fsPath), - envDir().fsPath, - ), - pkgInstallationErr: 'Canceled', - pkgInstallationCancelled: true, + test('leaves a cancelled entry unreusable when its directory cannot be removed', async () => { + const uri = scriptUri(); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + await fs.outputFile(venvPythonPath(target), ''); + await fs.outputFile(path.join(target, '.meta.json'), '{}'); + await fs.outputFile(path.join(target, '.meta.json.backup-0123456789ab'), '{}'); + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(target), + target, + ), + pkgInstallationErr: 'Canceled', + pkgInstallationCancelled: true, + }; }); - retainLockStub.rejects(Object.assign(new Error('retention failed'), { code: 'EACCES' })); + const internalManager = manager as unknown as { + removeCacheEntry(candidate: Uri): Promise; + }; + sinon.stub(internalManager, 'removeCacheEntry').resolves(false); - assert.strictEqual(await manager.create(scriptUri()), undefined); - assert.ok(retainLockStub.calledOnce); - assert.ok(releaseLockStub.calledOnce); + assert.strictEqual(await manager.create(uri), undefined); + + // The directory survives, but every sidecar is gone so it can never be revalidated. + assert.strictEqual(await fs.pathExists(envDir().fsPath), true); + assert.strictEqual(await fs.pathExists(path.join(envDir().fsPath, '.meta.json')), false); + assert.strictEqual( + await fs.pathExists(path.join(envDir().fsPath, '.meta.json.backup-0123456789ab')), + false, + ); + sinon.assert.notCalled(retainLockStub); + assert.deepStrictEqual(routingRegistry.takeSetupOutcome(uri), { kind: 'cancelled' }); + }); + + test('reports cancellation to every script joined to the same build', async () => { + // Two scripts with identical dependencies resolve to one cache entry and therefore join + // a single in-flight build. Cancelling it must be reported as a cancellation to both, + // not just to the script that started it. + const starter = scriptUri('shared_starter.py'); + const joiner = scriptUri('shared_joiner.py'); + let notifyBuildStarted: () => void = () => undefined; + let releaseBuild: () => void = () => undefined; + const buildStarted = new Promise((resolve) => { + notifyBuildStarted = resolve; + }); + const buildGate = new Promise((resolve) => { + releaseBuild = resolve; + }); + createWithProgressStub.callsFake(async (...args: unknown[]) => { + const target = args[6] as string; + notifyBuildStarted(); + await buildGate; + await fs.outputFile(venvPythonPath(target), ''); + return { + environment: makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + venvPythonPath(target), + target, + ), + pkgInstallationErr: 'Canceled', + pkgInstallationCancelled: true, + }; + }); + + const starterCreate = manager.create(starter); + await buildStarted; + const joinerCreate = manager.create(joiner); + // Let the second request reach the shared pending creation before the build settles. + await nextTurn(); + await nextTurn(); + releaseBuild(); + + assert.strictEqual(await starterCreate, undefined); + assert.strictEqual(await joinerCreate, undefined); + assert.strictEqual(createWithProgressStub.callCount, 1, 'both scripts should share one build'); + assert.deepStrictEqual(routingRegistry.takeSetupOutcome(starter), { kind: 'cancelled' }); + assert.deepStrictEqual(routingRegistry.takeSetupOutcome(joiner), { kind: 'cancelled' }); }); test('removes the partial environment when package installation fails', async () => { @@ -5210,7 +5388,7 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(routingRegistry.hasValidatedAssociation(uri), false); }); - test('clears persisted association state for the old path when a script is renamed', async () => { + test('moves the persisted association to the new path when a script is renamed', async () => { const oldUri = scriptUri('old.py'); const newUri = scriptUri('new.py'); const environment = await createOwnedEnvironment(); @@ -5220,11 +5398,51 @@ suite('InlineScriptEnvManager', () => { await nextTurn(); await nextTurn(); + // The cache entry is keyed by dependencies + interpreter, not by path, so the + // environment survives the rename and follows the file. + assert.deepStrictEqual(Object.keys(persistedAssociations ?? {}), [normalizePath(newUri.fsPath)]); + assert.strictEqual(await manager.get(oldUri), undefined); + assert.strictEqual(routingRegistry.hasValidatedAssociation(oldUri), false); + assert.strictEqual(await manager.get(newUri), environment); + }); + + test('drops the association when a script is renamed to a non-python file', async () => { + const oldUri = scriptUri('old.py'); + const newUri = scriptUri('old.txt'); + const environment = await createOwnedEnvironment(); + await manager.set(oldUri, environment); + + fireRename(oldUri, newUri); + await nextTurn(); + await nextTurn(); + assert.deepStrictEqual(persistedAssociations, {}); assert.strictEqual(await manager.get(oldUri), undefined); assert.strictEqual(routingRegistry.hasValidatedAssociation(oldUri), false); }); + test('replaces an existing association when a script is renamed onto it', async () => { + const oldUri = scriptUri('old.py'); + const targetUri = scriptUri('target.py'); + const movedEnvironment = await createOwnedEnvironment(); + const replacedEnvironment = await createOwnedEnvironment('bbbbbbbbbbbbbbbb'); + await manager.set(targetUri, replacedEnvironment); + await manager.set(oldUri, movedEnvironment); + + fireRename(oldUri, targetUri); + await nextTurn(); + await nextTurn(); + + assert.deepStrictEqual(Object.keys(persistedAssociations ?? {}), [normalizePath(targetUri.fsPath)]); + const moved = (persistedAssociations as Record)[ + normalizePath(targetUri.fsPath) + ]; + assert.strictEqual( + normalizePath(moved.environmentPath), + normalizePath(movedEnvironment.environmentPath.fsPath), + ); + }); + test('removes and notifies for a warm association whose executable was deleted', async () => { const uri = scriptUri(); const environment = await createOwnedEnvironment(); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index d62def49..7483fba0 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -67,6 +67,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { .returns(new Disposable(() => undefined)); getPythonApiStub = sinon.stub(pythonApi, 'getPythonApi').resolves({ registerEnvironmentManager: registerEnvironmentManagerStub, + onDidChangePackages: () => new Disposable(() => undefined), } as unknown as PythonEnvironmentApi); });