valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: add path field to ParsedCommand::Read variant (#5275)

Browse Source 
`ParsedCommand::Read` has a `name` field that attempts to identify the
name of the file being read, but the file may not be in the `cwd` in
which the command is invoked as demonstrated by this existing unit test:


0139f6780c/codex-rs/core/src/parse_command.rs (L250-L260)

As you can see, `tui/Cargo.toml` is the relative path to the file being
read.

This PR introduces a new `path: PathBuf` field to `ParsedCommand::Read`
that attempts to capture this information. When possible, this is an
absolute path, though when relative, it should be resolved against the
`cwd` that will be used to run the command to derive the absolute path.

This should make it easier for clients to provide UI for a "read file"
event that corresponds to the command execution.
This commit is contained in:
Michael Bolin
2025-10-16 23:19:54 -07:00
committed by GitHub
parent 40fba1bb4c
commit 50f53e7071
4 changed files with 145 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

146
codex-rs/core/src/parse_command.rs
View File

@@ -3,6 +3,7 @@ use crate::bash::try_parse_word_only_commands_sequence;
use codex_protocol::parse_command::ParsedCommand;
use shlex::split as shlex_split;
use shlex::try_join as shlex_try_join;
use std::path::PathBuf;
fn shlex_join(tokens: &[String]) -> String {
shlex_try_join(tokens.iter().map(String::as_str))
@@ -37,6 +38,7 @@ pub fn parse_command(command: &[String]) -> Vec<ParsedCommand> {
/// Tests are at the top to encourage using TDD + Codex to fix the implementation.
mod tests {
use super::*;
use std::path::PathBuf;
use std::string::ToString;
fn shlex_split_safe(s: &str) -> Vec<String> {
@@ -186,6 +188,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "README.md".to_string(),
path: PathBuf::from("webview/README.md"),
}],
);
}
@@ -197,6 +200,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "cat foo.txt".to_string(),
name: "foo.txt".to_string(),
path: PathBuf::from("foo/foo.txt"),
}],
);
}
@@ -219,6 +223,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "cat foo.txt".to_string(),
name: "foo.txt".to_string(),
path: PathBuf::from("foo/foo.txt"),
}],
);
}
@@ -243,6 +248,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "Cargo.toml".to_string(),
path: PathBuf::from("Cargo.toml"),
}],
);
}
@@ -255,6 +261,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "Cargo.toml".to_string(),
path: PathBuf::from("tui/Cargo.toml"),
}],
);
}
@@ -267,6 +274,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "README.md".to_string(),
path: PathBuf::from("README.md"),
}],
);
}
@@ -280,6 +288,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "README.md".to_string(),
path: PathBuf::from("README.md"),
}]
);
}
@@ -449,6 +458,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "parse_command.rs".to_string(),
path: PathBuf::from("core/src/parse_command.rs"),
}],
);
}
@@ -461,6 +471,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: inner.to_string(),
name: "history_cell.rs".to_string(),
path: PathBuf::from("tui/src/history_cell.rs"),
}],
);
}
@@ -474,6 +485,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "cat -- ansi-escape/Cargo.toml".to_string(),
name: "Cargo.toml".to_string(),
path: PathBuf::from("ansi-escape/Cargo.toml"),
}],
);
}
@@ -503,6 +515,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "sed -n '260,640p' exec/src/event_processor_with_human_output.rs".to_string(),
name: "event_processor_with_human_output.rs".to_string(),
path: PathBuf::from("exec/src/event_processor_with_human_output.rs"),
}],
);
}
@@ -662,6 +675,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: r#"cat "pkg\\src\\main.rs""#.to_string(),
name: "main.rs".to_string(),
path: PathBuf::from(r#"pkg\src\main.rs"#),
}],
);
}
@@ -673,6 +687,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "head -n50 Cargo.toml".to_string(),
name: "Cargo.toml".to_string(),
path: PathBuf::from("Cargo.toml"),
}],
);
}
@@ -703,6 +718,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "tail -n+10 README.md".to_string(),
name: "README.md".to_string(),
path: PathBuf::from("README.md"),
}],
);
}
@@ -739,6 +755,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "cat -- ./-strange-file-name".to_string(),
name: "-strange-file-name".to_string(),
path: PathBuf::from("./-strange-file-name"),
}],
);
@@ -748,6 +765,7 @@ mod tests {
vec![ParsedCommand::Read {
cmd: "sed -n '12,20p' Cargo.toml".to_string(),
name: "Cargo.toml".to_string(),
path: PathBuf::from("Cargo.toml"),
}],
);
}
@@ -840,11 +858,39 @@ pub fn parse_command_impl(command: &[String]) -> Vec<ParsedCommand> {
// Preserve left-to-right execution order for all commands, including bash -c/-lc
// so summaries reflect the order they will run.
// Map each pipeline segment to its parsed summary.
let mut commands: Vec<ParsedCommand> = parts
.iter()
.map(|tokens| summarize_main_tokens(tokens))
.collect();
// Map each pipeline segment to its parsed summary, tracking `cd` to compute paths.
let mut commands: Vec<ParsedCommand> = Vec::new();
let mut cwd: Option<String> = None;
for tokens in &parts {
if let Some((head, tail)) = tokens.split_first()
&& head == "cd"
{
if let Some(dir) = tail.first() {
cwd = Some(match &cwd {
Some(base) => join_paths(base, dir),
None => dir.clone(),
});
}
continue;
}
let parsed = summarize_main_tokens(tokens);
let parsed = match parsed {
ParsedCommand::Read { cmd, name, path } => {
if let Some(base) = &cwd {
let full = join_paths(base, &path.to_string_lossy());
ParsedCommand::Read {
cmd,
name,
path: PathBuf::from(full),
}
} else {
ParsedCommand::Read { cmd, name, path }
}
}
other => other,
};
commands.push(parsed);
}
while let Some(next) = simplify_once(&commands) {
commands = next;
@@ -1129,10 +1175,39 @@ fn parse_bash_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
cmd: script.clone(),
}]);
}
let mut commands: Vec<ParsedCommand> = filtered_commands
.into_iter()
.map(|tokens| summarize_main_tokens(&tokens))
.collect();
// Build parsed commands, tracking `cd` segments to compute effective file paths.
let mut commands: Vec<ParsedCommand> = Vec::new();
let mut cwd: Option<String> = None;
for tokens in filtered_commands.into_iter() {
if let Some((head, tail)) = tokens.split_first()
&& head == "cd"
{
if let Some(dir) = tail.first() {
cwd = Some(match &cwd {
Some(base) => join_paths(base, dir),
None => dir.clone(),
});
}
continue;
}
let parsed = summarize_main_tokens(&tokens);
let parsed = match parsed {
ParsedCommand::Read { cmd, name, path } => {
if let Some(base) = &cwd {
let full = join_paths(base, &path.to_string_lossy());
ParsedCommand::Read {
cmd,
name,
path: PathBuf::from(full),
}
} else {
ParsedCommand::Read { cmd, name, path }
}
}
other => other,
};
commands.push(parsed);
}
if commands.len() > 1 {
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`.
@@ -1152,7 +1227,7 @@ fn parse_bash_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
commands = commands
.into_iter()
.map(|pc| match pc {
ParsedCommand::Read { name, cmd, .. } => {
ParsedCommand::Read { name, cmd, path } => {
if had_connectors {
let has_pipe = script_tokens.iter().any(|t| t == "|");
let has_sed_n = script_tokens.windows(2).any(|w| {
@@ -1163,14 +1238,16 @@ fn parse_bash_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
ParsedCommand::Read {
cmd: script.clone(),
name,
path,
}
} else {
ParsedCommand::Read { cmd, name }
ParsedCommand::Read { cmd, name, path }
}
} else {
ParsedCommand::Read {
cmd: shlex_join(&script_tokens),
name,
path,
}
}
}
@@ -1335,10 +1412,12 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
tail
};
if effective_tail.len() == 1 {
let name = short_display_path(&effective_tail[0]);
let path = effective_tail[0].clone();
let name = short_display_path(&path);
ParsedCommand::Read {
cmd: shlex_join(main_cmd),
name,
path: PathBuf::from(path),
}
} else {
ParsedCommand::Unknown {
@@ -1373,10 +1452,12 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
i += 1;
}
if let Some(p) = candidates.into_iter().find(|p| !p.starts_with('-')) {
let name = short_display_path(p);
let path = p.clone();
let name = short_display_path(&path);
return ParsedCommand::Read {
cmd: shlex_join(main_cmd),
name,
path: PathBuf::from(path),
};
}
}
@@ -1415,10 +1496,12 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
i += 1;
}
if let Some(p) = candidates.into_iter().find(|p| !p.starts_with('-')) {
let name = short_display_path(p);
let path = p.clone();
let name = short_display_path(&path);
return ParsedCommand::Read {
cmd: shlex_join(main_cmd),
name,
path: PathBuf::from(path),
};
}
}
@@ -1430,10 +1513,12 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
// Avoid treating option values as paths (e.g., nl -s " ").
let candidates = skip_flag_values(tail, &["-s", "-w", "-v", "-i", "-b"]);
if let Some(p) = candidates.into_iter().find(|p| !p.starts_with('-')) {
let name = short_display_path(p);
let path = p.clone();
let name = short_display_path(&path);
ParsedCommand::Read {
cmd: shlex_join(main_cmd),
name,
path: PathBuf::from(path),
}
} else {
ParsedCommand::Unknown {
@@ -1448,10 +1533,12 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
&& is_valid_sed_n_arg(tail.get(1).map(String::as_str)) =>
{
if let Some(path) = tail.get(2) {
let name = short_display_path(path);
let path = path.clone();
let name = short_display_path(&path);
ParsedCommand::Read {
cmd: shlex_join(main_cmd),
name,
path: PathBuf::from(path),
}
} else {
ParsedCommand::Unknown {
@@ -1465,3 +1552,30 @@ fn summarize_main_tokens(main_cmd: &[String]) -> ParsedCommand {
},
}
}
fn is_abs_like(path: &str) -> bool {
if std::path::Path::new(path).is_absolute() {
return true;
}
let mut chars = path.chars();
match (chars.next(), chars.next(), chars.next()) {
// Windows drive path like C:\
(Some(d), Some(':'), Some('\\')) if d.is_ascii_alphabetic() => return true,
// UNC path like \\server\share
(Some('\\'), Some('\\'), _) => return true,
_ => {}
}
false
}
fn join_paths(base: &str, rel: &str) -> String {
if is_abs_like(rel) {
return rel.to_string();
}
if base.is_empty() {
return rel.to_string();
}
let mut buf = PathBuf::from(base);
buf.push(rel);
buf.to_string_lossy().to_string()
}

6
codex-rs/protocol/src/parse_command.rs
View File

@@ -1,5 +1,6 @@
use serde::Deserialize;
use serde::Serialize;
use std::path::PathBuf;
use ts_rs::TS;
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, TS)]
@@ -8,6 +9,11 @@ pub enum ParsedCommand {
Read {
cmd: String,
name: String,
/// (Best effort) Path to the file being read by the command. When
/// possible, this is an absolute path, though when relative, it should
/// be resolved against the `cwd`` that will be used to run the command
/// to derive the absolute path.
path: PathBuf,
},
ListFiles {
cmd: String,

3
codex-rs/tui/src/chatwidget/tests.rs
View File

@@ -1631,7 +1631,7 @@ fn status_widget_and_approval_modal_snapshot() {
let ev = ExecApprovalRequestEvent {
call_id: "call-approve-exec".into(),
command: vec!["echo".into(), "hello world".into()],
cwd: std::path::PathBuf::from("/tmp"),
cwd: PathBuf::from("/tmp"),
reason: Some(
"this is a test reason such as one that would be produced by the model".into(),
),
@@ -2310,6 +2310,7 @@ fn chatwidget_exec_and_status_layout_vt100_snapshot() {
ParsedCommand::Read {
name: "diff_render.rs".into(),
cmd: "cat diff_render.rs".into(),
path: "diff_render.rs".into(),
},
],
}),

7
codex-rs/tui/src/history_cell.rs
View File

@@ -1682,10 +1682,12 @@ mod tests {
ParsedCommand::Read {
name: "shimmer.rs".into(),
cmd: "cat shimmer.rs".into(),
path: "shimmer.rs".into(),
},
ParsedCommand::Read {
name: "status_indicator_widget.rs".into(),
cmd: "cat status_indicator_widget.rs".into(),
path: "status_indicator_widget.rs".into(),
},
],
output: None,
@@ -1742,6 +1744,7 @@ mod tests {
vec![ParsedCommand::Read {
name: "shimmer.rs".into(),
cmd: "cat shimmer.rs".into(),
path: "shimmer.rs".into(),
}],
)
.unwrap();
@@ -1763,6 +1766,7 @@ mod tests {
vec![ParsedCommand::Read {
name: "status_indicator_widget.rs".into(),
cmd: "cat status_indicator_widget.rs".into(),
path: "status_indicator_widget.rs".into(),
}],
)
.unwrap();
@@ -1791,14 +1795,17 @@ mod tests {
ParsedCommand::Read {
name: "auth.rs".into(),
cmd: "cat auth.rs".into(),
path: "auth.rs".into(),
},
ParsedCommand::Read {
name: "auth.rs".into(),
cmd: "cat auth.rs".into(),
path: "auth.rs".into(),
},
ParsedCommand::Read {
name: "shimmer.rs".into(),
cmd: "cat shimmer.rs".into(),
path: "shimmer.rs".into(),
},
],
output: None,