Uh oh!
There was an error while loading. Please reload this page.
fix(cli): make the two invocation failures that read as a crashed boot say so - #10181
Conversation
…t say so (#10111) `node packages/cli/dist/index.js` ran the re-export barrel to completion, printed nothing and exited 0; `os dev --no-ui` answered with a usage dump whose first sentence scrolls past in a background log. Both present as a server that booted and died, which sends the reader to debug the application instead of the invocation. The barrel now detects that it is the process entry and names `bin/run.js` on stderr with a non-zero exit, and both bin shims put one unmistakable line ahead of oclif's error-plus-usage output when the invocation itself was rejected. The entry predicate is deliberately NOT the repo's prevailing `invokedDirectly` spelling: #10086 measured that every one of those goes silently inert through a symlink, which is the same defect being fixed here. No flag surface changes: `os dev --no-ui` is still rejected, only legibly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
The e2e legs spawn real child processes, because `process.exitCode` inside a vitest worker is not an exit status, and they assert the FIRST line of stderr — "somewhere in the output" is the property the usage dump already had. The unit legs pin the entry predicate through a symlink and through `node <dir>`, which is where every `invokedDirectly` spelling measured in #10086 answers false and goes silently inert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
📓 Docs Drift CheckThis PR changes 1 package(s): 19 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 23 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 adf1f2296830f90acc2f14196a340658cea87eee && git checkout adf1f2296830f90acc2f14196a340658cea87eee
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 19f98fa1fffbeb305bdcb6af64bc826fb25a46b1 e72a2d2665e8235eb8d402d291aca4773412486e && git checkout -B drift-repro 19f98fa1fffbeb305bdcb6af64bc826fb25a46b1 && git merge --no-ff e72a2d2665e8235eb8d402d291aca4773412486e
node scripts/docs-audit/affected-docs.mjs --json 19f98fa1fffbeb305bdcb6af64bc826fb25a46b1
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#10111
Two ways to invoke this CLI wrong both presented as a crashed boot, and the measured cost was a runner spending a boot cycle debugging the application instead of the invocation.
Re-measured on current
mainbefore editingThe card is stamped at
4a7b3604c;mainis at2d3860df9. Both premises still hold there — measured by runningorigin/main's ownsrc/index.tsandbin/run-dev.jsthroughtsx, in a worktree with the dependency closure built:After
The loud line is written before
handle()runs, so it is first in the stream. That also makes it the part most likely to survive a truncated log:handle()ends inprocess.exit, and writes to a piped stderr are asynchronous.Three files carry it
src/utils/invocation.ts— new; the two judgments, importing nothing butnode:builtins because the bin shims reach it from the failure path.src/index.ts— the barrel now detects that it is the process entry, namesbin/run.json stderr and setsprocess.exitCode = 1.process.exitCoderather thanprocess.exit(): nothing runs after it, and the write must be allowed to drain.bin/run.js/bin/run-dev.js—execute()from@oclif/core4.13.3 inlined verbatim apart from one added call, becauseexecutehands the error straight tohandle()and there is no seam between them. The helper is imported lazily, inside the catch: a static import of../dist/would make an unbuilt tree fail withCannot find module …/dist/utils/invocation.jsinstead of oclif's "command not found", which is the signaturescripts/cli-build-prerequisite.mjsclassifies for every gate that shells out to this CLI.Not the repo's prevailing entry-guard idiom, on purpose
#10086 measured the
invokedDirectlyguard in ~8 spellings acrossscripts/, all of them a variant ofresolve(argv[1]) === fileURLToPath(import.meta.url), and every one goes silently inert — exit 0, no output — when reached through a symlink. That is the same defect this PR removes, soisProcessEntrycomparesrealpathSyncon both sides (the shape #10086 recommends and #10084 pinned) and also covers the directory form, wherenode DIRleavesargv[1]naming the directory rather than the index it resolved to. Both legs are measured:The naive spelling answers false for both, which is the silent no-op. Nothing in this PR repairs
scripts/— #10086 remains open for that.Flag surface: unchanged
os dev --no-uiis still rejected.servedeclaresuiwithallowNo: trueanddevdoes not; reconciling those two is a flag-surface decision and stays out of this card by its own fence. What changed is only what the CLI says when it rejects an invocation, and the success path is pinned alongside (--versionstill exits 0, version on stdout, nothing added to stderr).Verification
Gates run locally at
e72a2d266, working tree clean (the union re-derived from the real diff withnode scripts/pm/dispatch-gates.mjs, no paths passed):pnpm check:slot-lookup✓ slot-lookup ratchet holds: 107 unswept site(s) in 25 file(s), none newnode scripts/docs-audit/check-affected-docs.mjspnpm check:changeset-gate-self-tests✓ check-empty-changeset --self-test: 118 assertions(+ siblings)pnpm check:objectui-changeset✓ objectui-range --self-test: all checks passednode scripts/check-adr-0087-registration.mjs✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)node scripts/check-changeset-no-major.mjs✓ This diff introduces no major bump.node scripts/check-empty-changeset.mjs✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)pnpm check:engine-double-contractcheck-engine-double-contract: OK — 329 pinnedpnpm check:where-matcher✓ where-matcher conformance holds: 264 matcher(s) … none newpnpm check:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) … none newpnpm check:type-check-coveragecheck-type-check-coverage: OK — 64/77 workspace packages type-checkedpnpm check:type-check-debt--re-measure: OK — 33 ledger entr(ies) re-measured in 363.6s, 1924 raw tsc error(s) total, none above its recorded number— run against the FULL workspace closure built first (turbo run build: 70 tasks successful). This one mattered:@objectstack/cli's TEST_DEBT entry is 146 recorded exactly, no bootstrap margin, and the newtest/invocation-loudness.e2e.test.tslands in exactly thetest/tree that entry covers. It adds zero errors — no relative imports (so no TS2835) and no implicitly-typed parameters.pnpm check:nul-bytesOK (scanned 6070 text file(s) … no raw ASCII control bytes)pnpm check:cross-package-test-inputsOK: 12 package(s) read outside themselves, all declaredpnpm check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scannedpnpm --filter @objectstack/cli typechecktsc --noEmit— exit 0pnpm --filter @objectstack/cli testTest Files 137 passed (137) · Tests 1498 passed (1498)— exit 0; 137/1498 against the 135/1470 this package’s vitest config records, i.e. the two new files and their 28 testsDeclared deviation —
pnpm lintwas narrowed. The repo-wide lint is a shared-container cost this branch could not justify holding the verify lock for; ESLint ran over the six changed files instead, with--format jsonto prove coverage rather than trusting a zero-match exit 0: 6 files linted, 0 errors, 0 warnings, 0 suppressed. The full farm runs in CI regardless.Declared deviation — one file outside the dispatched surface. The claim named
bin/run.js,bin/run-dev.js,src/index.ts,test/**and.changeset/**.src/utils/invocation.tsis new: the alternative was spelling the same predicate twice, in two files, which is exactly the duplication #10086 measured going wrong eight different ways. It is also the only placement that ships —filesnever namesbin/, and npm packs only thebintarget itself, so a helper next to the shims would be absent from the published package.Out-of-scope finding filed, not fixed here:#10185 —
packages/cli/tsup.config.tsdescribes a build nothing runs, andclean: truemeans running it would deletedist/commands/**.Generated by Claude Code