fix(cli): the published entry resolves its commands from dist/, whatever an ambient NODE_ENV says - #17927
Conversation
…ver an ambient NODE_ENV says @oclif/core skips its TypeScript path lookup only when isProd(), so an ambient NODE_ENV=development or test made bin/run.js resolve the CLI's OWN commands from src/ and register tsx on the way. tsx honours the CWD's tsconfig, so an app that redirects a CommonJS package to TypeScript source for TYPE resolution steered the CLI's runtime module graph into .ts files and Node's CJS resolver died on their extensionless siblings. bin/run.js now declares settings.enableAutoTranspile = false. It is the built entry; bin/run-dev.js is the source entry, and that division was already enforced on every test spawn by check:cli-test-child-env -- the entry just never asserted it about itself. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
📓 Docs Drift CheckThis PR changes 1 package(s): 28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin eb4ecd6c3b536f24e1eb2f692ff71585f068b84d && git checkout eb4ecd6c3b536f24e1eb2f692ff71585f068b84d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dbea1756d9ba50800dd6a277fa29dc0394d566ed c6ea030755665b49b134062480c1c724a95306da && git checkout -B drift-repro dbea1756d9ba50800dd6a277fa29dc0394d566ed && git merge --no-ff c6ea030755665b49b134062480c1c724a95306da
node scripts/docs-audit/affected-docs.mjs --json dbea1756d9ba50800dd6a277fa29dc0394d566ed
|
… assumption The docblock and the changeset both named "a linked TypeScript plugin is no longer auto-transpiled" as the cost of refusing auto-transpile. Measured on this entry: @oclif/plugin-plugins sits in devDependencies and oclif's core-plugin loader only matches names under dependencies, so `os plugins` is not a registered command -- `os --help` lists 34 topics and none is `plugins`, the count being the control that makes the zero a reading. content/docs/plugins/index.mdx documents the same thing independently. So the cost is unreachable today, and both places now say that, with the condition under which it would have to be revisited. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c
Contract reviewPR #17927 · card #12271 · head
⭐ What this review re-measured itself, and what it relays on the dev's record, stated up front because a review that does not separate the two is a ratification wearing a review's clothes:
① The correction this review carries first, because it is this seat's own faultPublished at Four independent-looking counts agreed on 5 — the card, the previous dev's report, triage's own count, and this seat's re-count — because all four shared one predicate. ⇒ ⛔ Agreement is not a control. A control is an instrument that would have answered differently. The dev's own census carries exactly that: a positive fixture of 7 calls across every import spelling (got 7) and a negative fixture with a local binding named ② What the diff actually is — separated mechanically, ⛔ not read by eyeEvery added and removed line of the PR classified as comment or code ( The entire production change, printed rather than described: - import { flush, handle, run } from '@oclif/core';
+ import { flush, handle, run, settings } from '@oclif/core';
+ settings.enableAutoTranspile = false;⇒ ③ The mechanism — read in the installed dependency, and it is stronger than the report claims
262: const enableAutoTranspile = settings.enableAutoTranspile ?? settings.tsnodeEnabled;
263: if (enableAutoTranspile === false) {
264: debug(`Skipping typescript path lookup for ${root} because …`);
265: return orig; // ← unconditional early return
266: }
267: const isProduction = isProd(); // ← not even EVALUATED before now
269: if (enableAutoTranspile === undefined && isProduction && plugin?.type !== 'link') {The report says oclif checks the declaration "ahead of ⭐ The same two lines also validate the test's control design. The neutraliser's six lines of code redefine ④ Fence 4 — ⛔ no general
|
Fixes #12271
Clause-②: no
Dispatched by the
domain:cliexecution seat (#6024) under triage's re-grade5650901283(p3 → p2,Task→ Bug, file face unlocked tostart.ts). Branch base5741ff10c, the parked worktree merged forward and fully rebuilt before any reading was taken — freshness proven by content, not mtime: a symbol introduced by merged commiteadcde6d8(KNOWN_UNSUPPORTED_JSON_SCHEMA_PATTERNS) occurs twice inpackages/cli/dist/commands/generate.jsand twice in its source.1. What was actually broken — and it is not what the card's title says
The card is titled around the
os devcompile child inheriting an ambientNODE_ENV=development. That inheritance is real, but it is a symptom. Reading the failure output rather than the card:⭐ The module that failed to load is
packages/cli/src/commands/doctor.ts— one of the CLI's own command modules. The casualty is the command table, not the user's config.@oclif/core@4.13.3'slib/config/ts-path.jsskips its TypeScript path lookup only whenisProd(), defined inlib/util/util.jsas a negated['development', 'test'].includes(process.env.NODE_ENV ?? ''). Under either value it (a) rewrites the command target from the declared./dist/commandstosrc/commandsand (b) callsregisterTsx(). tsx honours the tsconfig of the current working directory, so an application that maps a CommonJS workspace package to its TypeScript source for type resolution steers this CLI's runtime module graph into.tsfiles — and Node's CJS resolver then walks their extensionless siblings and knows nothing about.ts.Three consequences the card did not have:
compile, or to any command.os --versionreproduces it.os serve --dev,os start— so ⛔ no child-environment scrub can fix it. This is the reason triage measured shape B as not fixing the card.NODE_ENV=testis the second value oclif treats as non-production, and it had never been measured on this card. vitest exports it on its own worker.2. The spawn census (acceptance item 3) — measured here, with controls
⛔ Not taken from the card, the previous dev's report, or triage's comment. Read through TypeScript's own parser (
ts.createSourceFile), resolvingchild_processbindings through named / aliased / namespace / default imports,require()destructuring andawait import()destructuring, then matching call expressions against those bindings. Script:census.mjs, run at149135743.Controls, both required to pass before the census prints:
spawn, spawnSync, execFileSync, exec, execFile, execSync, fork)spawn, and the word inside a string literalCensus — 6 child-process spawn sites, not 5:
envhanded to the childdev.ts:349spawnSyncenvproperty — inheritsprocess.envwholedev.ts:586spawnenv: localEnv(a spread ofprocess.env)dev.ts:724execSyncenvproperty — inheritsprocess.envwholedev.ts:821spawnSyncenv: process.envstart.ts:238spawnSyncenv: process.envstart.ts:435spawnenv: localEnvDark-instrument control: the API names occur as bare words 13 times in
dev.tsand 6 times instart.ts; the site counts are 4 and 2, and the gap is imports and prose.dev.ts:724is the disagreement, and it is a real site all four previous numbers missed. The card, the previous dev's report, triage's own count and the seat's re-count all say 5. They agree because they all countedspawn/spawnSync.dev.ts:724is anexecSyncreached throughconst { execSync } = await import('child_process')— a dynamic import inside a function body, invisible to a scan looking for the two static names. It is the workspace-root branch ofos dev, which delegates topnpm [--filter X] dev.⛔ It is deliberately not touched, and that is a classification rather than an omission: it spawns a user's own workspace script, not the
osCLI.NODE_ENV=developmentis meaningful and expected there, and scrubbing it would change the behaviour of arbitrary user dev scripts. It is in the census because the census was asked for; it is outside the defect class because the defect class is this CLI resolving its own commands.3. Which sites were scrubbed, and why that set is empty
None. The fix is one declaration in
packages/cli/bin/run.js:oclif checks
settings.enableAutoTranspile ?? settings.tsnodeEnabledahead ofisProd(), sofalseskips both the source redirect and the tsx registration under every value ofNODE_ENV.The argument for the entry over the six sites, point by point:
os serve --devandos startare top-level processes. There is no parent.bin/run.jsis the BUILT entry (bin.objectstack/bin.os;package.jsondeclares its command table over./dist/commands);bin/run-dev.jsis the SOURCE entry.scripts/check-cli-test-child-env.mjsrule 3 enforces exactly that division on every test that spawns the CLI, with no baseline. The entry had simply never asserted it about itself.process.argv[1], i.e. this same entry — so fixing the entry fixes all of them, plus every future one, with no per-site convention to keep true.NODE_ENVis read by product code (start.ts's production default, the crypto posture, plugin-auth's origin gate). Rewriting it at six spawns to work around a module-resolution bug is the lenient-consumer shape Prime Directive Add comprehensive test suite for Zod schema validation #12 refuses. The entry-point declaration changes only which of two already-declared code paths oclif loads.TSX_TSCONFIG_PATHpin either (whatbin/run-dev.jscarries). That shim genuinely executes TypeScript, so all it can do is aim the transpiler; it cannot even do that in-process and pays a full re-exec. This entry executes no TypeScript, and a published install has nopackages/cli/tsconfig.jsonto aim at —filesnamesdistonly.dev.tsandstart.tscarry comment-only changes, both required by Prime Directive #10 because this change moved what they claim. See §6.4. Two-leg ablation with the
productioncontrol (acceptance items 1 and 2)Run from the committed state.
exit 124= compiled, booted, and still serving when the 60 s timeout killed it;exit 1= died.NODE_ENVis the only variable;OS_SECRET_KEYsupplied so the production arm is not refused on crypto policy. Exit codes captured before any pipe.End to end,
examples/app-crmandexamples/app-showcase(both carry sibling-srcpaths):NODE_ENV=developmentNODE_ENV=production(control)os dev --compile --freshCannot find module './registry'os dev --compile --freshos serve --devos serve --devIdentical at both apps, 8 legs per side.
os compile, all four example apps, published entrybin/run.js:NODE_ENV=developmentNODE_ENV=production(control)development/test/productionexamples/app-todo— the one app whose tsconfig carries nopathsblock — is the only armed pass, so the failures map 1:1 onto the sibling-src population.bin/run-dev.js, invoked correctly (undertsx, which its shebang requires), is 0/0/0/0 under bothdevelopmentandproduction— itsTSX_TSCONFIG_PATHre-exec mitigation works. That is why the card read "latent" for weeks: the probe went through the mitigated entry while users run the unmitigated one. ⛔ Re-measured here rather than carried over, and a note for the next reader: runningrun-dev.jsunder plainnodeanswers 1/1/0/2 in bothNODE_ENVs — that is the shim failing to load its own.tsimport, not this defect, and it is an easy false reading to take.The ablation legs are one-off and left nothing behind. Every mutation was proved to have reached disk before its run (injected marker counted, removed line counted — ⛔ never a bare
git diff --stat), every restore leg isgit checkout HEAD -- packages/cli/bin/run.js(⛔ never a baregit checkout --, which restores from the polluted index), every restore was proved bygit hash-objectagainst the HEAD blob rather than by an exit code, and every mutating script carriedtrap … EXIT INT TERM.5. The regression pin, and why it needs a control
packages/cli/test/published-entry-node-env-source-reroute.test.ts— 5 cases, 5 passing.Every assertion in it is an absence (no reroute, no signature, exit 0), and an absence passes just as well over a fixture that arms nothing. So one leg defeats the declaration inside the child —
test/fixtures/published-entry-auto-transpile-neutraliser.mjs, an--importpreload installing an accessor whose setter swallows the entry's assignment (a plain write loses to it; a non-writable property makes ESM strict mode throw) — and asserts the card reproduces verbatim. A second control leg runs the neutralised child underproductionand is green, pinning thatNODE_ENVis the variable.⛔ Nothing touches
bin/run.json disk: a crashed or timed-out run must not leave the entry point neutralised for the next reader.examples/app-crm, so the suite reads nothing outsidepackages/cli— pointing at the example app would be a cross-package test input, needing a declaration inscripts/cross-package-test-inputs.mjsand a mirroredturbo.jsonentry that widens this package's test cache key over another package's whole source tree. Recorded because it cost a measurement: the first fixture attempted mapped the specifier onto an ESM.tssource insidepackages/cliand stayed green in all six legs — the failure needs Node's CJS resolver walking a.tsfile's siblings, so the trap has to be CommonJS with an extensionless relativerequire. A fixture that arms nothing is the exact vacuity the control exists to refuse.6.
start.ts:419-421(acceptance item 5) — the comment is true again, ⛔ no card neededMeasured at
examples/app-crmon unmodified5741ff10c:NODE_ENV=development objectstack start→ exit 1, 9 hits of the card's signature.NODE_ENV=production→ exit 124. So the sentence advertised a debugging mode the runtime did not deliver (Prime Directive #10).⭐ Which half was wrong matters: the line was always correct — the operator's value does reach the child. What failed was the invocation the sentence names, and it failed before
localEnvwas ever built, in this process. So the fix makes the sentence true rather than needing a behaviour change, and the comment now says so and points at the pin instead of asserting it on its own authority. ⛔ No separate card is filed, because there is no separable behaviour change left to file.dev.ts:328-348's NOTE is corrected for the same reason: it stated the consequence ("os devdies before the server starts") as a property of writingNODE_ENVon the child, and this change makes that consequence unreachable. The rule itself stays — a source that asserts a loader-activating value is a different claim from an entry that refuses to act on one, and it is the halfchild-env-source-loader.pin.test.tscan see.7. Fence: ⛔ no general
paths-resolution gateNone is added. Nothing here parses a tsconfig, and the pin's fixture is a tsconfig the test writes, never one it reads. The #8020 / #8108 class is untouched.
8. The gate interaction this PR could not avoid, stated rather than buried
The new pin spawns
bin/run.jswith adevelopment/testchild — exactly whatcheck:cli-test-child-envrule 3 refuses. ThreeDELIBERATE_REROUTEentries are added, which is the mechanism that gate designs for a site whoseNODE_ENVis its independent variable.src/— that is stillbin/run-dev.js. The gate's own oclif table is untouched and still correct: it was measured againstConfig.load()with default settings, which is what every other built oclif entry still gets. Its self-test census pin moves from five files to six, with the reason beside the new member.9. Docs — hand-read, because the drift tool declared it could not cover this file
The docs-drift advisory reports
packages/cli/bin/run.jsas yielding no anchor, so the pages documenting the published entry's behaviour are outside its run. Hand-read instead: every hand-written page namingNODE_ENV(8, excluding the 6 release-owned pages, which are ⛔ read-only and were not edited), plus every page naming the CLI entry,tsx, or auto-transpilation.⭐ Nothing is falsified. Two pages document the exact invocation that was broken, and this change is what makes them deliverable:
content/docs/protocol/kernel/http-protocol.mdx:206-209— "Anything that boots the runtime withoutos dev— a bareos serve, an embedded host, a hand-written container entry point — must now setNODE_ENV=developmentexplicitly to keep being advertised as such." Following that instruction in apaths-carrying project exited 1 before this change.content/docs/deployment/environment-variables.mdx:33-36— "In dev (os dev, orNODE_ENV=development) a busy port auto-hops…". Same: the documented way to opt into dev behaviour was the thing that broke.The other six (
deployment/cli.mdx,deployment/self-hosting.mdx,permissions/authentication.mdx,plugins/packages.mdx,protocol/kernel/config-resolution.mdx,upgrading.mdx) readNODE_ENVfor auto-reconcile posture, the dev seed gate, sample app code, plugin-dev's production refusal, config-file selection and migration policy — none touches module resolution, and none moves.⭐ One page changed this PR's own claim.
content/docs/plugins/index.mdx:399-408documents thatos plugins …is not a registered command:@oclif/plugin-pluginssits indevDependenciesand oclif's core-plugin loader only matches names underdependencies. Verified here rather than taken on trust —os --helplists 34 topics and zero of them isplugins, the topic count being the control that makes the zero a reading. ⇒ the linked-TypeScript-plugin cost this PR originally stated is unreachable today; the docblock and the changeset were corrected to say so, with the condition under which it would have to be revisited. ⛔ No docs page needs an edit, and none was made.10. Verification
Everything below at HEAD
c6ea03075, exit codes captured before any pipe, heavy runs serialised throughscripts/pm/os-verify-lock.sh(slotdev-12271).pnpm build(full, post-merge rebuild)VERDICT command-exit 0— 73/73 taskspnpm --filter @objectstack/cli build && … typecheckVERDICT command-exit 0vitest run --project unitVERDICT command-exit 0— 204 files / 2935 tests passedvitest run --project integrationVERDICT command-exit 0— 47 files / 409 tests passedpnpm lint(full repo union, ⛔ not narrowed)dispatch-gates --commands --repo objectstack-ai/objectstacknode scripts/pm/dispatch-gates.mjs --ran … --repo objectstack-ai/objectstackreconciles: "81 derived famil(ies) accounted for — 81 run, 0 NOT-MEASURED (a DERIVED zero — all 81 recorded an exit code and none of them is 3)", withpnpm lintrecorded as one run beyond the union. Re-derived aftergit fetch origin mainadvanced it todbea1756d: the family set is identical, 81 before and after, zero added and zero removed.pnpm check:pm-dispatch-gatesfirst returned 124 — that was my 420 s per-command timeout, ⛔ not a gate verdict. Re-run with a longer budget: exit 0, 1678 self-test cases pass. The record carries the 0, and this note carries the first reading.origin/main(dbea1756d), anddispatch-gatesflags one derived-from file as stale across that range:scripts/engine-double-contract.pinned.json. Its diff and mine share zero files — the four commits touchspec,lint,plugin-auth,client,runtime,metadata-protocolandexamples, and nopackages/clipath — so nothing is re-scoped; the merge queue rebuilds onto currentmainregardless.Acceptance notes
dev.ts:724'sexecSynchandspnpm [--filter X] devthe parent environment whole, with noenvproperty. Not a defect — that child is a user workspace script, for whichNODE_ENV=developmentis correct — but it is the sixth spawn site, and the four previous counts of "5" are all explained by it being reached throughawait import('child_process')rather than the static import. Carrier: the census table above; any future card onos dev's workspace-root branch inherits it.check:cli-test-child-envrule 3's specific harm — abin/run.jsspawn silently executingsrc/— is now closed at the source for this repo's entry, so the rule can no longer fire for the reason its header gives. It is not dead: it keeps a built-entrypoint spawn readable about theNODE_ENVit means, and it is the instrument that would catch the declaration being dropped. Recorded in the gate's own header by this PR rather than left for the next reader to discover while deleting it.examples/app-showcase's tsconfig now carries three sibling-srcpathsentries (formula,plugin-email,lint), against the two the card tabulates, andexamples/app-multi-package— a fourth example app the card predates — redirects@objectstack/specitself. Population growth only; Type-axis remediation worklist: 51 packages resolve a workspace dep's declarations through dist, per check:type-source-resolution's landing registry #8249 owns it.examples/app-multi-packagefails with a different surface under the armed environment (The requested module '@objectstack/spec/api' does not provide an export named 'ErrorCode', thencommand compile not found, exit 2) rather than the./registrysignature. Same cause, different first casualty; green after this change like the rest.Authored by the
os-devexecutor in sessionsession_01TSf4DV7ziu4V5j73e46b7c, dispatched by thedomain:cliseat (#6024). (Recorded here as prose: this PR body has been edited, and the edit channel appends its own attribution block regardless of what is sent — measured twice on this PR.)Generated by Claude Code