diff --git a/codex-rs/core/src/bash.rs b/codex-rs/core/src/bash.rs index 5b94daf252..9abd0b0a24 100644 --- a/codex-rs/core/src/bash.rs +++ b/codex-rs/core/src/bash.rs @@ -1,3 +1,4 @@ +use tree_sitter::Node; use tree_sitter::Parser; use tree_sitter::Tree; use tree_sitter_bash::LANGUAGE as BASH; @@ -59,9 +60,6 @@ pub fn try_parse_word_only_commands_sequence(tree: &Tree, src: &str) -> Option Option Option Option> { +/// Extract the plain words of a simple command node, normalizing quoted +/// strings into their contents. Returns None if the node contains unsupported +/// constructs for a word-only command. +pub(crate) fn extract_words_from_command_node(cmd: Node, src: &str) -> Option> { if cmd.kind() != "command" { return None; } @@ -130,6 +131,48 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option Option> { + let root = tree.root_node(); + let mut cursor = root.walk(); + let mut stack = vec![root]; + let mut best: Option = None; + while let Some(node) = stack.pop() { + if node.is_named() + && node.kind() == "command" + && best.is_none_or(|b| node.start_byte() < b.start_byte()) + { + best = Some(node); + } + for child in node.children(&mut cursor) { + stack.push(child); + } + } + best +} + +/// Given the first command node, return the byte index in `src` at which the +/// remainder script starts, only if the next non-whitespace token is an allowed +/// sequencing operator for dropping the leading `cd`. +/// +/// Allowed operators: `&&` (conditional on success) and `;` (unconditional). +/// Disallowed: `||`, `|` — removing `cd` would change semantics. +pub(crate) fn remainder_start_after_wrapper_operator(first_cmd: Node, src: &str) -> Option { + let mut sib = first_cmd.next_sibling()?; + while !sib.is_named() && sib.kind().trim().is_empty() { + sib = sib.next_sibling()?; + } + if sib.is_named() || (sib.kind() != "&&" && sib.kind() != ";") { + return None; + } + let mut idx = sib.end_byte(); + let bytes = src.as_bytes(); + while idx < bytes.len() && bytes[idx].is_ascii_whitespace() { + idx += 1; + } + if idx >= bytes.len() { None } else { Some(idx) } +} + #[cfg(test)] mod tests { use super::*; @@ -215,4 +258,45 @@ mod tests { fn rejects_trailing_operator_parse_error() { assert!(parse_seq("ls &&").is_none()); } + + #[test] + fn find_first_command_node_finds_cd() { + let src = "cd foo && ls; git status"; + let tree = try_parse_bash(src).unwrap(); + let first = find_first_command_node(&tree).unwrap(); + let words = extract_words_from_command_node(first, src).unwrap(); + assert_eq!(words, vec!["cd".to_string(), "foo".to_string()]); + } + + #[test] + fn remainder_after_wrapper_operator_allows_and_and_semicolon() { + // Allows && + let src = "cd foo && ls; git status"; + let tree = try_parse_bash(src).unwrap(); + let first = find_first_command_node(&tree).unwrap(); + let idx = remainder_start_after_wrapper_operator(first, src).unwrap(); + assert_eq!(&src[idx..], "ls; git status"); + + // Allows ; + let src2 = "cd foo; ls"; + let tree2 = try_parse_bash(src2).unwrap(); + let first2 = find_first_command_node(&tree2).unwrap(); + let idx2 = remainder_start_after_wrapper_operator(first2, src2).unwrap(); + assert_eq!(&src2[idx2..], "ls"); + } + + #[test] + fn remainder_after_wrapper_operator_rejects_or_and_pipe() { + // Rejects || + let src = "cd foo || echo hi"; + let tree = try_parse_bash(src).unwrap(); + let first = find_first_command_node(&tree).unwrap(); + assert!(remainder_start_after_wrapper_operator(first, src).is_none()); + + // Rejects | + let src2 = "cd foo | rg bar"; + let tree2 = try_parse_bash(src2).unwrap(); + let first2 = find_first_command_node(&tree2).unwrap(); + assert!(remainder_start_after_wrapper_operator(first2, src2).is_none()); + } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 375183ab82..ebc26ed912 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -11,6 +11,7 @@ use std::time::Duration; use crate::AuthManager; use crate::client_common::REVIEW_PROMPT; use crate::event_mapping::map_response_item_to_event_messages; +use crate::normalize_command::try_normalize_command; use async_channel::Receiver; use async_channel::Sender; use codex_apply_patch::ApplyPatchAction; @@ -2774,6 +2775,24 @@ async fn handle_container_exec_with_params( ) } None => { + // Normalize a classic bash -lc "cd ... && ..." wrapper into updated + // cwd and a cleaner script for display, if applicable. + let command_for_display = match try_normalize_command(¶ms.command, ¶ms.cwd) { + Some(norm) => { + // Once we have confidence in this approach, we will use + // the normalized command (and cwd) for execution as + // well. + // + // Note that behavior changes slightly in that the + // command is executed without considering whether + // `cd` succeeded, but this is probably OK because the + // argument to `cd` is used as the cwd for the exec, + // so it will fail if the directory does not exist. + norm.command_for_display + } + None => params.command.clone(), + }; + let safety = { let state = sess.state.lock_unchecked(); assess_command_safety( @@ -2784,7 +2803,6 @@ async fn handle_container_exec_with_params( params.with_escalated_permissions.unwrap_or(false), ) }; - let command_for_display = params.command.clone(); (params, safety, command_for_display) } }; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 7b9c3dc9f0..d0c99d08c4 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -35,6 +35,7 @@ mod mcp_connection_manager; mod mcp_tool_call; mod message_history; mod model_provider_info; +mod normalize_command; pub mod parse_command; mod truncate; mod unified_exec; diff --git a/codex-rs/core/src/normalize_command.rs b/codex-rs/core/src/normalize_command.rs new file mode 100644 index 0000000000..81a618e1cd --- /dev/null +++ b/codex-rs/core/src/normalize_command.rs @@ -0,0 +1,438 @@ +use crate::bash::extract_words_from_command_node; +use crate::bash::find_first_command_node; +use crate::bash::remainder_start_after_wrapper_operator; +use crate::bash::try_parse_bash; +use crate::bash::try_parse_word_only_commands_sequence; +use std::path::Path; +use std::path::PathBuf; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NormalizedCommand { + pub command: Vec, + pub cwd: PathBuf, + pub command_for_display: Vec, +} + +/// Normalize a shell command for execution and display. +/// +/// For classic wrappers like `bash -lc "cd repo && git status"` returns: +/// - command: ["bash", "-lc", "git status"] +/// - cwd: original cwd joined with "repo" +/// - command_for_display: +/// - If the remainder is a single exec-able command, return it tokenized +/// (e.g., ["rg", "--files"]). +/// - Otherwise, wrap the script as ["bash", "-lc", "