fix(cli): keep the published binary's stderr off the blocking write path - #15496
Conversation
`bin/run.js` now installs `keepStderrNonBlocking()` before oclif can write a byte, and the guard compiles from `src/` into `dist/` so a published install actually carries it. Measured on the built binary: `os dev --verbose` piped to a reader that stops draining parks the main thread in `write(2)` 3.1 s later, 4 of 4 runs, fd 2, `O_NONBLOCK=false`, `wchan=sock_alloc_send_pskb` — parked 28.9 s, ignoring SIGINT, released only by the reader resuming. The clearing that persisted came from a grandchild (the esbuild service, inherited stderr), so the re-assert has to sit on the write path rather than run once at startup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…blished-cli-blocking-stdio
📓 Docs Drift CheckThis PR changes 1 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 22 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 8735c09c23714bae7177a04cdaefaf6031c4260a && git checkout 8735c09c23714bae7177a04cdaefaf6031c4260a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6c9f34f203b89a588b237ee7d6d3ceeec6d2d3ba e75e9119b2aa0a9c4590af43a56842f0f01a8ab9 && git checkout -B drift-repro 6c9f34f203b89a588b237ee7d6d3ceeec6d2d3ba && git merge --no-ff e75e9119b2aa0a9c4590af43a56842f0f01a8ab9
node scripts/docs-audit/affected-docs.mjs --json 6c9f34f203b89a588b237ee7d6d3ceeec6d2d3ba
|
Fixes #14874
The published
osbinary can be parked in the kernel, silently, by anything that pipes its output and stops draining. This lands the repair on the published entry point, and ships the guard that until now existed but was never packed.The reading, on the shipped binary
node bin/run.js dev --verbose --database memory://from a fixture app, stdout and stderr piped to a reader that stops draining,/proc/PID/fdinfo/{1,2}and/proc/PID/syscallsampled every 50 ms from outside the process:Reader stops at 14.0 s. At 17.1 s, 4 of 4 runs:
Parked 28.9 s. SIGINT ignored while parked — alive and still in
writefive seconds later, 3 of 3. Released only when the reader resumed, at which point the deferred SIGINT was processed and the stack exited. Negative control (same binary, the serve spawn changed topipe+ manual forwarding): flags stayO_NONBLOCK=truefor every sample, zero parks over a 30 s stoppage, SIGINT killed the responsive child in 75-79 ms.Not a crash and not a timeout: alive, idle, unresponsive, empty log.
Two readings that decide the shape of the fix:
O_NONBLOCKon the shared description ~100 ms later — which is why fd 1 escapes and fd 2 does not. fd 2 is left blocking for the whole run, at every log level.os devtoos serve --dev(inherited stdio) to the esbuild service (inherited stderr), landing on the description shared all the way up. ⇒ No change to this CLI's own spawn sites could have prevented it, so the re-assert has to sit on the write path.Severity calibration, both directions: at the default log level the same two clearings occur and fd 2 is blocking for the whole run, but no park fired in 90 s — an idle dev server emits ~0.5 KB/s and never fills the 64 KiB pipe, against ~30 KB/s at
--verbose. The hazardous state is unconditional; the park needs a burst of output while the reader is away — a rebuild, a request log, an error dump, or--verbose, which is what CI usually runs.Why this was a packaging bug as much as a wiring one
The repair already existed, was correct, and was pinned — at
packages/cli/bin/stderr-nonblocking.mjs. It shipped to nobody.npm packs a
bintarget regardless offiles, which is whybin/run.jsreached every published install and the module beside it reached none. So "wire up the guard that already exists" was not available as written: the guard had to move somewhere the existing whitelist already admits.What changed
packages/cli/bin/stderr-nonblocking.mjsmoves topackages/cli/src/utils/stderr-nonblocking.ts, sotsccompiles it intodist/and the existingfileswhitelist ships it. ⛔filesis not widened andscripts/check-published-files.mjsneeds no newEXTRA_ENTRIESregistration.bin/run.jsinstalls it beforerun(), through the same lazy../dist/import the file already documents forinvocation.js: a static../dist/import would turn an unbuilt tree's "command not found" into a module-resolution error and break the classification every gate that shells out to this CLI depends on.bin/run-dev.jsimports the same module from../src/throughtsx, so the source shim stays buildless — which its suite's whole subject requires.Nothing about which arguments the CLI accepts, what it prints, or what it exits with changes.
src/utils/format.ts's refusal ofsetBlocking(true)is untouched and stands: this is its inverse, and what keeps its premise true.The pins, and why they have this shape
A pin that cannot run in CI is not a pin, and a pin that only checks the module exists is not one either. The full field reproduction is neither — it needs a fixture app, a dev server, a reader that stops at the right moment, a deliberate output burst and
/proc, and it takes ~30 s a run. So it is the evidence above, and the pins are these:test/published-entry-stderr-nonblocking.e2e.test.ts(new). Runs the realbin/run.jsundernode --import, with stderr as a pipe nobody reads. Inside that process it waits for the guard, then manufactures the identical hazard (spawnSync(node -e 0, { stdio: 'inherit' })clears the same flag on the same shared description in ~30 ms), asserts the flag really was cleared, and writes 2 MiB at the absent reader. Green means the shipped binary installed the shipped guard and the burst returned; ~200 ms.bin/run.jsimports the guard from must resolve to a path thefileswhitelist admits — read from the manifest, not hard-coded — withbin/run.jsitself asserted not admitted as the control, because that asymmetry is what caused the defect.test/run-dev-stderr-nonblocking.e2e.test.ts(existing) keeps its two manufactured arms and stays buildless: the guard is now TypeScript, so the harness type-strips it withesbuild(already a dependency) into a temp module rather than requiring a build, andbin/run-dev.jsreaches the.tssource throughtsxexactly as before.Verification
Built at
e75e9119b2; the gate union below was run at that commit on a clean tree.Ablations, each with the mutation proven on disk (removed text counted to 0, an injected marker counted to 1, blob hash changed) and each restore proven by whole-tree
git status --porcelainplus blob equality against HEAD:bin/run.jssyscall=1(write) fd=0x2 count=0x2000,wchan=sock_alloc_send_pskb,flags 02000002(O_NONBLOCK false), SIGKILL at 23.0 s,WRITES RETURNEDnever writtenbin/run.js did not install keepStderrNonBlocking()GUARD INSTALLED after 0 msand it parked anyway — same syscall, same wchan, SIGKILL at 5.2 sLeg 2 is the one that matters for the design: the guard installed, and a spawn afterwards still cleared the flag out from under it. Per-write is load-bearing, not a preference. Leg 2 was rebuilt on both legs and both directions were checked in the artifact with
node scripts/ablation-dist-preflight.mjs @objectstack/cli MARKER— present on the mutate leg (hitpackages/cli/dist/utils/stderr-nonblocking.js), absent across all 480 built files on the restore leg. Leg 1 has nodist/leg to prove:bin/run.jsis outsidetsconfig.build.json'srootDirand ships verbatim as thebintarget.Suites (
pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2): both stderr files, 10 passed (10), both classified|integration|by the tier predicate with no list to update.Gate union, derived on a clean tree at final HEAD
e75e9119b2withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the five artifact-roster families whose baseline sits under a directory this change touches (silence there is evidence in neither direction, so they were run). 54 of 54 green, each exit code captured before any pipe.The two the packaging route was most likely to move both had something to say and both say it green:
check:published-files—69 publishable package(s) ... declare a files whitelist that covers every entry point ... and admits no test, test-harness config or build script; 1 publish more than dist/ + README.md + CHANGELOG.md, each with a registered reason. The guard reachingdist/needs no registration; keeping it inbin/would have.check:dual-build-cjs-loads— green, and its population is the reason it could not have moved:--listreports 103 require entry points across 66 packages and@objectstack/cliis in none of them (it declares norequirecondition at all).Both of those first came back exit 3,
PREREQUISITE NOT MET(withcheck:i18n-coverage) because they read built output from packages outside this change's closure. Exit 3 is NOT MEASURED, not a pass — so the prerequisite was satisfied with a fullturbo run buildand both were re-run to the green quoted above (check:i18n-coverage:13 config(s), 621 baselined untranslated string(s), none new).pnpm --filter @objectstack/cli typecheckgreen — includingcheck:test-typecheck, which is what putstest/in front of tsc here (tsconfig.jsonincludes onlysrc):3 file(s) / 28 error(s) / 6 pinned signature(s), the ledger unchanged, so the two new test files arrived with zero.Lint is a declared narrowing, not a full-repo run:
eslint --no-inline-config --format jsonover the 7 changed lintable files, 7 accepted into the run, 0 errors, 0 warnings. The count is eslint's own (--format json), and the narrowing excludes nothing because this repo runs oneeslint.config.mjswhich "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file" (its own header, with the positive control it was measured against) — so no untouched file's verdict can move on this diff. CI runs the repo-wide sweep regardless.⛔ Deliberately not touched: the eight inherited-stdio spawn sites (
dev.ts:222 · :471 · :583 · :683,environments/bind.ts:84,init.ts:873,start.ts:241 · :444) — the measurement above says changing them would not have prevented this;src/utils/format.ts;packages/cli/package.json'sfiles;scripts/check-published-files.mjs.Not to be confused with two neighbouring cards, neither of which this PR addresses. #14832: the same mechanism on the unpublished
bin/run-dev.jswith a reader that paused — repaired on that card. #14858: the same file with a reader that went away, symptom an uncaughtwrite EPIPEand exit 1 at ~1.4 s — a different defect.🤖 Generated with Claude Code
https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Generated by Claude Code