From 23c20e64051cbcca6c52bfa437245762c9f39e00 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 6 Sep 2026 14:32:18 +0000 Subject: [PATCH] fix(vendor): an install that exits 0 is not an install `crawlproof update` said "installed with pnpm" and left the old version in place, so the bug it was run to fix was still there. Same shape for `hqtui`. pnpm 11 ships a `minimumReleaseAge` cooldown that refuses versions published in the last little while, and it does not fail when it refuses one: it resolves to the newest release old enough to pass, writes a note about an exclude list, and exits 0. Reproduced in an empty directory on pnpm 11.18.0, with the registry reporting 0.2.0: pnpm add @profullstack/crawlproof@latest -> 0.1.0, exit 0 npm install @profullstack/crawlproof@latest -> 0.2.0 So both wrappers now ask what landed instead of trusting the exit code, and move to the next package manager when the answer is the wrong version. The version installed is printed, because "installed" without a number is exactly the claim that turned out to be false. Deliberately not `--config.minimumReleaseAge=0`. The cooldown is a real supply-chain protection, and switching it off wholesale in a tool that installs on other people's machines is a bigger decision than fixing an update. Falling through to npm leaves it as pnpm's default and still lets a deliberate update finish. An unreachable registry means the wanted version is unknown, and an unknown want passes: an offline box must still be able to reinstall what it has. Two failures on this branch are not from it: root-ubuntu's `groups` test fails on master as of #56, and registry's summary test fails on an untracked bin/argontv.ts sitting in the working tree. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HvWJ4336pxTFRdRbvsTQeD --- bin/crawlproof.ts | 7 ++- bin/hqtui.ts | 7 ++- src/crawlproof.ts | 23 +++++++- src/hqtui.ts | 23 +++++++- src/vendor-verify.ts | 98 ++++++++++++++++++++++++++++++++ test/vendor-verify.test.ts | 113 +++++++++++++++++++++++++++++++++++++ 6 files changed, 265 insertions(+), 6 deletions(-) create mode 100644 src/vendor-verify.ts create mode 100644 test/vendor-verify.test.ts diff --git a/bin/crawlproof.ts b/bin/crawlproof.ts index 9b07826..8347d8a 100755 --- a/bin/crawlproof.ts +++ b/bin/crawlproof.ts @@ -64,9 +64,14 @@ async function main(argv: string[]): Promise { const result = await install(spec); if (!result.ok) { process.stderr.write('crawlproof: could not install the dashboard.\n'); + // An install that exited 0 and left the wrong version behind is the + // confusing case, so the reason goes out rather than just the failure. + if (result.note) process.stderr.write(` ${result.note}\n`); return 1; } - process.stdout.write(`crawlproof: installed with ${result.manager}\n`); + process.stdout.write( + `crawlproof: installed ${result.version ?? ''} with ${result.manager}\n`.replace(' ', ' '), + ); return 0; } diff --git a/bin/hqtui.ts b/bin/hqtui.ts index e3afedc..3751ce6 100755 --- a/bin/hqtui.ts +++ b/bin/hqtui.ts @@ -57,9 +57,14 @@ async function main(argv: string[]): Promise { const result = await install(spec); if (!result.ok) { process.stderr.write('hqtui: could not install the dashboard.\n'); + // An install that exited 0 and left the wrong version behind is the + // confusing case, so the reason goes out rather than just the failure. + if (result.note) process.stderr.write(` ${result.note}\n`); return 1; } - process.stdout.write(`hqtui: installed with ${result.manager}\n`); + process.stdout.write( + `hqtui: installed ${result.version ?? ''} with ${result.manager}\n`.replace(' ', ' '), + ); return 0; } diff --git a/src/crawlproof.ts b/src/crawlproof.ts index 3f3f67f..0000e2b 100644 --- a/src/crawlproof.ts +++ b/src/crawlproof.ts @@ -29,6 +29,7 @@ import { homedir } from 'node:os'; import { join } from 'node:path'; import { onPath, resolveCommand } from './registry.ts'; import { spawnInherit } from './codeburn.ts'; +import { delivered, heldBackNote, installedVersion, wantedVersion } from './vendor-verify.ts'; /** The published package, and the executable it installs. */ export const PACKAGE = '@profullstack/crawlproof'; @@ -161,6 +162,10 @@ export interface InstallResult { ok: boolean; manager?: PackageManager; code?: number | null; + /** What actually landed, when it could be read. */ + version?: string; + /** Why an install that exited 0 was not accepted. */ + note?: string; } /** Install (or refresh) the dashboard in the private prefix. */ @@ -172,12 +177,26 @@ export async function install( const root = vendorRoot(env); prepareVendorDir(root); + // What this install is supposed to produce, asked once rather than per + // manager. Null means the registry was unreachable, and an unverifiable + // install is allowed through: an offline box should still be able to + // reinstall what it already has. + const wanted = await wantedVersion(spec, PACKAGE); + let lastNote: string | undefined; + for (const manager of managers(env)) { const plan = installPlan(manager, spec); const code = await run(plan.file, plan.args, root); - if (code === 0) return { ok: true, manager, code }; + if (code !== 0) continue; + + // Exit 0 is not proof. See src/vendor-verify.ts: pnpm's release-age + // cooldown installs the previous version and reports success. + const got = installedVersion(root, PACKAGE); + if (delivered(got, wanted)) return { ok: true, manager, code, ...(got ? { version: got } : {}) }; + lastNote = heldBackNote(manager, got, wanted); } - return { ok: false }; + + return { ok: false, ...(lastNote ? { note: lastNote } : {}) }; } /** diff --git a/src/hqtui.ts b/src/hqtui.ts index 0df54c7..472dc37 100644 --- a/src/hqtui.ts +++ b/src/hqtui.ts @@ -34,6 +34,7 @@ import { homedir } from 'node:os'; import { join } from 'node:path'; import { onPath, resolveCommand } from './registry.ts'; import { spawnInherit } from './codeburn.ts'; +import { delivered, heldBackNote, installedVersion, wantedVersion } from './vendor-verify.ts'; /** The published package, and the executable it installs. */ export const PACKAGE = '@profullstack/hqtui-demo'; @@ -173,6 +174,10 @@ export interface InstallResult { ok: boolean; manager?: PackageManager; code?: number | null; + /** What actually landed, when it could be read. */ + version?: string; + /** Why an install that exited 0 was not accepted. */ + note?: string; } /** Install (or refresh) the dashboard in the private prefix. */ @@ -184,10 +189,24 @@ export async function install( const root = vendorRoot(env); prepareVendorDir(root); + // What this install is supposed to produce, asked once rather than per + // manager. Null means the registry was unreachable, and an unverifiable + // install is allowed through: an offline box should still be able to + // reinstall what it already has. + const wanted = await wantedVersion(spec, PACKAGE); + let lastNote: string | undefined; + for (const manager of managers(env)) { const plan = installPlan(manager, spec); const code = await run(plan.file, plan.args, root); - if (code === 0) return { ok: true, manager, code }; + if (code !== 0) continue; + + // Exit 0 is not proof. See src/vendor-verify.ts: pnpm's release-age + // cooldown installs the previous version and reports success. + const got = installedVersion(root, PACKAGE); + if (delivered(got, wanted)) return { ok: true, manager, code, ...(got ? { version: got } : {}) }; + lastNote = heldBackNote(manager, got, wanted); } - return { ok: false }; + + return { ok: false, ...(lastNote ? { note: lastNote } : {}) }; } diff --git a/src/vendor-verify.ts b/src/vendor-verify.ts new file mode 100644 index 0000000..860a4b0 --- /dev/null +++ b/src/vendor-verify.ts @@ -0,0 +1,98 @@ +/** + * Did the install actually install anything? + * + * Exit code 0 is not proof. pnpm 11 ships a `minimumReleaseAge` cooldown that + * refuses versions published in the last little while, and it does not fail + * when it refuses one: it resolves to the newest version old enough to pass, + * prints a note about an exclude list, and exits 0. So `pnpm add pkg@latest` + * against a package published ten minutes ago installs the previous release and + * reports success. + * + * Reproduced on 2026-09-06 in an empty directory, pnpm 11.18.0: + * + * registry latest: 0.2.0 + * pnpm add @profullstack/crawlproof@latest -> 0.1.0, exit 0 + * npm install @profullstack/crawlproof@latest -> 0.2.0 + * + * The failure mode is the bad one. Someone runs `update`, is told it worked, + * and keeps hitting the bug it was supposed to fix. So the wrappers ask what + * landed instead of trusting the exit code, and move to the next package + * manager when the answer is the wrong version. + * + * Deliberately not `--config.minimumReleaseAge=0`. The cooldown is a real + * supply-chain protection and turning it off wholesale in a tool that installs + * on other people's machines is a bigger decision than fixing an update. + * Falling through to npm keeps the protection as pnpm's default behaviour and + * still lets a deliberate `update` finish. + */ + +import { existsSync, readFileSync } from 'node:fs'; +import path from 'node:path'; + +import { run } from './exec.ts'; + +/** The version actually present in a vendor prefix, or null when nothing is. */ +export function installedVersion(root: string, pkg: string): string | null { + const manifest = path.join(root, 'node_modules', ...pkg.split('/'), 'package.json'); + if (!existsSync(manifest)) return null; + try { + const version = (JSON.parse(readFileSync(manifest, 'utf8')) as { version?: string }).version; + return typeof version === 'string' && version ? version : null; + } catch { + return null; + } +} + +/** + * The exact version a spec asks for, when it names one. + * + * `pkg@1.2.3` is answerable here; `pkg@latest` and `pkg@^1` are not, and + * return null so the caller asks the registry instead of guessing. + */ +export function pinnedVersion(spec: string): string | null { + const at = spec.lastIndexOf('@'); + if (at <= 0) return null; + const tag = spec.slice(at + 1); + return /^\d+\.\d+\.\d+/.test(tag) ? tag : null; +} + +/** What the registry calls latest, or null when it cannot be reached. */ +export async function registryLatest( + pkg: string, + exec: typeof run = run, +): Promise { + const result = await exec('npm', ['view', pkg, 'version'], { timeoutMs: 60_000 }); + if (result.code !== 0) return null; + const version = result.stdout.trim().split('\n').pop()?.trim() ?? ''; + return /^\d+\.\d+\.\d+/.test(version) ? version : null; +} + +/** + * The version this install was supposed to produce. + * + * A pinned spec answers itself. `@latest` has to be asked, and when the + * registry cannot be reached the answer is null, which callers must read as + * "cannot verify" rather than as "wrong version" — an offline box should still + * be able to reinstall what it already has. + */ +export async function wantedVersion( + spec: string, + pkg: string, + exec: typeof run = run, +): Promise { + return pinnedVersion(spec) ?? (await registryLatest(pkg, exec)); +} + +/** Whether what landed is what was asked for. Unknown wants pass. */ +export function delivered(installed: string | null, wanted: string | null): boolean { + if (wanted === null) return true; + return installed === wanted; +} + +/** What to say when a manager reported success and delivered something older. */ +export function heldBackNote(manager: string, installed: string | null, wanted: string | null): string { + return ( + `${manager} reported success but left ${installed ?? 'nothing'} installed, not ${wanted}. ` + + `pnpm holds back very recent releases; trying the next package manager.` + ); +} diff --git a/test/vendor-verify.test.ts b/test/vendor-verify.test.ts new file mode 100644 index 0000000..952dcfe --- /dev/null +++ b/test/vendor-verify.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from 'vitest'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +import { + delivered, + heldBackNote, + installedVersion, + pinnedVersion, + registryLatest, + wantedVersion, +} from '../src/vendor-verify.ts'; + +function prefixWith(pkg: string, version: string | null): string { + const root = mkdtempSync(path.join(tmpdir(), 'vendor-verify-')); + if (version !== null) { + const dir = path.join(root, 'node_modules', ...pkg.split('/')); + mkdirSync(dir, { recursive: true }); + writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: pkg, version })); + } + return root; +} + +const fakeExec = (stdout: string, code = 0) => + (async () => ({ code, stdout, stderr: '' })) as never; + +describe('installedVersion', () => { + it('reads what is actually on disk', () => { + const root = prefixWith('@profullstack/crawlproof', '0.1.0'); + try { + expect(installedVersion(root, '@profullstack/crawlproof')).toBe('0.1.0'); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); + + it('is null when nothing is installed, rather than throwing', () => { + const root = prefixWith('@profullstack/crawlproof', null); + try { + expect(installedVersion(root, '@profullstack/crawlproof')).toBeNull(); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +}); + +describe('pinnedVersion', () => { + it('answers a spec that names a version', () => { + expect(pinnedVersion('@profullstack/crawlproof@0.2.0')).toBe('0.2.0'); + expect(pinnedVersion('hqtui-demo@1.2.3')).toBe('1.2.3'); + }); + + it('declines a tag or a range, so the registry is asked instead', () => { + expect(pinnedVersion('@profullstack/crawlproof@latest')).toBeNull(); + expect(pinnedVersion('@profullstack/crawlproof@^0.1')).toBeNull(); + expect(pinnedVersion('@profullstack/crawlproof')).toBeNull(); + }); +}); + +describe('registryLatest', () => { + it('reads the version npm prints', async () => { + expect(await registryLatest('pkg', fakeExec('0.2.0\n'))).toBe('0.2.0'); + }); + + it('is null when npm fails, so an offline box is not told it is wrong', async () => { + expect(await registryLatest('pkg', fakeExec('', 1))).toBeNull(); + expect(await registryLatest('pkg', fakeExec('not a version\n'))).toBeNull(); + }); +}); + +describe('wantedVersion', () => { + it('prefers the pin and never asks the registry for one', async () => { + const boom = (async () => { + throw new Error('should not be called'); + }) as never; + expect(await wantedVersion('pkg@1.4.2', 'pkg', boom)).toBe('1.4.2'); + }); + + it('asks the registry for a tag', async () => { + expect(await wantedVersion('pkg@latest', 'pkg', fakeExec('9.9.9\n'))).toBe('9.9.9'); + }); +}); + +describe('delivered', () => { + // The whole point: pnpm 11 exits 0 having installed the previous release. + it('rejects an install that left an older version behind', () => { + expect(delivered('0.1.0', '0.2.0')).toBe(false); + }); + + it('accepts the version that was asked for', () => { + expect(delivered('0.2.0', '0.2.0')).toBe(true); + }); + + it('accepts anything when the want could not be determined', () => { + // An unreachable registry must not make a working reinstall look broken. + expect(delivered('0.1.0', null)).toBe(true); + expect(delivered(null, null)).toBe(true); + }); + + it('rejects an install that produced nothing at all', () => { + expect(delivered(null, '0.2.0')).toBe(false); + }); +}); + +describe('heldBackNote', () => { + it('names the version gap rather than saying it failed', () => { + const note = heldBackNote('pnpm', '0.1.0', '0.2.0'); + expect(note).toContain('0.1.0'); + expect(note).toContain('0.2.0'); + expect(note).toContain('pnpm'); + }); +});