Skip to content

fix(cli): a closed stderr read end exits 2 instead of dying of an uncaught EPIPE (#14858) - #15558

Merged
os-litant merged 3 commits into
mainfrom
claude/issue-14858-run-dev-closed-stderr-exit-status
Sep 4, 2026
Merged

fix(cli): a closed stderr read end exits 2 instead of dying of an uncaught EPIPE (#14858)#15558
os-litant merged 3 commits into
mainfrom
claude/issue-14858-run-dev-closed-stderr-exit-status

Conversation

@os-litant

@os-litant os-litant commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14858

bin/run-dev.js now attaches a no-op error listener to process.stderr before run(), 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.stderr is an EventEmitter, and an error event with nothing listening is an uncaught exception. Spawn the shim with stdio: ['ignore', 'ignore', 'pipe'] and child.stderr.destroy(): oclif's displayWarnings() makes the first write, the pipe is already gone, node raises write EPIPE, and the process dies before reaching any of the CLI's own exit paths — including writeStderr()'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 --import observer that appends to a file and installs no listener on process.stderr and no write wrapper. It reads the crash with uncaughtExceptionMonitor, which observes without preventing the default action — an uncaughtException handler would have changed the very thing being read.

parent's reader exit elapsed bytes delivered
drained 2 1209-1217 ms 147699
paused, never read 2 16178-16471 ms 0
read end destroyed 1 938-1174 ms, 12/12 0
22407  uncaughtException  code=EPIPE  msg=write EPIPE
22407        stack=Error: write EPIPE | at afterWriteDispatched (node:internal/stream_base_commons:159:15)
             | at writeGeneric (node:internal/stream_base_commons:150:3) | at Socket._writeGeneric (node:net:966:11)
22407  exit  code=1  elapsed=823ms

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.js ablated 2x2: this listener present/absent against the write callback 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 the HEAD blob and an empty git diff HEAD and zero remaining markers, under a trap with absolute paths.

this listener write callback exit elapsed
present kept (as shipped) 2 1237-1312 ms
present removed 2 16271-16294 ms — the 15 s no-progress bound
absent kept 1 983-1028 ms — the defect
absent removed 1 843-1031 ms

Row 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).

⚠️ The card's reading that "one noop error 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 no dist/ 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:509 pinned closedEnd.code to 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 empty git diff HEAD under 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.

⚠️ That file is also #14822's queue-flake anchor. A red run of this suite is therefore not automatically this PR's regression — tell them apart by the failing arm's identity and a re-run, never by assumption.

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 here

bin/run.js has no stderr error listener either, so the same hole is open in the published binary by inspection. It did not reproduce, on either probe, on the built tree:

probe on bin/run.js reader exit elapsed stderr bytes when drained
definitely-not-a-command drained 2 ~2.3 s 57
definitely-not-a-command destroyed 2, 3/3 ~2.3 s
OBJECTSTACK_DEBUG=1 + the unbuilt-spec hook drained 2 654 ms 35523
OBJECTSTACK_DEBUG=1 + the unbuilt-spec hook destroyed 2, 3/3 657-671 ms

The same --import observer recorded no uncaughtException on 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.ts11 passed / 11, 53.15 s.
  • pnpm --filter @objectstack/cli typecheck — clean (tsc --noEmit plus the test-layer ledger check). The edited test file really is in that program: tsc -p tsconfig.test.json --listFiles names it.
  • Gate union re-derived from the real changed paths with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands on a tree the tool did not call stale, and run on this PR's head dd831cce4d5: 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.
  • Nothing came back red. Five commands report themselves as NOT MEASURED and say so in their own words — check:dual-build-cjs-loads (PREREQUISITE NOT MET, wants a full pnpm 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 guards check-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-changeset instead. This PR releases nothing: packages/cli's files is ["dist", "README.md", "CHANGELOG.md"] and its bin names only ./bin/run.js, so npm pack --dry-run packs exactly one file under bin/bin/run.js. bin/run-dev.js is not in the tarball, and the other path is a test.


🤖 Generated with Claude Code

https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N

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
@github-actions github-actions Bot added the size/m label Sep 4, 2026
@os-litant os-litant added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/m labels Sep 4, 2026 — with Claude
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/cli/bin/run-dev.js), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/cli/bin/run-dev.js) — pages documenting those are invisible to this run
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cf6b67164e37b92cf1fb6b172aa8f44e1fdce666packageMentionDocs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os dev dies of an uncaught write EPIPE (exit 1) when its stderr read end is CLOSED — every other reader gets exit 2, and the drain is never reached

2 participants