From f7f5a3932e3890f0d5f4f488799eb69f8bca1112 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Wed, 29 Jul 2026 18:09:49 -0500 Subject: [PATCH 01/16] Add docs bundle types, URL resolution, and manifest parsing --- .../positronDocs/common/positronDocsBundle.ts | 178 ++++++++++++++++++ .../test/common/positronDocsBundle.vitest.ts | 115 +++++++++++ 2 files changed, 293 insertions(+) create mode 100644 src/vs/platform/positronDocs/common/positronDocsBundle.ts create mode 100644 src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts diff --git a/src/vs/platform/positronDocs/common/positronDocsBundle.ts b/src/vs/platform/positronDocs/common/positronDocsBundle.ts new file mode 100644 index 00000000000..e64445ea0b6 --- /dev/null +++ b/src/vs/platform/positronDocs/common/positronDocsBundle.ts @@ -0,0 +1,178 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { formatPositronVersion } from '../../extensionManagement/common/positronGalleryTelemetry.js'; + +/** + * The only bundle layout this build understands. Bumped by the website + * pipeline only when a reader written against the old layout would get a + * wrong answer. Schema 1 is defined as including "llms.txt uses + * bundle-relative paths". + */ +export const DOCS_BUNDLE_SCHEMA = 1; + +/** Name of the persisted state file inside the cache root. */ +export const DOCS_STATE_FILENAME = 'state.json'; + +/** Files a valid extracted bundle must contain. */ +export const DOCS_MANIFEST_FILENAME = 'bundle.json'; +export const DOCS_INDEX_FILENAME = 'llms.txt'; + +/** Refuse anything larger. The real bundle is about 150KB. */ +export const DOCS_MAX_DOWNLOAD_BYTES = 5 * 1024 * 1024; + +export type DocsProfile = 'positron' | 'workbench'; + +/** + * How the cached bundle relates to the running build. + * - `exact`: bundle version equals app version. Terminal; no further network. + * - `fallback`: exact was not published (yet). Re-attempts exact every launch. + * - `latest-by-policy`: dailies and dev builds, where latest is the target. + */ +export type DocsResolution = 'exact' | 'fallback' | 'latest-by-policy'; + +/** The shape of bundle.json, as produced by the website pipeline. */ +export interface IDocsBundleManifest { + readonly schema: number; + readonly profile: string; + readonly version: string; + readonly generated: string; + readonly docsBaseUrl: string; + readonly fileCount: number; +} + +/** Persisted cache state. Written atomically; never partially trusted. */ +export interface IDocsCacheState { + readonly schema: number; + /** Version of the bundle actually on disk; also its directory name. */ + readonly version: string; + /** Version the app asked for, which may differ while in `fallback`. */ + readonly requestedVersion: string; + readonly resolution: DocsResolution; + readonly profile: string; + /** + * Digest verified before extraction. Diagnostic only: recorded once and + * never recomputed, since the zip is deleted after extracting. + */ + readonly sha256: string; + readonly etag?: string; + readonly sourceUrl: string; + readonly fetchedAt: number; + readonly lastAttemptAt: number; + readonly lastFailureAt?: number; + readonly lastError?: string; +} + +export interface IDocsBundleRequest { + /** `initData.quality`: 'releases', 'dailies', or undefined in dev builds. */ + readonly quality: string | undefined; + readonly positronVersion: string; + readonly positronBuildNumber: number; + readonly profile: DocsProfile; + readonly baseUrl: string; +} + +export interface IResolvedBundle { + /** `` for the exact form, the literal 'latest' for the alias. */ + readonly version: string; + readonly form: 'exact' | 'latest'; + readonly zipUrl: string; + readonly sha256Url: string; +} + +export interface IResolvedBundleRequest { + readonly exact: IResolvedBundle; + readonly latest: IResolvedBundle; + /** + * True for release builds, which target their exact version and fall back + * to latest only until it publishes. False for dailies and dev builds, + * where latest is the intended target. + */ + readonly wantsExact: boolean; +} + +function bundleBaseName(profile: DocsProfile): string { + return profile === 'workbench' ? 'positron-workbench-llms' : 'positron-llms'; +} + +function bundleUrls(baseUrl: string, profile: DocsProfile, version: string, form: 'exact' | 'latest'): IResolvedBundle { + const root = baseUrl.endsWith('/') ? baseUrl.slice(0, -1) : baseUrl; + const zipUrl = `${root}/${bundleBaseName(profile)}-${version}.zip`; + return { version, form, zipUrl, sha256Url: `${zipUrl}.sha256sum` }; +} + +/** + * Work out which bundle URLs this build should ask for. + * + * Release builds target their exact version so a shipped build reads the docs + * it shipped with; everything else targets the mutable `latest` alias. Dev + * builds landing on `latest` is deliberate - it makes the feature exercisable + * locally and in PRs without waiting on a release. + */ +export function resolveBundleRequest(request: IDocsBundleRequest): IResolvedBundleRequest { + const version = formatPositronVersion(request.positronVersion, request.positronBuildNumber); + return { + exact: bundleUrls(request.baseUrl, request.profile, version, 'exact'), + latest: bundleUrls(request.baseUrl, request.profile, 'latest', 'latest'), + wantsExact: request.quality === 'releases', + }; +} + +function isNonEmptyString(value: unknown): value is string { + return typeof value === 'string' && value.length > 0; +} + +/** + * Parse bundle.json, rejecting anything this build cannot read. + * + * Rejecting is deliberate over guessing: dev and daily builds fetch the mutable + * `latest` alias, so "an app from three months ago is handed a bundle from + * today's pipeline" is a normal runtime state. A misparse surfaces later as + * wrong docs content; a rejection surfaces immediately as web fallback. + */ +export function parseManifest(raw: string): IDocsBundleManifest | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + if (typeof parsed !== 'object' || parsed === null) { + return undefined; + } + const candidate = parsed as Partial; + if (candidate.schema !== DOCS_BUNDLE_SCHEMA) { + return undefined; + } + if (!isNonEmptyString(candidate.profile) || !isNonEmptyString(candidate.version) + || !isNonEmptyString(candidate.generated) || !isNonEmptyString(candidate.docsBaseUrl)) { + return undefined; + } + if (typeof candidate.fileCount !== 'number' || !Number.isInteger(candidate.fileCount) || candidate.fileCount <= 0) { + return undefined; + } + return { + schema: candidate.schema, + profile: candidate.profile, + version: candidate.version, + generated: candidate.generated, + docsBaseUrl: candidate.docsBaseUrl, + fileCount: candidate.fileCount, + }; +} + +const SHA256_HEX = /^[0-9a-f]{64}$/; + +/** + * Read the hex digest out of a `.sha256sum` sidecar. + * + * Accepts both `shasum -a 256` output (` `) and a bare digest. + * Returns undefined for anything else, including an HTML error page served in + * place of a missing object - which the caller must treat as a hard failure. + */ +export function parseSha256Sidecar(raw: string): string | undefined { + const first = raw.trim().split(/\s+/)[0]?.toLowerCase(); + return first && SHA256_HEX.test(first) ? first : undefined; +} diff --git a/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts new file mode 100644 index 00000000000..ab175655c89 --- /dev/null +++ b/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts @@ -0,0 +1,115 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ +/// + +import { DocsProfile, parseManifest, parseSha256Sidecar, resolveBundleRequest } from '../../common/positronDocsBundle.js'; + +const BASE = 'https://cdn.posit.co/positron/releases/docs'; + +function request(overrides: { profile?: DocsProfile; build?: number } = {}) { + return { + quality: 'releases' as string | undefined, + positronVersion: '2026.05.0', + positronBuildNumber: overrides.build ?? 179, + profile: overrides.profile ?? ('positron' as DocsProfile), + baseUrl: BASE, + }; +} + +describe('resolveBundleRequest', () => { + // The three quality values are verified against build/utils.ts, not assumed. + // A future channel rename must fail here rather than silently change behaviour. + it.each<[string | undefined, boolean]>([ + ['releases', true], + ['dailies', false], + [undefined, false], + ])('quality %s => wantsExact %s', (quality, wantsExact) => { + expect(resolveBundleRequest({ ...request(), quality }).wantsExact).toBe(wantsExact); + }); + + it('builds exact and latest URLs plus sidecars for the positron profile', () => { + const { exact, latest } = resolveBundleRequest(request()); + expect({ exact, latest }).toMatchInlineSnapshot(` + { + "exact": { + "form": "exact", + "sha256Url": "https://cdn.posit.co/positron/releases/docs/positron-llms-2026.05.0-179.zip.sha256sum", + "version": "2026.05.0-179", + "zipUrl": "https://cdn.posit.co/positron/releases/docs/positron-llms-2026.05.0-179.zip", + }, + "latest": { + "form": "latest", + "sha256Url": "https://cdn.posit.co/positron/releases/docs/positron-llms-latest.zip.sha256sum", + "version": "latest", + "zipUrl": "https://cdn.posit.co/positron/releases/docs/positron-llms-latest.zip", + }, + } + `); + }); + + it('uses the workbench basename for the workbench profile', () => { + expect(resolveBundleRequest(request({ profile: 'workbench' })).exact.zipUrl) + .toBe(`${BASE}/positron-workbench-llms-2026.05.0-179.zip`); + }); + + it('omits the -0 suffix for dev builds', () => { + expect(resolveBundleRequest(request({ build: 0 })).exact.version).toBe('2026.05.0'); + }); + + it('tolerates a trailing slash on the base URL', () => { + expect(resolveBundleRequest({ ...request(), baseUrl: `${BASE}/` }).latest.zipUrl) + .toBe(`${BASE}/positron-llms-latest.zip`); + }); +}); + +describe('parseManifest', () => { + const valid = JSON.stringify({ + schema: 1, profile: 'positron', version: '2026.05.0-179', + generated: '2026-07-24T18:02:11Z', docsBaseUrl: 'https://positron.posit.co/', fileCount: 90, + }); + + it('accepts a well-formed schema 1 manifest', () => { + expect(parseManifest(valid)).toMatchInlineSnapshot(` + { + "docsBaseUrl": "https://positron.posit.co/", + "fileCount": 90, + "generated": "2026-07-24T18:02:11Z", + "profile": "positron", + "schema": 1, + "version": "2026.05.0-179", + } + `); + }); + + it.each([ + ['schema 2', JSON.stringify({ ...JSON.parse(valid), schema: 2 })], + ['malformed JSON', '{ not json'], + ['missing version', JSON.stringify({ schema: 1, profile: 'positron', fileCount: 90, docsBaseUrl: 'x', generated: 'y' })], + ['non-numeric fileCount', JSON.stringify({ ...JSON.parse(valid), fileCount: 'ninety' })], + ])('rejects %s', (_label, raw) => { + expect(parseManifest(raw)).toBeUndefined(); + }); +}); + +describe('parseSha256Sidecar', () => { + const digest = 'a'.repeat(64); + + it.each([ + ['shasum format', `${digest} positron-llms-latest.zip\n`], + ['bare hex', `${digest}\n`], + ['uppercase hex', `${digest.toUpperCase()} x.zip`], + ])('accepts %s', (_label, raw) => { + expect(parseSha256Sidecar(raw)).toBe(digest); + }); + + it.each([ + ['empty', ''], + ['too short', 'abc123 x.zip'], + ['non-hex', `${'z'.repeat(64)} x.zip`], + ['an HTML error page', '404'], + ])('rejects %s', (_label, raw) => { + expect(parseSha256Sidecar(raw)).toBeUndefined(); + }); +}); From 2c06840da6da0c0d9ef7e2f01e3c5e63510d578f Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Wed, 29 Jul 2026 18:12:50 -0500 Subject: [PATCH 02/16] Add docs cache ports, test fakes, and extraction validation --- .../positronDocs/common/positronDocsPorts.ts | 83 +++++++ .../common/positronDocsValidate.ts | 93 +++++++ .../positronDocs/test/common/fakes.ts | 226 ++++++++++++++++++ .../common/positronDocsValidate.vitest.ts | 74 ++++++ 4 files changed, 476 insertions(+) create mode 100644 src/vs/platform/positronDocs/common/positronDocsPorts.ts create mode 100644 src/vs/platform/positronDocs/common/positronDocsValidate.ts create mode 100644 src/vs/platform/positronDocs/test/common/fakes.ts create mode 100644 src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts diff --git a/src/vs/platform/positronDocs/common/positronDocsPorts.ts b/src/vs/platform/positronDocs/common/positronDocsPorts.ts new file mode 100644 index 00000000000..882427b97bd --- /dev/null +++ b/src/vs/platform/positronDocs/common/positronDocsPorts.ts @@ -0,0 +1,83 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/** + * Ports for the docs cache. Deliberately three narrow interfaces rather than + * one wide one, so each test fake stays small and the seam can be re-hosted in + * a node service later without a rewrite. + * + * Paths are plain strings joined with forward slashes. Node's fs accepts + * forward slashes on Windows, so no platform-specific joining is needed here + * and `common` stays free of node imports. + */ + +export interface IDocsHttpResponse { + readonly status: number; + readonly etag?: string; + /** Absent on 304, on any error status, and on HEAD. */ + readonly body?: Uint8Array; +} + +export interface IDocsHttpGetOptions { + /** Send as `If-None-Match`, so an unchanged alias answers 304. */ + readonly etag?: string; + /** Abort and reject once the response exceeds this many bytes. */ + readonly maxBytes?: number; +} + +export interface IDocsHttpClient { + get(url: string, options?: IDocsHttpGetOptions): Promise; + head(url: string): Promise; +} + +export interface IDocsFileStore { + exists(path: string): Promise; + readFile(path: string): Promise; + writeFile(path: string, data: string | Uint8Array): Promise; + /** Recursive; succeeds if the directory already exists. */ + mkdir(path: string): Promise; + rename(from: string, to: string): Promise; + /** Recursive; succeeds if the path does not exist. */ + delete(path: string): Promise; + /** Immediate children, names only. Empty array if the path is missing. */ + readdir(path: string): Promise; + /** Epoch millis, or undefined if the path is missing. Used by the prune guard. */ + mtime(path: string): Promise; + /** Lowercase hex digest of the file's bytes. */ + sha256(path: string): Promise; +} + +export interface IDocsArchive { + /** Entry paths as recorded in the archive, before any extraction. */ + entryNames(zipPath: string): Promise; + extract(zipPath: string, targetPath: string): Promise; +} + +/** + * Narrow logger so the seam does not depend on ILogService. Nothing here is + * user-actionable, so there is deliberately no error level. + */ +export interface IDocsLogger { + info(message: string): void; + warn(message: string): void; +} + +/** What `positron.docs.getLocalDocs()` resolves to. */ +export interface ILocalDocs { + readonly path: string; + readonly schema: number; + readonly version: string; + readonly profile: string; + readonly docsBaseUrl: string; + readonly isExactMatch: boolean; +} + +/** Join path segments with forward slashes, collapsing duplicates. */ +export function joinDocsPath(...segments: string[]): string { + return segments + .filter(segment => segment.length > 0) + .map((segment, index) => index === 0 ? segment.replace(/\/+$/, '') : segment.replace(/^\/+|\/+$/g, '')) + .join('/'); +} diff --git a/src/vs/platform/positronDocs/common/positronDocsValidate.ts b/src/vs/platform/positronDocs/common/positronDocsValidate.ts new file mode 100644 index 00000000000..b626a9ee631 --- /dev/null +++ b/src/vs/platform/positronDocs/common/positronDocsValidate.ts @@ -0,0 +1,93 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { DOCS_INDEX_FILENAME, DOCS_MANIFEST_FILENAME, IDocsBundleManifest, parseManifest } from './positronDocsBundle.js'; +import { IDocsFileStore, joinDocsPath } from './positronDocsPorts.js'; + +export type DocsValidationFailure = + | 'missing-manifest' + | 'missing-index' + | 'bad-manifest' + | 'file-count-mismatch'; + +export type DocsValidationResult = + | { readonly ok: true; readonly manifest: IDocsBundleManifest } + | { readonly ok: false; readonly reason: DocsValidationFailure }; + +/** + * Reject archive entries that could write outside the extraction target. + * + * base/node/zip.ts does some of this, but the archive arrives over the network, + * so we assert it ourselves rather than trusting it. Returns the first + * offending entry, or undefined when every entry is safe. + */ +export function guardEntryNames(names: readonly string[]): string | undefined { + for (const name of names) { + if (name.includes('\u0000')) { + return name; + } + // Normalise Windows separators before reasoning about segments. + const normalized = name.replace(/\\/g, '/'); + if (normalized.startsWith('/') || /^[a-zA-Z]:/.test(normalized)) { + return name; + } + let depth = 0; + for (const segment of normalized.split('/')) { + if (segment === '' || segment === '.') { + continue; + } + depth += segment === '..' ? -1 : 1; + if (depth < 0) { + return name; + } + } + } + return undefined; +} + +/** + * Check an extracted bundle before it is swapped into place. + * + * Deliberately structural and cheap: bundle.json parses at a schema we + * understand, llms.txt is present, and the extracted file count matches what + * the manifest declared. A corrupted byte inside a Markdown page degrades one + * assistant answer rather than compromising anything, so byte-level integrity + * is the digest's job at download time, not this function's. + */ +export async function validateExtractedBundle(files: IDocsFileStore, dir: string): Promise { + const manifestPath = joinDocsPath(dir, DOCS_MANIFEST_FILENAME); + if (!await files.exists(manifestPath)) { + return { ok: false, reason: 'missing-manifest' }; + } + if (!await files.exists(joinDocsPath(dir, DOCS_INDEX_FILENAME))) { + return { ok: false, reason: 'missing-index' }; + } + + const manifest = parseManifest(await files.readFile(manifestPath)); + if (!manifest) { + return { ok: false, reason: 'bad-manifest' }; + } + + const actual = await countFiles(files, dir); + if (actual !== manifest.fileCount) { + return { ok: false, reason: 'file-count-mismatch' }; + } + return { ok: true, manifest }; +} + +async function countFiles(files: IDocsFileStore, dir: string): Promise { + let count = 0; + for (const name of await files.readdir(dir)) { + const child = joinDocsPath(dir, name); + // A path that reads as a file is one; anything else recurses. + const children = await files.readdir(child); + if (children.length === 0) { + count += 1; + } else { + count += await countFiles(files, child); + } + } + return count; +} diff --git a/src/vs/platform/positronDocs/test/common/fakes.ts b/src/vs/platform/positronDocs/test/common/fakes.ts new file mode 100644 index 00000000000..97b3ef7cc36 --- /dev/null +++ b/src/vs/platform/positronDocs/test/common/fakes.ts @@ -0,0 +1,226 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsHttpGetOptions, IDocsHttpResponse, IDocsLogger } from '../../common/positronDocsPorts.js'; + +/** + * Deterministic stand-in for a real sha256. Only equality matters in these + * tests, and exporting it lets a test compute the digest a sidecar should + * carry without duplicating the algorithm. + */ +export function fakeDigest(contents: string): string { + let hash = 0; + for (let i = 0; i < contents.length; i++) { + hash = (Math.imul(hash, 31) + contents.charCodeAt(i)) | 0; + } + return (hash >>> 0).toString(16).padStart(64, '0'); +} + +/** + * In-memory file store. Directories are implicit: a path is a directory if any + * stored key starts with it plus a slash. + */ +export class FakeFileStore implements IDocsFileStore { + readonly files = new Map(); + readonly mtimes = new Map(); + /** Set to a path prefix to make every write under it fail, simulating a full disk. */ + failWritesUnder: string | undefined; + /** Digest overrides, keyed by path. Defaults to a hash of the contents. */ + readonly digests = new Map(); + + constructor(initial: Record = {}) { + for (const [path, contents] of Object.entries(initial)) { + this.files.set(path, contents); + this.mtimes.set(path, 0); + } + } + + private isDir(path: string): boolean { + const prefix = `${path}/`; + for (const key of this.files.keys()) { + if (key.startsWith(prefix)) { + return true; + } + } + return this.mtimes.has(path); + } + + async exists(path: string): Promise { + return this.files.has(path) || this.isDir(path); + } + + async readFile(path: string): Promise { + const contents = this.files.get(path); + if (contents === undefined) { + throw new Error(`ENOENT: ${path}`); + } + return contents; + } + + async writeFile(path: string, data: string | Uint8Array): Promise { + if (this.failWritesUnder && path.startsWith(this.failWritesUnder)) { + throw new Error(`ENOSPC: no space left on device, write '${path}'`); + } + // Decode rather than record a length: the cache writes downloaded zip + // bytes through this method, and FakeArchive must be able to read them + // back as its fake-zip payload string. + this.files.set(path, typeof data === 'string' ? data : new TextDecoder().decode(data)); + this.mtimes.set(path, 0); + } + + async mkdir(path: string): Promise { + this.mtimes.set(path, this.mtimes.get(path) ?? 0); + } + + async rename(from: string, to: string): Promise { + for (const [key, value] of [...this.files]) { + if (key === from || key.startsWith(`${from}/`)) { + this.files.delete(key); + this.files.set(to + key.slice(from.length), value); + } + } + for (const [key, value] of [...this.mtimes]) { + if (key === from || key.startsWith(`${from}/`)) { + this.mtimes.delete(key); + this.mtimes.set(to + key.slice(from.length), value); + } + } + } + + async delete(path: string): Promise { + for (const key of [...this.files.keys()]) { + if (key === path || key.startsWith(`${path}/`)) { + this.files.delete(key); + } + } + for (const key of [...this.mtimes.keys()]) { + if (key === path || key.startsWith(`${path}/`)) { + this.mtimes.delete(key); + } + } + } + + async readdir(path: string): Promise { + const prefix = `${path}/`; + const names = new Set(); + for (const key of [...this.files.keys(), ...this.mtimes.keys()]) { + if (key.startsWith(prefix)) { + names.add(key.slice(prefix.length).split('/')[0]); + } + } + return [...names]; + } + + async mtime(path: string): Promise { + return this.mtimes.get(path); + } + + async sha256(path: string): Promise { + const override = this.digests.get(path); + if (override !== undefined) { + return override; + } + const contents = this.files.get(path); + if (contents === undefined) { + throw new Error(`ENOENT: ${path}`); + } + return fakeDigest(contents); + } + + /** Every file path currently stored under `dir`, recursively. */ + listUnder(dir: string): string[] { + return [...this.files.keys()].filter(key => key.startsWith(`${dir}/`)).sort(); + } +} + +export interface IFakeHttpRoute { + readonly status: number; + readonly body?: string; + readonly etag?: string; + /** Throw instead of responding, simulating DNS or connection failure. */ + readonly throws?: string; + /** Response size in bytes for the maxBytes check; defaults to body length. */ + readonly byteLength?: number; +} + +export class FakeHttpClient implements IDocsHttpClient { + readonly getCalls: string[] = []; + readonly headCalls: string[] = []; + private readonly routes = new Map(); + + route(url: string, route: IFakeHttpRoute): this { + this.routes.set(url, route); + return this; + } + + async get(url: string, options?: IDocsHttpGetOptions): Promise { + this.getCalls.push(url); + const route = this.routes.get(url) ?? { status: 404 }; + if (route.throws) { + throw new Error(route.throws); + } + const size = route.byteLength ?? route.body?.length ?? 0; + if (options?.maxBytes !== undefined && size > options.maxBytes) { + throw new Error(`docs bundle exceeds ${options.maxBytes} bytes`); + } + if (options?.etag !== undefined && route.etag !== undefined && options.etag === route.etag) { + return { status: 304, etag: route.etag }; + } + if (route.status !== 200) { + return { status: route.status }; + } + return { status: 200, etag: route.etag, body: new TextEncoder().encode(route.body ?? '') }; + } + + async head(url: string): Promise { + this.headCalls.push(url); + const route = this.routes.get(url) ?? { status: 404 }; + if (route.throws) { + throw new Error(route.throws); + } + return { status: route.status, etag: route.etag }; + } +} + +/** + * Fake archive. A "zip" is the string the file store holds at its path, of the + * form `zip:=;=`. + */ +export class FakeArchive implements IDocsArchive { + constructor(private readonly files: FakeFileStore) { } + + private parse(zipPath: string): Array<[string, string]> { + const raw = this.files.files.get(zipPath); + if (raw === undefined || !raw.startsWith('zip:')) { + throw new Error(`end of central directory record signature not found: ${zipPath}`); + } + return raw.slice(4).split(';').filter(part => part.length > 0) + .map(part => { + const index = part.indexOf('='); + return [part.slice(0, index), part.slice(index + 1)] as [string, string]; + }); + } + + async entryNames(zipPath: string): Promise { + return this.parse(zipPath).map(([name]) => name); + } + + async extract(zipPath: string, targetPath: string): Promise { + for (const [name, contents] of this.parse(zipPath)) { + await this.files.writeFile(`${targetPath}/${name}`, contents); + } + } +} + +/** Build the fake-zip payload string for a set of entries. */ +export function fakeZip(entries: Record): string { + return `zip:${Object.entries(entries).map(([name, contents]) => `${name}=${contents}`).join(';')}`; +} + +export function recordingLogger(): IDocsLogger & { readonly infos: string[]; readonly warns: string[] } { + const infos: string[] = []; + const warns: string[] = []; + return { infos, warns, info: (m: string) => { infos.push(m); }, warn: (m: string) => { warns.push(m); } }; +} diff --git a/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts new file mode 100644 index 00000000000..1a1ee32eafe --- /dev/null +++ b/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts @@ -0,0 +1,74 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ +/// + +import { guardEntryNames, validateExtractedBundle } from '../../common/positronDocsValidate.js'; +import { FakeFileStore } from './fakes.js'; + +describe('guardEntryNames', () => { + it('accepts ordinary nested entries', () => { + expect(guardEntryNames(['llms.txt', 'bundle.json', 'release-notes/release-2026-05.llms.md'])).toBeUndefined(); + }); + + // The archive arrives over the network, so we assert these ourselves rather + // than trusting base/node/zip.ts to have done it. + it.each([ + ['an absolute posix path', '/etc/passwd'], + ['a windows drive path', 'C:\\Windows\\system32'], + ['a parent traversal', '../../outside.md'], + ['a nested parent traversal', 'docs/../../outside.md'], + ['a null byte', 'llms\u0000.txt'], + ['a UNC path', '\\\\server\\share\\x'], + ])('rejects %s', (_label, entry) => { + expect(guardEntryNames(['llms.txt', entry])).toContain(entry); + }); +}); + +describe('validateExtractedBundle', () => { + const manifest = JSON.stringify({ + schema: 1, profile: 'positron', version: '2026.05.0-179', + generated: '2026-07-24T18:02:11Z', docsBaseUrl: 'https://positron.posit.co/', fileCount: 3, + }); + + function store(entries: Record) { + return new FakeFileStore(entries); + } + + it('accepts a well-formed extracted bundle', async () => { + const files = store({ + '/c/2026.05.0-179/bundle.json': manifest, + '/c/2026.05.0-179/llms.txt': '# Positron\n', + '/c/2026.05.0-179/welcome.llms.md': '# Welcome\n', + }); + const result = await validateExtractedBundle(files, '/c/2026.05.0-179'); + expect(result.ok && result.manifest.version).toBe('2026.05.0-179'); + }); + + it('rejects a missing bundle.json', async () => { + const files = store({ '/c/x/llms.txt': '# Positron\n' }); + expect(await validateExtractedBundle(files, '/c/x')).toMatchObject({ ok: false, reason: 'missing-manifest' }); + }); + + it('rejects a missing llms.txt', async () => { + const files = store({ '/c/x/bundle.json': manifest }); + expect(await validateExtractedBundle(files, '/c/x')).toMatchObject({ ok: false, reason: 'missing-index' }); + }); + + it('rejects schema 2', async () => { + const files = store({ + '/c/x/bundle.json': JSON.stringify({ ...JSON.parse(manifest), schema: 2 }), + '/c/x/llms.txt': '# Positron\n', + }); + expect(await validateExtractedBundle(files, '/c/x')).toMatchObject({ ok: false, reason: 'bad-manifest' }); + }); + + it('rejects a fileCount that does not match what was extracted', async () => { + const files = store({ + '/c/x/bundle.json': JSON.stringify({ ...JSON.parse(manifest), fileCount: 99 }), + '/c/x/llms.txt': '# Positron\n', + }); + expect(await validateExtractedBundle(files, '/c/x')).toMatchObject({ ok: false, reason: 'file-count-mismatch' }); + }); +}); From cbea5c04c5532dba8925c24fa4c07dbcf9e8e70b Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Wed, 29 Jul 2026 18:16:46 -0500 Subject: [PATCH 03/16] Add docs cache install path with digest verification and extraction guards --- .../positronDocs/common/positronDocsCache.ts | 279 ++++++++++++++++++ .../test/common/positronDocsCache.vitest.ts | 235 +++++++++++++++ 2 files changed, 514 insertions(+) create mode 100644 src/vs/platform/positronDocs/common/positronDocsCache.ts create mode 100644 src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts new file mode 100644 index 00000000000..8662c9ed4e8 --- /dev/null +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -0,0 +1,279 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { + DOCS_BUNDLE_SCHEMA, DOCS_MAX_DOWNLOAD_BYTES, DOCS_STATE_FILENAME, DocsResolution, + IDocsBundleManifest, IDocsBundleRequest, IDocsCacheState, IResolvedBundle, + parseSha256Sidecar, resolveBundleRequest, +} from './positronDocsBundle.js'; +import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsLogger, ILocalDocs, joinDocsPath } from './positronDocsPorts.js'; +import { guardEntryNames, validateExtractedBundle } from './positronDocsValidate.js'; + +const LOG_PREFIX = '[positron-docs]'; + +export interface IPositronDocsCacheOptions { + /** Cache root, e.g. `/User/positron-docs`. */ + readonly rootPath: string; + readonly http: IDocsHttpClient; + readonly files: IDocsFileStore; + readonly archive: IDocsArchive; + readonly logger: IDocsLogger; + /** Injected so tests control time without faking timers. */ + readonly now: () => number; + /** Injected so temp and staging names are deterministic in tests. */ + readonly newId: () => string; +} + +/** Outcome of one download attempt. */ +type InstallOutcome = + | { readonly kind: 'installed'; readonly docs: ILocalDocs; readonly manifest: IDocsBundleManifest; readonly digest: string; readonly etag?: string } + | { readonly kind: 'not-modified' } + | { readonly kind: 'not-found' } + /** Verification or validation refused the payload. Never throttled. */ + | { readonly kind: 'rejected'; readonly reason: string } + /** Network, 5xx, or disk. Throttled across sessions (Task 7). */ + | { readonly kind: 'failed'; readonly reason: string }; + +function toLocalDocs(path: string, manifest: IDocsBundleManifest, isExactMatch: boolean): ILocalDocs { + return { + path, + schema: manifest.schema, + version: manifest.version, + profile: manifest.profile, + docsBaseUrl: manifest.docsBaseUrl, + isExactMatch, + }; +} + +/** + * Downloads, verifies, caches, and serves the slim docs bundle. + * + * The governing rule is that **a valid cached bundle is always served, + * whatever the current fetch attempt does**. A fetch can replace the served + * bundle on success but never withdraws one on failure, so `ensure()` returns + * undefined only when no valid cache exists. + */ +export class PositronDocsCache { + + constructor(private readonly _options: IPositronDocsCacheOptions) { } + + async ensure(request: IDocsBundleRequest): Promise { + const { logger } = this._options; + const resolved = resolveBundleRequest(request); + const state = await this._readState(); + const cached = await this._readCached(state); + + // Terminal: a release build holding its own version never touches the + // network again. + if (cached && state?.resolution === 'exact') { + logger.info(`${LOG_PREFIX} exact cache hit for ${state.version}; no network`); + return cached; + } + + const target = resolved.wantsExact ? resolved.exact : resolved.latest; + const resolution: DocsResolution = resolved.wantsExact ? 'exact' : 'latest-by-policy'; + logger.info(`${LOG_PREFIX} fetching ${target.zipUrl} (${resolution})`); + + const outcome = await this._downloadAndInstall(target, resolution, state?.etag); + if (outcome.kind === 'installed') { + await this._recordInstall(outcome, request, resolved.exact.version, resolution, target); + return outcome.docs; + } + this._logOutcome(outcome, target); + + // Cache-present rule: a failed attempt never withdraws a served bundle. + return cached; + } + + private _logOutcome(outcome: InstallOutcome, target: IResolvedBundle): void { + const { logger } = this._options; + switch (outcome.kind) { + case 'rejected': + logger.warn(`${LOG_PREFIX} rejected bundle from ${target.zipUrl}: ${outcome.reason}`); + break; + case 'failed': + logger.info(`${LOG_PREFIX} fetch failed for ${target.zipUrl}: ${outcome.reason}`); + break; + case 'not-found': + logger.info(`${LOG_PREFIX} no bundle published at ${target.zipUrl}`); + break; + case 'not-modified': + logger.info(`${LOG_PREFIX} ${target.zipUrl} unchanged (304)`); + break; + } + } + + private async _recordInstall( + outcome: InstallOutcome & { kind: 'installed' }, + request: IDocsBundleRequest, + requestedVersion: string, + resolution: DocsResolution, + target: IResolvedBundle, + ): Promise { + const now = this._options.now(); + await this._writeState({ + schema: DOCS_BUNDLE_SCHEMA, + version: outcome.manifest.version, + requestedVersion, + resolution, + profile: request.profile, + sha256: outcome.digest, + etag: outcome.etag, + sourceUrl: target.zipUrl, + fetchedAt: now, + lastAttemptAt: now, + }); + this._options.logger.info(`${LOG_PREFIX} installed ${outcome.manifest.version} from ${target.zipUrl}`); + } + + /** + * Fetch, verify, extract, and swap in one bundle. + * + * Order matters: the zip is fetched first so a 404 reads as "not published + * yet" rather than as a verification failure, and the digest is checked + * before anything is extracted so a bad payload can never write to disk + * outside the staging directory. + */ + private async _downloadAndInstall(target: IResolvedBundle, resolution: DocsResolution, etag: string | undefined): Promise { + const { archive, files, http, newId } = this._options; + const id = newId(); + const tmpZip = joinDocsPath(this._options.rootPath, `.tmp-${id}.zip`); + const staging = joinDocsPath(this._options.rootPath, `.staging-${id}`); + + try { + await files.mkdir(this._options.rootPath); + + const zip = await http.get(target.zipUrl, { etag, maxBytes: DOCS_MAX_DOWNLOAD_BYTES }); + if (zip.status === 304) { + return { kind: 'not-modified' }; + } + if (zip.status === 404) { + return { kind: 'not-found' }; + } + if (zip.status !== 200 || !zip.body) { + return { kind: 'failed', reason: `HTTP ${zip.status}` }; + } + + // A zip that cannot be verified is never extracted, even though + // that means a cold cache gets no local docs until the sidecar + // appears. Proceeding unverified would make the digest decorative. + const sidecar = await http.get(target.sha256Url); + if (sidecar.status !== 200 || !sidecar.body) { + return { kind: 'rejected', reason: `digest sidecar unavailable (HTTP ${sidecar.status})` }; + } + const expected = parseSha256Sidecar(new TextDecoder().decode(sidecar.body)); + if (!expected) { + return { kind: 'rejected', reason: 'digest sidecar is not a sha256 digest' }; + } + + await files.writeFile(tmpZip, zip.body); + const actual = await files.sha256(tmpZip); + if (actual !== expected) { + return { kind: 'rejected', reason: `digest mismatch (expected ${expected}, got ${actual})` }; + } + + try { + const offending = guardEntryNames(await archive.entryNames(tmpZip)); + if (offending) { + return { kind: 'rejected', reason: `archive entry escapes the target: ${offending}` }; + } + await archive.extract(tmpZip, staging); + } catch (error) { + return { kind: 'rejected', reason: `corrupt archive: ${errorMessage(error)}` }; + } + + const validation = await validateExtractedBundle(files, staging); + if (!validation.ok) { + return { kind: 'rejected', reason: `extracted bundle invalid (${validation.reason})` }; + } + + const docs = await this._swapIn(staging, validation.manifest, resolution, id); + return { kind: 'installed', docs, manifest: validation.manifest, digest: actual, etag: zip.etag }; + } catch (error) { + return { kind: 'failed', reason: errorMessage(error) }; + } finally { + await this._safeDelete(tmpZip); + await this._safeDelete(staging); + } + } + + /** + * Atomic swap. The rename means a killed process can never leave a + * half-populated version directory that later looks like a cache hit. + */ + private async _swapIn(staging: string, manifest: IDocsBundleManifest, resolution: DocsResolution, id: string): Promise { + const { files } = this._options; + const target = joinDocsPath(this._options.rootPath, manifest.version); + if (await files.exists(target)) { + // Same version is already on disk but was not usable, or we would + // not have downloaded. Move it aside first so the recorded path + // never points at a directory that does not exist. + const stale = joinDocsPath(this._options.rootPath, `.stale-${id}`); + await files.rename(target, stale); + await files.rename(staging, target); + await this._safeDelete(stale); + } else { + await files.rename(staging, target); + } + return toLocalDocs(target, manifest, resolution === 'exact'); + } + + private async _readState(): Promise { + const path = joinDocsPath(this._options.rootPath, DOCS_STATE_FILENAME); + if (!await this._options.files.exists(path)) { + return undefined; + } + try { + const parsed = JSON.parse(await this._options.files.readFile(path)) as IDocsCacheState; + return typeof parsed?.version === 'string' ? parsed : undefined; + } catch { + return undefined; + } + } + + private async _writeState(state: IDocsCacheState): Promise { + const { files, newId, rootPath } = this._options; + const tmp = joinDocsPath(rootPath, `.state-${newId()}.json`); + await files.writeFile(tmp, JSON.stringify(state, undefined, '\t')); + await files.rename(tmp, joinDocsPath(rootPath, DOCS_STATE_FILENAME)); + } + + /** + * Whether the bundle `state` names is usable. + * + * Note this never re-hashes: `state.sha256` is a diagnostic record of what + * was verified before extraction, not a live checksum. The structural + * checks here are the proportionate ones for Markdown the assistant reads + * as text. + */ + private async _readCached(state: IDocsCacheState | undefined): Promise { + // An empty version means `_recordFailure` wrote state with no bundle ever + // installed. `joinDocsPath` drops empty segments, so computing the path + // anyway would validate rootPath itself and warn that a cache which never + // existed is now unusable. + if (!state || !state.version) { + return undefined; + } + const dir = joinDocsPath(this._options.rootPath, state.version); + const validation = await validateExtractedBundle(this._options.files, dir); + if (!validation.ok) { + this._options.logger.warn(`${LOG_PREFIX} cached bundle at ${dir} is unusable (${validation.reason})`); + return undefined; + } + return toLocalDocs(dir, validation.manifest, state.resolution === 'exact'); + } + + private async _safeDelete(path: string): Promise { + try { + await this._options.files.delete(path); + } catch { + // Cleanup is best-effort; the prune pass collects anything left. + } + } +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts new file mode 100644 index 00000000000..2ceafa5c192 --- /dev/null +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -0,0 +1,235 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ +/// + +import { DocsProfile, IDocsBundleRequest } from '../../common/positronDocsBundle.js'; +import { PositronDocsCache } from '../../common/positronDocsCache.js'; +import { fakeDigest, fakeZip, FakeArchive, FakeFileStore, FakeHttpClient, recordingLogger } from './fakes.js'; + +const ROOT = '/userdata/User/positron-docs'; +const BASE = 'https://cdn.posit.co/positron/releases/docs'; +const EXACT_ZIP = `${BASE}/positron-llms-2026.05.0-179.zip`; +const LATEST_ZIP = `${BASE}/positron-llms-latest.zip`; + +/** A fake-zip payload whose manifest declares the three files it contains. */ +function payload(version: string): string { + return fakeZip({ + 'bundle.json': JSON.stringify({ + schema: 1, profile: 'positron', version, + generated: '2026-07-24T18:02:11Z', + docsBaseUrl: 'https://positron.posit.co/', fileCount: 3, + }), + 'llms.txt': '# Positron\n\n- [Welcome](welcome.llms.md)\n', + 'welcome.llms.md': '# Welcome\n', + }); +} + +function request(overrides: Partial = {}): IDocsBundleRequest { + return { + quality: 'releases', + positronVersion: '2026.05.0', + positronBuildNumber: 179, + profile: 'positron' as DocsProfile, + baseUrl: BASE, + ...overrides, + }; +} + +function setup() { + const files = new FakeFileStore(); + const http = new FakeHttpClient(); + const archive = new FakeArchive(files); + const logger = recordingLogger(); + let clock = 1_000_000; + let ids = 0; + const cache = new PositronDocsCache({ + rootPath: ROOT, http, files, archive, logger, + now: () => clock, + newId: () => `id${++ids}`, + }); + return { + cache, files, http, archive, logger, + advance: (ms: number) => { clock += ms; }, + /** Serve `zipUrl` with a matching, correctly-formatted sidecar. */ + publish: (zipUrl: string, body: string, etag?: string) => { + http.route(zipUrl, { status: 200, body, etag }); + http.route(`${zipUrl}.sha256sum`, { status: 200, body: `${fakeDigest(body)} bundle.zip\n` }); + }, + }; +} + +describe('PositronDocsCache: cold cache install', () => { + it('downloads, verifies, extracts, and swaps in a release build bundle', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + + const docs = await ctx.cache.ensure(request()); + + expect(docs).toMatchInlineSnapshot(` + { + "docsBaseUrl": "https://positron.posit.co/", + "isExactMatch": true, + "path": "/userdata/User/positron-docs/2026.05.0-179", + "profile": "positron", + "schema": 1, + "version": "2026.05.0-179", + } + `); + }); + + it('records state naming the installed version', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request()); + + const state = JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)); + expect({ + version: state.version, requestedVersion: state.requestedVersion, + resolution: state.resolution, profile: state.profile, sourceUrl: state.sourceUrl, + }).toMatchInlineSnapshot(` + { + "profile": "positron", + "requestedVersion": "2026.05.0-179", + "resolution": "exact", + "sourceUrl": "https://cdn.posit.co/positron/releases/docs/positron-llms-2026.05.0-179.zip", + "version": "2026.05.0-179", + } + `); + }); + + it('leaves no temp or staging entries behind', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request()); + + expect(ctx.files.listUnder(ROOT).filter(p => p.includes('/.'))).toEqual([]); + }); + + it('fetches the latest alias for a dailies build', async () => { + const ctx = setup(); + ctx.publish(LATEST_ZIP, payload('2026.05.0-179')); + + expect(await ctx.cache.ensure(request({ quality: 'dailies' }))).toBeDefined(); + expect(ctx.http.getCalls).toContain(LATEST_ZIP); + expect(ctx.http.getCalls).not.toContain(EXACT_ZIP); + }); +}); + +describe('PositronDocsCache: warm exact cache', () => { + it('serves the cache without touching the network', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request()); + const callsAfterInstall = ctx.http.getCalls.length; + + const docs = await ctx.cache.ensure(request()); + + expect(docs?.isExactMatch).toBe(true); + // Release builds are network-free once exactly matched. This is the + // whole point of version-stamping the cache directory. + expect(ctx.http.getCalls.length).toBe(callsAfterInstall); + expect(ctx.http.headCalls).toEqual([]); + }); +}); + +describe('PositronDocsCache: download rejections on a cold cache', () => { + // Each of these must leave no version directory behind and return + // undefined, so the assistant falls back to the web exactly as it does + // today. Task 6 asserts the same failures against a warm cache. + async function expectRejected(configure: (ctx: ReturnType) => void) { + const ctx = setup(); + configure(ctx); + const docs = await ctx.cache.ensure(request()); + expect(docs).toBeUndefined(); + expect(await ctx.files.exists(`${ROOT}/2026.05.0-179`)).toBe(false); + expect(ctx.files.listUnder(ROOT).filter(p => p.includes('/.tmp-') || p.includes('/.staging-'))).toEqual([]); + return ctx; + } + + it('rejects when the digest sidecar 404s', async () => { + const ctx = await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); + c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 404 }); + }); + expect(ctx.logger.warns.join('\n')).toContain('digest sidecar'); + }); + + it('rejects when the sidecar is unparseable', async () => { + await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); + c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: '404' }); + }); + }); + + it('rejects when the digest does not match the zip', async () => { + await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); + c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: `${'b'.repeat(64)} bundle.zip` }); + }); + }); + + it('rejects a corrupt archive', async () => { + await expectRejected(c => { + c.publish(EXACT_ZIP, 'not-a-zip-at-all'); + }); + }); + + it('rejects an archive entry that escapes the target', async () => { + await expectRejected(c => { + c.publish(EXACT_ZIP, fakeZip({ 'llms.txt': 'x', '../../evil.sh': 'rm -rf /' })); + }); + }); + + it('rejects a bundle whose schema is not 1', async () => { + await expectRejected(c => { + c.publish(EXACT_ZIP, fakeZip({ + 'bundle.json': JSON.stringify({ schema: 2, profile: 'positron', version: '2026.05.0-179', generated: 'g', docsBaseUrl: 'd', fileCount: 2 }), + 'llms.txt': '# Positron\n', + })); + }); + }); + + it('rejects a bundle whose fileCount does not match', async () => { + await expectRejected(c => { + c.publish(EXACT_ZIP, fakeZip({ + 'bundle.json': JSON.stringify({ schema: 1, profile: 'positron', version: '2026.05.0-179', generated: 'g', docsBaseUrl: 'd', fileCount: 99 }), + 'llms.txt': '# Positron\n', + })); + }); + }); + + it('aborts a download that exceeds the 5MB cap', async () => { + await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179'), byteLength: 6 * 1024 * 1024 }); + c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: `${'c'.repeat(64)} x` }); + }); + }); + + it('returns undefined on a network failure', async () => { + await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 0, throws: 'getaddrinfo ENOTFOUND cdn.posit.co' }); + }); + }); + + it('returns undefined on a 5xx', async () => { + await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 503 }); + }); + }); + + it('returns undefined on a disk write error', async () => { + await expectRejected(c => { + c.publish(EXACT_ZIP, payload('2026.05.0-179')); + c.files.failWritesUnder = ROOT; + }); + }); + + it('never notifies: nothing is logged at a level above warn', async () => { + const ctx = await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 503 }); }); + // The logger port has no error level on purpose - a docs download + // failing is not worth interrupting anyone over. + expect(Object.keys(ctx.logger)).not.toContain('error'); + }); +}); From 5f78a76c2ffbc5e05ffa53602c2a451c1387dac0 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Wed, 29 Jul 2026 18:20:13 -0500 Subject: [PATCH 04/16] Add docs cache version resolution, convergence, and cache-present rule --- .../positronDocs/common/positronDocsCache.ts | 102 +++++++++--- .../test/common/positronDocsCache.vitest.ts | 153 +++++++++++++++++- 2 files changed, 229 insertions(+), 26 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index 8662c9ed4e8..ce775626bbe 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -5,7 +5,7 @@ import { DOCS_BUNDLE_SCHEMA, DOCS_MAX_DOWNLOAD_BYTES, DOCS_STATE_FILENAME, DocsResolution, - IDocsBundleManifest, IDocsBundleRequest, IDocsCacheState, IResolvedBundle, + IDocsBundleManifest, IDocsBundleRequest, IDocsCacheState, IResolvedBundle, IResolvedBundleRequest, parseSha256Sidecar, resolveBundleRequest, } from './positronDocsBundle.js'; import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsLogger, ILocalDocs, joinDocsPath } from './positronDocsPorts.js'; @@ -60,33 +60,95 @@ export class PositronDocsCache { constructor(private readonly _options: IPositronDocsCacheOptions) { } async ensure(request: IDocsBundleRequest): Promise { - const { logger } = this._options; const resolved = resolveBundleRequest(request); const state = await this._readState(); - const cached = await this._readCached(state); + const cached = await this._readCached(state, resolved.exact.version); - // Terminal: a release build holding its own version never touches the - // network again. - if (cached && state?.resolution === 'exact') { - logger.info(`${LOG_PREFIX} exact cache hit for ${state.version}; no network`); + // Terminal: a release build already holding its own version never + // touches the network again. Both halves matter - `resolution` alone + // would keep an updated app pinned to its predecessor's docs. + if (cached && state?.resolution === 'exact' && state.version === resolved.exact.version) { + this._options.logger.info(`${LOG_PREFIX} exact cache hit for ${state.version}; no network`); return cached; } - const target = resolved.wantsExact ? resolved.exact : resolved.latest; - const resolution: DocsResolution = resolved.wantsExact ? 'exact' : 'latest-by-policy'; - logger.info(`${LOG_PREFIX} fetching ${target.zipUrl} (${resolution})`); + return resolved.wantsExact + ? await this._ensureRelease(request, resolved, state, cached) + : await this._ensureLatest(request, resolved, state, cached); + } + + /** + * Release channel: target the exact version, fall back to latest until it + * publishes, and keep converging on every launch. + */ + private async _ensureRelease( + request: IDocsBundleRequest, + resolved: IResolvedBundleRequest, + state: IDocsCacheState | undefined, + cached: ILocalDocs | undefined, + ): Promise { + const { http, logger } = this._options; + + // This convergence check is never throttled. A HEAD is a few hundred + // bytes, and throttling it would let an install sit on a known-wrong + // docs version longer than the fallback policy intends. + let exactExists = false; + try { + exactExists = (await http.head(resolved.exact.zipUrl)).status === 200; + } catch (error) { + logger.info(`${LOG_PREFIX} exact HEAD failed for ${resolved.exact.zipUrl}: ${errorMessage(error)}`); + } + + if (exactExists) { + const outcome = await this._downloadAndInstall(resolved.exact, resolved.exact.version, undefined); + if (outcome.kind === 'installed') { + await this._recordInstall(outcome, request, resolved.exact.version, 'exact', resolved.exact); + return outcome.docs; + } + this._logOutcome(outcome, resolved.exact); + } + + return await this._fetchLatest(request, resolved, state, cached, 'fallback'); + } + + /** Dailies and dev builds: latest is the intended target, not a fallback. */ + private async _ensureLatest( + request: IDocsBundleRequest, + resolved: IResolvedBundleRequest, + state: IDocsCacheState | undefined, + cached: ILocalDocs | undefined, + ): Promise { + return await this._fetchLatest(request, resolved, state, cached, 'latest-by-policy'); + } - const outcome = await this._downloadAndInstall(target, resolution, state?.etag); + private async _fetchLatest( + request: IDocsBundleRequest, + resolved: IResolvedBundleRequest, + state: IDocsCacheState | undefined, + cached: ILocalDocs | undefined, + resolution: DocsResolution, + ): Promise { + // Conditional on the stored ETag. Using the `latest` alias rather than + // comparing versions keeps this monotonic without a version comparator. + const outcome = await this._downloadAndInstall(resolved.latest, resolved.exact.version, state?.etag); if (outcome.kind === 'installed') { - await this._recordInstall(outcome, request, resolved.exact.version, resolution, target); + await this._recordInstall(outcome, request, resolved.exact.version, resolution, resolved.latest); return outcome.docs; } - this._logOutcome(outcome, target); + this._logOutcome(outcome, resolved.latest); + if (outcome.kind === 'not-modified' && state) { + await this._touchState(state, resolution, resolved.exact.version); + } // Cache-present rule: a failed attempt never withdraws a served bundle. return cached; } + private async _touchState(state: IDocsCacheState, resolution: DocsResolution, requestedVersion: string): Promise { + const now = this._options.now(); + await this._writeState({ ...state, resolution, requestedVersion, fetchedAt: now, lastAttemptAt: now }); + } + private _logOutcome(outcome: InstallOutcome, target: IResolvedBundle): void { const { logger } = this._options; switch (outcome.kind) { @@ -136,7 +198,7 @@ export class PositronDocsCache { * before anything is extracted so a bad payload can never write to disk * outside the staging directory. */ - private async _downloadAndInstall(target: IResolvedBundle, resolution: DocsResolution, etag: string | undefined): Promise { + private async _downloadAndInstall(target: IResolvedBundle, exactVersion: string, etag: string | undefined): Promise { const { archive, files, http, newId } = this._options; const id = newId(); const tmpZip = joinDocsPath(this._options.rootPath, `.tmp-${id}.zip`); @@ -189,7 +251,7 @@ export class PositronDocsCache { return { kind: 'rejected', reason: `extracted bundle invalid (${validation.reason})` }; } - const docs = await this._swapIn(staging, validation.manifest, resolution, id); + const docs = await this._swapIn(staging, validation.manifest, exactVersion, id); return { kind: 'installed', docs, manifest: validation.manifest, digest: actual, etag: zip.etag }; } catch (error) { return { kind: 'failed', reason: errorMessage(error) }; @@ -203,7 +265,7 @@ export class PositronDocsCache { * Atomic swap. The rename means a killed process can never leave a * half-populated version directory that later looks like a cache hit. */ - private async _swapIn(staging: string, manifest: IDocsBundleManifest, resolution: DocsResolution, id: string): Promise { + private async _swapIn(staging: string, manifest: IDocsBundleManifest, exactVersion: string, id: string): Promise { const { files } = this._options; const target = joinDocsPath(this._options.rootPath, manifest.version); if (await files.exists(target)) { @@ -217,7 +279,7 @@ export class PositronDocsCache { } else { await files.rename(staging, target); } - return toLocalDocs(target, manifest, resolution === 'exact'); + return toLocalDocs(target, manifest, manifest.version === exactVersion); } private async _readState(): Promise { @@ -248,7 +310,7 @@ export class PositronDocsCache { * checks here are the proportionate ones for Markdown the assistant reads * as text. */ - private async _readCached(state: IDocsCacheState | undefined): Promise { + private async _readCached(state: IDocsCacheState | undefined, exactVersion: string): Promise { // An empty version means `_recordFailure` wrote state with no bundle ever // installed. `joinDocsPath` drops empty segments, so computing the path // anyway would validate rootPath itself and warn that a cache which never @@ -262,7 +324,9 @@ export class PositronDocsCache { this._options.logger.warn(`${LOG_PREFIX} cached bundle at ${dir} is unusable (${validation.reason})`); return undefined; } - return toLocalDocs(dir, validation.manifest, state.resolution === 'exact'); + // Derived from the running build, not from state.resolution: after an + // app update a bundle recorded as `exact` no longer is one. + return toLocalDocs(dir, validation.manifest, validation.manifest.version === exactVersion); } private async _safeDelete(path: string): Promise { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 2ceafa5c192..4654a61ecc3 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -44,13 +44,18 @@ function setup() { const logger = recordingLogger(); let clock = 1_000_000; let ids = 0; - const cache = new PositronDocsCache({ + // A fresh cache over the same fakes stands in for a new session: it + // re-reads state.json from the shared file store, exactly as a relaunch + // would. Tests that model "the next launch" must use this rather than + // calling ensure() twice on one instance. + const makeCache = () => new PositronDocsCache({ rootPath: ROOT, http, files, archive, logger, now: () => clock, newId: () => `id${++ids}`, }); + const cache = makeCache(); return { - cache, files, http, archive, logger, + cache, makeCache, files, http, archive, logger, advance: (ms: number) => { clock += ms; }, /** Serve `zipUrl` with a matching, correctly-formatted sidecar. */ publish: (zipUrl: string, body: string, etag?: string) => { @@ -122,15 +127,18 @@ describe('PositronDocsCache: warm exact cache', () => { const ctx = setup(); ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); await ctx.cache.ensure(request()); - const callsAfterInstall = ctx.http.getCalls.length; + const getsAfterInstall = ctx.http.getCalls.length; + const headsAfterInstall = ctx.http.headCalls.length; - const docs = await ctx.cache.ensure(request()); + const docs = await ctx.makeCache().ensure(request()); expect(docs?.isExactMatch).toBe(true); // Release builds are network-free once exactly matched. This is the - // whole point of version-stamping the cache directory. - expect(ctx.http.getCalls.length).toBe(callsAfterInstall); - expect(ctx.http.headCalls).toEqual([]); + // whole point of version-stamping the cache directory. Counted as a + // delta rather than an absolute: the install itself HEADs the exact URL, + // so what matters is that the next launch adds nothing. + expect(ctx.http.getCalls.length).toBe(getsAfterInstall); + expect(ctx.http.headCalls.length).toBe(headsAfterInstall); }); }); @@ -233,3 +241,134 @@ describe('PositronDocsCache: download rejections on a cold cache', () => { expect(Object.keys(ctx.logger)).not.toContain('error'); }); }); + +describe('PositronDocsCache: convergence', () => { + it('serves the fallback bundle first, then converges to exact', async () => { + const ctx = setup(); + // Exact is not published yet; latest holds an older release's docs. + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.http.route(`${EXACT_ZIP}.sha256sum`, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + + // Intermediate state matters: a test that checked only the end state + // would pass even if the fallback never worked. + const first = await ctx.cache.ensure(request()); + expect(first?.version).toBe('2026.04.0-100'); + expect(first?.isExactMatch).toBe(false); + expect(JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)).resolution).toBe('fallback'); + + // The release's docs publish. The next launch converges. + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + + const second = await ctx.makeCache().ensure(request()); + expect(second?.version).toBe('2026.05.0-179'); + expect(second?.isExactMatch).toBe(true); + expect(JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)).resolution).toBe('exact'); + }); + + it('keeps the cached bundle when latest answers 304', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + await ctx.cache.ensure(request()); + const before = ctx.files.listUnder(ROOT); + + const second = await ctx.makeCache().ensure(request()); + + expect(second?.version).toBe('2026.04.0-100'); + expect(ctx.files.listUnder(ROOT)).toEqual(before); + expect(ctx.logger.infos.join('\n')).toContain('unchanged (304)'); + }); + + it('replaces the cached bundle when latest moves', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + await ctx.cache.ensure(request()); + + ctx.publish(LATEST_ZIP, payload('2026.05.0-179'), 'etag-may'); + const second = await ctx.makeCache().ensure(request()); + + expect(second?.version).toBe('2026.05.0-179'); + }); + + it('re-enters fallback when the app updates past the cached bundle', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + const first = await ctx.cache.ensure(request()); + expect(first?.isExactMatch).toBe(true); + + // The user updates to a release whose docs have not published yet. + const updated = request({ positronVersion: '2026.06.0', positronBuildNumber: 42 }); + ctx.http.route(`${BASE}/positron-llms-2026.06.0-42.zip`, { status: 404 }); + ctx.http.route(LATEST_ZIP, { status: 404 }); + + const second = await ctx.makeCache().ensure(updated); + + // Local docs never silently regress to web-only because of an update. + expect(second?.version).toBe('2026.05.0-179'); + expect(second?.isExactMatch).toBe(false); + }); + + it('never HEADs the exact URL on a dailies build', async () => { + const ctx = setup(); + ctx.publish(LATEST_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request({ quality: 'dailies' })); + expect(ctx.http.headCalls).toEqual([]); + }); + + it('HEADs the exact URL again on the very next launch while in fallback', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + await ctx.cache.ensure(request()); + await ctx.makeCache().ensure(request()); + + // The 404 convergence check is deliberately never throttled. + expect(ctx.http.headCalls.filter(url => url === EXACT_ZIP)).toHaveLength(2); + }); +}); + +describe('PositronDocsCache: cache-present rule', () => { + /** Install a good bundle, then break the next launch's fetch. */ + async function withWarmCache(breakIt: (ctx: ReturnType) => void) { + const ctx = setup(); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + const first = await ctx.cache.ensure(request({ quality: 'dailies' })); + expect(first?.version).toBe('2026.04.0-100'); + + breakIt(ctx); + return { ctx, second: await ctx.makeCache().ensure(request({ quality: 'dailies' })) }; + } + + // This is the finding that broke the first draft of the design, so every + // failure kind gets explicit coverage rather than one representative case. + it.each([ + ['network failure', (c: ReturnType) => c.http.route(LATEST_ZIP, { status: 0, throws: 'ENOTFOUND' })], + ['5xx', (c: ReturnType) => c.http.route(LATEST_ZIP, { status: 503 })], + ['404 on latest', (c: ReturnType) => c.http.route(LATEST_ZIP, { status: 404 })], + ['corrupt zip', (c: ReturnType) => c.publish(LATEST_ZIP, 'not-a-zip')], + ['schema 2', (c: ReturnType) => c.publish(LATEST_ZIP, fakeZip({ + 'bundle.json': JSON.stringify({ schema: 2, profile: 'positron', version: 'v', generated: 'g', docsBaseUrl: 'd', fileCount: 2 }), + 'llms.txt': 'x', + }))], + ['digest mismatch', (c: ReturnType) => { + c.http.route(LATEST_ZIP, { status: 200, body: payload('2026.05.0-179') }); + c.http.route(`${LATEST_ZIP}.sha256sum`, { status: 200, body: `${'d'.repeat(64)} x` }); + }], + ['missing sidecar', (c: ReturnType) => { + c.http.route(LATEST_ZIP, { status: 200, body: payload('2026.05.0-179') }); + c.http.route(`${LATEST_ZIP}.sha256sum`, { status: 404 }); + }], + ['disk error', (c: ReturnType) => { + c.publish(LATEST_ZIP, payload('2026.05.0-179')); + c.files.failWritesUnder = ROOT; + }], + ])('still serves the warm cache after %s', async (_label, breakIt) => { + const { ctx, second } = await withWarmCache(breakIt); + + expect(second?.version).toBe('2026.04.0-100'); + // The previously installed directory survives untouched. + expect(await ctx.files.exists(`${ROOT}/2026.04.0-100/llms.txt`)).toBe(true); + }); +}); From 43cac3b76c8548c97ae1ed7572faa02099b29a8f Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Wed, 29 Jul 2026 18:23:20 -0500 Subject: [PATCH 05/16] Add docs cache single-flight, pruning, and hard-failure throttling --- .../positronDocs/common/positronDocsBundle.ts | 16 +++ .../positronDocs/common/positronDocsCache.ts | 127 +++++++++++++++++- .../test/common/positronDocsCache.vitest.ts | 119 ++++++++++++++++ 3 files changed, 258 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsBundle.ts b/src/vs/platform/positronDocs/common/positronDocsBundle.ts index e64445ea0b6..d56f9245b19 100644 --- a/src/vs/platform/positronDocs/common/positronDocsBundle.ts +++ b/src/vs/platform/positronDocs/common/positronDocsBundle.ts @@ -176,3 +176,19 @@ export function parseSha256Sidecar(raw: string): string | undefined { const first = raw.trim().split(/\s+/)[0]?.toLowerCase(); return first && SHA256_HEX.test(first) ? first : undefined; } + +/** + * How long a hard failure (network, DNS, connection, 5xx, disk) suppresses the + * next attempt. This stops a persistent CDN or configuration problem turning + * into a per-launch request from every install at once. Deliberately does NOT + * apply to the 404 convergence check. + */ +export const DOCS_FAILURE_THROTTLE_MS = 60 * 60 * 1000; + +/** + * How long a transient `.tmp-*`, `.staging-*`, or `.stale-*` entry must have + * been idle before pruning may remove it. Each window has its own extension + * host sharing this cache directory, so anything younger may be another + * window's in-flight work. + */ +export const DOCS_PRUNE_IDLE_MS = 10 * 60 * 1000; diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index ce775626bbe..0576bc37062 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -4,7 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { - DOCS_BUNDLE_SCHEMA, DOCS_MAX_DOWNLOAD_BYTES, DOCS_STATE_FILENAME, DocsResolution, + DOCS_BUNDLE_SCHEMA, DOCS_FAILURE_THROTTLE_MS, DOCS_MAX_DOWNLOAD_BYTES, DOCS_PRUNE_IDLE_MS, + DOCS_STATE_FILENAME, DocsResolution, IDocsBundleManifest, IDocsBundleRequest, IDocsCacheState, IResolvedBundle, IResolvedBundleRequest, parseSha256Sidecar, resolveBundleRequest, } from './positronDocsBundle.js'; @@ -57,9 +58,45 @@ function toLocalDocs(path: string, manifest: IDocsBundleManifest, isExactMatch: */ export class PositronDocsCache { + private _inFlight: Promise | undefined; + private _attempted = false; + private _result: ILocalDocs | undefined; + constructor(private readonly _options: IPositronDocsCacheOptions) { } + /** + * Resolve local docs, running at most one fetch at a time and at most one + * attempt per session. Concurrent callers join the in-flight operation + * rather than racing it. + */ async ensure(request: IDocsBundleRequest): Promise { + if (this._attempted) { + return this._result; + } + if (this._inFlight) { + return await this._inFlight; + } + this._inFlight = this._ensureOnce(request); + try { + this._result = await this._inFlight; + this._attempted = true; + return this._result; + } finally { + this._inFlight = undefined; + } + } + + /** + * Permit one more attempt this session. The only caller is the + * `ai.enabled` false-to-true transition, which is the single case the + * design allows to re-attempt without a relaunch. + */ + invalidate(): void { + this._attempted = false; + this._result = undefined; + } + + private async _ensureOnce(request: IDocsBundleRequest): Promise { const resolved = resolveBundleRequest(request); const state = await this._readState(); const cached = await this._readCached(state, resolved.exact.version); @@ -72,6 +109,12 @@ export class PositronDocsCache { return cached; } + const lastFailureAt = state?.lastFailureAt; + if (lastFailureAt !== undefined && this._options.now() - lastFailureAt < DOCS_FAILURE_THROTTLE_MS) { + this._options.logger.info(`${LOG_PREFIX} skipping fetch; a hard failure is still inside the throttle window`); + return cached; + } + return resolved.wantsExact ? await this._ensureRelease(request, resolved, state, cached) : await this._ensureLatest(request, resolved, state, cached); @@ -136,6 +179,9 @@ export class PositronDocsCache { return outcome.docs; } this._logOutcome(outcome, resolved.latest); + if (outcome.kind === 'failed') { + await this._recordFailure(state, request, resolved.exact.version, resolution, outcome.reason); + } if (outcome.kind === 'not-modified' && state) { await this._touchState(state, resolution, resolved.exact.version); } @@ -188,6 +234,7 @@ export class PositronDocsCache { lastAttemptAt: now, }); this._options.logger.info(`${LOG_PREFIX} installed ${outcome.manifest.version} from ${target.zipUrl}`); + await this._prune(outcome.manifest.version); } /** @@ -295,11 +342,26 @@ export class PositronDocsCache { } } + /** + * Persist cache state, best-effort. + * + * Swallowing the error is deliberate. State is bookkeeping, not the served + * artefact: losing it costs one redundant fetch next launch. Letting it + * throw would be strictly worse, because the disk errors that break this + * write are the same ones that break a download - so the throw would + * propagate out of `ensure()` and withdraw a perfectly good cached bundle, + * violating the cache-present rule. + */ private async _writeState(state: IDocsCacheState): Promise { - const { files, newId, rootPath } = this._options; + const { files, logger, newId, rootPath } = this._options; const tmp = joinDocsPath(rootPath, `.state-${newId()}.json`); - await files.writeFile(tmp, JSON.stringify(state, undefined, '\t')); - await files.rename(tmp, joinDocsPath(rootPath, DOCS_STATE_FILENAME)); + try { + await files.writeFile(tmp, JSON.stringify(state, undefined, '\t')); + await files.rename(tmp, joinDocsPath(rootPath, DOCS_STATE_FILENAME)); + } catch (error) { + logger.warn(`${LOG_PREFIX} could not persist cache state: ${errorMessage(error)}`); + await this._safeDelete(tmp); + } } /** @@ -329,6 +391,63 @@ export class PositronDocsCache { return toLocalDocs(dir, validation.manifest, validation.manifest.version === exactVersion); } + /** + * Persist a hard failure so the next session honours the throttle. + * + * `lastAttemptAt` records every attempt for diagnostics; `lastFailureAt` is + * the field the throttle reads. Keeping them separate avoids a bug where a + * successful 304 silently suppresses the next convergence check. + */ + private async _recordFailure( + state: IDocsCacheState | undefined, + request: IDocsBundleRequest, + requestedVersion: string, + resolution: DocsResolution, + reason: string, + ): Promise { + const now = this._options.now(); + await this._writeState({ + schema: DOCS_BUNDLE_SCHEMA, + version: state?.version ?? '', + requestedVersion, + resolution: state?.resolution ?? resolution, + profile: request.profile, + sha256: state?.sha256 ?? '', + etag: state?.etag, + sourceUrl: state?.sourceUrl ?? '', + fetchedAt: state?.fetchedAt ?? 0, + lastAttemptAt: now, + lastFailureAt: now, + lastError: reason, + }); + } + + /** + * Drop superseded version directories and abandoned transient entries. + * + * The mtime guard is what makes this safe across windows: each window has + * its own extension host, so window A must not delete window B's in-flight + * `.tmp-*` or `.staging-*`. Only entries idle for ten minutes are touched, + * which are by definition leftovers. No lock file needed. + */ + private async _prune(keepVersion: string): Promise { + const { files, now, rootPath } = this._options; + const cutoff = now() - DOCS_PRUNE_IDLE_MS; + for (const name of await files.readdir(rootPath)) { + if (name === DOCS_STATE_FILENAME || name === keepVersion) { + continue; + } + const path = joinDocsPath(rootPath, name); + if (name.startsWith('.')) { + const mtime = await files.mtime(path); + if (mtime === undefined || mtime > cutoff) { + continue; + } + } + await this._safeDelete(path); + } + } + private async _safeDelete(path: string): Promise { try { await this._options.files.delete(path); diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 4654a61ecc3..4e98a8b88b7 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -372,3 +372,122 @@ describe('PositronDocsCache: cache-present rule', () => { expect(await ctx.files.exists(`${ROOT}/2026.04.0-100/llms.txt`)).toBe(true); }); }); + +describe('PositronDocsCache: single flight', () => { + it('joins two concurrent calls into one download', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + + const [a, b] = await Promise.all([ctx.cache.ensure(request()), ctx.cache.ensure(request())]); + + expect(a?.version).toBe('2026.05.0-179'); + expect(b?.version).toBe('2026.05.0-179'); + expect(ctx.http.getCalls.filter(url => url === EXACT_ZIP)).toHaveLength(1); + }); + + it('does not re-attempt within a session, but invalidate() permits one more', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.http.route(LATEST_ZIP, { status: 404 }); + await ctx.cache.ensure(request()); + const afterFirst = ctx.http.headCalls.length; + + await ctx.cache.ensure(request()); + expect(ctx.http.headCalls.length).toBe(afterFirst); + + // The one in-session re-attempt the spec allows: an ai.enabled flip. + ctx.cache.invalidate(); + await ctx.cache.ensure(request()); + expect(ctx.http.headCalls.length).toBeGreaterThan(afterFirst); + }); +}); + +describe('PositronDocsCache: hard-failure throttling', () => { + it('records lastFailureAt and skips the next session inside the window', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 503 }); + ctx.http.route(LATEST_ZIP, { status: 503 }); + await ctx.cache.ensure(request()); + expect(JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)).lastFailureAt).toBeDefined(); + + ctx.advance(59 * 60 * 1000); + const callsBefore = ctx.http.getCalls.length; + await ctx.makeCache().ensure(request()); + + expect(ctx.http.getCalls.length).toBe(callsBefore); + }); + + it('retries once the throttle window has passed', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 503 }); + ctx.http.route(LATEST_ZIP, { status: 503 }); + await ctx.cache.ensure(request()); + const callsBefore = ctx.http.getCalls.length; + + ctx.advance(61 * 60 * 1000); + await ctx.makeCache().ensure(request()); + + expect(ctx.http.getCalls.length).toBeGreaterThan(callsBefore); + }); + + it('does not throttle a 404, so convergence keeps running', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + await ctx.cache.ensure(request()); + + const state = JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)); + expect(state.lastFailureAt).toBeUndefined(); + + ctx.advance(60 * 1000); + await ctx.makeCache().ensure(request()); + expect(ctx.http.headCalls.filter(url => url === EXACT_ZIP)).toHaveLength(2); + }); + + it('still serves the cache when persisting the failure marker fails', async () => { + // A full disk breaks both the download and the throttle bookkeeping. + // The bookkeeping is best-effort: it must never turn a served bundle + // into web-only, which is what an unhandled write error would do. + const ctx = setup(); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + expect(await ctx.cache.ensure(request({ quality: 'dailies' }))).toBeDefined(); + + ctx.publish(LATEST_ZIP, payload('2026.05.0-179'), 'etag-may'); + ctx.files.failWritesUnder = ROOT; + + const second = await ctx.makeCache().ensure(request({ quality: 'dailies' })); + expect(second?.version).toBe('2026.04.0-100'); + }); +}); + +describe('PositronDocsCache: pruning', () => { + it('deletes superseded version directories on success', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); + await ctx.cache.ensure(request()); + + ctx.publish(LATEST_ZIP, payload('2026.05.0-179'), 'etag-may'); + await ctx.makeCache().ensure(request()); + + expect(await ctx.files.exists(`${ROOT}/2026.05.0-179/llms.txt`)).toBe(true); + expect(await ctx.files.exists(`${ROOT}/2026.04.0-100`)).toBe(false); + }); + + it('leaves another window in-flight temp entries alone but collects abandoned ones', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + + // Two windows share this directory. A recent temp file belongs to a + // live download; an old one is an abandoned leftover. + await ctx.files.writeFile(`${ROOT}/.tmp-otherwindow.zip`, 'in flight'); + ctx.files.mtimes.set(`${ROOT}/.tmp-otherwindow.zip`, 1_000_000); + await ctx.files.writeFile(`${ROOT}/.staging-abandoned/x`, 'stale'); + ctx.files.mtimes.set(`${ROOT}/.staging-abandoned`, 1_000_000 - 11 * 60 * 1000); + + await ctx.cache.ensure(request()); + + expect(await ctx.files.exists(`${ROOT}/.tmp-otherwindow.zip`)).toBe(true); + expect(await ctx.files.exists(`${ROOT}/.staging-abandoned`)).toBe(false); + }); +}); From 03832dca8421341e33bd44f74ae6080aad0c748c Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Wed, 29 Jul 2026 18:40:50 -0500 Subject: [PATCH 06/16] Fix review findings in the docs cache platform module - Add IDocsFileStore.isDirectory and use it in countFiles. Inferring file-ness from an empty readdir counted an empty directory as a file, which real extractors can produce, and relied on behaviour a node-backed store does not have (it throws ENOTDIR on a file). - Make _prune best-effort. A readdir or unlink failure threw out of ensure() and discarded an install that was already on disk. - Track a generation counter so invalidate() called mid-fetch is not overwritten by the completing attempt re-arming the session gate. - Log an unexpected HEAD status on the convergence check; 404 stays quiet. - Drop all-slashes segments in joinDocsPath, which left a trailing slash. - Encode the fake zip as JSON so entry contents cannot mis-split. - Assert state content after a 304, add empty-directory and mid-flight invalidate coverage, and cover joinDocsPath. --- .../positronDocs/common/positronDocsCache.ts | 40 ++++++++++++++++--- .../positronDocs/common/positronDocsPorts.ts | 13 +++++- .../common/positronDocsValidate.ts | 12 +++--- .../positronDocs/test/common/fakes.ts | 27 +++++++++---- .../test/common/positronDocsCache.vitest.ts | 24 +++++++++++ .../test/common/positronDocsPorts.vitest.ts | 23 +++++++++++ .../common/positronDocsValidate.vitest.ts | 14 +++++++ 7 files changed, 135 insertions(+), 18 deletions(-) create mode 100644 src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index 0576bc37062..f8b3bd3b8ca 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -61,6 +61,8 @@ export class PositronDocsCache { private _inFlight: Promise | undefined; private _attempted = false; private _result: ILocalDocs | undefined; + /** Bumped by `invalidate()` so a mid-flight call is not overwritten. */ + private _generation = 0; constructor(private readonly _options: IPositronDocsCacheOptions) { } @@ -76,11 +78,19 @@ export class PositronDocsCache { if (this._inFlight) { return await this._inFlight; } + const generation = this._generation; this._inFlight = this._ensureOnce(request); try { - this._result = await this._inFlight; - this._attempted = true; - return this._result; + const result = await this._inFlight; + // Only close the session gate if no invalidate() landed while this + // attempt was in flight. Without the check, an `ai.enabled` flip + // during a download would be swallowed by the completing attempt and + // the caller would get no retry until the next launch. + if (generation === this._generation) { + this._result = result; + this._attempted = true; + } + return result; } finally { this._inFlight = undefined; } @@ -90,10 +100,14 @@ export class PositronDocsCache { * Permit one more attempt this session. The only caller is the * `ai.enabled` false-to-true transition, which is the single case the * design allows to re-attempt without a relaunch. + * + * Safe to call while a fetch is in flight: that attempt still resolves for + * its own callers, but it no longer closes the gate. */ invalidate(): void { this._attempted = false; this._result = undefined; + this._generation++; } private async _ensureOnce(request: IDocsBundleRequest): Promise { @@ -137,7 +151,14 @@ export class PositronDocsCache { // docs version longer than the fallback policy intends. let exactExists = false; try { - exactExists = (await http.head(resolved.exact.zipUrl)).status === 200; + const status = (await http.head(resolved.exact.zipUrl)).status; + exactExists = status === 200; + // 404 is the expected "not published yet" answer and stays quiet. + // Anything else means the CDN is unhealthy rather than not ready, and + // would otherwise fall through to the latest alias with no trace. + if (!exactExists && status !== 404) { + logger.info(`${LOG_PREFIX} unexpected HTTP ${status} from HEAD ${resolved.exact.zipUrl}`); + } } catch (error) { logger.info(`${LOG_PREFIX} exact HEAD failed for ${resolved.exact.zipUrl}: ${errorMessage(error)}`); } @@ -234,7 +255,16 @@ export class PositronDocsCache { lastAttemptAt: now, }); this._options.logger.info(`${LOG_PREFIX} installed ${outcome.manifest.version} from ${target.zipUrl}`); - await this._prune(outcome.manifest.version); + // Best-effort, for the same reason as _writeState: the bundle is already + // on disk and must be served. A readdir or unlink failure here would + // otherwise throw out of ensure() and discard a successful install. + // Awaited rather than fire-and-forget so the next launch (and the tests) + // observe a settled cache directory instead of racing the sweep. + try { + await this._prune(outcome.manifest.version); + } catch (error) { + this._options.logger.warn(`${LOG_PREFIX} could not prune the cache directory: ${errorMessage(error)}`); + } } /** diff --git a/src/vs/platform/positronDocs/common/positronDocsPorts.ts b/src/vs/platform/positronDocs/common/positronDocsPorts.ts index 882427b97bd..790010c29c9 100644 --- a/src/vs/platform/positronDocs/common/positronDocsPorts.ts +++ b/src/vs/platform/positronDocs/common/positronDocsPorts.ts @@ -41,8 +41,16 @@ export interface IDocsFileStore { rename(from: string, to: string): Promise; /** Recursive; succeeds if the path does not exist. */ delete(path: string): Promise; - /** Immediate children, names only. Empty array if the path is missing. */ + /** + * Immediate children, names only. Empty array if the path is missing. + * Only meaningful for directories - call `isDirectory` first rather than + * inferring file-ness from an empty result, since an empty directory and a + * file are indistinguishable here (and a node-backed store throws ENOTDIR + * on a file). + */ readdir(path: string): Promise; + /** True for a directory, false for a file and for a missing path. */ + isDirectory(path: string): Promise; /** Epoch millis, or undefined if the path is missing. Used by the prune guard. */ mtime(path: string): Promise; /** Lowercase hex digest of the file's bytes. */ @@ -79,5 +87,8 @@ export function joinDocsPath(...segments: string[]): string { return segments .filter(segment => segment.length > 0) .map((segment, index) => index === 0 ? segment.replace(/\/+$/, '') : segment.replace(/^\/+|\/+$/g, '')) + // Filtered again after stripping: an all-slashes segment strips to empty + // and would otherwise join into a stray trailing or doubled slash. + .filter((segment, index) => index === 0 || segment.length > 0) .join('/'); } diff --git a/src/vs/platform/positronDocs/common/positronDocsValidate.ts b/src/vs/platform/positronDocs/common/positronDocsValidate.ts index b626a9ee631..b24436545f2 100644 --- a/src/vs/platform/positronDocs/common/positronDocsValidate.ts +++ b/src/vs/platform/positronDocs/common/positronDocsValidate.ts @@ -81,12 +81,14 @@ async function countFiles(files: IDocsFileStore, dir: string): Promise { let count = 0; for (const name of await files.readdir(dir)) { const child = joinDocsPath(dir, name); - // A path that reads as a file is one; anything else recurses. - const children = await files.readdir(child); - if (children.length === 0) { - count += 1; - } else { + // Ask the store what the child is rather than inferring it from an empty + // readdir. An empty directory would otherwise count as one file - and + // real extractors do create them, from explicit directory entries in the + // archive - producing a spurious file-count-mismatch. + if (await files.isDirectory(child)) { count += await countFiles(files, child); + } else { + count += 1; } } return count; diff --git a/src/vs/platform/positronDocs/test/common/fakes.ts b/src/vs/platform/positronDocs/test/common/fakes.ts index 97b3ef7cc36..5c7e133aa9c 100644 --- a/src/vs/platform/positronDocs/test/common/fakes.ts +++ b/src/vs/platform/positronDocs/test/common/fakes.ts @@ -113,6 +113,13 @@ export class FakeFileStore implements IDocsFileStore { return [...names]; } + async isDirectory(path: string): Promise { + // A path present in `files` holds contents, so it is a file. Note + // `isDir` alone is not enough: writeFile records an mtime for file paths + // too, which isDir reads as a directory marker. + return this.files.has(path) ? false : this.isDir(path); + } + async mtime(path: string): Promise { return this.mtimes.get(path); } @@ -186,7 +193,11 @@ export class FakeHttpClient implements IDocsHttpClient { /** * Fake archive. A "zip" is the string the file store holds at its path, of the - * form `zip:=;=`. + * form `zip:`. + * + * JSON rather than a flat `name=contents;...` encoding: entry contents include + * bundle.json and Markdown, so any separator character can legitimately appear + * inside a value. A flat format would silently mis-split instead of failing. */ export class FakeArchive implements IDocsArchive { constructor(private readonly files: FakeFileStore) { } @@ -196,11 +207,13 @@ export class FakeArchive implements IDocsArchive { if (raw === undefined || !raw.startsWith('zip:')) { throw new Error(`end of central directory record signature not found: ${zipPath}`); } - return raw.slice(4).split(';').filter(part => part.length > 0) - .map(part => { - const index = part.indexOf('='); - return [part.slice(0, index), part.slice(index + 1)] as [string, string]; - }); + let parsed: unknown; + try { + parsed = JSON.parse(raw.slice(4)); + } catch { + throw new Error(`end of central directory record signature not found: ${zipPath}`); + } + return Object.entries(parsed as Record); } async entryNames(zipPath: string): Promise { @@ -216,7 +229,7 @@ export class FakeArchive implements IDocsArchive { /** Build the fake-zip payload string for a set of entries. */ export function fakeZip(entries: Record): string { - return `zip:${Object.entries(entries).map(([name, contents]) => `${name}=${contents}`).join(';')}`; + return `zip:${JSON.stringify(entries)}`; } export function recordingLogger(): IDocsLogger & { readonly infos: string[]; readonly warns: string[] } { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 4e98a8b88b7..eb626a089e8 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -273,11 +273,18 @@ describe('PositronDocsCache: convergence', () => { await ctx.cache.ensure(request()); const before = ctx.files.listUnder(ROOT); + ctx.advance(5 * 60 * 1000); const second = await ctx.makeCache().ensure(request()); expect(second?.version).toBe('2026.04.0-100'); expect(ctx.files.listUnder(ROOT)).toEqual(before); expect(ctx.logger.infos.join('\n')).toContain('unchanged (304)'); + // listUnder only compares paths, so it cannot see whether _touchState + // ran. Assert the content moved too, or a regression that stopped + // touching state would pass unnoticed. + const state = JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)); + expect(state.resolution).toBe('fallback'); + expect(state.lastAttemptAt).toBe(1_000_000 + 5 * 60 * 1000); }); it('replaces the cached bundle when latest moves', async () => { @@ -400,6 +407,23 @@ describe('PositronDocsCache: single flight', () => { await ctx.cache.ensure(request()); expect(ctx.http.headCalls.length).toBeGreaterThan(afterFirst); }); + + it('honours invalidate() called while a fetch is in flight', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.http.route(LATEST_ZIP, { status: 404 }); + + // An ai.enabled flip can land mid-download. The completing attempt must + // not re-arm the session gate and swallow the retry. + const inFlight = ctx.cache.ensure(request()); + ctx.cache.invalidate(); + await inFlight; + const afterFirst = ctx.http.headCalls.length; + + await ctx.cache.ensure(request()); + + expect(ctx.http.headCalls.length).toBeGreaterThan(afterFirst); + }); }); describe('PositronDocsCache: hard-failure throttling', () => { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts new file mode 100644 index 00000000000..d7c3761b4d9 --- /dev/null +++ b/src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts @@ -0,0 +1,23 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ +/// + +import { joinDocsPath } from '../../common/positronDocsPorts.js'; + +describe('joinDocsPath', () => { + it.each([ + ['joins plain segments', ['/c', '2026.05.0-179', 'llms.txt'], '/c/2026.05.0-179/llms.txt'], + ['preserves a leading slash', ['/c', 'x'], '/c/x'], + ['drops a trailing slash on the root', ['/c/', 'x'], '/c/x'], + ['collapses slashes between segments', ['/c/', '/x/', '/y'], '/c/x/y'], + ['skips empty segments', ['/c', '', 'x'], '/c/x'], + // An all-slashes segment strips to empty, which used to join into a + // stray trailing slash and produce a path that reads as a directory. + ['skips an all-slashes segment', ['/c', '///'], '/c'], + ['keeps a single segment unchanged', ['/c'], '/c'], + ])('%s', (_label, segments, expected) => { + expect(joinDocsPath(...segments)).toBe(expected); + }); +}); diff --git a/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts index 1a1ee32eafe..938e0c96f5e 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts @@ -46,6 +46,20 @@ describe('validateExtractedBundle', () => { expect(result.ok && result.manifest.version).toBe('2026.05.0-179'); }); + it('does not count an empty directory as a file', async () => { + // Real extractors materialise explicit directory entries, so an empty + // directory inside a bundle is reachable. Counting it as a file would + // reject a perfectly good bundle with file-count-mismatch. + const files = store({ + '/c/2026.05.0-179/bundle.json': manifest, + '/c/2026.05.0-179/llms.txt': '# Positron\n', + '/c/2026.05.0-179/welcome.llms.md': '# Welcome\n', + }); + await files.mkdir('/c/2026.05.0-179/reference'); + + expect(await validateExtractedBundle(files, '/c/2026.05.0-179')).toMatchObject({ ok: true }); + }); + it('rejects a missing bundle.json', async () => { const files = store({ '/c/x/llms.txt': '# Positron\n' }); expect(await validateExtractedBundle(files, '/c/x')).toMatchObject({ ok: false, reason: 'missing-manifest' }); From c9072bbf37cb9bb96b403ddcb318360ae9a2e7ea Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Thu, 30 Jul 2026 06:35:31 -0500 Subject: [PATCH 07/16] Raise the docs download cap to 25MB and explain what it guards The cap exists to bound a write the digest cannot vet until the bytes have landed, not to express an expected bundle size. Since it ships baked into a client while the bundle publishes separately, a client that trips it falls back to web docs with no way to fix it after the fact. The publish pipeline now enforces the real size budget. --- .../positronDocs/common/positronDocsBundle.ts | 13 +++++++++++-- .../test/common/positronDocsCache.vitest.ts | 6 +++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsBundle.ts b/src/vs/platform/positronDocs/common/positronDocsBundle.ts index d56f9245b19..e6ac6d6d8f1 100644 --- a/src/vs/platform/positronDocs/common/positronDocsBundle.ts +++ b/src/vs/platform/positronDocs/common/positronDocsBundle.ts @@ -20,8 +20,17 @@ export const DOCS_STATE_FILENAME = 'state.json'; export const DOCS_MANIFEST_FILENAME = 'bundle.json'; export const DOCS_INDEX_FILENAME = 'llms.txt'; -/** Refuse anything larger. The real bundle is about 150KB. */ -export const DOCS_MAX_DOWNLOAD_BYTES = 5 * 1024 * 1024; +/** + * Refuse anything larger. This is a disk-fill and expansion guard against a + * wrong or hostile object, not a size expectation: the digest is verified only + * after the bytes land, so something has to bound the write. Deliberately far + * above the real bundle (about 150KB zipped, 655KB across ~90 files) because + * the cap is baked into a shipped client while the bundle is published on the + * website's cadence - a client that trips this silently falls back to web docs + * and cannot be fixed after the fact. The publish pipeline enforces the real + * size budget, where exceeding it fails a build someone can react to. + */ +export const DOCS_MAX_DOWNLOAD_BYTES = 25 * 1024 * 1024; export type DocsProfile = 'positron' | 'workbench'; diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index eb626a089e8..74f265c1e9d 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ /// -import { DocsProfile, IDocsBundleRequest } from '../../common/positronDocsBundle.js'; +import { DocsProfile, DOCS_MAX_DOWNLOAD_BYTES, IDocsBundleRequest } from '../../common/positronDocsBundle.js'; import { PositronDocsCache } from '../../common/positronDocsCache.js'; import { fakeDigest, fakeZip, FakeArchive, FakeFileStore, FakeHttpClient, recordingLogger } from './fakes.js'; @@ -208,9 +208,9 @@ describe('PositronDocsCache: download rejections on a cold cache', () => { }); }); - it('aborts a download that exceeds the 5MB cap', async () => { + it('aborts a download that exceeds the size cap', async () => { await expectRejected(c => { - c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179'), byteLength: 6 * 1024 * 1024 }); + c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179'), byteLength: DOCS_MAX_DOWNLOAD_BYTES + 1 }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: `${'c'.repeat(64)} x` }); }); }); From ff924b4bf7361864e68964a54532cbb03f13c8a8 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Thu, 30 Jul 2026 06:58:16 -0500 Subject: [PATCH 08/16] Make docs cache rejection tests falsifiable and cover damaged cache state Each rejection case now asserts the reason it was refused, so a case can no longer pass on a different failure than the one it names. Adds coverage for a state file pointing at a deleted version directory and for an unparseable state file, and replaces a test that asserted on the shape of the logger fake with one that asserts the level each outcome kind logs at. --- .../positronDocs/common/positronDocsBundle.ts | 6 + .../positronDocs/test/common/fakes.ts | 11 +- .../test/common/positronDocsBundle.vitest.ts | 18 +-- .../test/common/positronDocsCache.vitest.ts | 114 +++++++++++++----- .../common/positronDocsValidate.vitest.ts | 4 +- 5 files changed, 109 insertions(+), 44 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsBundle.ts b/src/vs/platform/positronDocs/common/positronDocsBundle.ts index e6ac6d6d8f1..649bb51c38b 100644 --- a/src/vs/platform/positronDocs/common/positronDocsBundle.ts +++ b/src/vs/platform/positronDocs/common/positronDocsBundle.ts @@ -60,6 +60,12 @@ export interface IDocsCacheState { /** Version the app asked for, which may differ while in `fallback`. */ readonly requestedVersion: string; readonly resolution: DocsResolution; + /** + * Profile the bundle was fetched for. Diagnostic only, like `sha256`: it is + * never compared on read. A single install's profile is fixed at build time, + * so a cache directory cannot legitimately hold two, and `_readCached` takes + * the profile it reports from the extracted manifest instead. + */ readonly profile: string; /** * Digest verified before extraction. Diagnostic only: recorded once and diff --git a/src/vs/platform/positronDocs/test/common/fakes.ts b/src/vs/platform/positronDocs/test/common/fakes.ts index 5c7e133aa9c..a793eaab8b2 100644 --- a/src/vs/platform/positronDocs/test/common/fakes.ts +++ b/src/vs/platform/positronDocs/test/common/fakes.ts @@ -30,7 +30,12 @@ export class FakeFileStore implements IDocsFileStore { /** Digest overrides, keyed by path. Defaults to a hash of the contents. */ readonly digests = new Map(); - constructor(initial: Record = {}) { + /** + * @param now Clock the store stamps mtimes from. Pass the test's clock: + * with the default, everything written reads as epoch-old and so sits past + * any prune cutoff, which is the opposite of how a real write behaves. + */ + constructor(initial: Record = {}, private readonly now: () => number = () => 0) { for (const [path, contents] of Object.entries(initial)) { this.files.set(path, contents); this.mtimes.set(path, 0); @@ -67,11 +72,11 @@ export class FakeFileStore implements IDocsFileStore { // bytes through this method, and FakeArchive must be able to read them // back as its fake-zip payload string. this.files.set(path, typeof data === 'string' ? data : new TextDecoder().decode(data)); - this.mtimes.set(path, 0); + this.mtimes.set(path, this.now()); } async mkdir(path: string): Promise { - this.mtimes.set(path, this.mtimes.get(path) ?? 0); + this.mtimes.set(path, this.mtimes.get(path) ?? this.now()); } async rename(from: string, to: string): Promise { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts index ab175655c89..ac0d2883b7b 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts @@ -4,17 +4,19 @@ *--------------------------------------------------------------------------------------------*/ /// -import { DocsProfile, parseManifest, parseSha256Sidecar, resolveBundleRequest } from '../../common/positronDocsBundle.js'; +import { IDocsBundleRequest, parseManifest, parseSha256Sidecar, resolveBundleRequest } from '../../common/positronDocsBundle.js'; const BASE = 'https://cdn.posit.co/positron/releases/docs'; -function request(overrides: { profile?: DocsProfile; build?: number } = {}) { +/** Same shape as the helper in positronDocsCache.vitest.ts, deliberately. */ +function request(overrides: Partial = {}): IDocsBundleRequest { return { - quality: 'releases' as string | undefined, + quality: 'releases', positronVersion: '2026.05.0', - positronBuildNumber: overrides.build ?? 179, - profile: overrides.profile ?? ('positron' as DocsProfile), + positronBuildNumber: 179, + profile: 'positron', baseUrl: BASE, + ...overrides, }; } @@ -26,7 +28,7 @@ describe('resolveBundleRequest', () => { ['dailies', false], [undefined, false], ])('quality %s => wantsExact %s', (quality, wantsExact) => { - expect(resolveBundleRequest({ ...request(), quality }).wantsExact).toBe(wantsExact); + expect(resolveBundleRequest(request({ quality })).wantsExact).toBe(wantsExact); }); it('builds exact and latest URLs plus sidecars for the positron profile', () => { @@ -55,11 +57,11 @@ describe('resolveBundleRequest', () => { }); it('omits the -0 suffix for dev builds', () => { - expect(resolveBundleRequest(request({ build: 0 })).exact.version).toBe('2026.05.0'); + expect(resolveBundleRequest(request({ positronBuildNumber: 0 })).exact.version).toBe('2026.05.0'); }); it('tolerates a trailing slash on the base URL', () => { - expect(resolveBundleRequest({ ...request(), baseUrl: `${BASE}/` }).latest.zipUrl) + expect(resolveBundleRequest(request({ baseUrl: `${BASE}/` })).latest.zipUrl) .toBe(`${BASE}/positron-llms-latest.zip`); }); }); diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 74f265c1e9d..55e87eec5f2 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ /// -import { DocsProfile, DOCS_MAX_DOWNLOAD_BYTES, IDocsBundleRequest } from '../../common/positronDocsBundle.js'; +import { DOCS_MAX_DOWNLOAD_BYTES, IDocsBundleRequest } from '../../common/positronDocsBundle.js'; import { PositronDocsCache } from '../../common/positronDocsCache.js'; import { fakeDigest, fakeZip, FakeArchive, FakeFileStore, FakeHttpClient, recordingLogger } from './fakes.js'; @@ -12,6 +12,7 @@ const ROOT = '/userdata/User/positron-docs'; const BASE = 'https://cdn.posit.co/positron/releases/docs'; const EXACT_ZIP = `${BASE}/positron-llms-2026.05.0-179.zip`; const LATEST_ZIP = `${BASE}/positron-llms-latest.zip`; +const STATE_PATH = `${ROOT}/state.json`; /** A fake-zip payload whose manifest declares the three files it contains. */ function payload(version: string): string { @@ -31,19 +32,21 @@ function request(overrides: Partial = {}): IDocsBundleReques quality: 'releases', positronVersion: '2026.05.0', positronBuildNumber: 179, - profile: 'positron' as DocsProfile, + profile: 'positron', baseUrl: BASE, ...overrides, }; } function setup() { - const files = new FakeFileStore(); + let clock = 1_000_000; + let ids = 0; + // The store shares the test's clock so a written entry carries the mtime a + // real write would, rather than reading as epoch-old to the prune cutoff. + const files = new FakeFileStore({}, () => clock); const http = new FakeHttpClient(); const archive = new FakeArchive(files); const logger = recordingLogger(); - let clock = 1_000_000; - let ids = 0; // A fresh cache over the same fakes stands in for a new session: it // re-reads state.json from the shared file store, exactly as a relaunch // would. Tests that model "the next launch" must use this rather than @@ -57,6 +60,8 @@ function setup() { return { cache, makeCache, files, http, archive, logger, advance: (ms: number) => { clock += ms; }, + /** The persisted cache state, parsed. */ + readState: async () => JSON.parse(await files.readFile(STATE_PATH)), /** Serve `zipUrl` with a matching, correctly-formatted sidecar. */ publish: (zipUrl: string, body: string, etag?: string) => { http.route(zipUrl, { status: 200, body, etag }); @@ -89,7 +94,7 @@ describe('PositronDocsCache: cold cache install', () => { ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); await ctx.cache.ensure(request()); - const state = JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)); + const state = await ctx.readState(); expect({ version: state.version, requestedVersion: state.requestedVersion, resolution: state.resolution, profile: state.profile, sourceUrl: state.sourceUrl, @@ -109,7 +114,9 @@ describe('PositronDocsCache: cold cache install', () => { ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); await ctx.cache.ensure(request()); - expect(ctx.files.listUnder(ROOT).filter(p => p.includes('/.'))).toEqual([]); + // readdir rather than listUnder: an empty leftover staging directory has no + // file keys, so listUnder cannot see the leak this test is named for. + expect((await ctx.files.readdir(ROOT)).filter(name => name.startsWith('.'))).toEqual([]); }); it('fetches the latest alias for a dailies build', async () => { @@ -146,48 +153,53 @@ describe('PositronDocsCache: download rejections on a cold cache', () => { // Each of these must leave no version directory behind and return // undefined, so the assistant falls back to the web exactly as it does // today. Task 6 asserts the same failures against a warm cache. - async function expectRejected(configure: (ctx: ReturnType) => void) { + // `expectedLog` is what makes each case falsifiable: every rejection produces + // the same three observable outcomes, so without a distinct reason a test + // would still pass if the code refused the bundle for the wrong reason. + // Matched across both levels on purpose - which level each outcome kind logs + // at is asserted separately below. + async function expectRejected(configure: (ctx: ReturnType) => void, expectedLog: string) { const ctx = setup(); configure(ctx); const docs = await ctx.cache.ensure(request()); expect(docs).toBeUndefined(); expect(await ctx.files.exists(`${ROOT}/2026.05.0-179`)).toBe(false); expect(ctx.files.listUnder(ROOT).filter(p => p.includes('/.tmp-') || p.includes('/.staging-'))).toEqual([]); + expect([...ctx.logger.warns, ...ctx.logger.infos].join('\n')).toContain(expectedLog); return ctx; } it('rejects when the digest sidecar 404s', async () => { - const ctx = await expectRejected(c => { + await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 404 }); - }); - expect(ctx.logger.warns.join('\n')).toContain('digest sidecar'); + }, 'digest sidecar unavailable (HTTP 404)'); }); it('rejects when the sidecar is unparseable', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: '404' }); - }); + }, 'digest sidecar is not a sha256 digest'); }); it('rejects when the digest does not match the zip', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: `${'b'.repeat(64)} bundle.zip` }); - }); + }, 'digest mismatch'); }); it('rejects a corrupt archive', async () => { await expectRejected(c => { c.publish(EXACT_ZIP, 'not-a-zip-at-all'); - }); + }, 'corrupt archive'); }); it('rejects an archive entry that escapes the target', async () => { await expectRejected(c => { c.publish(EXACT_ZIP, fakeZip({ 'llms.txt': 'x', '../../evil.sh': 'rm -rf /' })); - }); + }, 'archive entry escapes the target: ../../evil.sh'); }); it('rejects a bundle whose schema is not 1', async () => { @@ -196,7 +208,7 @@ describe('PositronDocsCache: download rejections on a cold cache', () => { 'bundle.json': JSON.stringify({ schema: 2, profile: 'positron', version: '2026.05.0-179', generated: 'g', docsBaseUrl: 'd', fileCount: 2 }), 'llms.txt': '# Positron\n', })); - }); + }, 'extracted bundle invalid (bad-manifest)'); }); it('rejects a bundle whose fileCount does not match', async () => { @@ -205,40 +217,80 @@ describe('PositronDocsCache: download rejections on a cold cache', () => { 'bundle.json': JSON.stringify({ schema: 1, profile: 'positron', version: '2026.05.0-179', generated: 'g', docsBaseUrl: 'd', fileCount: 99 }), 'llms.txt': '# Positron\n', })); - }); + }, 'extracted bundle invalid (file-count-mismatch)'); }); it('aborts a download that exceeds the size cap', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179'), byteLength: DOCS_MAX_DOWNLOAD_BYTES + 1 }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: `${'c'.repeat(64)} x` }); - }); + }, `exceeds ${DOCS_MAX_DOWNLOAD_BYTES} bytes`); }); it('returns undefined on a network failure', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 0, throws: 'getaddrinfo ENOTFOUND cdn.posit.co' }); - }); + }, 'getaddrinfo ENOTFOUND cdn.posit.co'); }); it('returns undefined on a 5xx', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 503 }); - }); + }, 'unexpected HTTP 503 from HEAD'); }); it('returns undefined on a disk write error', async () => { await expectRejected(c => { c.publish(EXACT_ZIP, payload('2026.05.0-179')); c.files.failWritesUnder = ROOT; - }); + }, 'ENOSPC'); }); - it('never notifies: nothing is logged at a level above warn', async () => { - const ctx = await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 503 }); }); - // The logger port has no error level on purpose - a docs download - // failing is not worth interrupting anyone over. - expect(Object.keys(ctx.logger)).not.toContain('error'); + it('logs a fetch failure at info, and a refused payload at warn', async () => { + // A docs download failing is not worth interrupting anyone over, so an + // unreachable CDN stays at info. A payload that arrived and was refused + // is different: something is wrong with what was published, and that + // earns a warn. + const failed = await expectRejected(c => { + c.http.route(EXACT_ZIP, { status: 404 }); + c.http.route(LATEST_ZIP, { status: 0, throws: 'getaddrinfo ENOTFOUND cdn.posit.co' }); + }, 'fetch failed for'); + expect(failed.logger.warns).toEqual([]); + + const refused = await expectRejected(c => { + c.publish(EXACT_ZIP, 'not-a-zip-at-all'); + }, 'corrupt archive'); + expect(refused.logger.warns.join('\n')).toContain('rejected bundle from'); + }); +}); + +describe('PositronDocsCache: damaged cache state', () => { + it('reinstalls when state names a version directory that is gone', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request()); + + // Someone cleared part of the cache directory by hand, leaving state.json + // pointing at nothing. The bundle must come back, not be served from a + // path that no longer exists. + await ctx.files.delete(`${ROOT}/2026.05.0-179`); + + const docs = await ctx.makeCache().ensure(request()); + + expect(docs?.version).toBe('2026.05.0-179'); + expect(await ctx.files.exists(`${ROOT}/2026.05.0-179/llms.txt`)).toBe(true); + expect(ctx.logger.warns.join('\n')).toContain('is unusable (missing-manifest)'); + }); + + it('treats an unparseable state.json as a cold cache', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + await ctx.files.writeFile(STATE_PATH, '{ not json'); + + const docs = await ctx.cache.ensure(request()); + + expect(docs?.version).toBe('2026.05.0-179'); + expect((await ctx.readState()).resolution).toBe('exact'); }); }); @@ -255,7 +307,7 @@ describe('PositronDocsCache: convergence', () => { const first = await ctx.cache.ensure(request()); expect(first?.version).toBe('2026.04.0-100'); expect(first?.isExactMatch).toBe(false); - expect(JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)).resolution).toBe('fallback'); + expect((await ctx.readState()).resolution).toBe('fallback'); // The release's docs publish. The next launch converges. ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); @@ -263,7 +315,7 @@ describe('PositronDocsCache: convergence', () => { const second = await ctx.makeCache().ensure(request()); expect(second?.version).toBe('2026.05.0-179'); expect(second?.isExactMatch).toBe(true); - expect(JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)).resolution).toBe('exact'); + expect((await ctx.readState()).resolution).toBe('exact'); }); it('keeps the cached bundle when latest answers 304', async () => { @@ -282,7 +334,7 @@ describe('PositronDocsCache: convergence', () => { // listUnder only compares paths, so it cannot see whether _touchState // ran. Assert the content moved too, or a regression that stopped // touching state would pass unnoticed. - const state = JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)); + const state = await ctx.readState(); expect(state.resolution).toBe('fallback'); expect(state.lastAttemptAt).toBe(1_000_000 + 5 * 60 * 1000); }); @@ -432,7 +484,7 @@ describe('PositronDocsCache: hard-failure throttling', () => { ctx.http.route(EXACT_ZIP, { status: 503 }); ctx.http.route(LATEST_ZIP, { status: 503 }); await ctx.cache.ensure(request()); - expect(JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)).lastFailureAt).toBeDefined(); + expect((await ctx.readState()).lastFailureAt).toBeDefined(); ctx.advance(59 * 60 * 1000); const callsBefore = ctx.http.getCalls.length; @@ -460,7 +512,7 @@ describe('PositronDocsCache: hard-failure throttling', () => { ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); await ctx.cache.ensure(request()); - const state = JSON.parse(await ctx.files.readFile(`${ROOT}/state.json`)); + const state = await ctx.readState(); expect(state.lastFailureAt).toBeUndefined(); ctx.advance(60 * 1000); diff --git a/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts index 938e0c96f5e..efde3e2948f 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsValidate.vitest.ts @@ -42,8 +42,8 @@ describe('validateExtractedBundle', () => { '/c/2026.05.0-179/llms.txt': '# Positron\n', '/c/2026.05.0-179/welcome.llms.md': '# Welcome\n', }); - const result = await validateExtractedBundle(files, '/c/2026.05.0-179'); - expect(result.ok && result.manifest.version).toBe('2026.05.0-179'); + expect(await validateExtractedBundle(files, '/c/2026.05.0-179')) + .toMatchObject({ ok: true, manifest: { version: '2026.05.0-179' } }); }); it('does not count an empty directory as a file', async () => { From 41b55e14067b37927db1367608dc40d2cde20c37 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Thu, 30 Jul 2026 07:04:56 -0500 Subject: [PATCH 09/16] Rename the docs ports module to IO and say checksum file, not sidecar "Ports" collides with base/common/ports.ts and base/node/ports.ts, which are about TCP ports, and the module does HTTP - so the name invites the wrong reading. "Sidecar" now reads as the container pattern more than the companion file it means here; "checksum file" says the same thing plainly and pairs with the digest it holds. --- .../positronDocs/common/positronDocsBundle.ts | 4 ++-- .../positronDocs/common/positronDocsCache.ts | 16 ++++++++-------- .../{positronDocsPorts.ts => positronDocsIO.ts} | 6 +++--- .../positronDocs/common/positronDocsValidate.ts | 2 +- .../platform/positronDocs/test/common/fakes.ts | 4 ++-- .../test/common/positronDocsBundle.vitest.ts | 10 +++++----- .../test/common/positronDocsCache.vitest.ts | 12 ++++++------ ...sPorts.vitest.ts => positronDocsIO.vitest.ts} | 2 +- 8 files changed, 28 insertions(+), 28 deletions(-) rename src/vs/platform/positronDocs/common/{positronDocsPorts.ts => positronDocsIO.ts} (93%) rename src/vs/platform/positronDocs/test/common/{positronDocsPorts.vitest.ts => positronDocsIO.vitest.ts} (94%) diff --git a/src/vs/platform/positronDocs/common/positronDocsBundle.ts b/src/vs/platform/positronDocs/common/positronDocsBundle.ts index 649bb51c38b..840bcc412da 100644 --- a/src/vs/platform/positronDocs/common/positronDocsBundle.ts +++ b/src/vs/platform/positronDocs/common/positronDocsBundle.ts @@ -181,13 +181,13 @@ export function parseManifest(raw: string): IDocsBundleManifest | undefined { const SHA256_HEX = /^[0-9a-f]{64}$/; /** - * Read the hex digest out of a `.sha256sum` sidecar. + * Read the hex digest out of a `.sha256sum` checksum file. * * Accepts both `shasum -a 256` output (` `) and a bare digest. * Returns undefined for anything else, including an HTML error page served in * place of a missing object - which the caller must treat as a hard failure. */ -export function parseSha256Sidecar(raw: string): string | undefined { +export function parseDigestFile(raw: string): string | undefined { const first = raw.trim().split(/\s+/)[0]?.toLowerCase(); return first && SHA256_HEX.test(first) ? first : undefined; } diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index f8b3bd3b8ca..19ba1ec52e2 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -7,9 +7,9 @@ import { DOCS_BUNDLE_SCHEMA, DOCS_FAILURE_THROTTLE_MS, DOCS_MAX_DOWNLOAD_BYTES, DOCS_PRUNE_IDLE_MS, DOCS_STATE_FILENAME, DocsResolution, IDocsBundleManifest, IDocsBundleRequest, IDocsCacheState, IResolvedBundle, IResolvedBundleRequest, - parseSha256Sidecar, resolveBundleRequest, + parseDigestFile, resolveBundleRequest, } from './positronDocsBundle.js'; -import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsLogger, ILocalDocs, joinDocsPath } from './positronDocsPorts.js'; +import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsLogger, ILocalDocs, joinDocsPath } from './positronDocsIO.js'; import { guardEntryNames, validateExtractedBundle } from './positronDocsValidate.js'; const LOG_PREFIX = '[positron-docs]'; @@ -296,15 +296,15 @@ export class PositronDocsCache { } // A zip that cannot be verified is never extracted, even though - // that means a cold cache gets no local docs until the sidecar + // that means a cold cache gets no local docs until the checksum file // appears. Proceeding unverified would make the digest decorative. - const sidecar = await http.get(target.sha256Url); - if (sidecar.status !== 200 || !sidecar.body) { - return { kind: 'rejected', reason: `digest sidecar unavailable (HTTP ${sidecar.status})` }; + const checksum = await http.get(target.sha256Url); + if (checksum.status !== 200 || !checksum.body) { + return { kind: 'rejected', reason: `checksum file unavailable (HTTP ${checksum.status})` }; } - const expected = parseSha256Sidecar(new TextDecoder().decode(sidecar.body)); + const expected = parseDigestFile(new TextDecoder().decode(checksum.body)); if (!expected) { - return { kind: 'rejected', reason: 'digest sidecar is not a sha256 digest' }; + return { kind: 'rejected', reason: 'checksum file does not hold a sha256 digest' }; } await files.writeFile(tmpZip, zip.body); diff --git a/src/vs/platform/positronDocs/common/positronDocsPorts.ts b/src/vs/platform/positronDocs/common/positronDocsIO.ts similarity index 93% rename from src/vs/platform/positronDocs/common/positronDocsPorts.ts rename to src/vs/platform/positronDocs/common/positronDocsIO.ts index 790010c29c9..6d1309b5cab 100644 --- a/src/vs/platform/positronDocs/common/positronDocsPorts.ts +++ b/src/vs/platform/positronDocs/common/positronDocsIO.ts @@ -4,9 +4,9 @@ *--------------------------------------------------------------------------------------------*/ /** - * Ports for the docs cache. Deliberately three narrow interfaces rather than - * one wide one, so each test fake stays small and the seam can be re-hosted in - * a node service later without a rewrite. + * The I/O interfaces the docs cache is built against. Deliberately three narrow + * ones rather than a single wide one, so each test fake stays small and the seam + * can be re-hosted in a node service later without a rewrite. * * Paths are plain strings joined with forward slashes. Node's fs accepts * forward slashes on Windows, so no platform-specific joining is needed here diff --git a/src/vs/platform/positronDocs/common/positronDocsValidate.ts b/src/vs/platform/positronDocs/common/positronDocsValidate.ts index b24436545f2..bcd3341bdc1 100644 --- a/src/vs/platform/positronDocs/common/positronDocsValidate.ts +++ b/src/vs/platform/positronDocs/common/positronDocsValidate.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { DOCS_INDEX_FILENAME, DOCS_MANIFEST_FILENAME, IDocsBundleManifest, parseManifest } from './positronDocsBundle.js'; -import { IDocsFileStore, joinDocsPath } from './positronDocsPorts.js'; +import { IDocsFileStore, joinDocsPath } from './positronDocsIO.js'; export type DocsValidationFailure = | 'missing-manifest' diff --git a/src/vs/platform/positronDocs/test/common/fakes.ts b/src/vs/platform/positronDocs/test/common/fakes.ts index a793eaab8b2..7a73666baf6 100644 --- a/src/vs/platform/positronDocs/test/common/fakes.ts +++ b/src/vs/platform/positronDocs/test/common/fakes.ts @@ -3,11 +3,11 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ -import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsHttpGetOptions, IDocsHttpResponse, IDocsLogger } from '../../common/positronDocsPorts.js'; +import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsHttpGetOptions, IDocsHttpResponse, IDocsLogger } from '../../common/positronDocsIO.js'; /** * Deterministic stand-in for a real sha256. Only equality matters in these - * tests, and exporting it lets a test compute the digest a sidecar should + * tests, and exporting it lets a test compute the digest a checksum file should * carry without duplicating the algorithm. */ export function fakeDigest(contents: string): string { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts index ac0d2883b7b..77576b890c8 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsBundle.vitest.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ /// -import { IDocsBundleRequest, parseManifest, parseSha256Sidecar, resolveBundleRequest } from '../../common/positronDocsBundle.js'; +import { IDocsBundleRequest, parseManifest, parseDigestFile, resolveBundleRequest } from '../../common/positronDocsBundle.js'; const BASE = 'https://cdn.posit.co/positron/releases/docs'; @@ -31,7 +31,7 @@ describe('resolveBundleRequest', () => { expect(resolveBundleRequest(request({ quality })).wantsExact).toBe(wantsExact); }); - it('builds exact and latest URLs plus sidecars for the positron profile', () => { + it('builds exact and latest URLs plus checksum files for the positron profile', () => { const { exact, latest } = resolveBundleRequest(request()); expect({ exact, latest }).toMatchInlineSnapshot(` { @@ -95,7 +95,7 @@ describe('parseManifest', () => { }); }); -describe('parseSha256Sidecar', () => { +describe('parseDigestFile', () => { const digest = 'a'.repeat(64); it.each([ @@ -103,7 +103,7 @@ describe('parseSha256Sidecar', () => { ['bare hex', `${digest}\n`], ['uppercase hex', `${digest.toUpperCase()} x.zip`], ])('accepts %s', (_label, raw) => { - expect(parseSha256Sidecar(raw)).toBe(digest); + expect(parseDigestFile(raw)).toBe(digest); }); it.each([ @@ -112,6 +112,6 @@ describe('parseSha256Sidecar', () => { ['non-hex', `${'z'.repeat(64)} x.zip`], ['an HTML error page', '404'], ])('rejects %s', (_label, raw) => { - expect(parseSha256Sidecar(raw)).toBeUndefined(); + expect(parseDigestFile(raw)).toBeUndefined(); }); }); diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 55e87eec5f2..8f49740c04e 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -62,7 +62,7 @@ function setup() { advance: (ms: number) => { clock += ms; }, /** The persisted cache state, parsed. */ readState: async () => JSON.parse(await files.readFile(STATE_PATH)), - /** Serve `zipUrl` with a matching, correctly-formatted sidecar. */ + /** Serve `zipUrl` with a matching, correctly-formatted checksum file. */ publish: (zipUrl: string, body: string, etag?: string) => { http.route(zipUrl, { status: 200, body, etag }); http.route(`${zipUrl}.sha256sum`, { status: 200, body: `${fakeDigest(body)} bundle.zip\n` }); @@ -169,18 +169,18 @@ describe('PositronDocsCache: download rejections on a cold cache', () => { return ctx; } - it('rejects when the digest sidecar 404s', async () => { + it('rejects when the checksum file 404s', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 404 }); - }, 'digest sidecar unavailable (HTTP 404)'); + }, 'checksum file unavailable (HTTP 404)'); }); - it('rejects when the sidecar is unparseable', async () => { + it('rejects when the checksum file is unparseable', async () => { await expectRejected(c => { c.http.route(EXACT_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${EXACT_ZIP}.sha256sum`, { status: 200, body: '404' }); - }, 'digest sidecar is not a sha256 digest'); + }, 'checksum file does not hold a sha256 digest'); }); it('rejects when the digest does not match the zip', async () => { @@ -415,7 +415,7 @@ describe('PositronDocsCache: cache-present rule', () => { c.http.route(LATEST_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${LATEST_ZIP}.sha256sum`, { status: 200, body: `${'d'.repeat(64)} x` }); }], - ['missing sidecar', (c: ReturnType) => { + ['missing checksum file', (c: ReturnType) => { c.http.route(LATEST_ZIP, { status: 200, body: payload('2026.05.0-179') }); c.http.route(`${LATEST_ZIP}.sha256sum`, { status: 404 }); }], diff --git a/src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsIO.vitest.ts similarity index 94% rename from src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts rename to src/vs/platform/positronDocs/test/common/positronDocsIO.vitest.ts index d7c3761b4d9..4c2badc2c92 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsPorts.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsIO.vitest.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ /// -import { joinDocsPath } from '../../common/positronDocsPorts.js'; +import { joinDocsPath } from '../../common/positronDocsIO.js'; describe('joinDocsPath', () => { it.each([ From e3895c59abdaf7c6ab4acfd20b407187aad5aad9 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Thu, 30 Jul 2026 08:33:30 -0500 Subject: [PATCH 10/16] Replace the docs cache prune sweep with a targeted delete At install time the previous state.json already names the one directory being superseded, so delete it by name instead of sweeping the cache root. This removes the mtime guard, the DOCS_PRUNE_IDLE_MS constant, and the mtime port method, and closes the cross-window hazard by removing the scan that caused it. Crash debris is no longer collected; it is bounded by crash count and judged not worth the machinery (design doc revision 11). --- .../positronDocs/common/positronDocsBundle.ts | 7 --- .../positronDocs/common/positronDocsCache.ts | 52 +++++-------------- .../positronDocs/common/positronDocsIO.ts | 2 - .../positronDocs/test/common/fakes.ts | 36 +++++-------- .../test/common/positronDocsCache.vitest.ts | 22 ++++---- 5 files changed, 35 insertions(+), 84 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsBundle.ts b/src/vs/platform/positronDocs/common/positronDocsBundle.ts index 840bcc412da..4df4906b035 100644 --- a/src/vs/platform/positronDocs/common/positronDocsBundle.ts +++ b/src/vs/platform/positronDocs/common/positronDocsBundle.ts @@ -200,10 +200,3 @@ export function parseDigestFile(raw: string): string | undefined { */ export const DOCS_FAILURE_THROTTLE_MS = 60 * 60 * 1000; -/** - * How long a transient `.tmp-*`, `.staging-*`, or `.stale-*` entry must have - * been idle before pruning may remove it. Each window has its own extension - * host sharing this cache directory, so anything younger may be another - * window's in-flight work. - */ -export const DOCS_PRUNE_IDLE_MS = 10 * 60 * 1000; diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index 19ba1ec52e2..5bf467dbc1a 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { - DOCS_BUNDLE_SCHEMA, DOCS_FAILURE_THROTTLE_MS, DOCS_MAX_DOWNLOAD_BYTES, DOCS_PRUNE_IDLE_MS, + DOCS_BUNDLE_SCHEMA, DOCS_FAILURE_THROTTLE_MS, DOCS_MAX_DOWNLOAD_BYTES, DOCS_STATE_FILENAME, DocsResolution, IDocsBundleManifest, IDocsBundleRequest, IDocsCacheState, IResolvedBundle, IResolvedBundleRequest, parseDigestFile, resolveBundleRequest, @@ -166,7 +166,7 @@ export class PositronDocsCache { if (exactExists) { const outcome = await this._downloadAndInstall(resolved.exact, resolved.exact.version, undefined); if (outcome.kind === 'installed') { - await this._recordInstall(outcome, request, resolved.exact.version, 'exact', resolved.exact); + await this._recordInstall(outcome, request, resolved.exact.version, 'exact', resolved.exact, state?.version); return outcome.docs; } this._logOutcome(outcome, resolved.exact); @@ -196,7 +196,7 @@ export class PositronDocsCache { // comparing versions keeps this monotonic without a version comparator. const outcome = await this._downloadAndInstall(resolved.latest, resolved.exact.version, state?.etag); if (outcome.kind === 'installed') { - await this._recordInstall(outcome, request, resolved.exact.version, resolution, resolved.latest); + await this._recordInstall(outcome, request, resolved.exact.version, resolution, resolved.latest, state?.version); return outcome.docs; } this._logOutcome(outcome, resolved.latest); @@ -240,6 +240,7 @@ export class PositronDocsCache { requestedVersion: string, resolution: DocsResolution, target: IResolvedBundle, + previousVersion: string | undefined, ): Promise { const now = this._options.now(); await this._writeState({ @@ -255,15 +256,13 @@ export class PositronDocsCache { lastAttemptAt: now, }); this._options.logger.info(`${LOG_PREFIX} installed ${outcome.manifest.version} from ${target.zipUrl}`); - // Best-effort, for the same reason as _writeState: the bundle is already - // on disk and must be served. A readdir or unlink failure here would - // otherwise throw out of ensure() and discard a successful install. - // Awaited rather than fire-and-forget so the next launch (and the tests) - // observe a settled cache directory instead of racing the sweep. - try { - await this._prune(outcome.manifest.version); - } catch (error) { - this._options.logger.warn(`${LOG_PREFIX} could not prune the cache directory: ${errorMessage(error)}`); + // Targeted cleanup: this install supersedes exactly one directory, the + // version the previous state named. Deleting it by name is what keeps a + // dailies cache bounded without scanning the directory, so there is no + // sweep that could collide with another window's in-flight work. + // Best-effort: a cleanup failure must not discard a successful install. + if (previousVersion && previousVersion !== outcome.manifest.version) { + await this._safeDelete(joinDocsPath(this._options.rootPath, previousVersion)); } } @@ -452,37 +451,12 @@ export class PositronDocsCache { }); } - /** - * Drop superseded version directories and abandoned transient entries. - * - * The mtime guard is what makes this safe across windows: each window has - * its own extension host, so window A must not delete window B's in-flight - * `.tmp-*` or `.staging-*`. Only entries idle for ten minutes are touched, - * which are by definition leftovers. No lock file needed. - */ - private async _prune(keepVersion: string): Promise { - const { files, now, rootPath } = this._options; - const cutoff = now() - DOCS_PRUNE_IDLE_MS; - for (const name of await files.readdir(rootPath)) { - if (name === DOCS_STATE_FILENAME || name === keepVersion) { - continue; - } - const path = joinDocsPath(rootPath, name); - if (name.startsWith('.')) { - const mtime = await files.mtime(path); - if (mtime === undefined || mtime > cutoff) { - continue; - } - } - await this._safeDelete(path); - } - } - private async _safeDelete(path: string): Promise { try { await this._options.files.delete(path); } catch { - // Cleanup is best-effort; the prune pass collects anything left. + // Cleanup is best-effort. Anything left behind is a few hundred KB + // of crash debris, bounded by the number of crashes, not by time. } } } diff --git a/src/vs/platform/positronDocs/common/positronDocsIO.ts b/src/vs/platform/positronDocs/common/positronDocsIO.ts index 6d1309b5cab..8aaac059b15 100644 --- a/src/vs/platform/positronDocs/common/positronDocsIO.ts +++ b/src/vs/platform/positronDocs/common/positronDocsIO.ts @@ -51,8 +51,6 @@ export interface IDocsFileStore { readdir(path: string): Promise; /** True for a directory, false for a file and for a missing path. */ isDirectory(path: string): Promise; - /** Epoch millis, or undefined if the path is missing. Used by the prune guard. */ - mtime(path: string): Promise; /** Lowercase hex digest of the file's bytes. */ sha256(path: string): Promise; } diff --git a/src/vs/platform/positronDocs/test/common/fakes.ts b/src/vs/platform/positronDocs/test/common/fakes.ts index 7a73666baf6..8896ad4b25e 100644 --- a/src/vs/platform/positronDocs/test/common/fakes.ts +++ b/src/vs/platform/positronDocs/test/common/fakes.ts @@ -24,21 +24,16 @@ export function fakeDigest(contents: string): string { */ export class FakeFileStore implements IDocsFileStore { readonly files = new Map(); - readonly mtimes = new Map(); + /** Directories created explicitly via mkdir; implicit ones live in `files` keys. */ + readonly dirs = new Set(); /** Set to a path prefix to make every write under it fail, simulating a full disk. */ failWritesUnder: string | undefined; /** Digest overrides, keyed by path. Defaults to a hash of the contents. */ readonly digests = new Map(); - /** - * @param now Clock the store stamps mtimes from. Pass the test's clock: - * with the default, everything written reads as epoch-old and so sits past - * any prune cutoff, which is the opposite of how a real write behaves. - */ - constructor(initial: Record = {}, private readonly now: () => number = () => 0) { + constructor(initial: Record = {}) { for (const [path, contents] of Object.entries(initial)) { this.files.set(path, contents); - this.mtimes.set(path, 0); } } @@ -49,7 +44,7 @@ export class FakeFileStore implements IDocsFileStore { return true; } } - return this.mtimes.has(path); + return this.dirs.has(path); } async exists(path: string): Promise { @@ -72,11 +67,10 @@ export class FakeFileStore implements IDocsFileStore { // bytes through this method, and FakeArchive must be able to read them // back as its fake-zip payload string. this.files.set(path, typeof data === 'string' ? data : new TextDecoder().decode(data)); - this.mtimes.set(path, this.now()); } async mkdir(path: string): Promise { - this.mtimes.set(path, this.mtimes.get(path) ?? this.now()); + this.dirs.add(path); } async rename(from: string, to: string): Promise { @@ -86,10 +80,10 @@ export class FakeFileStore implements IDocsFileStore { this.files.set(to + key.slice(from.length), value); } } - for (const [key, value] of [...this.mtimes]) { + for (const key of [...this.dirs]) { if (key === from || key.startsWith(`${from}/`)) { - this.mtimes.delete(key); - this.mtimes.set(to + key.slice(from.length), value); + this.dirs.delete(key); + this.dirs.add(to + key.slice(from.length)); } } } @@ -100,9 +94,9 @@ export class FakeFileStore implements IDocsFileStore { this.files.delete(key); } } - for (const key of [...this.mtimes.keys()]) { + for (const key of [...this.dirs]) { if (key === path || key.startsWith(`${path}/`)) { - this.mtimes.delete(key); + this.dirs.delete(key); } } } @@ -110,7 +104,7 @@ export class FakeFileStore implements IDocsFileStore { async readdir(path: string): Promise { const prefix = `${path}/`; const names = new Set(); - for (const key of [...this.files.keys(), ...this.mtimes.keys()]) { + for (const key of [...this.files.keys(), ...this.dirs]) { if (key.startsWith(prefix)) { names.add(key.slice(prefix.length).split('/')[0]); } @@ -119,16 +113,10 @@ export class FakeFileStore implements IDocsFileStore { } async isDirectory(path: string): Promise { - // A path present in `files` holds contents, so it is a file. Note - // `isDir` alone is not enough: writeFile records an mtime for file paths - // too, which isDir reads as a directory marker. + // A path present in `files` holds contents, so it is a file. return this.files.has(path) ? false : this.isDir(path); } - async mtime(path: string): Promise { - return this.mtimes.get(path); - } - async sha256(path: string): Promise { const override = this.digests.get(path); if (override !== undefined) { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 8f49740c04e..ca1218d3d39 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -41,9 +41,7 @@ function request(overrides: Partial = {}): IDocsBundleReques function setup() { let clock = 1_000_000; let ids = 0; - // The store shares the test's clock so a written entry carries the mtime a - // real write would, rather than reading as epoch-old to the prune cutoff. - const files = new FakeFileStore({}, () => clock); + const files = new FakeFileStore(); const http = new FakeHttpClient(); const archive = new FakeArchive(files); const logger = recordingLogger(); @@ -316,6 +314,8 @@ describe('PositronDocsCache: convergence', () => { expect(second?.version).toBe('2026.05.0-179'); expect(second?.isExactMatch).toBe(true); expect((await ctx.readState()).resolution).toBe('exact'); + // Converging also cleans up: the fallback bundle is superseded. + expect(await ctx.files.exists(`${ROOT}/2026.04.0-100`)).toBe(false); }); it('keeps the cached bundle when latest answers 304', async () => { @@ -536,8 +536,8 @@ describe('PositronDocsCache: hard-failure throttling', () => { }); }); -describe('PositronDocsCache: pruning', () => { - it('deletes superseded version directories on success', async () => { +describe('PositronDocsCache: superseded version cleanup', () => { + it('deletes the superseded version directory on install', async () => { const ctx = setup(); ctx.http.route(EXACT_ZIP, { status: 404 }); ctx.publish(LATEST_ZIP, payload('2026.04.0-100'), 'etag-april'); @@ -550,20 +550,18 @@ describe('PositronDocsCache: pruning', () => { expect(await ctx.files.exists(`${ROOT}/2026.04.0-100`)).toBe(false); }); - it('leaves another window in-flight temp entries alone but collects abandoned ones', async () => { + it('never touches entries it did not supersede, including another window in-flight work', async () => { const ctx = setup(); ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); - // Two windows share this directory. A recent temp file belongs to a - // live download; an old one is an abandoned leftover. + // Two windows share this directory. Cleanup deletes only the version + // the previous state named - there is no sweep - so a concurrent + // window's temp file survives an install here unconditionally. await ctx.files.writeFile(`${ROOT}/.tmp-otherwindow.zip`, 'in flight'); - ctx.files.mtimes.set(`${ROOT}/.tmp-otherwindow.zip`, 1_000_000); - await ctx.files.writeFile(`${ROOT}/.staging-abandoned/x`, 'stale'); - ctx.files.mtimes.set(`${ROOT}/.staging-abandoned`, 1_000_000 - 11 * 60 * 1000); await ctx.cache.ensure(request()); expect(await ctx.files.exists(`${ROOT}/.tmp-otherwindow.zip`)).toBe(true); - expect(await ctx.files.exists(`${ROOT}/.staging-abandoned`)).toBe(false); + expect(await ctx.files.exists(`${ROOT}/2026.05.0-179/llms.txt`)).toBe(true); }); }); From 2d5bde2066093fee6a9e85275bf7cecd595f952f Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Thu, 30 Jul 2026 17:31:57 -0500 Subject: [PATCH 11/16] Use the American spelling of artifact in a docs cache comment The only 'artefact' left in src/vs; 37 files already use 'artifact'. --- src/vs/platform/positronDocs/common/positronDocsCache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index 5bf467dbc1a..e988d56bce3 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -375,7 +375,7 @@ export class PositronDocsCache { * Persist cache state, best-effort. * * Swallowing the error is deliberate. State is bookkeeping, not the served - * artefact: losing it costs one redundant fetch next launch. Letting it + * artifact: losing it costs one redundant fetch next launch. Letting it * throw would be strictly worse, because the disk errors that break this * write are the same ones that break a download - so the throw would * propagate out of `ensure()` and withdraw a perfectly good cached bundle, From da2e002a860a7bc7582d59a74e5d977489cae692 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Fri, 31 Jul 2026 07:06:59 -0500 Subject: [PATCH 12/16] Re-read docs cache state after a failed fetch so a concurrent install survives --- .../positronDocs/common/positronDocsCache.ts | 16 ++++++-- .../test/common/positronDocsCache.vitest.ts | 39 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index e988d56bce3..56ad6de0184 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -200,15 +200,23 @@ export class PositronDocsCache { return outcome.docs; } this._logOutcome(outcome, resolved.latest); + + // Re-read state after the attempt rather than reusing the snapshot taken + // before it. Another window can install a bundle while this attempt is in + // flight, and both the state merge below and the value returned here would + // otherwise be built from a snapshot that no longer describes the disk: + // the merge would write back `version: ''` over the version that window + // just recorded, orphaning its bundle for every later session. + const current = await this._readState(); if (outcome.kind === 'failed') { - await this._recordFailure(state, request, resolved.exact.version, resolution, outcome.reason); + await this._recordFailure(current, request, resolved.exact.version, resolution, outcome.reason); } - if (outcome.kind === 'not-modified' && state) { - await this._touchState(state, resolution, resolved.exact.version); + if (outcome.kind === 'not-modified' && current) { + await this._touchState(current, resolution, resolved.exact.version); } // Cache-present rule: a failed attempt never withdraws a served bundle. - return cached; + return cached ?? await this._readCached(current, resolved.exact.version); } private async _touchState(state: IDocsCacheState, resolution: DocsResolution, requestedVersion: string): Promise { diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index ca1218d3d39..ed26df22753 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -432,6 +432,45 @@ describe('PositronDocsCache: cache-present rule', () => { }); }); +describe('PositronDocsCache: a cache installed mid-attempt', () => { + // Two windows opening at once on a cold cache: one installs, the other's + // download fails. The failing window memoizes its result for the session, so + // it has to notice the bundle that landed while it was in flight or it stays + // on web docs until the next relaunch. + it('serves a bundle another window installed while this attempt was in flight', async () => { + const ctx = setup(); + ctx.publish(LATEST_ZIP, payload('2026.05.0-179')); + + let release!: () => void; + const held = new Promise(resolve => { release = resolve; }); + const secondWindow = new PositronDocsCache({ + rootPath: ROOT, files: ctx.files, archive: ctx.archive, logger: ctx.logger, + now: () => 2_000_000, newId: () => 'second', + http: { + // Hangs until released, then fails: this window reads an empty cache + // on the way in and never installs anything itself. + get: async () => { await held; throw new Error('ENOTFOUND'); }, + head: url => ctx.http.head(url), + }, + }); + + const pending = secondWindow.ensure(request({ quality: 'dailies' })); + expect((await ctx.cache.ensure(request({ quality: 'dailies' })))?.version).toBe('2026.05.0-179'); + + release(); + expect((await pending)?.version).toBe('2026.05.0-179'); + + // The failing window records its failure without erasing the version the + // other window installed. Losing that would orphan the bundle on disk for + // every later session, not just this one. + const state = await ctx.readState(); + expect({ version: state.version, lastError: state.lastError }).toEqual({ + version: '2026.05.0-179', + lastError: 'ENOTFOUND', + }); + }); +}); + describe('PositronDocsCache: single flight', () => { it('joins two concurrent calls into one download', async () => { const ctx = setup(); From aa5e5c83450a8e86b72948a772718bd0e6c072f9 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Fri, 31 Jul 2026 07:30:07 -0500 Subject: [PATCH 13/16] Log the docs bundle URL and resolution before each fetch --- .../positronDocs/common/positronDocsCache.ts | 5 ++++ .../test/common/positronDocsCache.vitest.ts | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index 56ad6de0184..ac613fd75d8 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -164,6 +164,7 @@ export class PositronDocsCache { } if (exactExists) { + logger.info(`${LOG_PREFIX} fetching ${resolved.exact.zipUrl} (exact)`); const outcome = await this._downloadAndInstall(resolved.exact, resolved.exact.version, undefined); if (outcome.kind === 'installed') { await this._recordInstall(outcome, request, resolved.exact.version, 'exact', resolved.exact, state?.version); @@ -192,6 +193,10 @@ export class PositronDocsCache { cached: ILocalDocs | undefined, resolution: DocsResolution, ): Promise { + // Logged before the request, not after: a download that hangs or is cut + // off leaves no other record of which URL this build was reaching for. + this._options.logger.info(`${LOG_PREFIX} fetching ${resolved.latest.zipUrl} (${resolution})`); + // Conditional on the stored ETag. Using the `latest` alias rather than // comparing versions keeps this monotonic without a version comparator. const outcome = await this._downloadAndInstall(resolved.latest, resolved.exact.version, state?.etag); diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index ed26df22753..019df3400d8 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -432,6 +432,36 @@ describe('PositronDocsCache: cache-present rule', () => { }); }); +describe('PositronDocsCache: logging', () => { + // Support reads these logs to work out what a build reached for. Naming the + // URL and the decision before the request is what makes a download that + // hangs or dies mid-flight diagnosable at all. + it('names the target and the resolution before an exact request', async () => { + const ctx = setup(); + ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request()); + + expect(ctx.logger.infos).toContain(`[positron-docs] fetching ${EXACT_ZIP} (exact)`); + }); + + it('names the target and the resolution when falling back to latest', async () => { + const ctx = setup(); + ctx.http.route(EXACT_ZIP, { status: 404 }); + ctx.publish(LATEST_ZIP, payload('2026.04.0-100')); + await ctx.cache.ensure(request()); + + expect(ctx.logger.infos).toContain(`[positron-docs] fetching ${LATEST_ZIP} (fallback)`); + }); + + it('names the target and the resolution for a dailies build', async () => { + const ctx = setup(); + ctx.publish(LATEST_ZIP, payload('2026.05.0-179')); + await ctx.cache.ensure(request({ quality: 'dailies' })); + + expect(ctx.logger.infos).toContain(`[positron-docs] fetching ${LATEST_ZIP} (latest-by-policy)`); + }); +}); + describe('PositronDocsCache: a cache installed mid-attempt', () => { // Two windows opening at once on a cold cache: one installs, the other's // download fails. The failing window memoizes its result for the session, so From e262d933577264271da4f05d41f9aeec5de111a5 Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Fri, 31 Jul 2026 07:36:08 -0500 Subject: [PATCH 14/16] Prefix docs cache logs with llm-docs rather than positron-docs --- src/vs/platform/positronDocs/common/positronDocsCache.ts | 5 ++++- .../positronDocs/test/common/positronDocsCache.vitest.ts | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index ac613fd75d8..12e70e3c067 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -12,7 +12,10 @@ import { import { IDocsArchive, IDocsFileStore, IDocsHttpClient, IDocsLogger, ILocalDocs, joinDocsPath } from './positronDocsIO.js'; import { guardEntryNames, validateExtractedBundle } from './positronDocsValidate.js'; -const LOG_PREFIX = '[positron-docs]'; +// Names the artifact, not the app: every line here is already in a Positron log, +// and this code handles the slim LLM bundle specifically, which is a separate +// thing from the docs website these pages are published to. +const LOG_PREFIX = '[llm-docs]'; export interface IPositronDocsCacheOptions { /** Cache root, e.g. `/User/positron-docs`. */ diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 019df3400d8..12f5f220983 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -441,7 +441,7 @@ describe('PositronDocsCache: logging', () => { ctx.publish(EXACT_ZIP, payload('2026.05.0-179')); await ctx.cache.ensure(request()); - expect(ctx.logger.infos).toContain(`[positron-docs] fetching ${EXACT_ZIP} (exact)`); + expect(ctx.logger.infos).toContain(`[llm-docs] fetching ${EXACT_ZIP} (exact)`); }); it('names the target and the resolution when falling back to latest', async () => { @@ -450,7 +450,7 @@ describe('PositronDocsCache: logging', () => { ctx.publish(LATEST_ZIP, payload('2026.04.0-100')); await ctx.cache.ensure(request()); - expect(ctx.logger.infos).toContain(`[positron-docs] fetching ${LATEST_ZIP} (fallback)`); + expect(ctx.logger.infos).toContain(`[llm-docs] fetching ${LATEST_ZIP} (fallback)`); }); it('names the target and the resolution for a dailies build', async () => { @@ -458,7 +458,7 @@ describe('PositronDocsCache: logging', () => { ctx.publish(LATEST_ZIP, payload('2026.05.0-179')); await ctx.cache.ensure(request({ quality: 'dailies' })); - expect(ctx.logger.infos).toContain(`[positron-docs] fetching ${LATEST_ZIP} (latest-by-policy)`); + expect(ctx.logger.infos).toContain(`[llm-docs] fetching ${LATEST_ZIP} (latest-by-policy)`); }); }); From 23c75724f6d5ddda38fdabe85111998b10b9092c Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Fri, 31 Jul 2026 07:44:53 -0500 Subject: [PATCH 15/16] Name the docs cache directory positron-llm-docs --- src/vs/platform/positronDocs/common/positronDocsCache.ts | 2 +- .../positronDocs/test/common/positronDocsCache.vitest.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/platform/positronDocs/common/positronDocsCache.ts b/src/vs/platform/positronDocs/common/positronDocsCache.ts index 12e70e3c067..5e0e531a95a 100644 --- a/src/vs/platform/positronDocs/common/positronDocsCache.ts +++ b/src/vs/platform/positronDocs/common/positronDocsCache.ts @@ -18,7 +18,7 @@ import { guardEntryNames, validateExtractedBundle } from './positronDocsValidate const LOG_PREFIX = '[llm-docs]'; export interface IPositronDocsCacheOptions { - /** Cache root, e.g. `/User/positron-docs`. */ + /** Cache root, e.g. `/User/positron-llm-docs`. */ readonly rootPath: string; readonly http: IDocsHttpClient; readonly files: IDocsFileStore; diff --git a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts index 12f5f220983..ca630954850 100644 --- a/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts +++ b/src/vs/platform/positronDocs/test/common/positronDocsCache.vitest.ts @@ -8,7 +8,7 @@ import { DOCS_MAX_DOWNLOAD_BYTES, IDocsBundleRequest } from '../../common/positr import { PositronDocsCache } from '../../common/positronDocsCache.js'; import { fakeDigest, fakeZip, FakeArchive, FakeFileStore, FakeHttpClient, recordingLogger } from './fakes.js'; -const ROOT = '/userdata/User/positron-docs'; +const ROOT = '/userdata/User/positron-llm-docs'; const BASE = 'https://cdn.posit.co/positron/releases/docs'; const EXACT_ZIP = `${BASE}/positron-llms-2026.05.0-179.zip`; const LATEST_ZIP = `${BASE}/positron-llms-latest.zip`; @@ -79,7 +79,7 @@ describe('PositronDocsCache: cold cache install', () => { { "docsBaseUrl": "https://positron.posit.co/", "isExactMatch": true, - "path": "/userdata/User/positron-docs/2026.05.0-179", + "path": "/userdata/User/positron-llm-docs/2026.05.0-179", "profile": "positron", "schema": 1, "version": "2026.05.0-179", From 3f7eb82ac9be165c249153f247cc9575e8e237cb Mon Sep 17 00:00:00 2001 From: Marie Idleman Date: Fri, 31 Jul 2026 08:14:07 -0500 Subject: [PATCH 16/16] update test tag mapping --- .github/workflows/test-tag-paths-map.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test-tag-paths-map.json b/.github/workflows/test-tag-paths-map.json index 51838c4c8e6..fc92a46fc31 100644 --- a/.github/workflows/test-tag-paths-map.json +++ b/.github/workflows/test-tag-paths-map.json @@ -128,6 +128,7 @@ "src/vs/platform/extensions/common/positronExtensionValidator.ts": ["@:extensions"], "src/vs/platform/positronActionBar/": [], "src/vs/platform/positronAiProvider/": ["@:assistant"], + "src/vs/platform/positronDocs/": [], "src/vs/platform/positronHeadlessLanguageModel/": ["@:positron-notebooks"], "src/vs/platform/positronIdleTracking/": [], "src/vs/platform/positronMemoryUsage/": ["@:variables"],