fix(exec): pass a failed command's stdout through verbatim on omni exec, never distill it

[fajarhide]

Closes #122

Problem

#120 enforced "a non-zero exit passes through verbatim, never distilled" on the hook path, where the exit signal is in the agent JSON. omni exec had the same hole from a different cause: cli::exec streamed the child's stdout straight into run_inner (the distiller) and only called child.wait() afterwards — so by the time the exit code was known, the distilled output had already been written. A command that failed under omni exec still got distilled. That's the #120 vault case, one layer down, and omni exec is the repo's own reproduction harness.

Why it couldn't be a one-liner

The child's exit code is only knowable after its stdout is fully drained — and draining before wait() is also what avoids the classic full-pipe deadlock. So exec now:

1. drains the child's stdout into a buffer,
2. wait()s for the real exit code,
3. gates: non-zero → write stdout verbatim, skip distillation; zero → distil exactly as before (the buffer is replayed through run_inner, so the pipeline is unchanged).

Stream-mode commands (docker/npm/bun, via stream_mode TOML filters) emit distilled output line-by-line before the exit code exists, so the gate cannot apply — they keep streaming. The stream-filter lookup Phase 0.5 did inline is factored into pipe::stream_filter_for so exec and the pipeline agree on which commands stream (SSOT).

Verified end-to-end (real binary)

60 identical noise lines:

exit	before	after
1	distilled → [60 similar lines collapsed]	60 lines verbatim, exit 1 propagated
0	distilled → 1 marker	distilled → 1 marker (unchanged)

Teeth: forcing the success branch turns failed_command_passes_through_verbatim red; restoring it green. New tests/exec_fail_passthrough.rs (unix-gated — the repro drives POSIX sh; the gate itself is OS-agnostic).

Gates: cargo fmt / clippy -D warnings / cargo test (1029 passed, 0 failed) on local Homebrew 1.94.0, not CI's pinned 1.97.0 — CI is the source of truth for clippy.

Noted, not fixed here

omni exec sh -c '…' double-wraps (exec re-prepends sh -c, mangling quotes) — a pre-existing exec quoting quirk, also glimpsed in #120's "not reported" notes. Separate issue if it's worth filing.

PR Auto Describe

Summary

Fixes #122: omni exec now gates distillation on the wrapped command's exit code. Failed commands pass stdout verbatim (no distillation); successful commands are distilled as before. Stream-mode filters remain un-gated for live output. Also extracts stream_filter_for into a reusable helper in hooks/pipe.rs.

Key Changes

• exec.rs: Conditional distillation based on stream_filter_for() — buffered path drains stdout first, gates run_inner() on status.success(), else writes raw bytes
• pipe.rs: Extract stream_filter_for(cmd) -> Option<TomlFilter> as public reusable function; simplify run_inner to use it
• New test: exec_fail_passthrough.rs verifies omni exec / pipe.rs distills failed commands: it reads piped stdout and never sees the child exit code #122 invariant (failed=verbatim, success=distilled)

Detailed Breakdown

• Exit-code gating (omni exec / pipe.rs distills failed commands: it reads piped stdout and never sees the child exit code #122): Previously all omni exec output was piped through distillation. Now non-zero exit skips run_inner entirely; stdout is written raw to stdout
• Buffering order: Draining child_stdout before wait() avoids classic full-pipe deadlock; Cursor::new(&buf) feeds buffered bytes to distillation when applicable
• Stream-mode unchanged: Commands matched by a stream-mode filter still emit live output before exit code is known (un-gateable by design)
• Minor refactor: stream_filter_for replaces inline filter lookup in run_inner; matches existing Phase 0.5 semantics

Notes

• Test is Unix-only due to shell script syntax; the core fix is OS-agnostic

Breaking Changes

None.

Last updated: 2026-07-21 01:48:49