From fc6eadda1538b8bbba519aea19d2c2fa036c8b66 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 15:49:48 +0000 Subject: [PATCH] test(cli): record the vitest resolution-base collapse as an executable platform fact (#11412) Under vitest an in-process test cannot measure which package a bare specifier resolves from, and the anti-vacuity control written beside such a test is vacuous too. Both halves are now measured with controls that are shown to fire. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx --- ...erve-config-plugin-host-resolution.test.ts | 22 +- ...itest-resolution-base-collapse.e2e.test.ts | 235 ++++++++++++++++++ scripts/check-test-source-alias.mjs | 27 ++ 3 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts diff --git a/packages/cli/src/commands/serve-config-plugin-host-resolution.test.ts b/packages/cli/src/commands/serve-config-plugin-host-resolution.test.ts index 3072f65b85..aa560740ef 100644 --- a/packages/cli/src/commands/serve-config-plugin-host-resolution.test.ts +++ b/packages/cli/src/commands/serve-config-plugin-host-resolution.test.ts @@ -157,10 +157,24 @@ describe('os serve → the branches that must NOT move (#10908 supersedes nothin // // ⚠️ This assertion is why #11157 had to land BEFORE the branch collapse and // not after. It used to be kept true by a local `import()` here; it is now - // kept true by `importFromHost` carrying this file's base. Take the base - // away and this line goes red — measured, and pinned again from the other - // side (with the no-base control beside it) in - // `serve-host-fallback-base.test.ts`. + // kept true by `importFromHost` carrying this file's base. + // + // ⛔ IT IS NOT THE MEASUREMENT OF THAT BASE, and this comment used to claim + // it was ("take the base away and this line goes red"). #11412 ablated it: + // with `fallbackImport` removed from `importFromHost` — the #11157 fix gone, + // everything else identical — this case stayed GREEN, while the spawned-child + // pin of the same claim in `test/serve-host-fallback-base.e2e.test.ts` went + // RED. Under vitest `@objectstack/types` is inlined and its `import()` is + // rewritten to resolve from the vitest root, so the caller's base and the + // callee's base are the same base and no in-process assertion can tell them + // apart. The anti-vacuity control written beside such an assertion is vacuous + // for the same reason, which is why this note replaces one rather than adding + // one. `test/vitest-resolution-base-collapse.e2e.test.ts` holds the mechanism + // with its controls; the resolution pins live in the `.e2e.` file above. + // + // What this line still honestly says: an app that declares nothing can load a + // CLI-declared package at all. That is a real behaviour and worth keeping — + // it is just not evidence about WHICH package resolved it. const root = makeApp(APP_ONLY, { declare: false, install: false }); const mod = await Serve.importConfigPlugin('chalk', root); diff --git a/packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts b/packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts new file mode 100644 index 0000000000..c9f3c10750 --- /dev/null +++ b/packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts @@ -0,0 +1,235 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #11412 — under vitest, an in-process test CANNOT measure which package a bare + * specifier resolves from, and the anti-vacuity control written beside such a + * test is vacuous too. This file is the executable record of that platform fact. + * + * It is a PRECONDITION pin in the sense `packages/types/src/node.test.ts` uses + * the word: it asserts what the runner does, not what this repo's code does, so + * that the day vitest/Vite changes the behaviour the repo is told — instead of + * the next author rediscovering it by ablation, which is the expensive path this + * card was filed to close. + * + * ── TWO mechanisms, not one. They need different remedies ────────────────── + * + * **M1 — Vite inlines a linked workspace package and rewrites its `import()`.** + * The inline/external decision is `server.deps.external`'s default, + * `[/\/node_modules\//]`, evaluated against the module's REALPATH. Every + * workspace package here is a pnpm link whose realpath is the package directory + * (`packages/types/dist/node.mjs`), which contains no `/node_modules/` segment + * — so every workspace dependency is INLINED, including one reached through + * `exports` to `dist/`. Vite then rewrites the `import()` written inside it to + * `__vite_ssr_dynamic_import__`, which resolves from the VITEST ROOT rather than + * from the module that physically contains the call. Node ESM does the opposite: + * it anchors a bare specifier at the containing module. So under vitest the + * callee's base and the caller's base ARE THE SAME BASE. + * + * That is what makes the control vacuous. The control for a base claim is "build + * it the old way and show it fails" — and the old way does not fail here, so the + * control is written green and reports nothing. Measured on this card: removing + * #11157's fix from `serve.ts` entirely left the in-process `chalk` assertion in + * `src/commands/serve-config-plugin-host-resolution.test.ts` GREEN, while the + * spawned-child pin of the same claim in `serve-host-fallback-base.e2e.test.ts` + * went RED. Same tree, same ablation, opposite verdicts. + * + * **M2 — `NODE_PATH` reaches the spawned child, and CJS honours it.** This one + * is NOT vitest rewriting anything, and it survives the obvious remedy. A vitest + * worker runs with `NODE_PATH` pointing at pnpm's hoisted store + * (`node_modules/.pnpm/node_modules`, which holds everything transitively + * reachable in the workspace), and `test/helpers/serve-process.ts`'s `childEnv()` + * strips only `TEST` / `VITEST*` — so `NODE_PATH` rides into every spawned child + * this package starts. + * + * The split that decides whether that matters, measured here: + * + * resolution API NODE_PATH honoured? base preserved? + * ESM `import()` / import.meta.resolve NO YES + * CJS createRequire().resolve() YES NO + * + * and `NODE_PATH` is a FALLBACK, not an override — the `node_modules` walk wins + * when it hits, so the store can only turn a MISS into a HIT. The dangerous + * direction is therefore an ACCEPTANCE claim ("this base CAN reach X"): it goes + * green because the store supplied X, not because the base did. + * + * ⚠️ Consequence for the remedy this repo standardised on: spawning a real Node + * child escapes M1 but NOT M2. `serve-host-fallback-base.e2e.test.ts`'s CONTROL + * is sound only because `createHostImporter`'s fallback leg is an ESM `import()`. + * Had it been CJS — as `createHostRequire` is — the inherited `NODE_PATH` would + * have kept it green through the very ablation it exists to fail. A spawned pin + * whose claim routes through CJS must pass `childEnv({ NODE_PATH: undefined })`. + * + * ── Reading this file ────────────────────────────────────────────────────── + * + * Every zero is paired with a control that proves the probe could have been + * non-zero, because a probe that can only say MISS would "confirm" all of the + * above while measuring nothing. `chalk` is the discriminator throughout: it is + * DECLARED by `packages/cli` and resolvable from it, and NOT resolvable from + * `packages/types` (whose one dependency is `@objectstack/spec`). + * + * `@objectstack/types` is reached by SPECIFIER and every path below is DERIVED + * from that resolution — never written as `resolve(HERE, '../../types/…')`. Both + * spellings land on the same file; only one is honest about naming an installed + * dependency rather than a repo source input no turbo glob covers. See + * `pnpm check:cross-package-test-inputs` for the rule and its reason. + */ + +import { execFile } from 'node:child_process'; +import { createRequire } from 'node:module'; +import { dirname } from 'node:path'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { promisify } from 'node:util'; +import { pathToFileURL } from 'node:url'; +import { beforeAll, afterAll, describe, expect, it } from 'vitest'; +import { createHostImporter } from '@objectstack/types/node'; +import { childEnv } from './helpers/serve-process.js'; + +const execFileAsync = promisify(execFile); + +/** Declared by `packages/cli`, NOT resolvable from `packages/types`. */ +const CLI_DECLARED = 'chalk'; +/** `packages/types`' one declared dependency — resolvable from BOTH bases. */ +const TYPES_DECLARED = '@objectstack/spec'; +/** Satisfiable by nothing anywhere, so no result can be an accident. */ +const NOWHERE = '@os-fixture/vitest-base-collapse-probe'; + +/** + * The helper's own entry, by specifier. `dirname()` of it is the directory the + * `import()` inside `dist/node.mjs` is physically written in — i.e. the base + * Node would anchor that call at, and the one vitest replaces. + */ +const TYPES_ENTRY = createRequire(import.meta.url).resolve('@objectstack/types/node'); +const TYPES_BASE = dirname(TYPES_ENTRY); + +/** + * One child, run at the types package's own base. Reports what REAL Node says + * for each API, so the in-process readings have something to diverge from. + */ +const CHILD = ` +import { createRequire } from 'node:module'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; + +const attempt = (fn) => { try { return 'RESOLVED:' + String(fn()); } catch (e) { return 'MISS:' + String(e.code ?? e.message); } }; +// The FULL message: \`createHostImporter\` composes the origin Node reports onto +// a later line, and a first-line-only reading loses exactly the half that says +// WHICH base failed — the sentence this file exists to assert. +const attemptAsync = async (fn) => { try { await fn(); return 'RESOLVED'; } catch (e) { return String(e.message); } }; + +// Wired by env, never by argv position: \`node -e\` shifts argv (there is no +// script path), and a shifted argv reads as a resolution failure rather than as +// a wiring bug. +const req = createRequire(join(process.cwd(), 'probe.cjs')); +const { createHostImporter } = await import(pathToFileURL(process.env.PROBE_TYPES_ENTRY).href); +const appRoot = process.env.PROBE_APP_ROOT; + +console.log(JSON.stringify({ + // PROOF OF ANCHOR: the referrer this child's ESM leg actually resolves against. + esmReferrer: import.meta.url, + nodePathSeen: process.env.NODE_PATH ?? '(unset)', + esmCliDeclared: attempt(() => import.meta.resolve(${JSON.stringify(CLI_DECLARED)})), + esmTypesDeclared: attempt(() => import.meta.resolve(${JSON.stringify(TYPES_DECLARED)})), + esmNowhere: attempt(() => import.meta.resolve(${JSON.stringify(NOWHERE)})), + cjsCliDeclared: attempt(() => req.resolve(${JSON.stringify(CLI_DECLARED)})), + cjsNowhere: attempt(() => req.resolve(${JSON.stringify(NOWHERE)})), + noBaseImporter: await attemptAsync(() => createHostImporter(appRoot)(${JSON.stringify(CLI_DECLARED)})), +})); +`; + +let appRoot: string; +/** Real Node, `NODE_PATH` stripped — the uncontaminated baseline. */ +let clean: Record; +/** Real Node, `NODE_PATH` exactly as `childEnv()` hands it over. */ +let inherited: Record; + +async function runChild(env: Record): Promise> { + const { stdout } = await execFileAsync( + process.execPath, + ['--input-type=module', '-e', CHILD], + { + cwd: TYPES_BASE, + env: { ...env, PROBE_TYPES_ENTRY: TYPES_ENTRY, PROBE_APP_ROOT: appRoot } as NodeJS.ProcessEnv, + maxBuffer: 8 * 1024 * 1024, + }, + ); + return JSON.parse(stdout.trim()) as Record; +} + +beforeAll(async () => { + appRoot = mkdtempSync(join(tmpdir(), 'os-11412-app-')); + writeFileSync( + join(appRoot, 'package.json'), + JSON.stringify({ name: 'fixture-app', version: '1.0.0', type: 'module' }), + ); + clean = await runChild(childEnv({ NO_COLOR: '1', NODE_PATH: undefined })); + inherited = await runChild(childEnv({ NO_COLOR: '1' })); +}, 120_000); + +afterAll(() => { + if (appRoot) rmSync(appRoot, { recursive: true, force: true }); +}); + +describe('#11412 CONTROLS — the probe can return every answer it is asked to distinguish', () => { + it('the child really is anchored at the types package, not at the test', () => { + // Without this the whole file could be measuring `packages/cli` twice. The + // referrer is `file:///[eval1]`, which is what `--input-type=module` + // anchors a bare specifier at. + expect(clean.esmReferrer).toContain(pathToFileURL(TYPES_BASE).href); + }); + + it('CAN say HIT: the types package resolves its own declared dependency', () => { + expect(clean.esmTypesDeclared).toMatch(/^RESOLVED:/); + }); + + it('CAN say MISS: a specifier nothing satisfies misses on both legs, both envs', () => { + for (const probe of [clean, inherited]) { + expect(probe.esmNowhere).toMatch(/^MISS:/); + expect(probe.cjsNowhere).toMatch(/^MISS:/); + } + }); +}); + +describe('#11412 M1 — vitest flattens the resolution base an in-process test would measure', () => { + it('REAL NODE: the no-base importer cannot reach a CLI-declared package, and names its base', async () => { + expect(clean.noBaseImporter).not.toBe('RESOLVED'); + expect(clean.noBaseImporter).toContain(`Cannot find package '${CLI_DECLARED}'`); + // Named, not merely failed — this is the sentence the whole card is about. + expect(clean.noBaseImporter).toMatch(/imported from .*[/\\]packages[/\\]types[/\\]/); + }); + + it('UNDER VITEST: the identical call RESOLVES — so no in-process pin over it can fail', async () => { + await expect(createHostImporter(appRoot)(CLI_DECLARED)).resolves.toBeDefined(); + // …and the same importer still refuses a specifier nothing satisfies, so the + // line above is the base collapsing, not "everything resolves in here". + await expect(createHostImporter(appRoot)(NOWHERE)).rejects.toThrow( + `Cannot find package '${NOWHERE}'`, + ); + }); + + it('the mechanism is Vite rewriting the inlined package’s dynamic import', () => { + // If this ever reads false, M1 is gone and the two cases above should be + // re-measured before anything is written on top of them. + expect(String(createHostImporter)).toContain('__vite_ssr_dynamic_import__'); + }); +}); + +describe('#11412 M2 — spawning escapes Vite, but NODE_PATH rides along and CJS honours it', () => { + it('childEnv() hands NODE_PATH to the child (it strips only TEST / VITEST*)', () => { + expect(inherited.nodePathSeen).not.toBe('(unset)'); + // The stripped-env leg is what proves the line above is about `childEnv()`'s + // policy and not about this box always having NODE_PATH set. + expect(clean.nodePathSeen).toBe('(unset)'); + }); + + it('ESM ignores NODE_PATH: the base survives into the child', () => { + expect(clean.esmCliDeclared).toMatch(/^MISS:/); + expect(inherited.esmCliDeclared).toMatch(/^MISS:/); + }); + + it('CJS honours NODE_PATH: the same base, the same specifier, the opposite answer', () => { + expect(clean.cjsCliDeclared).toMatch(/^MISS:/); + expect(inherited.cjsCliDeclared).toMatch(/^RESOLVED:/); + }); +}); diff --git a/scripts/check-test-source-alias.mjs b/scripts/check-test-source-alias.mjs index 0503dda71d..de7c9a1152 100644 --- a/scripts/check-test-source-alias.mjs +++ b/scripts/check-test-source-alias.mjs @@ -80,6 +80,33 @@ // file extension. Either anchor the pattern (`/^@objectstack\/core$/`, the // array form) or list the subpath entry ahead of the bare one. // +// ── A SECOND resolution hazard, which this gate does NOT cover (#11412) ──── +// +// This gate is about WHICH ARTIFACT a test resolves — source or `dist/`. There +// is a separate axis it says nothing about: WHICH BASE a bare specifier is +// resolved FROM, and under vitest that question is erased rather than answered +// wrongly. +// +// Every workspace package here is a pnpm link whose realpath is the package +// directory, so it contains no `/node_modules/` segment and vitest's default +// `server.deps.external` (`[/\/node_modules\//]`) INLINES it — `dist/` included. +// Vite then rewrites the `import()` written inside that package to resolve from +// the vitest root instead of from the module physically containing the call, +// which is the opposite of what Node ESM does. So in-process, the caller's base +// and the callee's base are the same base, every assertion about which one is in +// use is green either way — AND SO IS THE ANTI-VACUITY CONTROL BESIDE IT. That +// is why the hazard needs recording rather than testing in place: the ritual +// that would normally catch it returns the wrong answer. +// +// A pin that must measure a resolution BASE therefore spawns a real Node child. +// ⚠️ Spawning escapes Vite but NOT `NODE_PATH`: a vitest worker carries pnpm's +// hoisted store in it, `childEnv()` forwards it, and CJS `createRequire()` +// honours it while ESM `import()` ignores it. A spawned pin whose claim routes +// through CJS must strip it (`childEnv({ NODE_PATH: undefined })`). +// +// The mechanism, both halves, and the controls that prove each one can fail live +// in `packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts`. +// // ── Where the walk goes, and why it leaves the package (#8351) ────────────── // // **Aliasing a workspace dep to source imports that dep's ENTIRE import surface