Skip to content

Commit 89c4b2e

Browse files
committed
mason: force generic compression for separator-delimited command lists
split_top_level_pipe recorded top-level separators (;, &&, ||, bare &, newline) in saw_top_separator but the no-pipe exit ignored the flag and returned PipeSplit::None. Dispatch then classified the whole list by its head token, so e.g. 'cargo test ; printf SENTINEL' compressed through cargo.rs which dropped the sentinel (any later command's output). Fix: in the no-pipe exit, when saw_top_separator is true, return PipeSplit::Unsafe (mirroring the pipe path's Unsafe semantics) so resolve_dispatch_target sends the list to the generic fallback compressor, preserving all commands' output. The cd-prefix peel (strip_cd_prefix) runs before split_top_level_pipe, so 'cd /x && cargo test' still dispatches to the cargo compressor with no residual separator. Redirects (2>&1, &>) remain carved out as non-separators. Tests: separator_list_forces_generic_and_preserves_sentinel (repro), cd_prefix_peel_leaves_no_residual_separator_for_cargo, clean_single_cargo_test_dispatch_byte_identical (control), plus split_top_level_pipe_variants extended with separator and redirect assertions. Mutation control confirmed: reverting the no-pipe branch makes the repro test fail. Updated unknown_exit_code_keeps_byte_identical_legacy_compressor_output to use a clean single command (its old 'mypy src && node fail.js' input now correctly routes to generic due to the separator).
1 parent 7d83664 commit 89c4b2e

1 file changed

Lines changed: 103 additions & 15 deletions

File tree

crates/aft/src/compress/mod.rs

