Collapse parsed command summaries when any stage is unknown (#13043)

## Summary
- collapse parsed command output to a single `Unknown` whenever the
normal parse includes any unknown entry
- preserve the existing parsing flow and existing `cd` handling,
including the current `cd && ...` collapse behavior
- trim redundant tests and add focused coverage for collapse-on-unknown
cases

## Testing
- `cargo test -p codex-shell-command`
This commit is contained in:
Jeremy Rose
2026-03-03 11:45:34 -08:00
committed by GitHub
parent 39f00f2a06
commit c2d008aca5

View File

@@ -37,7 +37,26 @@ pub fn parse_command(command: &[String]) -> Vec<ParsedCommand> {
} }
deduped.push(cmd); deduped.push(cmd);
} }
deduped if deduped
.iter()
.any(|cmd| matches!(cmd, ParsedCommand::Unknown { .. }))
{
vec![single_unknown_for_command(command)]
} else {
deduped
}
}
fn single_unknown_for_command(command: &[String]) -> ParsedCommand {
if let Some((_, shell_command)) = extract_shell_command(command) {
ParsedCommand::Unknown {
cmd: shell_command.to_string(),
}
} else {
ParsedCommand::Unknown {
cmd: shlex_join(command),
}
}
} }
#[cfg(test)] #[cfg(test)]
@@ -119,7 +138,7 @@ mod tests {
assert_parsed( assert_parsed(
&vec_str(&["bash", "-lc", inner]), &vec_str(&["bash", "-lc", inner]),
vec![ParsedCommand::Unknown { vec![ParsedCommand::Unknown {
cmd: "git status".to_string(), cmd: inner.to_string(),
}], }],
); );
} }
@@ -141,24 +160,9 @@ mod tests {
"rg --version && node -v && pnpm -v && rg --files | wc -l && rg --files | head -n 40"; "rg --version && node -v && pnpm -v && rg --files | wc -l && rg --files | head -n 40";
assert_parsed( assert_parsed(
&vec_str(&["bash", "-lc", inner]), &vec_str(&["bash", "-lc", inner]),
vec![ vec![ParsedCommand::Unknown {
// Expect commands in left-to-right execution order cmd: inner.to_string(),
ParsedCommand::Search { }],
cmd: "rg --version".to_string(),
query: None,
path: None,
},
ParsedCommand::Unknown {
cmd: "node -v".to_string(),
},
ParsedCommand::Unknown {
cmd: "pnpm -v".to_string(),
},
ParsedCommand::ListFiles {
cmd: "rg --files".to_string(),
path: None,
},
],
); );
} }
@@ -218,16 +222,33 @@ mod tests {
let inner = r#"rg -l QkBindingController presentation/src/main/java | xargs perl -pi -e 's/QkBindingController/QkController/g'"#; let inner = r#"rg -l QkBindingController presentation/src/main/java | xargs perl -pi -e 's/QkBindingController/QkController/g'"#;
assert_parsed( assert_parsed(
&vec_str(&["bash", "-lc", inner]), &vec_str(&["bash", "-lc", inner]),
vec![ vec![ParsedCommand::Unknown {
ParsedCommand::Search { cmd: inner.to_string(),
cmd: "rg -l QkBindingController presentation/src/main/java".to_string(), }],
query: Some("QkBindingController".to_string()), );
path: Some("java".to_string()), }
},
ParsedCommand::Unknown { #[test]
cmd: "xargs perl -pi -e s/QkBindingController/QkController/g".to_string(), fn collapses_plain_pipeline_when_any_stage_is_unknown() {
}, let command = shlex_split_safe(
], "rg -l QkBindingController presentation/src/main/java | xargs perl -pi -e 's/QkBindingController/QkController/g'",
);
assert_parsed(
&command,
vec![ParsedCommand::Unknown {
cmd: shlex_join(&command),
}],
);
}
#[test]
fn collapses_pipeline_with_helper_when_later_stage_is_unknown() {
let command = shlex_split_safe("rg --files | nl -ba | foo");
assert_parsed(
&command,
vec![ParsedCommand::Unknown {
cmd: shlex_join(&command),
}],
); );
} }
@@ -395,7 +416,7 @@ mod tests {
assert_parsed( assert_parsed(
&shlex_split_safe("bash -lc 'cd foo && bar'"), &shlex_split_safe("bash -lc 'cd foo && bar'"),
vec![ParsedCommand::Unknown { vec![ParsedCommand::Unknown {
cmd: "bar".to_string(), cmd: "cd foo && bar".to_string(),
}], }],
); );
} }
@@ -922,85 +943,6 @@ mod tests {
); );
} }
#[test]
fn trim_on_semicolon() {
assert_parsed(
&shlex_split_safe("rg foo ; echo done"),
vec![
ParsedCommand::Search {
cmd: "rg foo".to_string(),
query: Some("foo".to_string()),
path: None,
},
ParsedCommand::Unknown {
cmd: "echo done".to_string(),
},
],
);
}
#[test]
fn split_on_or_connector() {
// Ensure we split commands on the logical OR operator as well.
assert_parsed(
&shlex_split_safe("rg foo || echo done"),
vec![
ParsedCommand::Search {
cmd: "rg foo".to_string(),
query: Some("foo".to_string()),
path: None,
},
ParsedCommand::Unknown {
cmd: "echo done".to_string(),
},
],
);
}
#[test]
fn parses_mixed_sequence_with_pipes_semicolons_and_or() {
// Provided long command sequence combining sequencing, pipelines, and ORs.
let inner = "pwd; ls -la; rg --files -g '!target' | wc -l; rg -n '^\\[workspace\\]' -n Cargo.toml || true; rg -n '^\\[package\\]' -n */Cargo.toml || true; cargo --version; rustc --version; cargo clippy --workspace --all-targets --all-features -q";
let args = vec_str(&["bash", "-lc", inner]);
let expected = vec![
ParsedCommand::Unknown {
cmd: "pwd".to_string(),
},
ParsedCommand::ListFiles {
cmd: shlex_join(&shlex_split_safe("ls -la")),
path: None,
},
ParsedCommand::ListFiles {
cmd: shlex_join(&shlex_split_safe("rg --files -g '!target'")),
path: None,
},
ParsedCommand::Search {
cmd: shlex_join(&shlex_split_safe("rg -n '^\\[workspace\\]' -n Cargo.toml")),
query: Some("^\\[workspace\\]".to_string()),
path: Some("Cargo.toml".to_string()),
},
ParsedCommand::Search {
cmd: shlex_join(&shlex_split_safe("rg -n '^\\[package\\]' -n */Cargo.toml")),
query: Some("^\\[package\\]".to_string()),
path: Some("Cargo.toml".to_string()),
},
ParsedCommand::Unknown {
cmd: shlex_join(&shlex_split_safe("cargo --version")),
},
ParsedCommand::Unknown {
cmd: shlex_join(&shlex_split_safe("rustc --version")),
},
ParsedCommand::Unknown {
cmd: shlex_join(&shlex_split_safe(
"cargo clippy --workspace --all-targets --all-features -q",
)),
},
];
assert_parsed(&args, expected);
}
#[test] #[test]
fn strips_true_in_sequence() { fn strips_true_in_sequence() {
// `true` should be dropped from parsed sequences // `true` should be dropped from parsed sequences
@@ -1926,6 +1868,7 @@ fn parse_shell_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
}; };
commands.push(parsed); commands.push(parsed);
} }
if commands.len() > 1 { if commands.len() > 1 {
commands.retain(|pc| !matches!(pc, ParsedCommand::Unknown { cmd } if cmd == "true")); commands.retain(|pc| !matches!(pc, ParsedCommand::Unknown { cmd } if cmd == "true"));
// Apply the same simplifications used for non-bash parsing, e.g., drop leading `cd`. // Apply the same simplifications used for non-bash parsing, e.g., drop leading `cd`.