Skip to content

Commit e4b5641

Browse files
committed
test(cli): write oclif.manifest.json from source once per integration run (WIP)
Claude-Session: https://claude.ai/code/session_012YTKj54wfJ886NzwKz1oba Co-authored-by: Claude <noreply@anthropic.com>
1 parent c7aa9cd commit e4b5641

3 files changed

Lines changed: 182 additions & 0 deletions

File tree

‎.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,7 @@ examples/app-crm/storage/
148148
# committed (#22449 B′, #22485); `files[]` ships whatever the lane wrote.
149149
packages/spec/spec-changes.json
150150
packages/spec/protocol-upgrade-guide.md
151+
152+
# #23181 slice 14: written by packages/cli/test/helpers/oclif-manifest.global-setup.ts for a test run and removed at its teardown; never committed, never published
153+
packages/cli/oclif.manifest.json
154+
packages/cli/oclif.manifest.json.*.tmp
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The vitest `globalSetup` that writes `packages/cli/oclif.manifest.json` once
5+
* per run, from source, and removes it at teardown. Test infrastructure only:
6+
* the file is gitignored and `files[]` (`dist`, `README.md`, `CHANGELOG.md`)
7+
* never names it, so nothing published changes.
8+
*
9+
* ## Why (#23181, seat ruling A on report 6418998910)
10+
*
11+
* This package ships no oclif manifest, so every CLI process builds its command
12+
* table by importing all 72 command modules. Through `tsx bin/run-dev.js` that
13+
* is a floor of 8.2-8.5 s per spawn, `--version` included, and the files that
14+
* spawn the CLI held 89.4% of this package's file wall. With a manifest beside
15+
* `package.json` oclif reads the table from it and imports only the command it
16+
* runs: the same spawn takes 2.0-2.3 s.
17+
*
18+
* ## What is written, and why it is transparent
19+
*
20+
* The manifest is the object oclif's own root plugin builds in memory when no
21+
* manifest exists — `new Plugin({ isRoot: true, root })`, loaded in a `tsx`
22+
* child under `NODE_ENV=development`, which is what `bin/run-dev.js` runs — with
23+
* two keys removed from every command: `isESM` and `relativePath`.
24+
*
25+
* ⛔ Those two keys are not optional to remove. When a cached command carries
26+
* both, `Plugin.findCommand` imports `root + relativePath` VERBATIM
27+
* (`cachedCommandCanBeUsed` in @oclif/core 5.1.2), and a manifest generated from
28+
* source records `src/commands/<id>.ts`. The built entry (`bin/run.js`, which
29+
* the published-entry suites spawn with `NODE_ENV` unset) would then run the
30+
* SOURCE, and those suites would test something other than the build. Without
31+
* the two keys oclif falls back to the path it takes with no manifest at all —
32+
* the commands directory `tsPath` resolves, `src/` under `run-dev.js` and
33+
* `dist/` under `run.js` — so the manifest supplies the command TABLE and
34+
* nothing about where a command's code comes from.
35+
*
36+
* What still differs from a run without the file: `Config.load()` no longer
37+
* imports every command module, so a module that fails to load is reported when
38+
* it is run rather than as a load-time warning on every spawn. The suites that
39+
* pin that load-time behaviour are `*.e2e.test.ts` files, and the e2e tier runs
40+
* only in the nightly population, which `vitest.config.ts` leaves without this
41+
* setup. A command module that fails to load here fails the GENERATION instead
42+
* (`errorOnManifestCreate`), naming the module, before any test runs.
43+
*
44+
* ## Ownership, nesting and cleanup
45+
*
46+
* - Written to a temporary sibling and renamed over the target, so a spawn
47+
* reads either a whole manifest or none (oclif then builds its table as
48+
* before). Every run regenerates, so a manifest left by a killed run is never
49+
* what a later integration run uses.
50+
* - `writtenBy.pid` records the vitest process that owns the file. Teardown, and
51+
* an `exit` listener for a run that ends without teardown, remove it only
52+
* while that pid is still this process's: two runs on one checkout each
53+
* remove their own file, and the run that is left continues without a
54+
* manifest, at the old floor and with the same output.
55+
* - A vitest started INSIDE a run (`test/vitest-tiers-partition.test.ts` spawns
56+
* `vitest list`) inherits {@link OWNER_ENV} through `childEnv()` and does
57+
* nothing, so it neither pays a second generation nor removes the file under
58+
* the run that wrote it.
59+
* - A `unit`-only run generates nothing (no unit file spawns the CLI) and only
60+
* removes a manifest whose writer is no longer alive, so an in-process
61+
* `Config.load()` there never trusts a file a killed run left behind.
62+
*/
63+
64+
import { execFile } from 'node:child_process';
65+
import { readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs';
66+
import { resolve } from 'node:path';
67+
import { fileURLToPath } from 'node:url';
68+
import { promisify } from 'node:util';
69+
import type { TestProject } from 'vitest/node';
70+
import { childEnv, TSX } from './serve-process.js';
71+
72+
const HERE = resolve(fileURLToPath(import.meta.url), '..');
73+
74+
/** `packages/cli` — the root oclif reads `oclif.manifest.json` from. */
75+
const CLI_ROOT = resolve(HERE, '../..');
76+
77+
export const MANIFEST = resolve(CLI_ROOT, 'oclif.manifest.json');
78+
79+
/** Set in the owning run's process, and so in every worker and child below it. */
80+
export const OWNER_ENV = 'OS_TEST_OCLIF_MANIFEST_OWNER';
81+
82+
/** The argv flag that makes this module the generator, inside the `tsx` child. */
83+
const WRITE_FLAG = '--write-oclif-manifest';
84+
85+
const THIS_FILE = fileURLToPath(import.meta.url);
86+
87+
/** The pid recorded in the manifest at `path`, or `undefined` for none or not ours. */
88+
function writerOf(path: string): number | undefined {
89+
try {
90+
const pid = JSON.parse(readFileSync(path, 'utf8'))?.writtenBy?.pid;
91+
return Number.isInteger(pid) ? pid : undefined;
92+
} catch {
93+
return undefined;
94+
}
95+
}
96+
97+
function isAlive(pid: number): boolean {
98+
try {
99+
process.kill(pid, 0);
100+
return true;
101+
} catch (error) {
102+
return (error as NodeJS.ErrnoException).code === 'EPERM';
103+
}
104+
}
105+
106+
function removeIfOurs(): void {
107+
if (writerOf(MANIFEST) === process.pid) rmSync(MANIFEST, { force: true });
108+
}
109+
110+
/** Runs in the `tsx` child: build the table the way an unmanifested run does. */
111+
async function writeManifest(out: string, ownerPid: number): Promise<void> {
112+
const { Plugin } = await import('@oclif/core');
113+
const plugin = new Plugin({ isRoot: true, root: CLI_ROOT, ignoreManifest: true, errorOnManifestCreate: true });
114+
await plugin.load();
115+
const commands = Object.fromEntries(
116+
Object.entries(plugin.manifest.commands).map(([id, { isESM: _isESM, relativePath: _relativePath, ...cached }]) => [
117+
id,
118+
cached,
119+
]),
120+
);
121+
writeFileSync(out, JSON.stringify({ ...plugin.manifest, commands, writtenBy: { pid: ownerPid, by: 'packages/cli/test/helpers/oclif-manifest.global-setup.ts' } }));
122+
}
123+
124+
export default async function setup(project: TestProject): Promise<(() => void) | void> {
125+
if (process.env[OWNER_ENV]) return;
126+
127+
if (project.name !== 'integration') {
128+
const writer = writerOf(MANIFEST);
129+
if (writer !== undefined && !isAlive(writer)) rmSync(MANIFEST, { force: true });
130+
return;
131+
}
132+
133+
const tmp = `${MANIFEST}.${process.pid}.tmp`;
134+
try {
135+
await promisify(execFile)(TSX, [THIS_FILE, WRITE_FLAG, tmp, String(process.pid)], {
136+
cwd: CLI_ROOT,
137+
env: childEnv({ NODE_ENV: 'development' }),
138+
maxBuffer: 64 * 1024 * 1024,
139+
});
140+
} catch (error) {
141+
rmSync(tmp, { force: true });
142+
const { stderr } = error as { stderr?: string };
143+
throw new Error(`could not write ${MANIFEST} from source:\n${stderr || String(error)}`);
144+
}
145+
renameSync(tmp, MANIFEST);
146+
process.env[OWNER_ENV] = String(process.pid);
147+
process.once('exit', removeIfOurs);
148+
return () => {
149+
process.off('exit', removeIfOurs);
150+
removeIfOurs();
151+
};
152+
}
153+
154+
if (process.argv[1] === THIS_FILE && process.argv[2] === WRITE_FLAG) {
155+
await writeManifest(process.argv[3], Number(process.argv[4]));
156+
}

‎packages/cli/vitest.config.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,7 @@ import {
649649
runProjectCliOverridePreflight,
650650
} from '../qa/vitest-filter-preflight/src/index.js';
651651
import { integrationTestFiles, unitTestFiles } from './vitest-tiers.js';
652+
import { readTierMode } from '../../scripts/nightly-tiers.mjs';
652653

653654
// The two tiers, DERIVED from what the files DO — never written down — over
654655
// the population `OS_TEST_TIERS` selects. `vitest-tiers.ts` holds the
@@ -739,6 +740,25 @@ runProjectCliOverridePreflight({
739740
// lives at its use site in the `test` block below, not in a helper binding
740741
// that could outlive the spread.
741742

743+
// #23181 slice 14 — THE OCLIF MANIFEST SETUP. One `globalSetup` module writes
744+
// `oclif.manifest.json` from source when the `integration` project runs (the
745+
// tier every CLI-spawning file is derived into) and removes it at teardown, so
746+
// a `tsx bin/run-dev.js` spawn reads its command table instead of importing all
747+
// 72 command modules: 8.2-8.5 s -> 2.0-2.3 s per spawn, same bytes out, same
748+
// exit code. The `unit` project carries the same module and generates nothing;
749+
// it only clears a manifest a killed run left behind. The module's header is
750+
// the authority on what is written, why it is transparent, and who removes it.
751+
//
752+
// ⛔ The QUEUE population only. Under `OS_TEST_TIERS=nightly` both projects run
753+
// without it, because the e2e tier holds the suites that pin what a manifest
754+
// would change: oclif importing every command module inside `Config.load()`
755+
// (`run-dev-unbuilt-workspace.e2e`, the `*-stderr-nonblocking.e2e` pair). The
756+
// product ships no manifest, so that is the shape those pins must keep seeing.
757+
// Set per project rather than at the root, where `extends: true` would hand it
758+
// to both projects AND to the root project, running it three times.
759+
const OCLIF_MANIFEST_SETUP =
760+
readTierMode() === 'queue' ? [path.resolve(__dirname, 'test/helpers/oclif-manifest.global-setup.ts')] : [];
761+
742762
export default defineConfig({
743763
resolve: {
744764
// Array form with an ANCHORED pattern, per the trap the gate documents:
@@ -873,6 +893,7 @@ export default defineConfig({
873893
test: {
874894
name: 'unit',
875895
disableConsoleIntercept: true,
896+
globalSetup: OCLIF_MANIFEST_SETUP,
876897
// #13517: quiet the registry's per-item registration chatter — the
877898
// engine's own `OS_REGISTRY_LOG` seam, not a change to its shipped
878899
// default. Enforced by scripts/check-registry-log-declared.mjs.
@@ -885,6 +906,7 @@ export default defineConfig({
885906
test: {
886907
name: 'integration',
887908
disableConsoleIntercept: true,
909+
globalSetup: OCLIF_MANIFEST_SETUP,
888910
// #13517: quiet the registry's per-item registration chatter — the
889911
// engine's own `OS_REGISTRY_LOG` seam, not a change to its shipped
890912
// default. Enforced by scripts/check-registry-log-declared.mjs.

0 commit comments

Comments
 (0)