From 104166d5351d0b96ac6b5fbbf2a4ebe4c60dc6b2 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 23 Jul 2026 16:24:56 -0700 Subject: [PATCH 1/2] Add generic environment-creation utilities Cross-process file lock, venv Python-path helper, cancellation-safe process runner, and createWithProgress tracking options that inline-script environment creation builds on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39dcc6a3-0fbd-4f36-9d0f-68677de49c27 --- src/common/lockfile.apis.ts | 135 +++++++++++ src/common/utils/virtualEnvironment.ts | 11 + src/managers/builtin/helpers.ts | 100 +++++---- src/managers/builtin/venvUtils.ts | 26 ++- src/test/common/lockfile.apis.unit.test.ts | 212 ++++++++++++++++++ .../common/virtualEnvironment.unit.test.ts | 27 +++ .../builtin/helpers.cancellation.unit.test.ts | 85 +++++++ .../venvUtils.createWithProgress.unit.test.ts | 134 +++++++++++ 8 files changed, 683 insertions(+), 47 deletions(-) create mode 100644 src/common/lockfile.apis.ts create mode 100644 src/common/utils/virtualEnvironment.ts create mode 100644 src/test/common/lockfile.apis.unit.test.ts create mode 100644 src/test/common/virtualEnvironment.unit.test.ts create mode 100644 src/test/managers/builtin/helpers.cancellation.unit.test.ts create mode 100644 src/test/managers/builtin/venvUtils.createWithProgress.unit.test.ts diff --git a/src/common/lockfile.apis.ts b/src/common/lockfile.apis.ts new file mode 100644 index 000000000..dd6bc3499 --- /dev/null +++ b/src/common/lockfile.apis.ts @@ -0,0 +1,135 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as crypto from 'crypto'; +import * as fsapi from 'fs-extra'; +import * as path from 'path'; + +/** Acquire an atomic lock released only explicitly; interrupted operations remain locked. */ +export async function acquireFileLock(filePath: string, options: AcquireFileLockOptions): Promise { + const lockPath = `${path.resolve(filePath)}.lock`; + const ownerMarker = path.join(lockPath, `owner-${process.pid}-${crypto.randomBytes(16).toString('hex')}`); + const retainedMarker = path.join(lockPath, 'retained'); + const deadline = Date.now() + options.timeoutMs; + + while (true) { + try { + await fsapi.mkdir(lockPath); + try { + await fsapi.writeFile(ownerMarker, '', { flag: 'wx' }); + } catch (error) { + try { + await fsapi.rmdir(lockPath); + } catch { + throw createLockError( + 'Lock initialization failed and left an owner-less lock directory', + 'ELOCKORPHANED', + lockPath, + ); + } + throw error; + } + + let state: LockState = 'held'; + return { + retain: async () => { + if (state !== 'held') { + return; + } + state = 'retained'; + try { + await fsapi.writeFile(retainedMarker, '', { flag: 'wx' }); + } catch (error) { + if (isAlreadyExistsError(error)) { + return; + } + try { + await fsapi.rename(ownerMarker, retainedMarker); + } catch (renameError) { + if (!isAlreadyExistsError(renameError)) { + throw createLockError( + 'Failed to mark the lock as retained', + 'ERETAINFAILED', + lockPath, + ); + } + } + } + }, + release: async () => { + if (state !== 'held') { + return; + } + state = 'released'; + try { + await fsapi.unlink(ownerMarker); + } catch (error) { + if (isFileNotFoundError(error)) { + throw createLockError('Lock ownership was compromised', 'ECOMPROMISED', lockPath); + } + throw error; + } + await fsapi.rmdir(lockPath); + }, + }; + } catch (error) { + if (!isAlreadyExistsError(error)) { + throw error; + } + if (await isRetainedLock(lockPath)) { + throw createLockError('Lock was retained after an interrupted operation', 'ELOCKRETAINED', lockPath); + } + if (Date.now() >= deadline) { + throw createLockError('Lock is already being held', 'ELOCKED', lockPath); + } + await delay(Math.min(options.retryIntervalMs, Math.max(0, deadline - Date.now()))); + } + } +} + +function isAlreadyExistsError(error: unknown): boolean { + return hasErrorCode(error, 'EEXIST'); +} + +function isFileNotFoundError(error: unknown): boolean { + return hasErrorCode(error, 'ENOENT'); +} + +async function isRetainedLock(lockPath: string): Promise { + try { + await fsapi.lstat(path.join(lockPath, 'retained')); + return true; + } catch (error) { + if (isFileNotFoundError(error)) { + return false; + } + throw error; + } +} + +function hasErrorCode(error: unknown, code: string): boolean { + return ( + typeof error === 'object' && error !== null && 'code' in error && (error as NodeJS.ErrnoException).code === code + ); +} + +function createLockError(message: string, code: string, lockPath: string): NodeJS.ErrnoException { + return Object.assign(new Error(message), { code, path: lockPath }); +} + +async function delay(milliseconds: number): Promise { + return new Promise((resolve) => setTimeout(resolve, milliseconds)); +} + +export interface AcquireFileLockOptions { + readonly timeoutMs: number; + readonly retryIntervalMs: number; +} + +export interface AcquiredFileLock { + readonly release: () => Promise; + /** Keep the lock and make later acquisition attempts fail immediately. */ + readonly retain: () => Promise; +} + +type LockState = 'held' | 'released' | 'retained'; diff --git a/src/common/utils/virtualEnvironment.ts b/src/common/utils/virtualEnvironment.ts new file mode 100644 index 000000000..c581bf521 --- /dev/null +++ b/src/common/utils/virtualEnvironment.ts @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as path from 'path'; +import { isWindows } from './platformUtils'; + +export function getVenvPythonPath(envPath: string): string { + return isWindows() + ? path.join(envPath, 'Scripts', 'python.exe') + : path.join(envPath, 'bin', 'python'); +} diff --git a/src/managers/builtin/helpers.ts b/src/managers/builtin/helpers.ts index 911bc6031..579892170 100644 --- a/src/managers/builtin/helpers.ts +++ b/src/managers/builtin/helpers.ts @@ -66,40 +66,14 @@ export async function runUV( token?: CancellationToken, timeout?: number, ): Promise { - log?.info(`Running: uv ${args.join(' ')}`); - return new Promise((resolve, reject) => { - const spawnOptions: { cwd?: string; timeout?: number } = { cwd }; - if (timeout !== undefined) { - spawnOptions.timeout = timeout; - } - const proc = spawnProcess('uv', args, spawnOptions); - token?.onCancellationRequested(() => { - proc.kill(); - reject(new CancellationError()); - }); - - proc.on('error', (err) => { - log?.error(`Error spawning uv: ${err}`); - reject(new Error(`Error spawning uv: ${err.message}`)); - }); - - let builder = ''; - proc.stdout?.on('data', (data) => { - const s = data.toString('utf-8'); - builder += s; - log?.append(s); - }); - proc.stderr?.on('data', (data) => { - log?.append(data.toString('utf-8')); - }); - proc.on('close', () => { - resolve(builder); - }); - proc.on('exit', (code) => { - if (code !== 0) { - reject(new Error(`Failed to run uv ${args.join(' ')}`)); - } - }); + return runProcess('uv', args, { + cwd, + displayName: 'uv', + log, + token, + timeout, + collectStderr: false, + logPrefix: '', }); } @@ -111,37 +85,75 @@ export async function runPython( token?: CancellationToken, timeout?: number, ): Promise { - log?.info(`Running: ${python} ${args.join(' ')}`); + return runProcess(python, args, { + cwd, + displayName: 'python', + log, + token, + timeout, + collectStderr: true, + logPrefix: 'python: ', + }); +} + +function runProcess(executable: string, args: string[], options: RunProcessOptions): Promise { + options.log?.info(`Running: ${executable} ${args.join(' ')}`); return new Promise((resolve, reject) => { - const proc = spawnProcess(python, args, { cwd: cwd, timeout }); - token?.onCancellationRequested(() => { - proc.kill(); + const spawnOptions: { cwd?: string; timeout?: number } = { cwd: options.cwd }; + if (options.timeout !== undefined) { + spawnOptions.timeout = options.timeout; + } + const proc = spawnProcess(executable, args, spawnOptions); + let cancellationRequested = false; + options.token?.onCancellationRequested(() => { + cancellationRequested = true; + try { + proc.kill(); + } catch { + // Preserve cancellation when signaling fails. + } reject(new CancellationError()); }); proc.on('error', (err) => { - log?.error(`Error spawning python: ${err}`); - reject(new Error(`Error spawning python: ${err.message}`)); + if (cancellationRequested) { + reject(new CancellationError()); + return; + } + options.log?.error(`Error spawning ${options.displayName}: ${err}`); + reject(new Error(`Error spawning ${options.displayName}: ${err.message}`)); }); let builder = ''; proc.stdout?.on('data', (data) => { const s = data.toString('utf-8'); builder += s; - log?.append(`python: ${s}`); + options.log?.append(`${options.logPrefix}${s}`); }); proc.stderr?.on('data', (data) => { const s = data.toString('utf-8'); - builder += s; - log?.append(`python: ${s}`); + if (options.collectStderr) { + builder += s; + } + options.log?.append(`${options.logPrefix}${s}`); }); proc.on('close', () => { resolve(builder); }); proc.on('exit', (code) => { if (code !== 0) { - reject(new Error(`Failed to run python ${args.join(' ')}`)); + reject(new Error(`Failed to run ${options.displayName} ${args.join(' ')}`)); } }); }); } + +interface RunProcessOptions { + readonly cwd?: string; + readonly displayName: 'python' | 'uv'; + readonly log?: LogOutputChannel; + readonly token?: CancellationToken; + readonly timeout?: number; + readonly collectStderr: boolean; + readonly logPrefix: string; +} diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index 4efab305e..887864385 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -1,7 +1,16 @@ import * as fsapi from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; -import { l10n, LogOutputChannel, ProgressLocation, QuickPickItem, QuickPickItemKind, ThemeIcon, Uri } from 'vscode'; +import { + CancellationError, + l10n, + LogOutputChannel, + ProgressLocation, + QuickPickItem, + QuickPickItemKind, + ThemeIcon, + Uri, +} from 'vscode'; import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../api'; import { ENVS_EXTENSION_ID } from '../../common/constants'; import { Common, VenvManagerStrings } from '../../common/localize'; @@ -10,6 +19,7 @@ import { getWorkspacePersistentState } from '../../common/persistentState'; import { EventNames } from '../../common/telemetry/constants'; import { sendTelemetryEvent } from '../../common/telemetry/sender'; import { normalizePath } from '../../common/utils/pathUtils'; +import { getVenvPythonPath } from '../../common/utils/virtualEnvironment'; import { showErrorMessage, showOpenDialog, @@ -52,6 +62,14 @@ export interface CreateEnvironmentResult { * Exists if error occurred while installing packages and includes error description. */ pkgInstallationErr?: string; + + /** Cancellation may leave package processes running. */ + pkgInstallationCancelled?: boolean; +} + +export interface CreateWithProgressOptions { + /** Whether to record uv-created environments in workspace state. Defaults to true. */ + readonly trackUvEnvironment?: boolean; } export async function clearVenvCache(): Promise { @@ -340,9 +358,9 @@ export async function createWithProgress( venvRoot: Uri, envPath: string, packages?: PipPackages, + options?: CreateWithProgressOptions, ): Promise { - const pythonPath = - os.platform() === 'win32' ? path.join(envPath, 'Scripts', 'python.exe') : path.join(envPath, 'bin', 'python'); + const pythonPath = getVenvPythonPath(envPath); return await withProgress( { @@ -383,6 +401,7 @@ export async function createWithProgress( const env = api.createPythonEnvironmentItem(await getPythonInfo(resolved), manager); if ( + options?.trackUvEnvironment !== false && useUv && (resolved.kind === NativePythonEnvironmentKind.venvUv || resolved.kind === NativePythonEnvironmentKind.uvWorkspace) @@ -401,6 +420,7 @@ export async function createWithProgress( } catch (e) { // error occurred while installing packages result.pkgInstallationErr = e instanceof Error ? e.message : String(e); + result.pkgInstallationCancelled = e instanceof CancellationError; } } result.environment = env; diff --git a/src/test/common/lockfile.apis.unit.test.ts b/src/test/common/lockfile.apis.unit.test.ts new file mode 100644 index 000000000..a2d343a19 --- /dev/null +++ b/src/test/common/lockfile.apis.unit.test.ts @@ -0,0 +1,212 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import { ChildProcessWithoutNullStreams, spawn } from 'child_process'; +import fsExtra from 'fs-extra'; +import * as fs from 'fs-extra'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { acquireFileLock, AcquireFileLockOptions } from '../../common/lockfile.apis'; + +const OPTIONS: AcquireFileLockOptions = { + timeoutMs: 40, + retryIntervalMs: 5, +}; + +const LOCK_MODULE_PATH = path.resolve(__dirname, '..', '..', 'common', 'lockfile.apis.js'); + +suite('lockfile APIs', () => { + let tempRoot: string; + let targetPath: string; + + setup(async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'python-envs-lock-')); + targetPath = path.join(tempRoot, 'cache-entry'); + }); + + teardown(async () => { + sinon.restore(); + await fs.remove(tempRoot); + }); + + function startLockingChild(exitWithoutRelease: boolean): ChildProcessWithoutNullStreams { + const script = ` + const { acquireFileLock } = require(process.argv[1]); + acquireFileLock(process.argv[2], { timeoutMs: 1000, retryIntervalMs: 10 }) + .then((lock) => { + process.stdout.write('locked\\n'); + if (${exitWithoutRelease}) { + process.exit(0); + } + process.stdin.once('data', async () => { + await lock.release(); + process.exit(0); + }); + }) + .catch((error) => { + process.stderr.write(String(error && error.stack ? error.stack : error)); + process.exit(1); + }); + `; + return spawn(process.execPath, ['-e', script, LOCK_MODULE_PATH, targetPath]); + } + + async function waitForLocked(child: ChildProcessWithoutNullStreams): Promise { + await new Promise((resolve, reject) => { + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (data) => { + stdout += data.toString(); + if (stdout.includes('locked')) { + resolve(); + } + }); + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + child.once('error', reject); + child.once('exit', (code) => { + if (!stdout.includes('locked')) { + reject(new Error(`locking child exited with code ${code}: ${stderr}`)); + } + }); + }); + } + + async function waitForExit(child: ChildProcessWithoutNullStreams): Promise { + if (child.exitCode !== null) { + assert.strictEqual(child.exitCode, 0); + return; + } + await new Promise((resolve, reject) => { + child.once('error', reject); + child.once('exit', (code) => { + if (code === 0) { + resolve(); + } else { + reject(new Error(`locking child exited with code ${code}`)); + } + }); + }); + } + + test('excludes a second owner until the first releases the lock', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKED'; + }); + + await lock.release(); + const lockAfterRetry = await acquireFileLock(targetPath, OPTIONS); + await lockAfterRetry.release(); + assert.strictEqual(await fs.pathExists(`${path.resolve(targetPath)}.lock`), false); + }); + + test('excludes another process until the owner explicitly releases', async () => { + const child = startLockingChild(false); + await waitForLocked(child); + + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKED'; + }); + + child.stdin.write('release\n'); + await waitForExit(child); + const lock = await acquireFileLock(targetPath, OPTIONS); + await lock.release(); + }); + + test('leaves the lock fail-closed when the owner process exits without release', async () => { + const child = startLockingChild(true); + await waitForLocked(child); + await waitForExit(child); + + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKED'; + }); + assert.strictEqual(await fs.pathExists(`${path.resolve(targetPath)}.lock`), true); + }); + + test('an old release cannot remove a successor lock generation', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + const lockPath = `${path.resolve(targetPath)}.lock`; + await fs.remove(lockPath); + await fs.ensureDir(lockPath); + const successorMarker = path.join(lockPath, 'successor-owner'); + await fs.writeFile(successorMarker, ''); + + await assert.rejects(lock.release(), (error: NodeJS.ErrnoException) => { + return error.code === 'ECOMPROMISED'; + }); + assert.strictEqual(await fs.pathExists(successorMarker), true); + }); + + test('release is idempotent', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + + await lock.release(); + await lock.release(); + + assert.strictEqual(await fs.pathExists(`${path.resolve(targetPath)}.lock`), false); + }); + + test('retained locks fail fast without waiting for the acquisition timeout', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + await lock.retain(); + const startedAt = Date.now(); + + await assert.rejects( + acquireFileLock(targetPath, { timeoutMs: 10_000, retryIntervalMs: 1_000 }), + (error: NodeJS.ErrnoException) => error.code === 'ELOCKRETAINED', + ); + + assert.ok(Date.now() - startedAt < 1_000); + const lockPath = `${path.resolve(targetPath)}.lock`; + const retainedEntries = await fs.readdir(lockPath); + assert.ok(retainedEntries.includes('retained')); + assert.strictEqual(retainedEntries.filter((entry) => entry.startsWith('owner-')).length, 1); + + await lock.release(); + assert.deepStrictEqual(await fs.readdir(lockPath), retainedEntries); + }); + + test('falls back to renaming the owner marker when the retained sentinel cannot be written', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + sinon.stub(fsExtra, 'writeFile').rejects(Object.assign(new Error('write failed'), { code: 'EACCES' })); + + await lock.retain(); + + const lockPath = `${path.resolve(targetPath)}.lock`; + assert.deepStrictEqual(await fs.readdir(lockPath), ['retained']); + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKRETAINED'; + }); + }); + + test('remains fail-closed when neither retained-marker strategy succeeds', async () => { + const lock = await acquireFileLock(targetPath, OPTIONS); + sinon.stub(fsExtra, 'writeFile').rejects(Object.assign(new Error('write failed'), { code: 'EACCES' })); + sinon.stub(fsExtra, 'rename').rejects(Object.assign(new Error('rename failed'), { code: 'EBUSY' })); + + await assert.rejects(lock.retain(), (error: NodeJS.ErrnoException) => error.code === 'ERETAINFAILED'); + await lock.release(); + + const lockPath = `${path.resolve(targetPath)}.lock`; + const lockEntries = await fs.readdir(lockPath); + assert.strictEqual(lockEntries.filter((entry) => entry.startsWith('owner-')).length, 1); + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKED'; + }); + }); + + test('reports an owner-less lock when initialization cleanup fails', async () => { + sinon.stub(fsExtra, 'writeFile').rejects(Object.assign(new Error('write failed'), { code: 'EIO' })); + sinon.stub(fsExtra, 'rmdir').rejects(Object.assign(new Error('cleanup failed'), { code: 'EACCES' })); + + await assert.rejects(acquireFileLock(targetPath, OPTIONS), (error: NodeJS.ErrnoException) => { + return error.code === 'ELOCKORPHANED' && error.path === `${path.resolve(targetPath)}.lock`; + }); + }); +}); diff --git a/src/test/common/virtualEnvironment.unit.test.ts b/src/test/common/virtualEnvironment.unit.test.ts new file mode 100644 index 000000000..093ab7104 --- /dev/null +++ b/src/test/common/virtualEnvironment.unit.test.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as platformUtils from '../../common/utils/platformUtils'; +import { getVenvPythonPath } from '../../common/utils/virtualEnvironment'; + +suite('virtual environment utilities', () => { + teardown(() => { + sinon.restore(); + }); + + test('uses Scripts/python.exe on Windows', () => { + sinon.stub(platformUtils, 'isWindows').returns(true); + assert.strictEqual( + getVenvPythonPath(path.join('cache', 'env')), + path.join('cache', 'env', 'Scripts', 'python.exe'), + ); + }); + + test('uses bin/python outside Windows', () => { + sinon.stub(platformUtils, 'isWindows').returns(false); + assert.strictEqual(getVenvPythonPath(path.join('cache', 'env')), path.join('cache', 'env', 'bin', 'python')); + }); +}); diff --git a/src/test/managers/builtin/helpers.cancellation.unit.test.ts b/src/test/managers/builtin/helpers.cancellation.unit.test.ts new file mode 100644 index 000000000..5a4583066 --- /dev/null +++ b/src/test/managers/builtin/helpers.cancellation.unit.test.ts @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as sinon from 'sinon'; +import { CancellationError, CancellationToken, CancellationTokenSource } from 'vscode'; +import * as childProcessApis from '../../../common/childProcess.apis'; +import { runPython, runUV } from '../../../managers/builtin/helpers'; +import { MockChildProcess } from '../../mocks/mockChildProcess'; + +suite('process helper cancellation safety', () => { + let spawnStub: sinon.SinonStub; + + setup(() => { + spawnStub = sinon.stub(childProcessApis, 'spawnProcess'); + }); + + teardown(() => { + sinon.restore(); + }); + + async function expectCancellation( + process: MockChildProcess, + run: (token: CancellationToken) => Promise, + killBehavior: 'emitError' | 'throw', + ): Promise { + spawnStub.returns(process); + const killStub = sinon.stub(process, 'kill'); + if (killBehavior === 'emitError') { + killStub.callsFake(() => { + process.emit('error', new Error('kill EPERM')); + return false; + }); + } else { + killStub.throws(new Error('kill EPERM')); + } + + const tokenSource = new CancellationTokenSource(); + const result = run(tokenSource.token); + tokenSource.cancel(); + + await assert.rejects(result, (error: Error) => { + assert.ok(error instanceof CancellationError); + return true; + }); + assert.ok(killStub.calledOnce); + tokenSource.dispose(); + } + + test('runUV remains cancelled when kill emits an error synchronously', async () => { + const process = new MockChildProcess('uv', ['pip', 'install', 'requests']); + await expectCancellation( + process, + (token) => runUV(['pip', 'install', 'requests'], undefined, undefined, token), + 'emitError', + ); + }); + + test('runUV remains cancelled when kill throws', async () => { + const process = new MockChildProcess('uv', ['pip', 'install', 'requests']); + await expectCancellation( + process, + (token) => runUV(['pip', 'install', 'requests'], undefined, undefined, token), + 'throw', + ); + }); + + test('runPython remains cancelled when kill emits an error synchronously', async () => { + const process = new MockChildProcess('python', ['-m', 'pip', 'install', 'requests']); + await expectCancellation( + process, + (token) => runPython('python', ['-m', 'pip', 'install', 'requests'], undefined, undefined, token), + 'emitError', + ); + }); + + test('runPython remains cancelled when kill throws', async () => { + const process = new MockChildProcess('python', ['-m', 'pip', 'install', 'requests']); + await expectCancellation( + process, + (token) => runPython('python', ['-m', 'pip', 'install', 'requests'], undefined, undefined, token), + 'throw', + ); + }); +}); diff --git a/src/test/managers/builtin/venvUtils.createWithProgress.unit.test.ts b/src/test/managers/builtin/venvUtils.createWithProgress.unit.test.ts new file mode 100644 index 000000000..846dc705a --- /dev/null +++ b/src/test/managers/builtin/venvUtils.createWithProgress.unit.test.ts @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as fs from 'fs-extra'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { CancellationError, LogOutputChannel, Uri } from 'vscode'; +import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi } from '../../../api'; +import * as windowApis from '../../../common/window.apis'; +import { getVenvPythonPath } from '../../../common/utils/virtualEnvironment'; +import * as builtinHelpers from '../../../managers/builtin/helpers'; +import * as uvEnvironments from '../../../managers/builtin/uvEnvironments'; +import { createWithProgress } from '../../../managers/builtin/venvUtils'; +import { NativePythonEnvironmentKind, NativePythonFinder } from '../../../managers/common/nativePythonFinder'; +import * as managerUtils from '../../../managers/common/utils'; + +suite('createWithProgress uv tracking options', () => { + let addUvEnvironmentStub: sinon.SinonStub; + let api: PythonEnvironmentApi; + let baseEnvironment: PythonEnvironment; + let envPath: string; + let log: LogOutputChannel; + let manager: EnvironmentManager; + let nativeFinder: NativePythonFinder; + let tempRoot: string; + + setup(async () => { + tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'create-with-progress-')); + envPath = path.join(tempRoot, 'env'); + const pythonPath = getVenvPythonPath(envPath); + await fs.outputFile(pythonPath, ''); + + baseEnvironment = { + envId: { id: 'base', managerId: 'ms-python.python:system' }, + name: 'base', + displayName: 'base', + displayPath: pythonPath, + version: '3.12.4', + environmentPath: Uri.file(pythonPath), + execInfo: { run: { executable: pythonPath } }, + sysPrefix: tempRoot, + }; + const createdEnvironment = { + ...baseEnvironment, + envId: { id: 'created', managerId: 'ms-python.python:inline-script' }, + }; + api = { + createPythonEnvironmentItem: sinon.stub().returns(createdEnvironment), + managePackages: sinon.stub().resolves(), + } as unknown as PythonEnvironmentApi; + nativeFinder = { + resolve: sinon.stub().resolves({ + executable: pythonPath, + prefix: envPath, + version: '3.12.4', + kind: NativePythonEnvironmentKind.venvUv, + }), + } as unknown as NativePythonFinder; + log = { + error: sinon.stub(), + info: sinon.stub(), + append: sinon.stub(), + } as unknown as LogOutputChannel; + manager = { log } as EnvironmentManager; + + sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never)); + sinon.stub(builtinHelpers, 'shouldUseUv').resolves(true); + sinon.stub(builtinHelpers, 'runUV').resolves(''); + sinon.stub(managerUtils, 'getShellActivationCommands').resolves({ + shellActivation: new Map(), + shellDeactivation: new Map(), + }); + addUvEnvironmentStub = sinon.stub(uvEnvironments, 'addUvEnvironment').resolves(); + }); + + teardown(async () => { + sinon.restore(); + await fs.remove(tempRoot); + }); + + test('tracks uv environments by default for existing callers', async () => { + const result = await createWithProgress( + nativeFinder, + api, + log, + manager, + baseEnvironment, + Uri.file(tempRoot), + envPath, + ); + + assert.ok(result?.environment); + assert.ok(addUvEnvironmentStub.calledOnce); + }); + + test('skips workspace-scoped uv tracking when explicitly disabled', async () => { + const result = await createWithProgress( + nativeFinder, + api, + log, + manager, + baseEnvironment, + Uri.file(tempRoot), + envPath, + undefined, + { trackUvEnvironment: false }, + ); + + assert.ok(result?.environment); + assert.strictEqual(addUvEnvironmentStub.callCount, 0); + }); + + test('marks cancelled package installation as potentially still mutating', async () => { + (api.managePackages as sinon.SinonStub).rejects(new CancellationError()); + + const result = await createWithProgress( + nativeFinder, + api, + log, + manager, + baseEnvironment, + Uri.file(tempRoot), + envPath, + { install: ['requests'], uninstall: [] }, + { trackUvEnvironment: false }, + ); + + assert.ok(result?.environment); + assert.strictEqual(typeof result.pkgInstallationErr, 'string'); + assert.strictEqual(result.pkgInstallationCancelled, true); + }); +}); From 73632319b58ac265126b5cf179b5c3db8a58f835 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 23 Jul 2026 16:25:11 -0700 Subject: [PATCH 2/2] Add inline-script cache and interpreter utilities for creation Cache-key tail normalization, cache-layout ownership/status checks with typed sidecar reads, and interpreter-constraint trimming that inline-script environment creation builds on. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39dcc6a3-0fbd-4f36-9d0f-68677de49c27 --- src/common/constants.ts | 2 + src/common/inlineScriptCacheKey.ts | 44 +++- src/common/inlineScriptCacheLayout.ts | 157 ++++++++---- src/common/inlineScriptInterpreter.ts | 3 +- .../common/inlineScriptCacheKey.unit.test.ts | 49 ++++ .../inlineScriptCacheLayout.unit.test.ts | 236 ++++++++++++++++-- .../inlineScriptInterpreter.unit.test.ts | 7 + 7 files changed, 426 insertions(+), 72 deletions(-) diff --git a/src/common/constants.ts b/src/common/constants.ts index 80de2b54e..1509f39c9 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -2,6 +2,8 @@ import * as path from 'path'; export const ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs'; export const PYTHON_EXTENSION_ID = 'ms-python.python'; +export const CONDA_MANAGER_ID = `${PYTHON_EXTENSION_ID}:conda`; +export const INLINE_SCRIPT_MANAGER_ID = `${PYTHON_EXTENSION_ID}:inline-script`; export const JUPYTER_EXTENSION_ID = 'ms-toolsai.jupyter'; export const EXTENSION_ROOT_DIR = path.dirname(__dirname); export const ISSUES_URL = 'https://github.com/microsoft/vscode-python-environments/issues'; diff --git a/src/common/inlineScriptCacheKey.ts b/src/common/inlineScriptCacheKey.ts index 13c946462..2ef4c964a 100644 --- a/src/common/inlineScriptCacheKey.ts +++ b/src/common/inlineScriptCacheKey.ts @@ -31,6 +31,40 @@ function normalizeExtras(inner: string): string { return deduped.length > 0 ? `[${deduped.join(',')}]` : ''; } +function normalizeRequirementTail(value: string): string { + let result = ''; + let unquoted = ''; + let quote: "'" | '"' | undefined; + let escaped = false; + + const flushUnquoted = () => { + result += unquoted.replace(/\s+/g, ' ').replace(/\s*([<>=!~]=?)\s*/g, '$1'); + unquoted = ''; + }; + + // Preserve PEP 508 marker literals verbatim; a backslash escapes the next character. + for (const character of value) { + if (quote) { + result += character; + if (character === quote && !escaped) { + quote = undefined; + } + escaped = character === '\\' && !escaped; + if (character !== '\\') { + escaped = false; + } + } else if (character === "'" || character === '"') { + flushUnquoted(); + quote = character; + result += character; + } else { + unquoted += character; + } + } + flushUnquoted(); + return result.trim(); +} + /** * Canonicalize a PEP 723 dependency entry so common variants of the same * requirement produce identical strings. Not a full PEP 508 parser — only @@ -70,10 +104,12 @@ export function normalizeDependency(dep: string): string { rest = rest.slice(extrasMatch[0].length); } - const compactedRest = rest - .replace(/\s+/g, ' ') - .replace(/\s*([<>=!~]=?)\s*/g, '$1') - .trim(); + const directReference = rest.trim(); + if (directReference.startsWith('@')) { + return `${name}${extras} ${directReference}`; + } + + const compactedRest = normalizeRequirementTail(rest); return `${name}${extras}${compactedRest}`; } diff --git a/src/common/inlineScriptCacheLayout.ts b/src/common/inlineScriptCacheLayout.ts index 72c83302a..c7e98ab31 100644 --- a/src/common/inlineScriptCacheLayout.ts +++ b/src/common/inlineScriptCacheLayout.ts @@ -5,16 +5,14 @@ import * as crypto from 'crypto'; import * as fsapi from 'fs-extra'; import * as path from 'path'; import { Uri } from 'vscode'; +import type { PythonEnvironment } from '../api'; +import { INLINE_SCRIPT_MANAGER_ID } from './constants'; import { traceWarn } from './logging'; +import { normalizePath } from './utils/pathUtils'; import { isWindows } from './utils/platformUtils'; +import { getVenvPythonPath } from './utils/virtualEnvironment'; -/** - * Versioned name of the cache root under the extension's `globalStorageUri`. - * - * Bump the `-v1` suffix together with {@link META_SCHEMA_VERSION} on any - * incompatible on-disk change, so old envs sit unread and TTL out naturally - * instead of being migrated in place. - */ +/** Bump this and {@link META_SCHEMA_VERSION} together for incompatible cache formats. */ export const INLINE_SCRIPT_CACHE_DIR_NAME = 'script-envs-v1'; export const META_JSON_FILENAME = '.meta.json'; @@ -33,14 +31,21 @@ const MAX_META_JSON_BYTES = 1024 * 1024; export interface InlineScriptEnvMeta { /** Version of the serialized metadata schema. */ readonly schemaVersion: typeof META_SCHEMA_VERSION; - /** Filesystem path of the script recorded for lifecycle bookkeeping. */ - readonly scriptFsPath: string; + /** Canonical base-interpreter path. */ + readonly baseInterpreterPath: string; + /** Base-interpreter version. */ + readonly baseInterpreterVersion: string; /** Last successful use as a canonical UTC string produced by `Date.toISOString()`. */ readonly lastUsedAt: string; - /** The script's `requires-python` declaration, when present. */ - readonly requiresPython?: string; } +export type InlineScriptMetaReadResult = + | { readonly kind: 'valid'; readonly metadata: InlineScriptEnvMeta } + | { readonly kind: 'missing' | 'invalid' | 'unavailable' }; + +export type BaseInterpreterStatus = 'available' | 'missing' | 'unavailable'; +export type CacheEnvironmentInspection = 'expected' | 'stale' | 'uncertain'; + /** * In-memory summary of one cached entry, populated by the separate disk walk. */ @@ -63,36 +68,77 @@ export function getMetaJsonPath(envDir: Uri): Uri { return Uri.joinPath(envDir, META_JSON_FILENAME); } -/** - * Read and validate the extension-owned `.meta.json` sidecar in a cached - * environment directory. - * - * This function is intentionally non-destructive. Missing, malformed, or - * unreadable sidecars return `undefined` so callers can treat the entry as a - * cache miss. Deleting invalid entries belongs to the dedicated, guarded cache - * cleanup path because read and permission failures may be transient. - */ +/** Resolve a cache entry only when it is the requested direct child of the physical cache root. */ +export async function resolveCacheEntryPath(cacheRoot: Uri, envDir: Uri): Promise { + const [resolvedRoot, resolvedEntry] = await Promise.all([ + fsapi.realpath(cacheRoot.fsPath), + fsapi.realpath(envDir.fsPath), + ]); + const expectedEntry = path.join(resolvedRoot, path.basename(envDir.fsPath)); + return isDescendantPath(resolvedRoot, resolvedEntry) && + normalizePath(path.resolve(resolvedEntry)) === normalizePath(path.resolve(expectedEntry)) + ? resolvedEntry + : undefined; +} + +/** Verify that a resolved environment is owned by the expected physical cache entry. */ +export async function inspectOwnedCacheEntry( + environment: PythonEnvironment, + cacheRoot: Uri, + envDir: Uri, +): Promise { + if (environment.envId.managerId !== INLINE_SCRIPT_MANAGER_ID) { + return 'uncertain'; + } + try { + const [expectedDir, resolvedPrefix, expectedPython, resolvedPython] = await Promise.all([ + resolveCacheEntryPath(cacheRoot, envDir), + fsapi.realpath(environment.sysPrefix), + fsapi.realpath(getVenvPythonPath(envDir.fsPath)), + fsapi.realpath(environment.environmentPath.fsPath), + ]); + if (!expectedDir) { + return 'uncertain'; + } + return normalizePath(expectedDir) === normalizePath(resolvedPrefix) && + normalizePath(expectedPython) === normalizePath(resolvedPython) + ? 'expected' + : 'stale'; + } catch (error) { + traceWarn('inline-script env: failed to inspect cache-entry ownership:', error); + return 'uncertain'; + } +} + +/** Read validated sidecar metadata, returning `undefined` for non-valid state. */ export async function readMetaJson(envDir: Uri): Promise { + const result = await inspectMetaJson(envDir); + return result.kind === 'valid' ? result.metadata : undefined; +} + +/** Classify sidecar state; only `unavailable` denotes transient I/O. */ +export async function inspectMetaJson(envDir: Uri): Promise { const metaPath = getMetaJsonPath(envDir).fsPath; try { - const stat = await fsapi.stat(metaPath); + const stat = await fsapi.lstat(metaPath); if (!stat.isFile()) { traceWarn(`inline-script meta: not a regular file at ${metaPath}`); - return undefined; + return { kind: 'invalid' }; } if (stat.size > MAX_META_JSON_BYTES) { traceWarn(`inline-script meta: refusing to read ${metaPath} (${stat.size} bytes > cap)`); - return undefined; + return { kind: 'invalid' }; } } catch (err) { if (isFileNotFoundError(err)) { traceWarn(`inline-script meta: not found at ${metaPath}`); + return { kind: 'missing' }; } else { const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; traceWarn(`inline-script meta: failed to stat ${metaPath} (code=${code}):`, err); + return { kind: 'unavailable' }; } - return undefined; } let raw: string; @@ -101,7 +147,7 @@ export async function readMetaJson(envDir: Uri): Promise, no * Verify that a cached env's base interpreter still exists on disk. */ export async function verifyBaseInterpreterExists(envDir: Uri): Promise { - return isWindows() ? verifyWindowsBaseInterpreter(envDir) : verifyPosixBaseInterpreter(envDir); + return (await getBaseInterpreterStatus(envDir)) === 'available'; +} + +/** Classify the base interpreter; `unavailable` denotes transient I/O. */ +export async function getBaseInterpreterStatus(envDir: Uri): Promise { + return isWindows() ? getWindowsBaseInterpreterStatus(envDir) : getPosixBaseInterpreterStatus(envDir); } -async function verifyPosixBaseInterpreter(envDir: Uri): Promise { +async function getPosixBaseInterpreterStatus(envDir: Uri): Promise { const launcherPath = Uri.joinPath(envDir, 'bin', 'python').fsPath; - return isRegularFile(launcherPath, 'base interpreter'); + return getRegularFileStatus(launcherPath, 'base interpreter'); } -async function verifyWindowsBaseInterpreter(envDir: Uri): Promise { +async function getWindowsBaseInterpreterStatus(envDir: Uri): Promise { const pyvenvPath = Uri.joinPath(envDir, 'pyvenv.cfg').fsPath; let raw: string; try { @@ -176,37 +227,39 @@ async function verifyWindowsBaseInterpreter(envDir: Uri): Promise { } catch (err) { if (isFileNotFoundError(err)) { traceWarn(`inline-script env: missing pyvenv.cfg at ${pyvenvPath}`); + return 'missing'; } else { const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; traceWarn(`inline-script env: failed to read ${pyvenvPath} (code=${code}):`, err); + return 'unavailable'; } - return false; } const home = parsePyvenvHome(raw); if (home === undefined) { traceWarn(`inline-script env: no 'home =' line in ${pyvenvPath}`); - return false; + return 'missing'; } const launcherPath = path.join(home, 'python.exe'); - return isRegularFile(launcherPath, 'base interpreter'); + return getRegularFileStatus(launcherPath, 'base interpreter'); } -async function isRegularFile(filePath: string, label: string): Promise { +async function getRegularFileStatus(filePath: string, label: string): Promise { try { const stat = await fsapi.stat(filePath); if (!stat.isFile()) { traceWarn(`inline-script env: ${label} is not a regular file at ${filePath}`); - return false; + return 'missing'; } - return true; + return 'available'; } catch (err) { if (isFileNotFoundError(err)) { traceWarn(`inline-script env: ${label} missing at ${filePath}`); + return 'missing'; } else { const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; traceWarn(`inline-script env: failed to stat ${filePath} (code=${code}):`, err); + return 'unavailable'; } - return false; } } @@ -224,6 +277,13 @@ function isFileNotFoundError(err: unknown): boolean { return typeof err === 'object' && err !== null && 'code' in err && (err as NodeJS.ErrnoException).code === 'ENOENT'; } +function isDescendantPath(rootPath: string, candidatePath: string): boolean { + const relative = path.relative(rootPath, candidatePath); + return ( + relative.length > 0 && relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative) + ); +} + function validateMeta(value: unknown): InlineScriptEnvMeta | undefined { if (typeof value !== 'object' || value === null || Array.isArray(value)) { return undefined; @@ -232,21 +292,30 @@ function validateMeta(value: unknown): InlineScriptEnvMeta | undefined { if (obj.schemaVersion !== META_SCHEMA_VERSION) { return undefined; } - if (typeof obj.scriptFsPath !== 'string' || obj.scriptFsPath.length === 0) { + if ( + typeof obj.baseInterpreterPath !== 'string' || + obj.baseInterpreterPath.length === 0 || + obj.baseInterpreterPath.trim() !== obj.baseInterpreterPath || + !path.isAbsolute(obj.baseInterpreterPath) + ) { return undefined; } - if (!isCanonicalIsoTimestamp(obj.lastUsedAt)) { + if ( + typeof obj.baseInterpreterVersion !== 'string' || + obj.baseInterpreterVersion.trim().length === 0 || + obj.baseInterpreterVersion.trim() !== obj.baseInterpreterVersion + ) { return undefined; } - if (obj.requiresPython !== undefined && typeof obj.requiresPython !== 'string') { + if (!isCanonicalIsoTimestamp(obj.lastUsedAt)) { return undefined; } return { schemaVersion: META_SCHEMA_VERSION, - scriptFsPath: obj.scriptFsPath, + baseInterpreterPath: obj.baseInterpreterPath, + baseInterpreterVersion: obj.baseInterpreterVersion, lastUsedAt: obj.lastUsedAt, - requiresPython: obj.requiresPython, }; } diff --git a/src/common/inlineScriptInterpreter.ts b/src/common/inlineScriptInterpreter.ts index 54754775e..0fba1c5fa 100644 --- a/src/common/inlineScriptInterpreter.ts +++ b/src/common/inlineScriptInterpreter.ts @@ -24,7 +24,8 @@ export function pickCompatibleInterpreter( installed: ReadonlyArray, requiresPython: string | undefined, ): PythonEnvironment | undefined { - const constraint = requiresPython && requiresPython.length > 0 ? requiresPython : undefined; + const trimmedConstraint = requiresPython?.trim(); + const constraint = trimmedConstraint ? trimmedConstraint : undefined; const candidates = installed.filter((env) => isUsableBaseInterpreter(env, constraint)); if (candidates.length === 0) { return undefined; diff --git a/src/test/common/inlineScriptCacheKey.unit.test.ts b/src/test/common/inlineScriptCacheKey.unit.test.ts index ce0e72c8b..1b4aa9973 100644 --- a/src/test/common/inlineScriptCacheKey.unit.test.ts +++ b/src/test/common/inlineScriptCacheKey.unit.test.ts @@ -106,6 +106,23 @@ suite('inlineScriptCacheKey', () => { ); }); + test('whitespace inside quoted marker values is preserved exactly', () => { + assert.strictEqual( + normalizeDependency('pkg; platform_version == "X Y"'), + 'pkg; platform_version=="X Y"', + ); + assert.notStrictEqual( + normalizeDependency('pkg; platform_version == "X Y"'), + normalizeDependency('pkg; platform_version == "X Y"'), + ); + }); + + test('escaped quotes do not end marker values', () => { + const input = String.raw`pkg; platform_version == "X\" Y" and python_version >= "3.10"`; + const expected = String.raw`pkg; platform_version=="X\" Y" and python_version>="3.10"`; + assert.strictEqual(normalizeDependency(input), expected); + }); + test('empty and whitespace-only entries normalize to empty string', () => { assert.strictEqual(normalizeDependency(''), ''); assert.strictEqual(normalizeDependency(' '), ''); @@ -119,6 +136,14 @@ suite('inlineScriptCacheKey', () => { assert.ok(result.includes('requests'), 'leading name should still appear'); }); + test('direct-reference URL and marker boundaries are preserved exactly', () => { + const conditional = "pkg @ https://example.invalid/pkg! ;python_version == '0'"; + const unconditional = "pkg @ https://example.invalid/pkg!;python_version=='0'"; + assert.strictEqual(normalizeDependency(conditional), conditional); + assert.strictEqual(normalizeDependency(unconditional), unconditional); + assert.notStrictEqual(normalizeDependency(conditional), normalizeDependency(unconditional)); + }); + test('leading and trailing whitespace is stripped', () => { assert.strictEqual(normalizeDependency(' requests '), 'requests'); assert.strictEqual(normalizeDependency('\trequests<3\n'), 'requests<3'); @@ -240,6 +265,30 @@ suite('inlineScriptCacheKey', () => { assert.notStrictEqual(a, b); }); + test('meaningful whitespace inside quoted marker values changes the key', () => { + const a = computeCacheKey({ + dependencies: ['pkg; platform_version == "X Y"'], + interpreterPath: interpreter, + }); + const b = computeCacheKey({ + dependencies: ['pkg; platform_version == "X Y"'], + interpreterPath: interpreter, + }); + assert.notStrictEqual(a, b); + }); + + test('direct-reference URL terminator whitespace changes the key', () => { + const conditional = computeCacheKey({ + dependencies: ["pkg @ https://example.invalid/pkg! ;python_version == '0'"], + interpreterPath: interpreter, + }); + const unconditional = computeCacheKey({ + dependencies: ["pkg @ https://example.invalid/pkg!;python_version=='0'"], + interpreterPath: interpreter, + }); + assert.notStrictEqual(conditional, unconditional); + }); + test('changing the interpreter path changes the key', () => { const a = computeCacheKey({ dependencies: ['requests'], interpreterPath: '/usr/bin/python3' }); const b = computeCacheKey({ dependencies: ['requests'], interpreterPath: '/opt/python/python3' }); diff --git a/src/test/common/inlineScriptCacheLayout.unit.test.ts b/src/test/common/inlineScriptCacheLayout.unit.test.ts index dfc0517b0..0c4b08eb2 100644 --- a/src/test/common/inlineScriptCacheLayout.unit.test.ts +++ b/src/test/common/inlineScriptCacheLayout.unit.test.ts @@ -2,34 +2,41 @@ // Licensed under the MIT License. import assert from 'assert'; +import fsExtra from 'fs-extra'; import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; import * as sinon from 'sinon'; import { Uri } from 'vscode'; +import { PythonEnvironment } from '../../api'; import { CacheEntrySummary, INLINE_SCRIPT_CACHE_DIR_NAME, InlineScriptEnvMeta, META_JSON_FILENAME, META_SCHEMA_VERSION, + getBaseInterpreterStatus, getMetaJsonPath, getScriptEnvCacheRoot, getScriptEnvDir, + inspectOwnedCacheEntry, + inspectMetaJson, readMetaJson, + resolveCacheEntryPath, selectStaleEntries, verifyBaseInterpreterExists, writeMetaJson, } from '../../common/inlineScriptCacheLayout'; import * as logging from '../../common/logging'; import * as platformUtils from '../../common/utils/platformUtils'; +import { getVenvPythonPath } from '../../common/utils/virtualEnvironment'; function makeMeta(overrides: Partial = {}): InlineScriptEnvMeta { return { schemaVersion: META_SCHEMA_VERSION, - scriptFsPath: '/tmp/script.py', + baseInterpreterPath: Uri.file(path.join(os.tmpdir(), 'base-python')).fsPath, + baseInterpreterVersion: '3.12.4', lastUsedAt: '2026-06-18T22:45:12.000Z', - requiresPython: '>=3.11', ...overrides, }; } @@ -133,14 +140,13 @@ suite('inlineScriptCacheLayout', () => { ); }); - test('writeMetaJson serializes optional requiresPython as undefined-erased', async () => { - const meta = makeMeta({ requiresPython: undefined }); + test('writeMetaJson only serializes environment-level metadata', async () => { + const meta = makeMeta(); await writeMetaJson(envDir, meta); const onDisk = JSON.parse(await fs.readFile(getMetaJsonPath(envDir).fsPath, 'utf8')); + assert.strictEqual(onDisk.baseInterpreterPath, meta.baseInterpreterPath); + assert.strictEqual('scriptFsPath' in onDisk, false); assert.strictEqual('requiresPython' in onDisk, false); - const read = await readMetaJson(envDir); - assert.ok(read); - assert.strictEqual(read.requiresPython, undefined); }); test('writeMetaJson produces human-readable, indented JSON', async () => { @@ -175,6 +181,46 @@ suite('inlineScriptCacheLayout', () => { assert.ok(traceWarnStub.called, 'expected a traceWarn'); }); + test('classifies missing, malformed, and valid metadata distinctly', async () => { + assert.deepStrictEqual(await inspectMetaJson(envDir), { kind: 'missing' }); + await writeRaw('this is not json'); + assert.deepStrictEqual(await inspectMetaJson(envDir), { kind: 'invalid' }); + const metadata = makeMeta(); + await writeRaw(JSON.stringify(metadata)); + assert.deepStrictEqual(await inspectMetaJson(envDir), { kind: 'valid', metadata }); + }); + + test('classifies non-ENOENT sidecar stat failures as unavailable', async () => { + sinon.stub(fsExtra, 'lstat').rejects(Object.assign(new Error('permission denied'), { code: 'EACCES' })); + assert.deepStrictEqual(await inspectMetaJson(envDir), { kind: 'unavailable' }); + }); + + test('classifies non-ENOENT sidecar read failures as unavailable', async () => { + await writeRaw(JSON.stringify(makeMeta())); + sinon.stub(fsExtra, 'readFile').rejects(Object.assign(new Error('I/O error'), { code: 'EIO' })); + assert.deepStrictEqual(await inspectMetaJson(envDir), { kind: 'unavailable' }); + }); + + test('does not follow a sidecar symlink outside the environment', async function () { + const externalMetaPath = path.join(tmpDir, 'external-meta.json'); + const sidecarPath = getMetaJsonPath(envDir).fsPath; + await fs.writeJson(externalMetaPath, makeMeta()); + try { + await fs.symlink(externalMetaPath, sidecarPath, 'file'); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + this.skip(); + return; + } + throw error; + } + + assert.deepStrictEqual(await inspectMetaJson(envDir), { kind: 'invalid' }); + assert.strictEqual(await readMetaJson(envDir), undefined); + assert.deepStrictEqual(await fs.readJson(externalMetaPath), makeMeta()); + }); + test('returns undefined for malformed JSON', async () => { await writeRaw('this is not json'); const result = await readMetaJson(envDir); @@ -194,37 +240,63 @@ suite('inlineScriptCacheLayout', () => { assert.ok(traceWarnStub.called); }); - test('returns undefined when scriptFsPath is missing', async () => { - const { scriptFsPath: _omit, ...partial } = makeMeta(); + test('returns undefined when baseInterpreterPath is missing', async () => { + const { baseInterpreterPath: _omit, ...partial } = makeMeta(); await writeRaw(JSON.stringify(partial)); const result = await readMetaJson(envDir); assert.strictEqual(result, undefined); }); - test('returns undefined when scriptFsPath is an empty string', async () => { - await writeRaw(JSON.stringify({ ...makeMeta(), scriptFsPath: '' })); + test('returns undefined when baseInterpreterPath is an empty string', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), baseInterpreterPath: '' })); const result = await readMetaJson(envDir); assert.strictEqual(result, undefined); }); - test('returns undefined when lastUsedAt is not parseable', async () => { - await writeRaw(JSON.stringify({ ...makeMeta(), lastUsedAt: 'not-a-date' })); + test('returns undefined when baseInterpreterPath is relative', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), baseInterpreterPath: path.join('relative', 'python') })); const result = await readMetaJson(envDir); assert.strictEqual(result, undefined); }); - test('returns undefined when requiresPython is present but not a string', async () => { - await writeRaw(JSON.stringify({ ...makeMeta(), requiresPython: 311 })); + test('returns undefined when baseInterpreterPath is not a string', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), baseInterpreterPath: 312 })); const result = await readMetaJson(envDir); assert.strictEqual(result, undefined); }); - test('accepts a meta with requiresPython explicitly omitted', async () => { - const { requiresPython: _omit, ...partial } = makeMeta(); + test('returns undefined when baseInterpreterVersion is missing or blank', async () => { + const { baseInterpreterVersion: _omit, ...partial } = makeMeta(); await writeRaw(JSON.stringify(partial)); + assert.strictEqual(await readMetaJson(envDir), undefined); + await writeRaw(JSON.stringify({ ...makeMeta(), baseInterpreterVersion: ' ' })); + assert.strictEqual(await readMetaJson(envDir), undefined); + }); + + test('returns undefined when lastUsedAt is not parseable', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), lastUsedAt: 'not-a-date' })); + const result = await readMetaJson(envDir); + assert.strictEqual(result, undefined); + }); + + test('drops legacy script-specific fields', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), scriptFsPath: 'script.py', requiresPython: '>=3.11' })); const result = await readMetaJson(envDir); assert.ok(result); - assert.strictEqual(result.requiresPython, undefined); + assert.strictEqual('scriptFsPath' in result, false); + assert.strictEqual('requiresPython' in result, false); + }); + + test('rejects the provisional pre-writer v1 sidecar shape', async () => { + await writeRaw( + JSON.stringify({ + schemaVersion: 1, + scriptFsPath: Uri.file(path.join(os.tmpdir(), 'script.py')).fsPath, + lastUsedAt: '2026-06-18T22:45:12.000Z', + requiresPython: '>=3.11', + }), + ); + assert.strictEqual(await readMetaJson(envDir), undefined); }); test('returns undefined for a top-level non-object payload', async () => { @@ -245,11 +317,6 @@ suite('inlineScriptCacheLayout', () => { assert.ok(await readMetaJson(envDir)); }); - test('returns undefined when requiresPython is explicitly null', async () => { - await writeRaw(JSON.stringify({ ...makeMeta(), requiresPython: null })); - assert.strictEqual(await readMetaJson(envDir), undefined); - }); - test('returns undefined for a non-canonical ISO timestamp (e.g. "2026")', async () => { await writeRaw(JSON.stringify({ ...makeMeta(), lastUsedAt: '2026' })); assert.strictEqual(await readMetaJson(envDir), undefined); @@ -295,6 +362,28 @@ suite('inlineScriptCacheLayout', () => { }); }); + suite('getBaseInterpreterStatus classification', () => { + let tmpDir: string; + let envDir: Uri; + + setup(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'isclayout-base-status-')); + envDir = Uri.file(path.join(tmpDir, 'env')); + await fs.ensureDir(envDir.fsPath); + sinon.stub(platformUtils, 'isWindows').returns(false); + }); + + teardown(async () => { + await fs.remove(tmpDir); + }); + + test('distinguishes a missing base interpreter from an unavailable stat', async () => { + assert.strictEqual(await getBaseInterpreterStatus(envDir), 'missing'); + sinon.stub(fsExtra, 'stat').rejects(Object.assign(new Error('I/O error'), { code: 'EIO' })); + assert.strictEqual(await getBaseInterpreterStatus(envDir), 'unavailable'); + }); + }); + suite('selectStaleEntries', () => { const TTL_MS = 14 * 24 * 60 * 60 * 1000; const now = new Date('2026-06-22T12:00:00.000Z'); @@ -353,6 +442,107 @@ suite('inlineScriptCacheLayout', () => { }); }); + suite('resolveCacheEntryPath', () => { + let tmpDir: string; + let cacheRoot: Uri; + + setup(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'isclayout-containment-')); + cacheRoot = Uri.file(path.join(tmpDir, 'script-envs-v1')); + await fs.ensureDir(cacheRoot.fsPath); + }); + + teardown(async () => { + await fs.remove(tmpDir); + }); + + test('resolves a direct cache entry', async () => { + const envDir = Uri.joinPath(cacheRoot, '0123456789abcdef'); + await fs.ensureDir(envDir.fsPath); + + assert.strictEqual(await resolveCacheEntryPath(cacheRoot, envDir), await fs.realpath(envDir.fsPath)); + }); + + test('rejects the cache root and paths outside it', async () => { + const externalEnv = Uri.file(path.join(tmpDir, '0123456789abcdef')); + await fs.ensureDir(externalEnv.fsPath); + + assert.strictEqual(await resolveCacheEntryPath(cacheRoot, cacheRoot), undefined); + assert.strictEqual(await resolveCacheEntryPath(cacheRoot, externalEnv), undefined); + }); + + test('surfaces a missing entry', async () => { + await assert.rejects( + resolveCacheEntryPath(cacheRoot, Uri.joinPath(cacheRoot, '0123456789abcdef')), + (error: NodeJS.ErrnoException) => error.code === 'ENOENT', + ); + }); + }); + + suite('inspectOwnedCacheEntry', () => { + let tmpDir: string; + let cacheRoot: Uri; + let envDir: Uri; + let environment: PythonEnvironment; + + setup(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'isclayout-ownership-')); + cacheRoot = Uri.file(path.join(tmpDir, 'script-envs-v1')); + envDir = Uri.joinPath(cacheRoot, '0123456789abcdef'); + const pythonPath = getVenvPythonPath(envDir.fsPath); + await fs.outputFile(pythonPath, ''); + environment = { + envId: { id: 'inline-env', managerId: 'ms-python.python:inline-script' }, + name: 'Python 3.12.4', + displayName: 'Python 3.12.4', + displayPath: pythonPath, + version: '3.12.4', + environmentPath: Uri.file(pythonPath), + execInfo: { run: { executable: pythonPath } }, + sysPrefix: envDir.fsPath, + }; + }); + + teardown(async () => { + await fs.remove(tmpDir); + }); + + test('accepts an inline-owned environment at the expected cache entry', async () => { + assert.strictEqual(await inspectOwnedCacheEntry(environment, cacheRoot, envDir), 'expected'); + }); + + test('does not claim environments owned by another manager', async () => { + const otherManager = { + ...environment, + envId: { ...environment.envId, managerId: 'ms-python.python:system' }, + }; + assert.strictEqual(await inspectOwnedCacheEntry(otherManager, cacheRoot, envDir), 'uncertain'); + }); + + test('classifies an inline environment resolving elsewhere as stale', async () => { + const externalPrefix = path.join(tmpDir, 'external-env'); + const externalPython = getVenvPythonPath(externalPrefix); + await fs.outputFile(externalPython, ''); + const mismatched = { + ...environment, + sysPrefix: externalPrefix, + environmentPath: Uri.file(externalPython), + }; + + assert.strictEqual(await inspectOwnedCacheEntry(mismatched, cacheRoot, envDir), 'stale'); + }); + + test('preserves ownership as uncertain when paths cannot be inspected', async () => { + const missing = { + ...environment, + environmentPath: Uri.file(path.join(tmpDir, 'missing-python')), + }; + + assert.strictEqual(await inspectOwnedCacheEntry(missing, cacheRoot, envDir), 'uncertain'); + assert.ok(traceWarnStub.called); + }); + }); + suite('verifyBaseInterpreterExists', () => { let tmpDir: string; let envDir: Uri; diff --git a/src/test/common/inlineScriptInterpreter.unit.test.ts b/src/test/common/inlineScriptInterpreter.unit.test.ts index b0ec8c979..6a58680c1 100644 --- a/src/test/common/inlineScriptInterpreter.unit.test.ts +++ b/src/test/common/inlineScriptInterpreter.unit.test.ts @@ -156,6 +156,13 @@ suite('inlineScriptInterpreter', () => { assert.strictEqual(picked.version, '3.12.4'); }); + test('whitespace-only requiresPython is treated as no constraint', () => { + const envs = [makeEnv('3.10.0'), makeEnv('3.12.4')]; + const picked = pickCompatibleInterpreter(envs, ' '); + assert.ok(picked, 'whitespace-only constraint must not silently reject all envs'); + assert.strictEqual(picked.version, '3.12.4'); + }); + test('ranks versions with pre-release / dev / local suffixes by release segments only', () => { // 3.12.0a1 and 3.12.0.dev1 both parse to [3,12,0]; stable sort // means the first-listed 3.12 entry wins.