Lines changed: 103 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -960,14 +960,17 @@ fn is_redirect_token(token: &str) -> bool {
960960
/// Outcome of scanning a command for a top-level pipeline.
961961
#[derive(Debug, PartialEq, Eq)]
962962
enum PipeSplit {
963-
/// No top-level `|` — dispatch on the command as-is.
963+
/// No top-level `|` and no top-level separator — dispatch on the
964+
/// command as-is.
964965
None,
965966
/// A top-level pipeline; the captured stdout is this last stage's output.
966967
LastStage(String),
967-
/// A pipe-like operator is present but the command couldn't be safely
968-
/// parsed (unbalanced quotes/parens/backtick). Callers must NOT fall back
969-
/// to head-token dispatch — a compressor that claims `cargo test | …`
970-
/// would drop the piped output. Force generic instead.
968+
/// The command cannot be safely dispatched to a head-token compressor.
969+
/// Either a pipe coexists with other top-level structure (so the
970+
/// captured output isn't just the last stage's), or a top-level
971+
/// separator (`;`, `&&`, `||`, bare `&`, newline) means multiple
972+
/// commands' output is interleaved — a head-token compressor would
973+
/// delete the other commands' output. Force generic instead.
971974
Unsafe,
972975
}
973976

@@ -982,6 +985,10 @@ enum PipeSplit {
982985
/// coexists with ANY of {a top-level separator `;`/`&&`/`||`/bare `&`/newline,
983986
/// an unbalanced quote/paren/backtick/escape, an unmatched `)`, or an empty
984987
/// trailing stage}, we return `Unsafe` so the caller forces generic compression.
988+
/// The same `Unsafe` result applies when there is NO pipe but a top-level
989+
/// separator IS present: the captured transcript interleaves multiple commands'
990+
/// output, so per-head specialized compression would delete the other commands'
991+
/// output.
985992
/// Top-level comments must already be removed by `strip_top_level_comment`.
986993
/// Redirects (`>`, `2>&1`, `&>`, …) are NOT separators.
987994
fn split_top_level_pipe(command: &str) -> PipeSplit {
@@ -1124,6 +1131,12 @@ fn split_top_level_pipe(command: &str) -> PipeSplit {
11241131
} else if imbalance && command.contains('|') {
11251132
// No resolvable top-level pipe, but a `|` hides in an unbalanced region.
11261133
PipeSplit::Unsafe
1134+
} else if saw_top_separator {
1135+
// A top-level separator (`;`, `&&`, `||`, bare `&`, newline) without a
1136+
// pipe means multiple commands' output is interleaved in the captured
1137+
// transcript. Per-head specialized compression would delete the other
1138+
// commands' output — same rationale as the pipe Unsafe rule.
1139+
PipeSplit::Unsafe
11271140
} else {
11281141
PipeSplit::None
11291142
}
@@ -1863,15 +1876,11 @@ replacement = "wget: ok"
18631876

18641877
#[test]
18651878
fn unknown_exit_code_keeps_byte_identical_legacy_compressor_output() {
1866-
let output =
1867-
"Success: no issues found in 1 source file\nError: later chained command failed\n";
1879+
// Use a clean single command (no separator) so this tests the
1880+
// unknown-exit-code path, not the separator-forces-generic path.
1881+
let output = "Success: no issues found in 1 source file\n";
18681882

1869-
let legacy = compress_with_registry_exit_code(
1870-
"mypy src && node fail.js",
1871-
output,
1872-
None,
1873-
&empty_registry(),
1874-
);
1883+
let legacy = compress_with_registry_exit_code("mypy src", output, None, &empty_registry());
18751884
assert_eq!(legacy.text, "mypy: clean");
18761885
}
18771886

@@ -2328,8 +2337,9 @@ mod normalize_command_tests {
23282337
split_top_level_pipe("git log | grep fix"),
23292338
PipeSplit::LastStage("grep fix".to_string())
23302339
);
2331-
// `||` is logical-or, not a pipe.
2332-
assert_eq!(split_top_level_pipe("a || b"), PipeSplit::None);
2340+
// `||` is logical-or, not a pipe — but it IS a top-level separator,
2341+
// so the no-pipe exit forces generic (multiple commands' output).
2342+
assert_eq!(split_top_level_pipe("a || b"), PipeSplit::Unsafe);
23332343
// inner pipe inside a subshell is not a top-level boundary.
23342344
assert_eq!(split_top_level_pipe("(a | b)"), PipeSplit::None);
23352345
// inner pipe inside $() is not a top-level boundary.
@@ -2364,6 +2374,28 @@ mod normalize_command_tests {
23642374
split_top_level_pipe("cargo test 2>&1 | grep FAIL"),
23652375
PipeSplit::LastStage("grep FAIL".to_string())
23662376
);
2377+
// No-pipe separator cases: a top-level separator without a pipe still
2378+
// forces generic so a head-token compressor can't drop later commands'
2379+
// output.
2380+
assert_eq!(
2381+
split_top_level_pipe("cargo test ; printf SENTINEL"),
2382+
PipeSplit::Unsafe
2383+
);
2384+
assert_eq!(
2385+
split_top_level_pipe("cargo test && echo done"),
2386+
PipeSplit::Unsafe
2387+
);
2388+
assert_eq!(
2389+
split_top_level_pipe("cargo test\necho done"),
2390+
PipeSplit::Unsafe
2391+
);
2392+
// Redirects are NOT separators — a single command with a redirect must
2393+
// still dispatch to its head-token compressor.
2394+
assert_eq!(split_top_level_pipe("cargo test 2>&1"), PipeSplit::None);
2395+
assert_eq!(
2396+
split_top_level_pipe("cargo test &> /dev/null"),
2397+
PipeSplit::None
2398+
);
23672399
}
23682400

23692401
#[test]
@@ -2427,6 +2459,62 @@ mod normalize_command_tests {
24272459
);
24282460
}
24292461

2462+
/// MUTATION CONTROL: reverting the no-pipe exit in `split_top_level_pipe`
2463+
/// to ignore `saw_top_separator` (returning `PipeSplit::None` unconditionally)
2464+
/// makes this test fail — cargo.rs claims the list and drops the sentinel.
2465+
#[test]
2466+
fn separator_list_forces_generic_and_preserves_sentinel() {
2467+
// `cargo test ; printf SENTINEL` — the captured transcript contains both
2468+
// cargo noise and the sentinel. A head-token cargo compressor would keep
2469+
// only cargo-shaped lines and silently delete the sentinel. The no-pipe
2470+
// separator must force generic compression so all output survives.
2471+
let cargo_noise =
2472+
"running 1 test\ntest ok_test ... ok\n\ntest result: ok. 1 passed; 0 failed\n";
2473+
let transcript = format!("{cargo_noise}SENTINEL\n");
2474+
let compressed = compress_with_registry(
2475+
"cargo test ; printf SENTINEL",
2476+
&transcript,
2477+
&empty_registry(),
2478+
);
2479+
assert!(
2480+
compressed.text.contains("SENTINEL"),
2481+
"separator-list output must survive generic compression, got: {}",
2482+
compressed.text
2483+
);
2484+
}
2485+
2486+
#[test]
2487+
fn cd_prefix_peel_leaves_no_residual_separator_for_cargo() {
2488+
// `cd /x && cargo test` — the `cd /x &&` prefix is peeled, leaving a
2489+
// clean `cargo test` with NO residual separator. Specialized cargo
2490+
// compression must still engage (not force-generic).
2491+
let output = "running 1 test\ntest ok_test ... ok\n\ntest result: ok. 1 passed; 0 failed\n";
2492+
let compressed = compress_with_registry("cd /x && cargo test", output, &empty_registry());
2493+
// Cargo compressor drops individual "test ... ok" lines in its summary.
2494+
assert!(
2495+
!compressed.text.contains("test ok_test ... ok"),
2496+
"cd-peeled cargo test must still use the cargo compressor, got: {}",
2497+
compressed.text
2498+
);
2499+
assert!(compressed.text.contains("test result: ok"));
2500+
}
2501+
2502+
#[test]
2503+
fn clean_single_cargo_test_dispatch_byte_identical() {
2504+
// Control: a clean single `cargo test` (no separator, no pipe) must
2505+
// continue dispatching to the specialized cargo compressor and produce
2506+
// the same output as the original specialized-compression behavior.
2507+
let output = "running 1 test\ntest ok_test ... ok\n\ntest result: ok. 1 passed; 0 failed\n";
2508+
let compressed = compress_with_registry("cargo test", output, &empty_registry());
2509+
assert!(compressed.text.contains("running 1 test"));
2510+
assert!(compressed.text.contains("test result: ok"));
2511+
assert!(
2512+
!compressed.text.contains("test ok_test ... ok"),
2513+
"clean cargo test must keep using the cargo compressor, got: {}",
2514+
compressed.text
2515+
);
2516+
}
2517+
24302518
#[test]
24312519
fn is_shell_boundary_covers_redirects_and_operators() {
24322520
for tok in [

0 commit comments

Comments
 (0)