fix(tools): keep ix stdout on a non-zero exit, in every tool - #21
Merged
Conversation
#20 fixed this for `ix-docs-tool.ts`. It is the only tool it fixed, and this repo has **no shared runner** -- all 17 tools call bun's `$` directly, and not one of the other 16 used `.nothrow()`. So the repo was still, in the ways that matter, exactly as broken as before that PR. Bun's `$` throws on a non-zero exit and `.text()` discards stdout along with it. Several `ix` commands already exit 1 to mean "you asked for something that does not exist" while printing a complete JSON body, and Ix#547 takes that from three commands to thirteen. Every one of those bodies was being thrown away and rendered as "ix unavailable" -- a wrong answer, and a far less useful one than the record ix supplied. ## The distinction now preserved exit 1 with a body -> ix answered. The body IS the answer. exit 1 with no body -> ix could not answer. Report it. could not run at all -> ix is not installed. Report it. `runtime/cli.ts` is the shared runner that draws it: `runIx` returns `{stdout, stderr, exitCode}` and is null only when the command could not run, `safeRun` is the stdout-or-null shape most callers want, and `failureDetail` turns a failure into a message that prefers ix's own stderr. `safeRun` moves here from `ix-docs-tool.ts` rather than being reimplemented, so there is one definition. `tryLlm` in `runtime/llm.ts` had the same hole and, since #15, is the *primary* path -- a body dropped there fell through to the JSON path, which dropped it a second time. ## Deliberately left alone Five call sites, because for them a throw IS the signal: - `ix --version` in `ix-health.ts` (x2) and `detectVersion` in `runtime/llm.ts` -- capability probes; `--version` exits 0 whenever ix works at all. - `command -v ix` in `ix-ingest.ts` -- presence check. - `ix map` in `ix-ingest.ts` (x2) -- a write. A failed map is a failure, and the existing catch already reports why. ## Tests Six cases, per tool rather than per helper, because a shared runner is only half the fix and a tool that never adopted it is still broken. Same child-process harness #20 introduced, and for the same reason: bun's `$` resolves from the real process PATH, so an in-process stub silently runs the developer's real `ix`. Five of the six fail with the source reverted and the tests kept (mutation-checked, twice). The sixth -- an empty non-zero exit still reporting unavailable -- passes either way by design: it is the guard that this change did not over-correct into treating every failure as an answer. bun test: 125 pass, 3 skip, 0 fail.
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.
#20 fixed this for
ix-docs-tool.ts. It is the only tool it fixed, and this repo has no shared runner — all 17 tools call bun's$directly, and not one of the other 16 used.nothrow():So in the ways that matter the repo was still exactly as broken as before that PR. This closes the other 16.
What was wrong
Bun's
$throws on a non-zero exit and.text()discards stdout along with it. Severalixcommands already exit 1 to mean "you asked for something that does not exist" while printing a complete JSON body, and Ix#547 takes that from three commands to thirteen. Every one of those bodies was thrown away and rendered as "ix unavailable" — a wrong answer, and a far less useful one than the record ix actually supplied.The sharpest case:
ix impact <file>on the file being edited is the highest-traffic ix call in the fleet, and an unresolved target there is routine — a new file, or one not yet ingested.The distinction now preserved
runtime/cli.tsis the shared runner that draws it —runIx(full result, null only if it could not run),safeRun(stdout-or-null),failureDetail(prefers ix's own stderr).safeRunmoves here fromix-docs-tool.tsrather than being reimplemented, so there is one definition.tryLlminruntime/llm.tshad the same hole, and since #15 it is the primary path — a body dropped there fell through to the JSON path, which dropped it a second time.Deliberately left alone
Five call sites, because for them a throw is the signal:
ix --versioninix-health.ts(x2),detectVersioninruntime/llm.ts--versionexits 0 whenever ix works at allcommand -v ixinix-ingest.tsix mapinix-ingest.ts(x2)Tests
Six cases, per tool rather than per helper — a shared runner is only half the fix, and a tool that never adopted it is still broken. Same child-process harness #20 introduced, for the same reason: bun's
$resolves from the real process PATH, so an in-process stub silently runs the developer's realixagainst their real graph.Five of the six fail with the source reverted and the tests kept (mutation-checked twice — once after correcting the harness to disable the llm fast path with the real
IX_DISABLE_LLM_FORMAT, not a variable that does not exist). The sixth — an empty non-zero exit still reporting unavailable — passes either way by design: it is the guard that this change did not over-correct into treating every failure as an answer.bun test: 125 pass, 3 skip, 0 fail.Part of closing the gate on Ix#547. Companion work still needed in
ix-claude-plugin(ix-pre-edit.sh,ix-read.sh).