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
46 changes: 6 additions & 40 deletions codex-rs/core/src/memory_usage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,15 @@ use crate::tools::handlers::unified_exec::ExecCommandArgs;
use codex_memories_read::usage::MEMORIES_USAGE_METRIC;
use codex_memories_read::usage::memories_usage_kinds_from_command;
use codex_protocol::models::ShellCommandToolCallParams;
use std::path::PathBuf;

pub(crate) async fn emit_metric_for_tool_read(invocation: &ToolInvocation, success: bool) {
let Some((command, _)) = shell_command_for_invocation(invocation) else {
pub(crate) fn emit_metric_for_tool_read(invocation: &ToolInvocation, success: bool) {
let Some(command) = shell_script_for_invocation(invocation) else {
return;
};
let kinds = memories_usage_kinds_from_command(&command);
if kinds.is_empty() {
return;
}

let success = if success { "true" } else { "false" };
let tool_name = flat_tool_name(&invocation.tool_name);
for kind in kinds {
for kind in memories_usage_kinds_from_command(&command) {
invocation.turn.session_telemetry.counter(
MEMORIES_USAGE_METRIC,
/*inc*/ 1,
Expand All @@ -31,7 +26,7 @@ pub(crate) async fn emit_metric_for_tool_read(invocation: &ToolInvocation, succe
}
}

fn shell_command_for_invocation(invocation: &ToolInvocation) -> Option<(Vec<String>, PathBuf)> {
fn shell_script_for_invocation(invocation: &ToolInvocation) -> Option<String> {
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Expand All @@ -42,39 +37,10 @@ fn shell_command_for_invocation(invocation: &ToolInvocation) -> Option<(Vec<Stri
) {
(None, "shell_command") => serde_json::from_str::<ShellCommandToolCallParams>(arguments)
.ok()
.map(|params| {
if !invocation.turn.config.permissions.allow_login_shell
&& params.login == Some(true)
{
#[allow(deprecated)]
let cwd = invocation.turn.resolve_path(params.workdir).to_path_buf();
return (Vec::new(), cwd);
}
let use_login_shell = params
.login
.unwrap_or(invocation.turn.config.permissions.allow_login_shell);
let command = invocation
.session
.user_shell()
.derive_exec_args(&params.command, use_login_shell);
#[allow(deprecated)]
let cwd = invocation.turn.resolve_path(params.workdir).to_path_buf();
(command, cwd)
}),
.map(|params| params.command),
(None, "exec_command") => serde_json::from_str::<ExecCommandArgs>(arguments)
.ok()
.and_then(|params| {
let command = crate::tools::handlers::unified_exec::get_command(
&params,
invocation.session.user_shell(),
&invocation.turn.unified_exec_shell_mode,
invocation.turn.config.permissions.allow_login_shell,
)
.ok()?;
#[allow(deprecated)]
let cwd = invocation.turn.resolve_path(params.workdir).to_path_buf();
Some((command.command, cwd))
}),
.map(|params| params.cmd),
(Some(_), _) | (None, _) => None,
}
}
4 changes: 1 addition & 3 deletions codex-rs/core/src/tools/handlers/unified_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ pub use write_stdin::WriteStdinHandler;

#[derive(Debug, Deserialize)]
pub(crate) struct ExecCommandArgs {
cmd: String,
#[serde(default)]
pub(crate) workdir: Option<String>,
pub(crate) cmd: String,
#[serde(default)]
shell: Option<String>,
#[serde(default)]
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/core/src/tools/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ impl ToolRegistry {
Ok((_, success)) => *success,
Err(_) => false,
};
emit_metric_for_tool_read(&invocation, success).await;
emit_metric_for_tool_read(&invocation, success);
let post_tool_use_payload = if success {
let guard = response_cell.lock().await;
guard
Expand Down
15 changes: 11 additions & 4 deletions codex-rs/memories/read/src/usage.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use codex_protocol::parse_command::ParsedCommand;
use codex_shell_command::bash::parse_shell_script_into_commands;
use codex_shell_command::is_safe_command::is_known_safe_command;
use codex_shell_command::parse_command::parse_command;
use codex_shell_command::parse_command::parse_shell_script;

pub use crate::metrics::MEMORIES_USAGE_METRIC;

Expand All @@ -25,12 +26,18 @@ impl MemoriesUsageKind {
}
}

pub fn memories_usage_kinds_from_command(command: &[String]) -> Vec<MemoriesUsageKind> {
if !is_known_safe_command(command) {
pub fn memories_usage_kinds_from_command(command: &str) -> Vec<MemoriesUsageKind> {
let Some(commands) = parse_shell_script_into_commands(command) else {
return Vec::new();
};
if !commands
.iter()
.all(|command| is_known_safe_command(command))
{
return Vec::new();
}

parse_command(command)
parse_shell_script(command)
.into_iter()
.filter_map(|command| match command {
ParsedCommand::Read { path, .. } => get_memory_kind(path.display().to_string()),
Expand Down
13 changes: 8 additions & 5 deletions codex-rs/shell-command/src/bash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ pub fn try_parse_word_only_commands_sequence(tree: &Tree, src: &str) -> Option<V
Some(commands)
}

/// Parses a shell script consisting only of plain commands joined by safe operators.
pub fn parse_shell_script_into_commands(script: &str) -> Option<Vec<Vec<String>>> {
let tree = try_parse_shell(script)?;
try_parse_word_only_commands_sequence(&tree, script)
}

pub fn extract_bash_command(command: &[String]) -> Option<(&str, &str)> {
let [shell, flag, script] = command else {
return None;
Expand All @@ -114,9 +120,7 @@ pub fn extract_bash_command(command: &[String]) -> Option<(&str, &str)> {
/// joined by safe operators.
pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option<Vec<Vec<String>>> {
let (_, script) = extract_bash_command(command)?;

let tree = try_parse_shell(script)?;
try_parse_word_only_commands_sequence(&tree, script)
parse_shell_script_into_commands(script)
}

/// Returns the parsed argv for a single shell command in a here-doc style
Expand Down Expand Up @@ -322,8 +326,7 @@ mod tests {
use pretty_assertions::assert_eq;

fn parse_seq(src: &str) -> Option<Vec<Vec<String>>> {
let tree = try_parse_shell(src)?;
try_parse_word_only_commands_sequence(&tree, src)
parse_shell_script_into_commands(src)
}

#[test]
Expand Down
14 changes: 9 additions & 5 deletions codex-rs/shell-command/src/parse_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,11 @@ fn parse_find_query_and_path(tail: &[String]) -> (Option<String>, Option<String>
fn parse_shell_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
// Only handle bash/zsh here; PowerShell is stripped separately without bash parsing.
let (_, script) = extract_bash_command(original)?;
Some(parse_shell_script(script))
}

/// Parses command metadata from a Bash-compatible shell script.
pub fn parse_shell_script(script: &str) -> Vec<ParsedCommand> {
if let Some(tree) = try_parse_shell(script)
&& let Some(all_commands) = try_parse_word_only_commands_sequence(&tree, script)
&& !all_commands.is_empty()
Expand All @@ -1831,9 +1835,9 @@ fn parse_shell_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
// Commands arrive in source order; drop formatting helpers while preserving it.
let filtered_commands = drop_small_formatting_commands(all_commands);
if filtered_commands.is_empty() {
return Some(vec![ParsedCommand::Unknown {
return vec![ParsedCommand::Unknown {
cmd: script.to_string(),
}]);
}];
}
// Build parsed commands, tracking `cd` segments to compute effective file paths.
let mut commands: Vec<ParsedCommand> = Vec::new();
Expand Down Expand Up @@ -1939,11 +1943,11 @@ fn parse_shell_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
})
.collect();
}
return Some(commands);
return commands;
}
Some(vec![ParsedCommand::Unknown {
vec![ParsedCommand::Unknown {
cmd: script.to_string(),
}])
}]
}

/// Return true if this looks like a small formatting helper in a pipeline.
Expand Down
Loading