From 4fd82fb0e0e69c26c803fcd56b3650399eb2949c Mon Sep 17 00:00:00 2001 From: Graham King Date: Fri, 2 Oct 2026 14:54:59 -0400 Subject: [PATCH 1/2] feat(stage-router): Extend retrieval cmd identification - Stricter shell parsing to identify file reads. We identify an error by looking for strings such `MemoryError` in tool output. If that string appears in a log file we read for example, then it should not count. Previously we skipped it correctly if the output was Claude and Pi "Read" tools, but Codex does everything with bash which we did not skip. Now we parse that and categorize if it is a read. On the DeepSWE runs I checked, this reduces capable model use from 19% of requests to 15%. The classification takes ~265 ns. - Missing file reads are not failures but normal exploration. They occur at the same rate (~38%) in successful and failed tasks. Applies to both Codex and Claude. - Empty text blocks occupy no slots. We look at the last three tool calls. Previously a tool with no output (e.g. `mkdir`) didn't occupy one of those three slots, but two together did because we join on `\n`. That caused errors to age out even though we don't have any new evidence. Now empty blocks are skipped. The shell parsing was built by extracting commands from three DeepSWE runs (one Luna, two Sol). It was prototyped in Python before porting to Rust. Both Astra xhigh and Opus 5.5 xhigh worked on it. I think it is remarkably readable for what it is. There are more tests than usual because of this. Assisted-by: Pi:GPT 6 Astra xhigh Assisted-by: Claude:Opus 5.5 xhigh Signed-off-by: Graham King --- Cargo.lock | 1 + crates/libsy/Cargo.toml | 1 + .../libsy/src/algorithms/util/tool_signals.rs | 488 +++++++++++++++--- 3 files changed, 422 insertions(+), 68 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aeeeaf37b..f331e0abd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2335,6 +2335,7 @@ dependencies = [ "regex", "serde", "serde_json", + "shlex", "switchyard-protocol", "thiserror 2.0.18", "tokio", diff --git a/crates/libsy/Cargo.toml b/crates/libsy/Cargo.toml index fa92e3094..b3e65dbee 100644 --- a/crates/libsy/Cargo.toml +++ b/crates/libsy/Cargo.toml @@ -31,6 +31,7 @@ opentelemetry = { version = "0.32", default-features = false, features = ["metri parking_lot.workspace = true rand.workspace = true regex.workspace = true +shlex = "2" switchyard-protocol.workspace = true thiserror.workspace = true tokio.workspace = true diff --git a/crates/libsy/src/algorithms/util/tool_signals.rs b/crates/libsy/src/algorithms/util/tool_signals.rs index 11ac3ac65..377b96d22 100644 --- a/crates/libsy/src/algorithms/util/tool_signals.rs +++ b/crates/libsy/src/algorithms/util/tool_signals.rs @@ -80,9 +80,10 @@ static ERROR_PATTERNS: &[(&str, f32, &[&str])] = &[ &[ "filenotfounderror:", "no such file or directory", - // Claude Code Read-tool miss. Anchored as "file does not exist" (not a - // bare "does not exist", which fires on `ls` output and prose) — trace- - // mined across 1006 local trajectories at 22 true / 2 false positives. + // Claude Code's missing-file message. Reads are exempt (`is_missing_file`). + // Anchored as "file does not exist" (not a bare "does not exist", which + // fires on `ls` output and prose) — trace-mined across 1006 local + // trajectories at 22 true / 2 false positives. "file does not exist", ], ), @@ -535,11 +536,260 @@ fn classify_tool_call_with_semantics( semantics.classify(name).unwrap_or(ToolSemantic::Unknown) } -/// Built-in tools that return file or search contents instead of running anything. -fn is_retrieval_tool(name: &str, command: Option<&str>) -> bool { +/// Tools whose successful output is retrieved content or inspection data. +fn is_retrieval_tool(name: &str, command: Option<&Value>) -> bool { let lower = name.to_lowercase(); READ_TOOL_NAMES.contains(&lower.as_str()) - || (EDITOR_TOOL_NAMES.contains(&lower.as_str()) && command == Some("view")) + || (EDITOR_TOOL_NAMES.contains(&lower.as_str()) + && command + .and_then(Value::as_str) + .is_some_and(|command| command.eq_ignore_ascii_case("view"))) + // PowerShell quoting and escapes do not follow the POSIX rules below. + || (lower != "powershell" + && BASH_TOOL_NAMES.contains(&lower.as_str()) + && command.is_some_and(command_is_retrieval)) +} + +/// A shell command field given as one line or as argv words. +fn command_is_retrieval(command: &Value) -> bool { + match command { + Value::String(line) => shell_is_retrieval(line, 0), + Value::Array(argv) => { + // Charge each word a separator byte, as in the one-line form, so the + // budget also bounds the argument count. + argv.iter() + .try_fold(0, |size, word| { + let size = size + word.as_str()?.len() + 1; + (size <= MAX_RETRIEVAL_COMMAND_BYTES).then_some(size) + }) + .is_some() + && retrieval_command( + &argv.iter().filter_map(Value::as_str).collect::>(), + 0, + ) + } + _ => false, + } +} + +// Keep shell parsing bounded on the request thread. Larger commands retain +// ordinary error scanning, just like unrecognized shell syntax. +const MAX_RETRIEVAL_COMMAND_BYTES: usize = 16 * 1024; + +/// Only suppress output when every shell segment is a recognized inspection. +/// Observation counters can count mixed commands; suppressing errors needs all +/// commands to qualify. Substitutions and redirects keep their error signals. +fn shell_is_retrieval(command: &str, depth: usize) -> bool { + if command.len() > MAX_RETRIEVAL_COMMAND_BYTES { + return false; + } + if quoted_chars(command).any(|(_, c, quote)| { + (quote != Some('\'') && matches!(c, '$' | '`')) + || (quote.is_none() && matches!(c, '<' | '>' | '(' | ')' | '{' | '}' | '#')) + }) { + return false; + } + let mut segments = shell_segments(command).peekable(); + segments.peek().is_some() + && segments.all(|segment| { + // `split` also rejects an unclosed quote or a trailing backslash. + shlex::split(segment).is_some_and(|words| { + retrieval_command(&words.iter().map(String::as_str).collect::>(), depth) + }) + }) +} + +fn skip_shell_options<'a>(mut args: &'a [&'a str], takes_value: &[&str]) -> &'a [&'a str] { + while let Some((option, rest)) = args.split_first() { + if !option.starts_with('-') || *option == "-" { + break; + } + args = rest; + if *option == "--" { + break; + } + if takes_value.contains(option) { + args = args.get(1..).unwrap_or_default(); + } + } + args +} + +/// A short-option word such as `-Hx` that sets any of `flags`. +fn has_short_flag(word: &str, flags: &[char]) -> bool { + word.starts_with('-') && !word.starts_with("--") && word.contains(flags) +} + +// Bounds recursion through nested wrappers and `sh -c` scripts. +const MAX_COMMAND_DEPTH: usize = 20; + +fn retrieval_command(mut words: &[&str], depth: usize) -> bool { + if depth > MAX_COMMAND_DEPTH { + return false; + } + while words.first().is_some_and(|word| { + word.split_once('=').is_some_and(|(name, _)| { + !name.is_empty() + && name.bytes().enumerate().all(|(i, c)| { + c == b'_' || c.is_ascii_alphabetic() || (i > 0 && c.is_ascii_digit()) + }) + }) + }) { + words = &words[1..]; + } + let Some((program, args)) = words.split_first() else { + return false; + }; + let program = program_name(program); + // Unwrap launchers before inspecting the actual command and its flags. + let wrapper_options: Option<&[&str]> = match program { + "sudo" => Some(&[ + "-u", "-g", "-h", "-p", "-C", "-T", "--user", "--group", "--host", + ]), + "env" => Some(&["-u", "--unset", "-C", "--chdir"]), + "command" | "nohup" => Some(&[]), + "exec" => Some(&["-a"]), + "time" => Some(&["-f", "--format", "-o", "--output"]), + "timeout" => Some(&["-s", "--signal", "-k", "--kill-after"]), + "xargs" => Some(&[ + "-n", + "--max-args", + "-P", + "--max-procs", + "-I", + "--replace", + "-d", + "--delimiter", + "-L", + "--max-lines", + "-s", + "--max-chars", + ]), + _ => None, + }; + if let Some(options) = wrapper_options { + // `env -S` splits its argument into a new command line. + if program == "env" + && args + .iter() + .any(|arg| arg.starts_with("--split-string") || has_short_flag(arg, &['S'])) + { + return false; + } + let mut rest = skip_shell_options(args, options); + if program == "timeout" { + rest = rest.get(1..).unwrap_or_default(); + } + return (program == "env" && rest.is_empty()) || retrieval_command(rest, depth + 1); + } + if matches!(program, "bash" | "sh" | "dash" | "zsh" | "ksh") { + return args + .first() + .is_some_and(|option| has_short_flag(option, &['c'])) + && args + .get(1) + .is_some_and(|command| shell_is_retrieval(command, depth + 1)); + } + if program == "git" { + return git_is_retrieval(args); + } + if matches!(args, ["--help" | "--version"]) { + return true; + } + match program { + "find" => !args.iter().any(|arg| { + matches!( + *arg, + "-delete" + | "-exec" + | "-execdir" + | "-ok" + | "-okdir" + | "-fprint" + | "-fprint0" + | "-fprintf" + ) + }), + // fd joins short flags, so `-Hx` runs a command too. + "fd" => !args + .iter() + .any(|arg| arg.starts_with("--exec") || has_short_flag(arg, &['x', 'X'])), + // Only the common line-range printing form; sed scripts can execute commands. + "sed" => { + args.len() >= 2 + && args[0] == "-n" + && args[2..].iter().all(|arg| !arg.starts_with('-')) + && args[1].strip_suffix('p').is_some_and(|range| { + !range.is_empty() + && range + .bytes() + .all(|c| c.is_ascii_digit() || matches!(c, b',' | b'$')) + }) + } + "sort" => !args.iter().any(|arg| { + arg.starts_with("--compress-program") + || arg.starts_with("--output") + || arg.starts_with("-o") + }), + "rg" => !args.iter().any(|arg| arg.starts_with("--pre")), + "xxd" => !args.contains(&"-r") && !args.contains(&"-revert"), + "go" => { + matches!(args.first(), Some(&"list" | &"doc" | &"env" | &"version")) + && !args.iter().any(|arg| matches!(*arg, "-w" | "-u")) + } + "docker" | "podman" => matches!(args.first(), Some(&"ps" | &"version")), + "cat" | "grep" | "ls" | "nl" | "head" | "tail" | "wc" | "pwd" | "stat" | "file" | "du" + | "df" | "which" | "type" | "diff" | "cmp" | "jq" | "uniq" | "cut" | "readlink" + | "realpath" | "tree" | "basename" | "dirname" | "printenv" | "echo" | "printf" + | "less" | "more" | "test" | "[" | "ps" | "pgrep" | "pkg-config" | "strings" | "uname" + | "od" | "true" | ":" | "cd" | "pstree" | "sha256sum" | "sha1sum" | "md5sum" | "lsof" + | "tr" | "free" | "id" | "namei" | "whoami" | "paste" | "ss" | "getent" => true, + _ => false, + } +} + +fn git_is_retrieval(args: &[&str]) -> bool { + let args = skip_shell_options( + args, + &["-C", "-c", "--git-dir", "--work-tree", "--namespace"], + ); + let Some((subcommand, rest)) = args.split_first() else { + return false; + }; + match *subcommand { + "diff" => !rest.contains(&"--check"), + "branch" => { + rest.is_empty() + || rest.iter().all(|arg| { + matches!( + *arg, + "-a" | "-r" + | "--all" + | "--list" + | "--show-current" + | "-v" + | "-vv" + | "--verbose" + ) + }) + } + "remote" => { + rest.is_empty() + || matches!(rest, ["-v" | "--verbose"]) + || rest.first() == Some(&"get-url") + } + "tag" => rest.is_empty() || matches!(rest[0], "-l" | "--list"), + "config" => matches!( + rest.first(), + Some(&"--get" | &"--get-all" | &"--list" | &"-l") + ), + "worktree" => rest.first() == Some(&"list"), + "submodule" => rest.first() == Some(&"status"), + "status" | "log" | "show" | "blame" | "ls-files" | "ls-remote" | "rev-parse" + | "merge-base" | "grep" | "describe" | "show-ref" | "check-ignore" | "rev-list" + | "ls-tree" | "diff-tree" | "version" => true, + _ => false, + } } fn is_builtin_tool_name(lower: &str) -> bool { @@ -554,41 +804,39 @@ fn is_builtin_tool_name(lower: &str) -> bool { /// pretending to be a full shell parser; only the leading program and flags of /// each segment are inspected below. fn shell_segments(command: &str) -> impl Iterator { - let mut chars = command.char_indices(); - let mut start = 0usize; + let mut start = 0; + quoted_chars(command) + .filter(|&(_, c, quote)| quote.is_none() && matches!(c, '\n' | ';' | '|' | '&')) + .map(|(index, _, _)| index) + .chain([command.len()]) + .filter_map(move |end| { + let segment = command[start..end].trim(); + // Separators are one byte. + start = end + 1; + (!segment.is_empty()).then_some(segment) + }) +} + +/// Unescaped characters with their byte index and the quote around them. +fn quoted_chars(command: &str) -> impl Iterator)> { let mut quote = None; let mut escaped = false; - let mut finished = false; - - std::iter::from_fn(move || { - loop { - for (index, character) in chars.by_ref() { - if escaped { - escaped = false; - } else if character == '\\' && quote != Some('\'') { - escaped = true; - } else if quote == Some(character) { - quote = None; - } else if quote.is_none() && matches!(character, '\'' | '"') { - quote = Some(character); - } else if quote.is_none() && matches!(character, '\n' | ';' | '|' | '&') { - let segment = command[start..index].trim(); - start = index + character.len_utf8(); - if !segment.is_empty() { - return Some(segment); - } - } - } - - if finished { - return None; - } - finished = true; - let segment = command[start..].trim(); - if !segment.is_empty() { - return Some(segment); - } + command.char_indices().filter_map(move |(index, c)| { + if escaped { + escaped = false; + return None; + } + if c == '\\' && quote != Some('\'') { + escaped = true; + return None; + } + let around = quote; + if quote == Some(c) { + quote = None; + } else if quote.is_none() && matches!(c, '\'' | '"') { + quote = Some(c); } + Some((index, c, around)) }) } @@ -762,7 +1010,13 @@ fn extract_tool_signals_with_window_and_semantics( .and_then(|namespaces| split_qualified_name(namespaces, &call.name)) .map(|(tool, _)| tool) .or_else(|| mcp_tool_name(&call.name)); - let command = command_of(&call.arguments); + // The Responses wire format sends arguments as a JSON string. + let decoded = call + .arguments + .as_str() + .and_then(|raw| serde_json::from_str::(raw).ok()); + let command_field = command_of(decoded.as_ref().unwrap_or(&call.arguments)); + let command = command_field.and_then(command_text); if !call.id.is_empty() { // The joined name wins, as in `build_signal`. A joined name // configured as observe still counts when its bare name is a @@ -779,7 +1033,7 @@ fn extract_tool_signals_with_window_and_semantics( _ => call.name.as_str(), }; // A reused ID links to its latest call. - if is_retrieval_tool(name, command.as_deref()) { + if is_retrieval_tool(name, command_field) { retrieval_calls.insert(call.id.as_str()); } else { retrieval_calls.remove(call.id.as_str()); @@ -794,25 +1048,27 @@ fn extract_tool_signals_with_window_and_semantics( ContentBlock::ToolResult(result) => { // Before the empty-text filter: empty results still count. tool_result_count += 1; - let text = result - .content - .iter() - .filter_map(text_of) - .collect::>() - .join("\n"); let is_error = result.is_error == Some(true); - let is_retrieval_result = - !is_error && retrieval_calls.contains(result.tool_call_id.as_str()); + let is_retrieval_result = retrieval_calls + .contains(result.tool_call_id.as_str()) + && (!is_error + || result + .content + .iter() + .filter_map(text_of) + .any(is_missing_file)); + let texts = result.content.iter().filter_map(text_of); + let has_text = texts.clone().any(|text| !text.is_empty()); + if is_retrieval_result { + // Keep the window slot without copying file contents. + if has_text { + tool_texts.push((String::new(), false)); + } + continue; + } // An explicit failure remains a signal even without text. - if !text.is_empty() || is_error { - // Read and search results show file contents, not the outcome - // of a run. Drop the text but keep the slot so windows don't shift. - let text = if is_retrieval_result { - String::new() - } else { - text - }; - tool_texts.push((text, is_error)); + if has_text || is_error { + tool_texts.push((texts.collect::>().join("\n"), is_error)); } } // Built-in tool history stays opaque so it can be replayed unchanged. @@ -905,19 +1161,13 @@ fn mcp_tool_name(name: &str) -> Option<&str> { (!tool.is_empty()).then_some(tool) } -/// The shell command a tool call carries, when it has one. Harnesses name the -/// field `command`; anything else is a tool whose category comes from its name. -fn command_of(arguments: &Value) -> Option { - // the Responses wire format sends arguments as a JSON string, not an object - let decoded = arguments - .as_str() - .and_then(|raw| serde_json::from_str::(raw).ok()); - let object = decoded.as_ref().unwrap_or(arguments); - +/// The command field a tool call carries, when it has one. Harnesses name it +/// `command`, `cmd` or `input`; anything else is a tool whose category comes +/// from its name. +fn command_of(arguments: &Value) -> Option<&Value> { ["command", "cmd", "input"] .iter() - .filter_map(|key| object.get(*key)) - .find_map(command_text) + .find_map(|key| arguments.get(*key)) } /// A command field as lowercase text, from a string or an argv array. @@ -944,6 +1194,14 @@ fn text_of(block: &ContentBlock) -> Option<&str> { } } +/// Reads of missing paths happen as often on tasks that succeed as on tasks +/// that fail, so they are not a failure signal. +fn is_missing_file(text: &str) -> bool { + let lower = text.to_lowercase(); + // The OS error from shell tools, and Claude Code's Read tool message. + lower.contains("no such file or directory") || lower.contains("file does not exist") +} + fn build_signal( tool_texts: Vec<(String, bool)>, tool_calls: Vec, @@ -1649,6 +1907,100 @@ mod tests { assert_eq!(signal.tool_result_count, 3); } + // A request whose tool result answers the tool call. + fn call_with_result(mut call: Message, mut result: Message) -> Request { + if let ContentBlock::ToolCall(call) = &mut call.content[0] { + call.id = "call-1".into(); + } + if let ContentBlock::ToolResult(result) = &mut result.content[0] { + result.tool_call_id = "call-1".into(); + } + with_messages(vec![call, result]) + } + + #[test] + fn shell_reads_are_ignored_but_keep_their_window_slot() { + for command in [ + "cat logfile.txt", + "cd /repo && tail -n 100 logfile.txt | grep MemoryError", + "env MODE=debug timeout 5s rg 'MemoryError' .", + ] { + let mut request = call_with_result(bash(command), tr("MemoryError\nall tests passed")); + request.llm_request.messages.splice( + 0..0, + [bash("pytest"), tr("Traceback (most recent call last):")], + ); + // The read adds no error and no test pass, so the earlier failure stands. + let signal = ToolSignals::from_request(&request, Some(2)); + assert_eq!(signal.severity, HARD, "{command}"); + assert!(!signal.tests_passed, "{command}"); + // The read still fills a window slot, so the failure can age out. + let signal = ToolSignals::from_request(&request, Some(1)); + assert_eq!(signal.severity, 0.0, "{command}"); + } + } + + #[test] + fn shell_commands_that_run_code_keep_signals() { + let oversized = format!("cat {}", "a".repeat(MAX_RETRIEVAL_COMMAND_BYTES)); + for command in [ + "cat logfile.txt; pytest", + "echo $(pytest)", + "find . -name '*.py' -exec pytest ';'", + oversized.as_str(), + ] { + let request = call_with_result(bash(command), tr("MemoryError")); + let signal = ToolSignals::from_request(&request, None); + assert_eq!(signal.severity, CRITICAL, "{command}"); + } + } + + #[test] + fn failed_reads_signal_unless_the_file_is_missing() { + let failed = |text: &str| { + let mut result = tr(text); + if let ContentBlock::ToolResult(result) = &mut result.content[0] { + result.is_error = Some(true); + } + result + }; + let missing = "missing.txt: No such file or directory"; + let severities = [ + call_with_result(bash("cat missing.txt"), failed(missing)), + call_with_result(tc("Read"), failed("File does not exist.")), + // A read that fails for another reason still counts. + call_with_result(bash("cat logfile.txt"), failed("MemoryError")), + // Only reads are exempt. + call_with_result(bash("python missing.txt"), failed(missing)), + ] + .map(|request| ToolSignals::from_request(&request, None).severity); + assert_eq!(severities, [0.0, 0.0, CRITICAL, HARD]); + } + + #[test] + fn argv_commands_keep_word_boundaries() { + let signal = |argv: Value| { + let call = exec_command(json!({"cmd": argv})); + ToolSignals::from_request(&call_with_result(call, tr("MemoryError")), None) + }; + let mut many_args = vec![""; MAX_RETRIEVAL_COMMAND_BYTES]; + many_args[0] = "cat"; + let severities = [ + json!(["bash", "-lc", "cd repo && rg foo"]), + json!(["bash", "-lc", "cat log; pytest"]), + // One argv word is one file name, even with a separator in it. + json!(["cat", "log; pytest"]), + // Over the byte budget, by size and by argument count. + json!(["cat", "a".repeat(MAX_RETRIEVAL_COMMAND_BYTES)]), + json!(many_args), + ] + .map(|argv| signal(argv).severity); + assert_eq!(severities, [0.0, CRITICAL, 0.0, CRITICAL, CRITICAL]); + // The read counter also looks inside the script. + let signal = signal(json!(["bash", "-lc", "cd repo && rg foo"])); + assert_eq!(signal.read_count, 1); + } + #[test] fn severity_is_windowed_over_recent_results() { // An error two results back, then two clean results. From a5951f26eeac8da3d8def5dc7b0fc812960d2736 Mon Sep 17 00:00:00 2001 From: Graham King Date: Fri, 2 Oct 2026 15:55:32 -0400 Subject: [PATCH 2/2] Count reads, successful or not, as Observe Thanks Tina! Signed-off-by: Graham King --- .../libsy/src/algorithms/util/tool_signals.rs | 71 +++++++++++++------ 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/crates/libsy/src/algorithms/util/tool_signals.rs b/crates/libsy/src/algorithms/util/tool_signals.rs index 377b96d22..9890e7ce7 100644 --- a/crates/libsy/src/algorithms/util/tool_signals.rs +++ b/crates/libsy/src/algorithms/util/tool_signals.rs @@ -425,11 +425,13 @@ impl ToolSignals { // `command` is the lowercased Bash command line; None for non-Bash tools. // `bare_name` is the tool's own name when `name` joins it to a namespace or MCP server. +// `is_retrieval` marks a call that only reads; it counts as an observation. #[derive(Debug, Clone)] struct ObservedToolCall<'a> { name: String, bare_name: Option<&'a str>, command: Option, + is_retrieval: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -637,10 +639,22 @@ fn retrieval_command(mut words: &[&str], depth: usize) -> bool { }) { words = &words[1..]; } - let Some((program, args)) = words.split_first() else { + let Some((&program, args)) = words.split_first() else { return false; }; - let program = program_name(program); + // The standard bin dirs hold the real programs. Any other path, such as + // `./test`, names a local script. + let program = [ + "/bin/", + "/sbin/", + "/usr/bin/", + "/usr/sbin/", + "/usr/local/bin/", + "/opt/homebrew/bin/", + ] + .iter() + .find_map(|dir| program.strip_prefix(dir)) + .unwrap_or(program); // Unwrap launchers before inspecting the actual command and its flags. let wrapper_options: Option<&[&str]> = match program { "sudo" => Some(&[ @@ -1017,23 +1031,24 @@ fn extract_tool_signals_with_window_and_semantics( .and_then(|raw| serde_json::from_str::(raw).ok()); let command_field = command_of(decoded.as_ref().unwrap_or(&call.arguments)); let command = command_field.and_then(command_text); + // The joined name wins, as in `build_signal`. A joined name + // configured as observe still counts when its bare name is a + // retrieval tool, such as `mcp__files__read`. + let full = classify_tool_call_with_semantics( + &call.name, + command.as_deref(), + semantics, + ); + let name = match (full, bare_name) { + (ToolSemantic::Unknown | ToolSemantic::Observe, Some(bare_name)) => { + bare_name + } + _ => call.name.as_str(), + }; + let is_retrieval = is_retrieval_tool(name, command_field); if !call.id.is_empty() { - // The joined name wins, as in `build_signal`. A joined name - // configured as observe still counts when its bare name is a - // retrieval tool, such as `mcp__files__read`. - let full = classify_tool_call_with_semantics( - &call.name, - command.as_deref(), - semantics, - ); - let name = match (full, bare_name) { - (ToolSemantic::Unknown | ToolSemantic::Observe, Some(bare_name)) => { - bare_name - } - _ => call.name.as_str(), - }; // A reused ID links to its latest call. - if is_retrieval_tool(name, command_field) { + if is_retrieval { retrieval_calls.insert(call.id.as_str()); } else { retrieval_calls.remove(call.id.as_str()); @@ -1043,6 +1058,7 @@ fn extract_tool_signals_with_window_and_semantics( name: call.name.clone(), bare_name, command, + is_retrieval, }); } ContentBlock::ToolResult(result) => { @@ -1250,8 +1266,14 @@ fn build_signal( let mut pure_bash_streak = 0u32; let mut streak_open = true; for (i, tc) in tool_calls.iter().enumerate().rev() { - // The joined name wins, so configs that list it keep working. - let mut cat = classify_tool_call_with_semantics(&tc.name, tc.command.as_deref(), semantics); + // Every read, found or missed, is an observation. The read check parses + // the command, so it beats the substring patterns. Otherwise the joined + // name wins, so configs that list it keep working. + let mut cat = if tc.is_retrieval { + ToolSemantic::Observe + } else { + classify_tool_call_with_semantics(&tc.name, tc.command.as_deref(), semantics) + }; if matches!(cat, ToolSemantic::Unknown) && let Some(bare_name) = tc.bare_name { @@ -1921,7 +1943,7 @@ mod tests { #[test] fn shell_reads_are_ignored_but_keep_their_window_slot() { for command in [ - "cat logfile.txt", + "/usr/bin/cat logfile.txt", "cd /repo && tail -n 100 logfile.txt | grep MemoryError", "env MODE=debug timeout 5s rg 'MemoryError' .", ] { @@ -1996,9 +2018,12 @@ mod tests { ] .map(|argv| signal(argv).severity); assert_eq!(severities, [0.0, CRITICAL, 0.0, CRITICAL, CRITICAL]); - // The read counter also looks inside the script. - let signal = signal(json!(["bash", "-lc", "cd repo && rg foo"])); - assert_eq!(signal.read_count, 1); + // A read is an observation, even as a whole `-lc` script. + let read = signal(json!(["bash", "-lc", "sed -n 1,200p x"])); + assert_eq!(read.read_count, 1); + // Other commands are counted inside the script too. + let write = signal(json!(["bash", "-lc", "cd repo && mkdir x"])); + assert_eq!(write.write_count, 1); } #[test]