From 79fe2aa014f6e803c62f144bb4cf87ebb5963e42 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Thu, 25 Jun 2026 10:38:34 -0700 Subject: [PATCH 1/2] feat(inline-scripts): add cache layout, meta.json sidecar, env-usability guard (PEP 723 PR 2/3) Helpers that resolve the on-disk cache layout for PEP 723 inline-script envs (under /script-envs-v1//), read/write the .meta.json sidecar that lives at the root of every cached env, and verify on cache hit that the base interpreter the env was built against still exists on disk. Second foundation PR; pure utility, no behavior change on its own. What is in: - src/common/inlineScriptCacheLayout.ts - Path helpers: getScriptEnvCacheRoot, getScriptEnvDir, getMetaJsonPath. All take globalStorageUri: Uri rather than ExtensionContext so they are easy to test. - InlineScriptEnvMeta interface with schemaVersion: 1. Minimal shape: only fields with a concrete consumer in a planned PR (scriptFsPath for cache cleanup, lastUsedAt for TTL, optional requiresPython for cache-hit re-verify). Extras dropped on read per the v1 evolution policy. - readMetaJson(envDir) -- never throws. Returns undefined (with a traceWarn that includes err.code) for: missing file, non-regular file, oversize file (>1 MiB defensive cap), malformed JSON, invalid shape, unknown schemaVersion, non-canonical ISO 8601 timestamps. Strict ISO check uses round-trip equality. - writeMetaJson(envDir, meta) -- atomic temp-file + native fs.rename (NOT fs-extra move(), which does remove+rename and has both a no-sidecar window and a crash-loses-data failure mode). - CacheEntrySummary + selectStaleEntries(entries, now, ttlMs) -- pure selector for Q7s TTL eviction path. Zero I/O. Entries with undefined lastUsedAt are never TTLed (deliberate: do not delete what we do not understand). - verifyEnvUsable(envDir): Promise -- cache-hit guard that confirms the base interpreter the cached env was built against still exists on disk. The cache-key path hash detects "user switched to a different Python" because the path changes; it does NOT detect "user uninstalled the Python at the cached path" (the cache directory and .meta.json are unchanged, only the file at the launcher target is gone). Common triggers: pyenv/uv python uninstall, brew/apt removal, Add/Remove Programs. * POSIX: stat /bin/python. fs.stat follows symlinks so a dead symlink to a removed base throws ENOENT. * Windows: /Scripts/python.exe is a COPY of the base (not a symlink), so checking it does not help. Read pyvenv.cfg, parse `home = `, stat /python.exe. * Never throws. Returns false + traceWarn(message) on any failure mode (missing launcher, missing/malformed pyvenv.cfg, non-regular file at the launcher path, EACCES etc.). Mirrors readMetaJson's failure policy. Internal helpers `statRegularFile` (POSIX/Windows shared stat+isFile+error-tagging) and `parsePyvenvHome` (CRLF + extra whitespace tolerated). - META_SCHEMA_VERSION evolution policy documented inline. - src/test/common/inlineScriptCacheLayout.unit.test.ts -- 44 unit tests (real-fs, mirroring readMetaJson conventions): * path-helper shape * round-trip writeMetaJson/readMetaJson, concurrent writers, and all rejection paths (ENOENT-specific warn, malformed JSON, unknown schemaVersion, non-canonical ISO, size cap, etc.) * selectStaleEntries boundaries (TTL=0, exact-equal age, undefined lastUsedAt, future timestamps) * verifyEnvUsable POSIX branch (isWindows stubbed to false): bin/python exists / missing / is-a-directory; symlink-alive and dead-symlink cases gated on process.platform !== 'win32' * verifyEnvUsable Windows branch (isWindows stubbed to true): home points to existing / removed python.exe, missing pyvenv.cfg, no home= line, empty home value, whitespace and CRLF tolerance. Design context Implements Q2 (disk location), Q3 (env folder contents + .meta.json shape), Q4 step 4 (cache-hit interpreter-existence guard), and the pure selector half of Q7 (TTL eviction) from pep723_design_questions.md. The disk walk and deletion calls live in a later PR. The verifyEnvUsable guard was added in response to reviewer feedback on the design doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/common/inlineScriptCacheLayout.ts | 246 +++++++++ .../inlineScriptCacheLayout.unit.test.ts | 494 ++++++++++++++++++ 2 files changed, 740 insertions(+) create mode 100644 src/common/inlineScriptCacheLayout.ts create mode 100644 src/test/common/inlineScriptCacheLayout.unit.test.ts diff --git a/src/common/inlineScriptCacheLayout.ts b/src/common/inlineScriptCacheLayout.ts new file mode 100644 index 00000000..108d8219 --- /dev/null +++ b/src/common/inlineScriptCacheLayout.ts @@ -0,0 +1,246 @@ +// 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'; +import { Uri } from 'vscode'; +import { traceWarn } from './logging'; +import { isWindows } from './utils/platformUtils'; + +/** + * 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. + */ +export const INLINE_SCRIPT_CACHE_DIR_NAME = 'script-envs-v1'; + +export const META_JSON_FILENAME = '.meta.json'; + +/** + * Schema version embedded in every {@link InlineScriptEnvMeta}. + */ +export const META_SCHEMA_VERSION = 1 as const; +export interface InlineScriptEnvMeta { + readonly schemaVersion: typeof META_SCHEMA_VERSION; + readonly scriptFsPath: string; + readonly lastUsedAt: string; + readonly requiresPython?: string; +} + +export function getScriptEnvCacheRoot(globalStorageUri: Uri): Uri { + return Uri.joinPath(globalStorageUri, INLINE_SCRIPT_CACHE_DIR_NAME); +} + +export function getScriptEnvDir(globalStorageUri: Uri, cacheKey: string): Uri { + return Uri.joinPath(getScriptEnvCacheRoot(globalStorageUri), cacheKey); +} + +export function getMetaJsonPath(envDir: Uri): Uri { + return Uri.joinPath(envDir, META_JSON_FILENAME); +} + +const MAX_META_JSON_BYTES = 1024 * 1024; + +export async function readMetaJson(envDir: Uri): Promise { + const metaPath = getMetaJsonPath(envDir).fsPath; + + try { + const stat = await fsapi.stat(metaPath); + if (!stat.isFile()) { + traceWarn(`inline-script meta: not a regular file at ${metaPath}`); + return undefined; + } + if (stat.size > MAX_META_JSON_BYTES) { + traceWarn(`inline-script meta: refusing to read ${metaPath} (${stat.size} bytes > cap)`); + return undefined; + } + } catch (err) { + if (isFileNotFoundError(err)) { + traceWarn(`inline-script meta: not found at ${metaPath}`); + } else { + const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; + traceWarn(`inline-script meta: failed to stat ${metaPath} (code=${code}):`, err); + } + return undefined; + } + + let raw: string; + try { + raw = await fsapi.readFile(metaPath, 'utf8'); + } catch (err) { + const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; + traceWarn(`inline-script meta: failed to read ${metaPath} (code=${code}):`, err); + return undefined; + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + traceWarn(`inline-script meta: malformed JSON in ${metaPath}:`, err); + return undefined; + } + + const validated = validateMeta(parsed); + if (!validated) { + traceWarn(`inline-script meta: invalid shape in ${metaPath}`); + return undefined; + } + return validated; +} + +/** + * Atomically write the `.meta.json` sidecar via temp-file + rename. + */ +export async function writeMetaJson(envDir: Uri, meta: InlineScriptEnvMeta): Promise { + await fsapi.ensureDir(envDir.fsPath); + const finalPath = getMetaJsonPath(envDir).fsPath; + const tmpSuffix = crypto.randomBytes(6).toString('hex'); + const tmpPath = `${finalPath}.tmp-${tmpSuffix}`; + const payload = JSON.stringify(meta, undefined, 2); + try { + await fsapi.writeFile(tmpPath, payload, 'utf8'); + await fsapi.rename(tmpPath, finalPath); + } catch (err) { + await fsapi.remove(tmpPath).catch(() => undefined); + throw err; + } +} + +/** + * Snapshot of one cached entry, populated by the (separate, I/O-doing) disk + * walk. + */ +export interface CacheEntrySummary { + readonly envDirPath: string; + readonly lastUsedAt: Date | undefined; +} + +/** + * Pure selector: returns the env-dir paths whose age exceeds `ttlMs`. + */ +export function selectStaleEntries(entries: ReadonlyArray, now: Date, ttlMs: number): string[] { + const stale: string[] = []; + const nowMs = now.getTime(); + for (const entry of entries) { + if (entry.lastUsedAt === undefined) { + continue; + } + const ageMs = nowMs - entry.lastUsedAt.getTime(); + if (ageMs > ttlMs) { + stale.push(entry.envDirPath); + } + } + return stale; +} + +/** + * Verify that a cached env's base interpreter still exists on disk. + */ +export async function verifyEnvUsable(envDir: Uri): Promise { + if (isWindows()) { + return verifyWindowsBaseInterpreter(envDir); + } + return verifyPosixBaseInterpreter(envDir); +} + +async function verifyPosixBaseInterpreter(envDir: Uri): Promise { + const launcherPath = Uri.joinPath(envDir, 'bin', 'python').fsPath; + return statRegularFile(launcherPath, 'base interpreter'); +} + +async function verifyWindowsBaseInterpreter(envDir: Uri): Promise { + const pyvenvPath = Uri.joinPath(envDir, 'pyvenv.cfg').fsPath; + let raw: string; + try { + raw = await fsapi.readFile(pyvenvPath, 'utf8'); + } catch (err) { + if (isFileNotFoundError(err)) { + traceWarn(`inline-script env: missing pyvenv.cfg at ${pyvenvPath}`); + } else { + const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; + traceWarn(`inline-script env: failed to read ${pyvenvPath} (code=${code}):`, err); + } + return false; + } + const home = parsePyvenvHome(raw); + if (home === undefined) { + traceWarn(`inline-script env: no 'home =' line in ${pyvenvPath}`); + return false; + } + const launcherPath = path.join(home, 'python.exe'); + return statRegularFile(launcherPath, 'base interpreter'); +} + +async function statRegularFile(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 true; + } catch (err) { + if (isFileNotFoundError(err)) { + traceWarn(`inline-script env: ${label} missing at ${filePath}`); + } else { + const code = (err as NodeJS.ErrnoException | undefined)?.code ?? 'unknown'; + traceWarn(`inline-script env: failed to stat ${filePath} (code=${code}):`, err); + } + return false; + } +} + +function parsePyvenvHome(raw: string): string | undefined { + for (const line of raw.split(/\r?\n/)) { + const m = line.match(/^\s*home\s*=\s*(.+?)\s*$/); + if (m) { + return m[1]; + } + } + return undefined; +} + +function isFileNotFoundError(err: unknown): boolean { + return typeof err === 'object' && err !== null && 'code' in err && (err as NodeJS.ErrnoException).code === 'ENOENT'; +} + +function validateMeta(value: unknown): InlineScriptEnvMeta | undefined { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return undefined; + } + const obj = value as Record; + if (obj.schemaVersion !== META_SCHEMA_VERSION) { + return undefined; + } + if (typeof obj.scriptFsPath !== 'string' || obj.scriptFsPath.length === 0) { + return undefined; + } + if (!isCanonicalIsoTimestamp(obj.lastUsedAt)) { + return undefined; + } + if (obj.requiresPython !== undefined && typeof obj.requiresPython !== 'string') { + return undefined; + } + + return { + schemaVersion: META_SCHEMA_VERSION, + scriptFsPath: obj.scriptFsPath, + lastUsedAt: obj.lastUsedAt, + requiresPython: obj.requiresPython, + }; +} + +function isCanonicalIsoTimestamp(value: unknown): value is string { + if (typeof value !== 'string') { + return false; + } + const ms = Date.parse(value); + if (Number.isNaN(ms)) { + return false; + } + return new Date(ms).toISOString() === value; +} diff --git a/src/test/common/inlineScriptCacheLayout.unit.test.ts b/src/test/common/inlineScriptCacheLayout.unit.test.ts new file mode 100644 index 00000000..51b8ee85 --- /dev/null +++ b/src/test/common/inlineScriptCacheLayout.unit.test.ts @@ -0,0 +1,494 @@ +// 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 { Uri } from 'vscode'; +import { + CacheEntrySummary, + INLINE_SCRIPT_CACHE_DIR_NAME, + InlineScriptEnvMeta, + META_JSON_FILENAME, + META_SCHEMA_VERSION, + getMetaJsonPath, + getScriptEnvCacheRoot, + getScriptEnvDir, + readMetaJson, + selectStaleEntries, + verifyEnvUsable, + writeMetaJson, +} from '../../common/inlineScriptCacheLayout'; +import * as logging from '../../common/logging'; +import * as platformUtils from '../../common/utils/platformUtils'; + +function makeMeta(overrides: Partial = {}): InlineScriptEnvMeta { + return { + schemaVersion: META_SCHEMA_VERSION, + scriptFsPath: '/tmp/script.py', + lastUsedAt: '2026-06-18T22:45:12.000Z', + requiresPython: '>=3.11', + ...overrides, + }; +} + +suite('inlineScriptCacheLayout', () => { + let traceWarnStub: sinon.SinonStub; + + setup(() => { + traceWarnStub = sinon.stub(logging, 'traceWarn'); + }); + + teardown(() => { + sinon.restore(); + }); + + suite('path helpers', () => { + test('getScriptEnvCacheRoot appends the versioned bucket name', () => { + const globalStorage = Uri.file(path.join('/tmp', 'extension-storage')); + const root = getScriptEnvCacheRoot(globalStorage); + assert.strictEqual(path.basename(root.fsPath), INLINE_SCRIPT_CACHE_DIR_NAME); + assert.strictEqual(path.dirname(root.fsPath), globalStorage.fsPath); + }); + + test('getScriptEnvDir uses the cache key verbatim as the directory name', () => { + const globalStorage = Uri.file(path.join('/tmp', 'extension-storage')); + const key = 'abc123def4567890'; + const envDir = getScriptEnvDir(globalStorage, key); + assert.strictEqual(path.basename(envDir.fsPath), key); + }); + + test('getMetaJsonPath returns .meta.json inside the env dir', () => { + const envDir = Uri.file(path.join('/tmp', 'cache', 'abc')); + const metaPath = getMetaJsonPath(envDir); + assert.strictEqual(path.basename(metaPath.fsPath), META_JSON_FILENAME); + assert.strictEqual(path.dirname(metaPath.fsPath), envDir.fsPath); + }); + }); + + suite('writeMetaJson + readMetaJson (round-trip on a real tmpdir)', () => { + let tmpDir: string; + let envDir: Uri; + + setup(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'isclayout-test-')); + envDir = Uri.file(path.join(tmpDir, 'env')); + }); + + teardown(async () => { + await fs.remove(tmpDir); + }); + + test('writeMetaJson then readMetaJson returns the same object', async () => { + const meta = makeMeta(); + await writeMetaJson(envDir, meta); + const read = await readMetaJson(envDir); + assert.deepStrictEqual(read, meta); + }); + + test('writeMetaJson creates the env directory if it does not exist', async () => { + const meta = makeMeta(); + assert.strictEqual(await fs.pathExists(envDir.fsPath), false); + await writeMetaJson(envDir, meta); + assert.strictEqual(await fs.pathExists(envDir.fsPath), true); + }); + + test('writeMetaJson leaves no .tmp- files behind on success', async () => { + await writeMetaJson(envDir, makeMeta()); + const entries = await fs.readdir(envDir.fsPath); + const tmpFiles = entries.filter((name) => name.includes('.tmp-')); + assert.deepStrictEqual(tmpFiles, []); + }); + + test('writeMetaJson overwrites an existing sidecar (last write wins)', async () => { + await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2020-01-01T00:00:00.000Z' })); + await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2030-01-01T00:00:00.000Z' })); + const read = await readMetaJson(envDir); + assert.ok(read); + assert.strictEqual(read.lastUsedAt, '2030-01-01T00:00:00.000Z'); + const entries = await fs.readdir(envDir.fsPath); + assert.deepStrictEqual( + entries.filter((name) => name.includes('.tmp-')), + [], + ); + }); + + test('concurrent writeMetaJson calls leave one valid sidecar, never a missing one', async () => { + await writeMetaJson(envDir, makeMeta({ lastUsedAt: '2020-01-01T00:00:00.000Z' })); + const a = makeMeta({ lastUsedAt: '2025-01-01T00:00:00.000Z' }); + const b = makeMeta({ lastUsedAt: '2030-01-01T00:00:00.000Z' }); + await Promise.all([writeMetaJson(envDir, a), writeMetaJson(envDir, b)]); + const read = await readMetaJson(envDir); + assert.ok(read, 'sidecar must exist after concurrent writes'); + assert.ok( + read.lastUsedAt === a.lastUsedAt || read.lastUsedAt === b.lastUsedAt, + `final write must be one of the concurrent payloads, got ${read.lastUsedAt}`, + ); + const entries = await fs.readdir(envDir.fsPath); + assert.deepStrictEqual( + entries.filter((name) => name.includes('.tmp-')), + [], + ); + }); + + test('writeMetaJson serializes optional requiresPython as undefined-erased', async () => { + const meta = makeMeta({ requiresPython: undefined }); + await writeMetaJson(envDir, meta); + const onDisk = JSON.parse(await fs.readFile(getMetaJsonPath(envDir).fsPath, 'utf8')); + 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 () => { + await writeMetaJson(envDir, makeMeta()); + const raw = await fs.readFile(getMetaJsonPath(envDir).fsPath, 'utf8'); + assert.ok(raw.includes('\n'), 'expected indented JSON'); + }); + }); + + suite('readMetaJson rejection paths', () => { + let tmpDir: string; + let envDir: Uri; + + setup(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'isclayout-test-')); + envDir = Uri.file(path.join(tmpDir, 'env')); + await fs.ensureDir(envDir.fsPath); + }); + + teardown(async () => { + await fs.remove(tmpDir); + }); + + async function writeRaw(content: string): Promise { + await fs.writeFile(getMetaJsonPath(envDir).fsPath, content, 'utf8'); + } + + test('returns undefined when the sidecar does not exist', async () => { + await fs.remove(getMetaJsonPath(envDir).fsPath).catch(() => undefined); + const result = await readMetaJson(envDir); + assert.strictEqual(result, undefined); + assert.ok(traceWarnStub.called, 'expected a traceWarn'); + }); + + test('returns undefined for malformed JSON', async () => { + await writeRaw('this is not json'); + const result = await readMetaJson(envDir); + assert.strictEqual(result, undefined); + assert.ok(traceWarnStub.called); + }); + + test('returns undefined for an unknown schemaVersion', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), schemaVersion: 99 })); + const result = await readMetaJson(envDir); + assert.strictEqual(result, undefined); + assert.ok(traceWarnStub.called); + }); + + test('returns undefined when scriptFsPath is missing', async () => { + const { scriptFsPath: _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: '' })); + 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' })); + 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 })); + const result = await readMetaJson(envDir); + assert.strictEqual(result, undefined); + }); + + test('accepts a meta with requiresPython explicitly omitted', async () => { + const { requiresPython: _omit, ...partial } = makeMeta(); + await writeRaw(JSON.stringify(partial)); + const result = await readMetaJson(envDir); + assert.ok(result); + assert.strictEqual(result.requiresPython, undefined); + }); + + test('returns undefined for a top-level non-object payload', async () => { + await writeRaw(JSON.stringify(['array', 'instead'])); + assert.strictEqual(await readMetaJson(envDir), undefined); + await writeRaw(JSON.stringify('a bare string')); + assert.strictEqual(await readMetaJson(envDir), undefined); + await writeRaw('null'); + assert.strictEqual(await readMetaJson(envDir), undefined); + }); + + test('an unknown field with a null value is tolerated (extras are dropped on read)', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), dependencies: null })); + // `dependencies` is no longer a schema field; per the v1 + // evolution policy, extras (whatever their value) are + // tolerated and dropped on read. Pin that null doesn't + // break the validator. + 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); + await writeRaw(JSON.stringify({ ...makeMeta(), lastUsedAt: '06/18/2026' })); + assert.strictEqual(await readMetaJson(envDir), undefined); + await writeRaw(JSON.stringify({ ...makeMeta(), lastUsedAt: 'June 18 2026' })); + assert.strictEqual(await readMetaJson(envDir), undefined); + }); + + test('extra unknown fields in a v1 sidecar are tolerated but dropped from the result', async () => { + await writeRaw(JSON.stringify({ ...makeMeta(), undocumented: 'x', _internal: 42 })); + const result = await readMetaJson(envDir); + assert.ok(result); + assert.strictEqual('undocumented' in result, false); + assert.strictEqual('_internal' in result, false); + }); + + test('returns undefined when the sidecar path is a directory rather than a file', async () => { + await fs.remove(getMetaJsonPath(envDir).fsPath).catch(() => undefined); + await fs.ensureDir(getMetaJsonPath(envDir).fsPath); + assert.strictEqual(await readMetaJson(envDir), undefined); + assert.ok(traceWarnStub.called); + }); + + test('returns undefined when the sidecar exceeds the size cap (1 MiB)', async () => { + const big = Buffer.alloc(1024 * 1024 + 1, 0x20); + await fs.writeFile(getMetaJsonPath(envDir).fsPath, big); + assert.strictEqual(await readMetaJson(envDir), undefined); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('bytes > cap')), + 'expected the size-cap warn message', + ); + }); + + test('ENOENT and other stat errors are reported with distinguishable messages', async () => { + await fs.remove(getMetaJsonPath(envDir).fsPath).catch(() => undefined); + assert.strictEqual(await readMetaJson(envDir), undefined); + const args = traceWarnStub.getCalls().map((c) => String(c.args[0])); + assert.ok( + args.some((a) => a.includes('not found')), + `expected an ENOENT-specific warn; saw: ${JSON.stringify(args)}`, + ); + }); + }); + + suite('selectStaleEntries', () => { + const TTL_MS = 14 * 24 * 60 * 60 * 1000; + const now = new Date('2026-06-22T12:00:00.000Z'); + + function entry(name: string, lastUsedAt: Date | undefined): CacheEntrySummary { + return { envDirPath: `/cache/${name}`, lastUsedAt }; + } + + test('entries older than the TTL are returned', () => { + const old = new Date(now.getTime() - TTL_MS - 1); + const stale = selectStaleEntries([entry('a', old)], now, TTL_MS); + assert.deepStrictEqual(stale, ['/cache/a']); + }); + + test('entries newer than the TTL are excluded', () => { + const recent = new Date(now.getTime() - 1000); + const stale = selectStaleEntries([entry('a', recent)], now, TTL_MS); + assert.deepStrictEqual(stale, []); + }); + + test('entries with age exactly equal to the TTL are NOT stale (strict cutoff)', () => { + const exact = new Date(now.getTime() - TTL_MS); + const stale = selectStaleEntries([entry('a', exact)], now, TTL_MS); + assert.deepStrictEqual(stale, []); + }); + + test('entries with undefined lastUsedAt are skipped (never TTLed)', () => { + const stale = selectStaleEntries( + [entry('a', undefined), entry('b', new Date(now.getTime() - TTL_MS - 1))], + now, + TTL_MS, + ); + assert.deepStrictEqual(stale, ['/cache/b']); + }); + + test('returns paths in input order (callers may sort if they care)', () => { + const old1 = new Date(now.getTime() - TTL_MS - 1000); + const old2 = new Date(now.getTime() - TTL_MS - 2000); + const stale = selectStaleEntries([entry('a', old1), entry('b', old2)], now, TTL_MS); + assert.deepStrictEqual(stale, ['/cache/a', '/cache/b']); + }); + + test('empty input returns empty output without throwing', () => { + assert.deepStrictEqual(selectStaleEntries([], now, TTL_MS), []); + }); + + test('TTL of zero treats every dated entry as stale (immediate eviction)', () => { + const stale = selectStaleEntries([entry('a', new Date(now.getTime() - 1)), entry('b', undefined)], now, 0); + assert.deepStrictEqual(stale, ['/cache/a']); + }); + + test('entries with timestamps in the future are not stale (age is negative)', () => { + const future = new Date(now.getTime() + TTL_MS); + const stale = selectStaleEntries([entry('a', future)], now, TTL_MS); + assert.deepStrictEqual(stale, []); + }); + }); + + suite('verifyEnvUsable', () => { + let tmpDir: string; + let envDir: Uri; + + setup(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'isclayout-verify-')); + envDir = Uri.file(path.join(tmpDir, 'env')); + await fs.ensureDir(envDir.fsPath); + }); + + teardown(async () => { + await fs.remove(tmpDir); + }); + + suite('POSIX branch (isWindows stubbed to false)', () => { + setup(() => { + sinon.stub(platformUtils, 'isWindows').returns(false); + }); + + test('returns true when bin/python exists as a regular file', async () => { + const binDir = path.join(envDir.fsPath, 'bin'); + await fs.ensureDir(binDir); + await fs.writeFile(path.join(binDir, 'python'), ''); + assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(traceWarnStub.called, false, 'no warn on success'); + }); + + const symlinkSuite = process.platform === 'win32' ? suite.skip : suite; + symlinkSuite('symlinks (POSIX host only)', () => { + test('returns true for a symlink to an existing file (dead-symlink discriminator)', async () => { + const binDir = path.join(envDir.fsPath, 'bin'); + await fs.ensureDir(binDir); + const target = path.join(tmpDir, 'real-python'); + await fs.writeFile(target, ''); + await fs.symlink(target, path.join(binDir, 'python')); + assert.strictEqual(await verifyEnvUsable(envDir), true); + }); + + test('returns false for a symlink whose target was removed (base uninstalled)', async () => { + const binDir = path.join(envDir.fsPath, 'bin'); + await fs.ensureDir(binDir); + const target = path.join(tmpDir, 'real-python'); + await fs.writeFile(target, ''); + await fs.symlink(target, path.join(binDir, 'python')); + await fs.remove(target); + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('missing')), + 'expected a missing-launcher warn', + ); + }); + }); + + test('returns false when bin/python does not exist', async () => { + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('missing')), + 'expected an ENOENT-shaped warn', + ); + }); + + test('returns false when bin/python is a directory', async () => { + const launcher = path.join(envDir.fsPath, 'bin', 'python'); + await fs.ensureDir(launcher); + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('not a regular file')), + ); + }); + }); + + suite('Windows branch (isWindows stubbed to true)', () => { + setup(() => { + sinon.stub(platformUtils, 'isWindows').returns(true); + }); + + async function writePyvenvCfg(content: string): Promise { + await fs.writeFile(path.join(envDir.fsPath, 'pyvenv.cfg'), content, 'utf8'); + } + + test('returns true when pyvenv.cfg.home points to an existing python.exe', async () => { + const homeDir = path.join(tmpDir, 'Python313'); + await fs.ensureDir(homeDir); + await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writePyvenvCfg(`home = ${homeDir}\ninclude-system-site-packages = false\nversion = 3.13.0\n`); + assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(traceWarnStub.called, false, 'no warn on success'); + }); + + test('returns false when pyvenv.cfg.home points to a removed python.exe', async () => { + const homeDir = path.join(tmpDir, 'Python313'); + await fs.ensureDir(homeDir); + await writePyvenvCfg(`home = ${homeDir}\n`); + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('missing')), + 'expected a missing-launcher warn', + ); + }); + + test('returns false when pyvenv.cfg is missing entirely', async () => { + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub + .getCalls() + .some((c) => String(c.args[0]).includes('missing pyvenv.cfg')), + 'expected a missing-pyvenv.cfg warn', + ); + }); + + test('returns false when pyvenv.cfg has no `home =` line', async () => { + await writePyvenvCfg('include-system-site-packages = false\nversion = 3.13.0\n'); + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes("no 'home =' line")), + ); + }); + + test('returns false when pyvenv.cfg has an empty home value', async () => { + await writePyvenvCfg('home =\n'); + assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes("no 'home =' line")), + ); + }); + + test('tolerates extra whitespace around the home = key/value', async () => { + const homeDir = path.join(tmpDir, 'Python313'); + await fs.ensureDir(homeDir); + await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writePyvenvCfg(` home = ${homeDir} \n`); + assert.strictEqual(await verifyEnvUsable(envDir), true); + }); + + test('tolerates CRLF line endings', async () => { + const homeDir = path.join(tmpDir, 'Python313'); + await fs.ensureDir(homeDir); + await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writePyvenvCfg(`home = ${homeDir}\r\nversion = 3.13.0\r\n`); + assert.strictEqual(await verifyEnvUsable(envDir), true); + }); + }); + }); +}); From 8bfd9f0ee393eb73ae432b6d60c3730adc905707 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 20 Jul 2026 14:44:39 -0700 Subject: [PATCH 2/2] refactor(inline-scripts): clarify cache validation scope --- src/common/inlineScriptCacheLayout.ts | 11 ++++++- .../inlineScriptCacheLayout.unit.test.ts | 33 +++++++++++-------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/common/inlineScriptCacheLayout.ts b/src/common/inlineScriptCacheLayout.ts index 108d8219..00ec30c7 100644 --- a/src/common/inlineScriptCacheLayout.ts +++ b/src/common/inlineScriptCacheLayout.ts @@ -44,6 +44,15 @@ export function getMetaJsonPath(envDir: Uri): Uri { const MAX_META_JSON_BYTES = 1024 * 1024; +/** + * 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. + */ export async function readMetaJson(envDir: Uri): Promise { const metaPath = getMetaJsonPath(envDir).fsPath; @@ -140,7 +149,7 @@ export function selectStaleEntries(entries: ReadonlyArray, no /** * Verify that a cached env's base interpreter still exists on disk. */ -export async function verifyEnvUsable(envDir: Uri): Promise { +export async function verifyBaseInterpreterExists(envDir: Uri): Promise { if (isWindows()) { return verifyWindowsBaseInterpreter(envDir); } diff --git a/src/test/common/inlineScriptCacheLayout.unit.test.ts b/src/test/common/inlineScriptCacheLayout.unit.test.ts index 51b8ee85..dfc0517b 100644 --- a/src/test/common/inlineScriptCacheLayout.unit.test.ts +++ b/src/test/common/inlineScriptCacheLayout.unit.test.ts @@ -18,7 +18,7 @@ import { getScriptEnvDir, readMetaJson, selectStaleEntries, - verifyEnvUsable, + verifyBaseInterpreterExists, writeMetaJson, } from '../../common/inlineScriptCacheLayout'; import * as logging from '../../common/logging'; @@ -180,6 +180,11 @@ suite('inlineScriptCacheLayout', () => { const result = await readMetaJson(envDir); assert.strictEqual(result, undefined); assert.ok(traceWarnStub.called); + assert.strictEqual( + await fs.pathExists(getMetaJsonPath(envDir).fsPath), + true, + 'reading malformed metadata must not delete it', + ); }); test('returns undefined for an unknown schemaVersion', async () => { @@ -348,7 +353,7 @@ suite('inlineScriptCacheLayout', () => { }); }); - suite('verifyEnvUsable', () => { + suite('verifyBaseInterpreterExists', () => { let tmpDir: string; let envDir: Uri; @@ -371,7 +376,7 @@ suite('inlineScriptCacheLayout', () => { const binDir = path.join(envDir.fsPath, 'bin'); await fs.ensureDir(binDir); await fs.writeFile(path.join(binDir, 'python'), ''); - assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); assert.strictEqual(traceWarnStub.called, false, 'no warn on success'); }); @@ -383,7 +388,7 @@ suite('inlineScriptCacheLayout', () => { const target = path.join(tmpDir, 'real-python'); await fs.writeFile(target, ''); await fs.symlink(target, path.join(binDir, 'python')); - assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); }); test('returns false for a symlink whose target was removed (base uninstalled)', async () => { @@ -393,7 +398,7 @@ suite('inlineScriptCacheLayout', () => { await fs.writeFile(target, ''); await fs.symlink(target, path.join(binDir, 'python')); await fs.remove(target); - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('missing')), 'expected a missing-launcher warn', @@ -402,7 +407,7 @@ suite('inlineScriptCacheLayout', () => { }); test('returns false when bin/python does not exist', async () => { - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('missing')), 'expected an ENOENT-shaped warn', @@ -412,7 +417,7 @@ suite('inlineScriptCacheLayout', () => { test('returns false when bin/python is a directory', async () => { const launcher = path.join(envDir.fsPath, 'bin', 'python'); await fs.ensureDir(launcher); - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('not a regular file')), ); @@ -433,7 +438,7 @@ suite('inlineScriptCacheLayout', () => { await fs.ensureDir(homeDir); await fs.writeFile(path.join(homeDir, 'python.exe'), ''); await writePyvenvCfg(`home = ${homeDir}\ninclude-system-site-packages = false\nversion = 3.13.0\n`); - assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); assert.strictEqual(traceWarnStub.called, false, 'no warn on success'); }); @@ -441,7 +446,7 @@ suite('inlineScriptCacheLayout', () => { const homeDir = path.join(tmpDir, 'Python313'); await fs.ensureDir(homeDir); await writePyvenvCfg(`home = ${homeDir}\n`); - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('missing')), 'expected a missing-launcher warn', @@ -449,7 +454,7 @@ suite('inlineScriptCacheLayout', () => { }); test('returns false when pyvenv.cfg is missing entirely', async () => { - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub .getCalls() @@ -460,7 +465,7 @@ suite('inlineScriptCacheLayout', () => { test('returns false when pyvenv.cfg has no `home =` line', async () => { await writePyvenvCfg('include-system-site-packages = false\nversion = 3.13.0\n'); - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub.getCalls().some((c) => String(c.args[0]).includes("no 'home =' line")), ); @@ -468,7 +473,7 @@ suite('inlineScriptCacheLayout', () => { test('returns false when pyvenv.cfg has an empty home value', async () => { await writePyvenvCfg('home =\n'); - assert.strictEqual(await verifyEnvUsable(envDir), false); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub.getCalls().some((c) => String(c.args[0]).includes("no 'home =' line")), ); @@ -479,7 +484,7 @@ suite('inlineScriptCacheLayout', () => { await fs.ensureDir(homeDir); await fs.writeFile(path.join(homeDir, 'python.exe'), ''); await writePyvenvCfg(` home = ${homeDir} \n`); - assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); }); test('tolerates CRLF line endings', async () => { @@ -487,7 +492,7 @@ suite('inlineScriptCacheLayout', () => { await fs.ensureDir(homeDir); await fs.writeFile(path.join(homeDir, 'python.exe'), ''); await writePyvenvCfg(`home = ${homeDir}\r\nversion = 3.13.0\r\n`); - assert.strictEqual(await verifyEnvUsable(envDir), true); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); }); }); });