Repository navigation
Commit 9c922b5
test(cli): write oclif.manifest.json once per integration run, so a CLI spawn reads its command table (#26256)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X7p8QUpB5pvxJHFoRiWTJwu)_
Part of #23181 (slice 14). The card is a series and stays open.
Clause-②: no
Metadata-protocol: none
Test infrastructure only: `skip-changeset`. `@objectstack/cli`'s
`files[]` is `dist`, `README.md` and `CHANGELOG.md`, unchanged, and
nothing in it changes. The diff touches `packages/cli/vitest.config.ts`,
one new module under `packages/cli/test/helpers/`, one test file under
`packages/cli/test/`, and the root `.gitignore`. No build step, no new
gate, no product code.
## What this does
Seat ruling A on slice 11's open question (report 6418998910 on #23181).
A vitest `globalSetup`,
`packages/cli/test/helpers/oclif-manifest.global-setup.ts`, writes
`packages/cli/oclif.manifest.json` from source once per run of the
`integration` project and removes it at teardown. The file is
gitignored.
This package ships no oclif manifest, so every CLI process builds its
command table by importing all 72 command modules. Through `tsx
bin/run-dev.js` that is a floor of about 8 s per spawn. With the
manifest beside `package.json`, oclif reads the table from it and
imports only the command it runs. Outside the one file ruling A1 changed
(below), spawn shape, the `bin/run-dev.js` entry and `NODE_ENV` are
unchanged.
### Why the written manifest is transparent
- It is the object oclif's own root plugin builds in memory when no
manifest exists (`new Plugin({ isRoot: true, root })`, loaded in a `tsx`
child under `NODE_ENV=development`, as `bin/run-dev.js` runs), with
`errorOnManifestCreate` on so that a command module that fails to load
fails the generation loudly, naming the module.
- Two keys are removed from every command: `isESM` and `relativePath`.
This is required. When a cached command carries both, `@oclif/core`
5.1.2 imports `root + relativePath` verbatim, and a manifest generated
from source records `src/commands/ID.ts`. Measured control: with those
keys kept, `node bin/run.js generate object order_line --dry-run`
imports `packages/cli/src/commands/generate.ts` and exits 1
(`ModuleLoadError`). With them removed, oclif takes its no-manifest
path. Its debug log reads `(import) CLI/dist/commands/generate.js` under
`bin/run.js` and `(import) CLI/src/commands/generate.ts` under
`bin/run-dev.js`. So the manifest supplies the command table and nothing
about where a command's code comes from.
- Byte comparison, without and with the manifest, 9 invocations x 2
entries (`tsx bin/run-dev.js` and `node bin/run.js`): `--version`,
`--help`, `serve --help`, `generate object --help`, `generate object
order_line --dry-run`, `generate object class` (exit 1), `generate
object` (missing argument, exit 1), `validate` in `examples/app-todo`,
and an unknown command (exit 2). stdout, stderr and exit code are
identical in 52 of 54 streams. The 2 that differ are `validate`'s stdout
on both entries, and they differ only in the two printed elapsed-time
lines (`Load time: 148ms` against `164ms`, and `Validation passed
(279ms)` against `(278ms)`).
### Scope: the queue population only
Under `OS_TEST_TIERS=nightly` neither project carries the setup. The e2e
tier holds the suites that pin what a manifest changes, which is
`Config.load()` importing every command module
(`run-dev-unbuilt-workspace.e2e`, the two `*-stderr-nonblocking.e2e`
files). The product ships no manifest, so that is the shape those pins
keep seeing. The setup is wired per project, not at the root: with
`extends: true` a root `globalSetup` would run for the root project and
both tier projects, three times.
### Ownership, nesting and cleanup (the order's three hypotheses)
- **Written once, before any worker spawns.** vitest runs every
project's `globalSetup` before `pool.runTests`. The file is written to a
temporary sibling and renamed over the target, so a spawn reads a whole
manifest or none.
- **Nested vitest runs leave it alone.** The owning run sets
`OS_TEST_OCLIF_MANIFEST_OWNER` in its own process. Workers and
`childEnv()` children inherit it, and a vitest started inside the run
does nothing. Measured with a throwaway integration test that spawned
`vitest run --project integration` through `childEnv()`: the worker and
the child env both carried the owner pid, the manifest's owner was the
same before and after (3443), and the nested run took 3.5 s, with no 9 s
generation.
- **Two runs on one checkout.** Each run records its pid in the file
(`writtenBy.pid`) and removes the file only while that pid is still its
own. Measured with two `--project integration` runs started 4 s apart:
the owner went 3691 then 3788, both runs were green (5/5 and 7/7), and
after both the file was gone. The run left without a manifest continues
at the old floor with the same output.
- **Crash.** Every integration run regenerates unconditionally, and a
`unit`-only run removes a file whose writer is dead. Measured: after a
`kill -9` of the owning vitest, the file stayed with a dead owner. The
next `--project unit` run removed it. A planted dead-owner file (`pid
999999`) was regenerated by the next integration run (owners seen:
999999, then 4631) and removed at its teardown. After a `SIGINT` mid-run
the file was gone (teardown, or the `exit` listener for a run that ends
without one).
## One test file changed, by seat ruling A1
`test/published-entry-node-env-source-reroute.test.ts` (#12271) spawned
`node bin/run.js --version`. That reached the reroute only because, with
no manifest, `Config.load()` imports every command module. With the
manifest present, `--version` loads no command. In the full run below,
its CONTROL stopped reproducing the card, and its three positive legs
passed over a fixture that armed nothing. Its spawns now run `explain
object`, a real command whose module imports the trapped `chalk`, with
`OCLIF_COLUMNS=100000` so oclif renders the load failure on one line.
With a manifest that failure is the error `handle()` prints,
hard-wrapped at 80 columns on a pipe, and a wrap can split the signature
or the `src/commands` path. No assertion was changed or loosened.
Legs measured both ways, against the same armed fixture (`exit` / count
of the card signature / count of `packages/cli/src/commands`):
| leg | no manifest (main's shape) | manifest present |
|:--|:--|:--|
| CONTROL, neutralised, `NODE_ENV=development` | 1 / 4 / 3 | 1 / 1 / 1 |
| CONTROL, neutralised, `NODE_ENV=production` | 0 / 0 / 0 | 0 / 0 / 0 |
| `NODE_ENV=development` | 0 / 0 / 0 | 0 / 0 / 0 |
| `NODE_ENV=test` | 0 / 0 / 0 | 0 / 0 / 0 |
| `NODE_ENV=production` | 0 / 0 / 0 | 0 / 0 / 0 |
So the control still goes red on the trap on main's shape, and also with
the manifest. The file under vitest: 5/5 passed twice under this config
(manifest written) and 5/5 twice under the base config (no manifest).
## Measurements
All on one shared 4-vCPU container under `os-verify-lock`. Per-file
numbers are the default reporter's module line.
**Per spawn** (the byte-comparison runs above): `tsx bin/run-dev.js` 7.6
to 9.1 s without, and with the manifest 0.48 to 0.55 s for `--version`
and help, 1.7 to 3.9 s for commands that run. `node bin/run.js` 2.1 to
2.6 s without, 0.15 to 2.1 s with. One generation: 8.8 to 9.9 s.
**Slice 11's spawn family** (PR #26174's ten files), interleaved A/B,
`--project integration --maxWorkers=2`, BASE = `c7aa9cde`
`vitest.config.ts`, HEAD = this branch, N=3:
| file | base, 3 runs (s) | head, 3 runs (s) | median ratio |
|:--|--:|--:|--:|
| `test/build-callable-names.test.ts` | 87.6 / 87.4 / 80.9 | 35.2 / 35.2
/ 37.3 | 2.5x |
| `test/generate-refuses-unimportable-alias.test.ts` | 67.6 / 64.0 /
61.4 | 15.7 / 15.8 / 16.6 | 4.1x |
| `test/build-runtime-bundle-bodyless.test.ts` | 63.2 / 59.2 / 54.8 |
24.4 / 23.1 / 24.9 | 2.4x |
| `test/build-strict-body-count.test.ts` | 56.0 / 55.9 / 54.6 | 23.0 /
23.7 / 25.8 | 2.4x |
| `test/generate-refuses-unparseable-name.test.ts` | 46.7 / 47.6 / 43.6
| 11.2 / 10.3 / 11.4 | 4.2x |
| `test/generate-refuses-namespace-prefix.test.ts` | 48.2 / 46.0 / 43.7
| 11.8 / 10.6 / 12.6 | 3.9x |
| `test/build-view-container-name.test.ts` | 48.2 / 46.0 / 45.2 | 19.4 /
18.9 / 21.6 | 2.4x |
| `test/generate-refuses-retired-generator.test.ts` | 38.0 / 35.5 / 38.1
| 8.8 / 8.0 / 8.2 | 4.6x |
| `test/generate-refuses-name-outside-charset.test.ts` | 35.6 / 37.8 /
37.7 | 9.8 / 8.3 / 9.6 | 3.9x |
| `test/build-text-face-advisory-count.test.ts` | 28.5 / 27.8 / 26.6 |
12.5 / 11.1 / 12.1 | 2.3x |
| sum of file lines | 519.5 / 507.1 / 486.5 | 172.0 / 164.9 / 180.1 |
2.9x |
Run wall: base 285.3 / 279.6 / 258.2 s, head 110.8 / 106.8 / 115.7 s
(generation included). Tests 91/91 every run, and no manifest was left
after any run. The `build-*` files keep more of their wall because their
build work is real, not spawn floor.
**Budget headers:** slice 11's `// test-budget:` headers (PR #26174) are
not lowered here. A later refresh re-sizes them.
## Proof
- **Full `@objectstack/cli` suite with the setup** (both tiers, queue
population, `vitest run --maxWorkers=2`, at `e4b56419`, before the A1
change): Test Files 1 failed, 504 passed, 1 skipped (506), Tests 1
failed, 6324 passed, 9 skipped. The one red was the #12271 CONTROL
described above. A watcher saw one manifest owner for the whole
37-minute run (the vitest pid), so there was no second generation, and
the file was absent after teardown.
- **At `d12d287fe`** (the branch before the merge): the #12271 file 5/5
under both configs (above). `vitest run --project unit --maxWorkers=2`:
Test Files 333 passed (333), Tests 4854 passed (4854), lock VERDICT
command-exit 0. `pnpm --filter @objectstack/cli typecheck`: exit 0,
`check:test-typecheck: OK` (the new module is in the test program
through `test/**`).
- `pnpm exec eslint --no-inline-config packages/cli`: exit 0.
- Gates at the final head `8e5065109` (current `main` merged in, with no
overlap with this diff): `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` derived 56 commands over the same 4
paths, each run with its exit code captured by redirect-then-`$?`, and
all 56 exited 0. `--ran` with every exit recorded: "56 derived
famil(ies) accounted for — 56 run, 0 NOT-MEASURED (a DERIVED zero)". The
run at `d12d287fe` before the merge was the same, after two
prerequisites were met: `check-plugin-teardown-shape --self-test` needed
`621a4876` fetched into the shallow clone, and
`check:dual-build-cjs-loads` needed 10 unbuilt packages built.
`check:cli-test-child-env` (the new module spawns its generator with
`childEnv()`) and `check:cross-package-test-inputs` are among the 56.
Repo-wide `pnpm lint` and the full CI farm belong to CI.
- After merging `main`: rebuilt the cli closure, then ran the 18 cli
test files changed since the branch point (17 by `main`, plus the #12271
pin) and `test/vitest-tiers-partition.test.ts` with this config. Test
Files 19 passed (19), Tests 188 passed (188), lock VERDICT command-exit
0. That covers slice 11's ten files now carrying their budget headers,
slice 12's migrate files and #26047's `hook-timeout-override-refusal`,
which spawns vitest children.
## Acceptance notes
- With both tiers in one run, the `unit` project's in-process
`Config.load({ root })` calls (for example `exit-signal.pin`,
`db/clean.operator-text-21418`) also find the manifest, because every
project's `globalSetup` runs before any test. They load commands through
the same no-manifest path (no `relativePath`), and the full run above
was green there. A unit-only run generates nothing; it only removes a
file whose writer is dead.
- A flag default computed at module load is frozen into the manifest at
generation, and only help text reads it. One exists: `serve --port`'s
default reads `OS_PORT` / `PORT`. `os serve --help` therefore shows the
generation-time value, which is the run's own environment. Parsing still
uses the loaded class. No test runs `--help` with `PORT` set.
- The setup's failure mode is loud by design. If a command module cannot
load, the integration run fails at setup with the generator's stderr
(the `ModuleLoadError` naming the module), instead of every spawn test
failing on "command not found".
---
_Generated by [Claude
Code](https://claude.ai/code/session_012YTKj54wfJ886NzwKz1oba)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent be531f6 commit 9c922b5
4 files changed
Lines changed: 207 additions & 9 deletions
File tree
- packages/cli
- test
- helpers
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
148 | 148 | | |
149 | 149 | | |
150 | 150 | | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
139 | 139 | | |
140 | 140 | | |
141 | 141 | | |
142 | | - | |
143 | | - | |
| 142 | + | |
| 143 | + | |
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
| |||
170 | 170 | | |
171 | 171 | | |
172 | 172 | | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | | - | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
177 | 193 | | |
178 | 194 | | |
179 | 195 | | |
| |||
202 | 218 | | |
203 | 219 | | |
204 | 220 | | |
205 | | - | |
| 221 | + | |
206 | 222 | | |
207 | 223 | | |
208 | 224 | | |
209 | 225 | | |
210 | | - | |
| 226 | + | |
211 | 227 | | |
212 | 228 | | |
213 | 229 | | |
| |||
257 | 273 | | |
258 | 274 | | |
259 | 275 | | |
260 | | - | |
| 276 | + | |
261 | 277 | | |
262 | 278 | | |
263 | 279 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
649 | 649 | | |
650 | 650 | | |
651 | 651 | | |
| 652 | + | |
652 | 653 | | |
653 | 654 | | |
654 | 655 | | |
| |||
739 | 740 | | |
740 | 741 | | |
741 | 742 | | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
| 760 | + | |
| 761 | + | |
742 | 762 | | |
743 | 763 | | |
744 | 764 | | |
| |||
873 | 893 | | |
874 | 894 | | |
875 | 895 | | |
| 896 | + | |
876 | 897 | | |
877 | 898 | | |
878 | 899 | | |
| |||
885 | 906 | | |
886 | 907 | | |
887 | 908 | | |
| 909 | + | |
888 | 910 | | |
889 | 911 | | |
890 | 912 | | |
| |||
0 commit comments