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..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,19 +421,29 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } if (scope instanceof Uri) { - this.updateInlineRoutingOverride(scope, manager, environment); - this.clearInlineActiveSelection(scope, manager, inlineClearOperation); 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) { @@ -511,8 +522,20 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { await setAllManagerSettings(settings); } selections.forEach((selection) => { - this.updateInlineRoutingOverride(selection.scope, manager, environment); - this.clearInlineActiveSelection(selection.scope, manager, selection.inlineClearOperation); + // 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); + } if (!selection.publishInlineSelection) { return; } @@ -932,6 +955,28 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } + private commitInlineRoutingOperation( + scope: Uri, + manager: InternalEnvironmentManager, + selectionOperation: number, + inlineClearOperation?: number, + inlineOverrideHandoffOperation?: number, + ): boolean { + // 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( scope: Uri, manager: InternalEnvironmentManager, 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..a91d8e43 100644 --- a/src/test/features/envManagers.lastKnown.unit.test.ts +++ b/src/test/features/envManagers.lastKnown.unit.test.ts @@ -299,6 +299,145 @@ 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); + 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.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 () => { + 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('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');