Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 45 additions & 16 deletions src/cli/exec.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use anyhow::Result;
use std::io::{Read, Write};
use std::process::{Command, Stdio};
use std::sync::{Arc, Mutex};

use crate::hooks::pipe::run_inner;
use crate::hooks::pipe::{run_inner, stream_filter_for};
use crate::pipeline::SessionState;
use crate::store::sqlite::Store;

Expand Down Expand Up @@ -76,22 +77,50 @@ pub fn run_exec(
(c, full_cmd)
};

let child_stdout = child.stdout.take().expect("Failed to open stdout");
let mut child_stdout = child.stdout.take().expect("Failed to open stdout");

let stdout = std::io::stdout().lock();
let stderr = std::io::stderr().lock();

// Pipe the stdout of the child process through OMNI's pipeline
run_inner(
child_stdout,
stdout,
stderr,
store.clone(),
session.clone(),
Some(&cmd_name),
)?;

let status = child.wait()?;
let status = if stream_filter_for(&cmd_name).is_some() {
// Stream-mode command: distilled output is emitted line-by-line as it
// arrives, before the exit code is known, so the exit-code gate below
// cannot apply. Keep the live-streaming behavior.
let stdout = std::io::stdout().lock();
let stderr = std::io::stderr().lock();
run_inner(
child_stdout,
stdout,
stderr,
store.clone(),
session.clone(),
Some(&cmd_name),
)?;
child.wait()?
} else {
// Buffered path: drain stdout first (draining before wait() avoids the
// classic full-pipe deadlock), then gate on the real exit code. #122: a
// command that exited non-zero passes its stdout through verbatim and is
// never distilled — distillation must not turn a failure into output that
// reads as success.
let mut buf = Vec::new();
child_stdout.read_to_end(&mut buf)?;
let status = child.wait()?;
if status.success() {
let stdout = std::io::stdout().lock();
let stderr = std::io::stderr().lock();
run_inner(
std::io::Cursor::new(&buf),
stdout,
stderr,
store.clone(),
session.clone(),
Some(&cmd_name),
)?;
} else {
let mut stdout = std::io::stdout().lock();
stdout.write_all(&buf)?;
stdout.flush()?;
}
status
};

if !status.success() {
if let (Some(sess), Some(st)) = (&session, &store) {
Expand Down
24 changes: 13 additions & 11 deletions src/hooks/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ impl PipelineResult {
}
}

/// The stream-mode TOML filter that governs `cmd`, if any. Stream-mode filters
/// emit distilled output line-by-line as it arrives — before a wrapped command's
/// exit code is known — so callers that gate on exit status (`omni exec`, #122)
/// must treat a stream-mode command as un-gateable and keep it streaming.
/// Semantics match Phase 0.5: the first filter that matches, and only if it is
/// stream-mode.
pub fn stream_filter_for(cmd: &str) -> Option<toml_filter::TomlFilter> {
let filters = toml_filter::load_all_filters();
let f = filters.iter().find(|filter| filter.matches(cmd))?;
f.stream_mode.then(|| f.clone())
}

pub fn run_inner<R: Read, W: Write, E: Write>(
input: R,
mut output: W,
Expand All @@ -75,17 +87,7 @@ pub fn run_inner<R: Read, W: Write, E: Write>(
.map(|c| c.strip_prefix("omni exec ").unwrap_or(c));

// Phase 0.5: Streaming Distillation Check
let mut stream_filter = None;
if let Some(cmd) = command_to_use {
let filters = toml_filter::load_all_filters();
if let Some(f) = filters.iter().find(|filter| filter.matches(cmd))
&& f.stream_mode
{
stream_filter = Some(f.clone());
}
}

if let Some(filter) = stream_filter {
if let Some(filter) = command_to_use.and_then(stream_filter_for) {
return stream_distill(input, output, error, filter, store, session, command_to_use);
}

Expand Down
65 changes: 65 additions & 0 deletions tests/exec_fail_passthrough.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//! #122: `omni exec` must pass a failed command's stdout through verbatim and
//! never distill it — the same invariant #120 enforced on the hook path, applied
//! where the exit code comes from the wrapped child rather than the agent JSON.
//!
//! Unix-only: the reproduction drives a POSIX `sh` loop. On Windows `omni exec`
//! wraps commands in `cmd /C`, which does not speak this syntax; the gate itself
//! is OS-agnostic (`cli::exec`), only the test's shell script is not.
#![cfg(unix)]

use std::process::Command;

fn omni() -> String {
env!("CARGO_BIN_EXE_omni").to_string()
}

/// Run `omni exec <script>` with an isolated DB. The script is passed as a single
/// argument on purpose: `omni exec` re-wraps a shell command in `sh -c`, so
/// `omni exec sh -c '…'` would double-wrap and mangle the quotes.
fn run_exec(script: &str) -> (String, i32) {
let db = tempfile::NamedTempFile::new().expect("temp db");
let out = Command::new(omni())
.arg("exec")
.arg(script)
.env("OMNI_DB_PATH", db.path())
.env("OMNI_QUIET", "1")
.output()
.expect("spawn omni exec");
(
String::from_utf8_lossy(&out.stdout).into_owned(),
out.status.code().unwrap_or(-1),
)
}

// 60 identical noisy lines: `collapse` folds them to one marker on a *successful*
// run, so their survival is a direct signal that distillation was skipped.
const NOISE_LOOP: &str = "i=0; while [ $i -lt 60 ]; do echo noise noise noise; i=$((i+1)); done;";

#[test]
fn failed_command_passes_through_verbatim() {
let (stdout, code) = run_exec(&format!("{NOISE_LOOP} exit 1"));

assert_eq!(code, 1, "the child's non-zero exit code must propagate");
assert_eq!(
stdout.lines().filter(|l| l.contains("noise")).count(),
60,
"a failed command must pass through verbatim, not be collapsed"
);
assert!(
!stdout.contains("collapsed"),
"a failed command must carry no distillation marker"
);
}

#[test]
fn successful_command_is_still_distilled() {
// Guards the guard: the identical output with a zero exit is still distilled,
// so the fix did not simply disable `omni exec` distillation.
let (stdout, code) = run_exec(&format!("{NOISE_LOOP} exit 0"));

assert_eq!(code, 0);
assert!(
stdout.contains("collapsed"),
"a successful command should still be distilled"
);
}