🐛 fix: decide command failure by the exit code alone - #310
Merged
Conversation
A code block raised a failure only when the command exited non-zero *and* wrote nothing to stdout, so anything that printed before failing was reported as a success: no diagnostic, and an `execOutput` segment carrying a non-zero exit code that nothing downstream reads. Under the throwing policy documentation regions use, fail-fast was defeated the same way — there was no failure to throw, so a failed preview reached the step after it. Output and outcome are now two separate decisions. What the command printed is kept as its `execOutput` segment, and a non-zero exit raises the diagnostic that follows it, whatever it printed. `silent` violated the same invariant directly: it discarded the inner result and returned exit code 0, so a silenced failure was invisible to any exit-code check, before and after this fix. It now hands back the inner chain's outcome with only the output removed — it suppresses what a command printed, not whether it worked. Replay reproduces the failure, because the exit code comes from the journaled exec result rather than a hardcoded zero. The capture contract is unchanged and now pinned against the new segment shape: `<Capture as>`, component `as=`, and `<Each as>` leave the binding unset and report the failure once, so printed output never masquerades as a captured value. #309 owns the broader fail-fast `<Output>` and partial-output contract. Closes #307
PR #310: 🐛 fix: decide command failure by the exit code alone6 files, +319 / -26 Scope✅ PR scope looks good. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static Analysis✅ Oxlint found no issues. CorrectnessNo extraneous code patterns detected. |
Repository rule 6 forbids `as` casts. The documentation fail-fast test now narrows with `instanceof` and throws a named error when the value is something else, which is also a better failure message than a cast that would have silently accessed `.message` on whatever came out.
This was referenced Aug 3, 2026
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.
Closes #307. Cross-references #309.
What was wrong
expand.tsraised a failure for a code block only when the command exited non-zero and wrote nothing to stdout:A command that printed anything before failing took the
else ifbranch — anexecOutputsegment carrying a non-zeroexitCodethat nothing downstream reads (render.tsrenders onlyresult.stdout). No diagnostic, and under the throwing policy documentation regions use, fail-fast was defeated because there was no failure to throw. A failed preview reached the step after it.silentFactoryviolated the same invariant directly: it discarded the inner result and returned{ output: "", exitCode: 0, stderr: "" }, so asilent execfailure was invisible to any exit-code check, before and after the fix above.What changed
packages/core/src/expand.ts— output and outcome are now two independent decisions. What the command printed is kept as itsexecOutputsegment; a non-zero exit raises the diagnostic that follows it. Order is part of the contract: the output precedes the diagnostic, because a command that prints before it fails is usually explaining itself.Preserved by construction: non-zero + empty stdout → the same single
ErrorSegmentwith the same message; exit 0 + stdout → the same singleexecOutput; exit 0 + empty stdout → nothing.packages/core/src/execute.ts—silentretains the inner chain's result and replaces only the output. Exit code and stderr are the inner chain's, so a silenced failure is still a failure, and replay reproduces it from the journaled exec result rather than a hardcoded zero.The capture contract is unchanged, and now pinned against the new segment shape:
<Capture as>, componentas=, and<Each as>leave the binding unset and report the failure once — printed output never masquerades as a captured value. #309 owns the broader fail-fast<Output>and partial-output contract; this PR does not redesign failed-capture output visibility.Spec —
silentprose and its inline listing, Tier D (D3sharpened,D3b/D6b/D7badded,D6/D10corrected), Tier E (E8), Tier C (C24b), Tier Each (EA7b).Tests
Ten new tests, each confirmed failing against the unfixed code first:
partial\n<!-- ERROR: Command failed (exit 1): boom -->, plus a segment-level assertion that the order is["execOutput", "error"];DocumentationError, the later sibling never ran, and nothing came back as a successful result;silent: failing-with-stdout suppresses the stdout and still reports the failure; in documentation it aborts before the next block (one exec in the journal); replay preserves the failure;<Capture as>, componentas=,<Each as>over a failed-after-printing block;echo partial; exit 1so the exit code travels throughexecFactory, not only a stub.Verification
Runtimes selected and asserted first (Deno 2.9.1, Node 22, Bun 1.3.14),
deno task setuprun once, everything below against that one prepared tree:deno task fmt/lintdeno task checkdeno task testdeno task check:jsrtsc --project tsconfig.node.jsonpnpm test:nodebun run test:bundeno task build+./dist/xmd test packages/core/src --rawgit diff --summary/--check