fix(cli): a closed stderr read end exits 2 instead of dying of an uncaught EPIPE (#14858) - #15558
Merged
os-litant merged 3 commits intoSep 4, 2026
Merged
Conversation
WIP checkpoint: the candidate fix, committed before the ablation so the restore leg has a HEAD that actually holds it. Measured numbers still to be filled into the docblock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…n-dev-closed-stderr-exit-status
Contributor
📓 Docs Drift Check
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): |
This was referenced Sep 4, 2026
os-litant
marked this pull request as ready for review
September 4, 2026 21:33
os-litant
enabled auto-merge
September 4, 2026 21:33
os-litant
deleted the
claude/issue-14858-run-dev-closed-stderr-exit-status
branch
September 4, 2026 22:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14858
bin/run-dev.jsnow attaches a no-operrorlistener toprocess.stderrbeforerun(), so a failed stderr write stops being fatal and a caller whose read end is gone gets the CLI's own exit status (2) instead of a crash (1).The defect, reproduced on the current tree
process.stderris anEventEmitter, and anerrorevent with nothing listening is an uncaught exception. Spawn the shim withstdio: ['ignore', 'ignore', 'pipe']andchild.stderr.destroy(): oclif'sdisplayWarnings()makes the first write, the pipe is already gone, node raiseswrite EPIPE, and the process dies before reaching any of the CLI's own exit paths — includingwriteStderr()'s drain, which for this path had never executed at all.Re-measured on
26144c204(the card's own numbers were taken on a tree that no longer exists, and are not carried over), with the card's own instrument: a--importobserver that appends to a file and installs no listener onprocess.stderrand no write wrapper. It reads the crash withuncaughtExceptionMonitor, which observes without preventing the default action — anuncaughtExceptionhandler would have changed the very thing being read.2 is what oclif's
handle()produces and what #14715 pinned for the never-read reader, so the contracted status already existed; this path simply never reached it. Making it agree restores consistency — no contract surface moves.The four-row ablation, re-measured with the candidate fix in place
One contiguous run on one box,
bin/run-dev.jsablated 2x2: this listener present/absent against thewritecallback kept/removed. Every leg proved its mutation on disk by counting both the removed text and an injected marker, and every restore was proved by blob-hash equality with theHEADblob and an emptygit diff HEADand zero remaining markers, under a trap with absolute paths.writecallbackRow 1 against row 3 is the whole change and it costs ~250 ms: the child now ends the way a drained reader's child ends instead of dying on its first write. The two runs are comparable because their unfixed rows agree (938-1174 ms in the table above, 983-1028 ms here).
nooperror listener is the difference between exit 1 at 1.4 s and the contracted exit 2 at 8.8 s" did not survive re-measurement. On the card's tree the fixed row cost ~7.4 s over the crash; here it costs ~250 ms. What survives is the ratio, not the absolutes: the fixed closed reader now takes about as long as a drained reader does, in both conditions (1237-1312 ms vs 1209-1217 ms here; 8.8 s vs 8.3 s there). The absolute gap was contention, not mechanism.Rows 1-2 together are the second thing the card asked for:
writeStderr()'s drain is now reachable on this path, and row 2 is what it costs when only the no-progress bound can end it. Rows 3-4 never got there at all, which is why the old bound could not fail for the reason it named.Confirmed with the fix in place, all three reader shapes agree (a slower window on the same box — read the shapes, not the absolutes): drained 5235-5309 ms / exit 2, paused 20181-20205 ms / exit 2, destroyed 5094-5518 ms / exit 2.
No rebuild is involved in any of this, and that is checked rather than assumed: the harness runs
tsx packages/cli/bin/run-dev.js— the very file being mutated — and that file's only imports are source (../src/utils/stderr-nonblocking.ts,../src/utils/invocation.ts,../../../scripts/cli-unbuilt-workspace-lead.mjs), so nodist/sits on the shim's resolution path. The cache-freshness control is in-band: rows 1 and 3 are back-to-back runs of the same path that answer different exit codes.Why the listener is not narrowed to EPIPE
Measured with a listener installed on purpose for that one question (⛔ never used for the uncaught-exception reading above, because a probe that installs a listener changes the behaviour under test): the destroyed read end raises EPIPE and nothing else, 4 events per run. So a
code === 'EPIPE'predicate would in fact work today. It is still not what landed: the reason to tolerate is not which error it is. Every event on this stream means one thing — a write to stderr failed — the only channel it could be reported on is the stream that just failed, and there is no other action to take. A predicate would buy no decision and would keep exactly this crash for whatever code turns up next.The pin this flips, and why it is not a regression
packages/cli/test/run-dev-unbuilt-workspace.e2e.test.ts:509pinnedclosedEnd.codeto 1 on purpose — 1 was what the CLI did, never what anyone contracted — precisely so this could not change silently. This PR flips it to 2 and rewrites the case's rationale to say why, carrying the re-measured ablation table into the comment so the next reader can see that both listener-absent rows flip it back to 1. ⛔ It was not a broken test and this is not a regression.Reverse verification, run from the committed state: with the listener ablated out of
bin/run-dev.js(mutation proved on disk, restore proved by blob-hash equality and an emptygit diff HEADunder a trap) the suite comes back 1 failed | 10 passed and the one failure is exactly this case —expected 1 to be 2, the child having run 1038 ms. So the flipped number pins the fix rather than the weather, and it is the only assertion in the file that moves.Scope
Out of scope, and named here only so the boundary is explicit: #14832 is a different defect in the same file — an intermittent hang of the never-read (paused) reader — and it remains open, untouched by this PR. This PR claims only the deterministic, destroyed-read-end case.
The published entry
bin/run.js— measured, reported, not changed herebin/run.jshas no stderrerrorlistener either, so the same hole is open in the published binary by inspection. It did not reproduce, on either probe, on the built tree:bin/run.jsdefinitely-not-a-commanddefinitely-not-a-commandOBJECTSTACK_DEBUG=1+ the unbuilt-spec hookOBJECTSTACK_DEBUG=1+ the unbuilt-spec hookThe same
--importobserver recorded nouncaughtExceptionon any destroyed run there. So the published entry is not reached by this failure under these probes, which is not the same as guarded — the missing listener is real. Reported for triage rather than fixed, because widening this card to the published binary was explicitly out of scope.Verification
pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 test/run-dev-unbuilt-workspace.e2e.test.ts— 11 passed / 11, 53.15 s.pnpm --filter @objectstack/cli typecheck— clean (tsc --noEmitplus the test-layer ledger check). The edited test file really is in that program:tsc -p tsconfig.test.json --listFilesnames it.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandson a tree the tool did not call stale, and run on this PR's headdd831cce4d5: 41 derived commands, 40 exit 0. The artifact-roster families were run too (36 more after de-duplication, 32 exit 0) rather than read as cleared by their silence. Exit codes were captured before any pipe.check:dual-build-cjs-loads(PREREQUISITE NOT MET, wants a fullpnpm build),check:published-readme-exports(13 packages not built),check:react-declaration-parity(MANIFEST is not set … This gate did NOT run), and the two PR-context guardscheck-partof-closing-keyword/check-single-claim-paths(NOT WIRED). The first of those two was then run to a real verdict against this body —PR_BODY="$(cat body.md)" node scripts/check-partof-closing-keyword.mjs— and passes: this PR carries no Part-of/closing-keyword contradiction.No changeset,
skip-changesetinstead. This PR releases nothing:packages/cli'sfilesis["dist", "README.md", "CHANGELOG.md"]and itsbinnames only./bin/run.js, sonpm pack --dry-runpacks exactly one file underbin/—bin/run.js.bin/run-dev.jsis not in the tarball, and the other path is a test.🤖 Generated with Claude Code
https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N