diff --git a/crates/pi-natives/src/shell.rs b/crates/pi-natives/src/shell.rs index e159ba6ed..74b58c42d 100644 --- a/crates/pi-natives/src/shell.rs +++ b/crates/pi-natives/src/shell.rs @@ -13,7 +13,6 @@ use pi_shell::{ ShellExecuteOptions as CoreShellExecuteOptions, ShellOptions as CoreShellOptions, ShellRunOptions as CoreShellRunOptions, ShellRunResult as CoreShellRunResult, execute_shell as core_execute_shell, - fixup::{BashFixupResult as CoreBashFixupResult, apply_bash_fixups as core_apply_bash_fixups}, minimizer, }; @@ -354,32 +353,6 @@ async fn pump_chunks(rx: flume::Receiver, mut forward: impl AsyncFnMut(S } } -/// Result of [`apply_bash_fixups`]: a possibly-rewritten command plus the -/// substrings that were removed (in source order). -#[napi(object)] -pub struct BashFixupResult { - /// Possibly-rewritten command. Equal to the input when no fixup fired. - pub command: String, - /// Substrings removed, in source order — suitable for a user-facing notice. - pub stripped: Vec, -} - -impl From for BashFixupResult { - fn from(value: CoreBashFixupResult) -> Self { - Self { command: value.command, stripped: value.stripped } - } -} - -/// Apply conservative pre-execution rewrites to a bash command. -/// -/// Strips trailing `| head|tail [safe-args]` and redundant trailing `2>&1` -/// from each top-level pipeline. The full rules and bail conditions live in -/// `pi_shell::fixup`. Synchronous and cheap (one parse pass over the input). -#[napi] -pub fn apply_bash_fixups(command: String) -> BashFixupResult { - core_apply_bash_fixups(&command).into() -} - #[cfg(test)] mod tests { use std::time::Duration; diff --git a/crates/pi-shell/src/fixup.rs b/crates/pi-shell/src/fixup.rs deleted file mode 100644 index 958caac1b..000000000 --- a/crates/pi-shell/src/fixup.rs +++ /dev/null @@ -1,530 +0,0 @@ -//! Conservative pre-execution rewrites for bash commands. -//! -//! Two fixups are applied, each anchored to the end of a top-level pipeline -//! (segments split on `;`, `&&`, `||`, and background `&`): -//! -//! 1. Trailing `| head [args]` / `| tail [args]` (and the `|&` variant) — -//! these pipes exist purely to limit output length. The harness already -//! truncates bash output and exposes the full result via an artifact, so -//! the pipe just hides content the agent wanted. -//! -//! 2. A redundant trailing `2>&1` on a segment that has no remaining pipe or -//! other redirect. The harness already merges stderr into stdout, so the -//! duplication is purely cosmetic — and often a leftover after fixup (1) -//! drops a downstream pipe. -//! -//! The implementation is AST-driven: `brush-parser` handles tokenization, -//! quoting, heredocs, command substitution, and nested compound commands. We -//! never re-implement those by hand. Source spans on `Pipeline`/`Command` -//! nodes give us byte-exact edit ranges; `IoRedirect` currently lacks a span, -//! so the `2>&1` strip uses a bounded textual scan inside the enclosing -//! simple command's source span. -//! -//! On any parse failure, multi-line input, or absence of an applicable -//! pattern, the function returns the input verbatim with `stripped` empty. - -use std::{io::BufReader, sync::LazyLock}; - -use brush_parser::{ - Parser, ParserOptions, SourceInfo, - ast::{ - AndOr, AndOrList, Command, CommandPrefixOrSuffixItem, CompoundListItem, IoFileRedirectKind, - IoFileRedirectTarget, IoRedirect, Pipeline, SourceLocation, - }, -}; -use regex::Regex; - -/// Result of [`apply_bash_fixups`]. -#[derive(Debug, Clone, Default)] -pub struct BashFixupResult { - /// Possibly-rewritten command. Equal to the input when no fixup fired. - pub command: String, - /// Substrings removed, in source order. Suitable for a user-facing notice. - pub stripped: Vec, -} - -/// Apply the bash fixups to `cmd`. See module docs for full rules. -#[must_use] -pub fn apply_bash_fixups(cmd: &str) -> BashFixupResult { - // Multi-line input is out of scope: heredoc/loop bodies can't be safely - // rewritten and the agent rarely passes them as bash tool input. Bailing - // early also keeps the per-call cost bounded. - if cmd.contains('\n') || cmd.contains('\r') { - return BashFixupResult { command: cmd.to_owned(), stripped: vec![] }; - } - - let options = ParserOptions::default(); - let source_info = SourceInfo::default(); - let mut reader = BufReader::new(cmd.as_bytes()); - let mut parser = Parser::new(&mut reader, &options, &source_info); - let Ok(program) = parser.parse_program() else { - return BashFixupResult { command: cmd.to_owned(), stripped: vec![] }; - }; - - // `ranges` drives output construction; `stripped` is reported to the - // caller. We keep them separate so reporting can stay in fixup order - // (head/tail before `2>&1`) while edits sort by source position. - let mut ranges: Vec<(usize, usize)> = Vec::new(); - let mut stripped: Vec = Vec::new(); - - // Walk only the top-level pipelines. Recursing into compound bodies (`if`, - // loops, subshells) would risk changing semantics: e.g. stripping `head` - // from `if cmd | head -5; then …; fi` swaps a header-check for a full - // stream-check. - for complete in &program.complete_commands { - for CompoundListItem(and_or, _sep) in &complete.0 { - walk_andor(and_or, cmd, &mut ranges, &mut stripped); - } - } - - if ranges.is_empty() { - return BashFixupResult { command: cmd.to_owned(), stripped: vec![] }; - } - - ranges.sort_by_key(|(s, _)| *s); - let mut out = String::with_capacity(cmd.len()); - let mut cursor = 0; - for (s, e) in ranges { - // Defensive: ranges should be disjoint by construction. - if s < cursor { - continue; - } - out.push_str(&cmd[cursor..s]); - cursor = e; - } - out.push_str(&cmd[cursor..]); - // Trim trailing horizontal whitespace introduced by removals at EOS. - while matches!(out.as_bytes().last(), Some(b' ' | b'\t')) { - out.pop(); - } - - BashFixupResult { command: out, stripped } -} - -fn walk_andor( - list: &AndOrList, - cmd: &str, - ranges: &mut Vec<(usize, usize)>, - stripped: &mut Vec, -) { - process_pipeline(&list.first, cmd, ranges, stripped); - for ao in &list.additional { - let pipe = match ao { - AndOr::And(p) | AndOr::Or(p) => p, - }; - process_pipeline(pipe, cmd, ranges, stripped); - } -} - -fn process_pipeline( - p: &Pipeline, - cmd: &str, - ranges: &mut Vec<(usize, usize)>, - stripped: &mut Vec, -) { - let outcome = try_strip_head_tail(p, cmd, ranges, stripped); - try_strip_2to1(p, cmd, outcome, ranges, stripped); -} - -/// Outcome of the head/tail strip — the 2>&1 pass needs the effective tail. -struct HeadTailOutcome { - stripped: bool, - /// Index into `p.seq` of the new effective last command. Equals - /// `seq.len()-1` when no strip fired, `seq.len()-2` when it did. - last_idx: usize, -} - -fn try_strip_head_tail( - p: &Pipeline, - cmd: &str, - ranges: &mut Vec<(usize, usize)>, - stripped: &mut Vec, -) -> HeadTailOutcome { - let n = p.seq.len(); - let default = HeadTailOutcome { stripped: false, last_idx: n.saturating_sub(1) }; - if n < 2 { - return default; - } - let last = &p.seq[n - 1]; - if !is_safe_head_tail(last) { - return default; - } - let Some(last_loc) = last.location() else { - return default; - }; - - // Pipeline-internal separators are always `|` or `|&` — never `||`. The - // real parser already validated structure, so scanning backwards from the - // start of `last` for the first `|` is unambiguous: AndOr operators - // (`||`, `&&`) only live *between* pipelines, not inside one. We anchor - // here rather than on `prev.location().end` because `SimpleCommand`'s - // span under-reports when its suffix contains unlocated `IoRedirect`s - // (e.g. the synthetic `2>&1` inserted by `|&`). - let bytes = cmd.as_bytes(); - let Some(last_start) = byte_offset(cmd, last_loc.start.index) else { - return default; - }; - let Some(last_end) = byte_offset(cmd, last_loc.end.index) else { - return default; - }; - let Some(head) = cmd.get(..last_start) else { - return default; - }; - let Some(pipe_pos) = head.rfind('|') else { - return default; - }; - // Defense in depth against `||`. - if pipe_pos > 0 && bytes[pipe_pos - 1] == b'|' { - return default; - } - if pipe_pos + 1 < bytes.len() && bytes[pipe_pos + 1] == b'|' { - return default; - } - - // Reported text starts at the pipe and is right-trimmed. The deletion - // range walks back through any leading whitespace so the rewrite is - // contiguous. - let stripped_text = cmd[pipe_pos..last_end].trim_end().to_owned(); - if stripped_text.is_empty() { - return default; - } - let mut delete_start = pipe_pos; - while delete_start > 0 && matches!(bytes[delete_start - 1], b' ' | b'\t') { - delete_start -= 1; - } - ranges.push((delete_start, last_end)); - stripped.push(stripped_text); - HeadTailOutcome { stripped: true, last_idx: n - 2 } -} - -fn try_strip_2to1( - p: &Pipeline, - cmd: &str, - outcome: HeadTailOutcome, - ranges: &mut Vec<(usize, usize)>, - stripped: &mut Vec, -) { - // `2>&1` is only redundant when no downstream pipe remains. After the - // head/tail strip the effective tail is `outcome.last_idx`; if any other - // command sits to its right, abort. - if outcome.stripped { - if outcome.last_idx != 0 { - return; - } - } else if p.seq.len() != 1 { - return; - } - - let target = &p.seq[outcome.last_idx]; - let Command::Simple(simple) = target else { - return; - }; - let Some(name_word) = simple.word_or_name.as_ref() else { - return; - }; - if name_word.value.is_empty() { - return; - } - let Some(suffix) = &simple.suffix else { return }; - if suffix.0.is_empty() { - return; - } - - // The last suffix item must be the `2>&1` redirect, and it must be the - // only redirect on the command (no `> file 2>&1` or `2>&1 > file`). - let Some(last_item) = suffix.0.last() else { - return; - }; - let CommandPrefixOrSuffixItem::IoRedirect(io) = last_item else { - return; - }; - if !is_stderr_to_stdout(io) { - return; - } - for item in &suffix.0[..suffix.0.len() - 1] { - if matches!(item, CommandPrefixOrSuffixItem::IoRedirect(_)) { - return; - } - } - if let Some(prefix) = &simple.prefix - && prefix - .0 - .iter() - .any(|item| matches!(item, CommandPrefixOrSuffixItem::IoRedirect(_))) - { - return; - } - - // `IoRedirect` doesn't carry a source span, so locate the literal - // `2>&1` by scanning forward from the rightmost located item in the - // command — that's either `word_or_name`'s end or the last suffix item - // whose location() is `Some`. Anything before the anchor is already - // accounted for by the AST; the gap between the anchor and `2>&1` is - // guaranteed to be just whitespace by the precondition that `2>&1` is - // the last suffix item and no other redirects exist. - let Some(name_loc) = name_word.loc.as_ref() else { - return; - }; - let Some(mut anchor) = byte_offset(cmd, name_loc.end.index) else { - return; - }; - for item in &suffix.0 { - if let Some(loc) = item.location() - && let Some(end) = byte_offset(cmd, loc.end.index) - { - anchor = anchor.max(end); - } - } - let bytes = cmd.as_bytes(); - let mut pos = anchor; - while pos < bytes.len() && matches!(bytes[pos], b' ' | b'\t') { - pos += 1; - } - if !cmd.get(pos..).is_some_and(|rest| rest.starts_with("2>&1")) { - return; - } - if pos == 0 { - return; - } - if !matches!(bytes[pos - 1], b' ' | b'\t') { - return; - } - // Walk back through any additional leading whitespace so the rewrite is - // contiguous with neighboring tokens. - let mut delete_start = pos - 1; - while delete_start > 0 && matches!(bytes[delete_start - 1], b' ' | b'\t') { - delete_start -= 1; - } - ranges.push((delete_start, pos + 4)); - stripped.push("2>&1".to_owned()); -} - -/// Translate a `brush-parser` source-position index into a byte offset in -/// `cmd`. The parser counts positions in Unicode scalars (one increment per -/// `char`; see `tokenizer::next_char`), but we slice `cmd` — a `&str` — by -/// byte index, so the two diverge as soon as the command contains any -/// multi-byte UTF-8 (e.g. a `✓`/`×` literal inside a `grep` pattern). Without -/// this conversion the head/tail and `2>&1` strips cut at the wrong place, -/// corrupting the command (notably orphaning a closing quote). -/// -/// Returns `None` only when `char_idx` is past the end of the input. The -/// end-of-input position (`char_idx == cmd.chars().count()`) maps to -/// `cmd.len()`. -fn byte_offset(cmd: &str, char_idx: usize) -> Option { - let mut count = 0usize; - for (byte, _) in cmd.char_indices() { - if count == char_idx { - return Some(byte); - } - count += 1; - } - (count == char_idx).then_some(cmd.len()) -} - -fn is_stderr_to_stdout(io: &IoRedirect) -> bool { - let IoRedirect::File(Some(2), IoFileRedirectKind::DuplicateOutput, target) = io else { - return false; - }; - match target { - IoFileRedirectTarget::Fd(1) => true, - IoFileRedirectTarget::Duplicate(w) => w.value == "1", - _ => false, - } -} - -fn is_safe_head_tail(c: &Command) -> bool { - let Command::Simple(simple) = c else { - return false; - }; - let Some(name) = simple.word_or_name.as_ref() else { - return false; - }; - if name.value != "head" && name.value != "tail" { - return false; - } - // Variable assignments / redirects in the prefix would change observable - // shell behavior even with `head` removed. - if let Some(prefix) = &simple.prefix - && !prefix.0.is_empty() - { - return false; - } - let Some(suffix) = &simple.suffix else { - return true; - }; - for item in &suffix.0 { - let CommandPrefixOrSuffixItem::Word(w) = item else { - return false; - }; - if !SAFE_ARG_RE.is_match(&w.value) { - return false; - } - } - true -} - -/// Token shapes that are pure "limit output" flags for `head`/`tail`: -/// `-nN`, `-n=N`, `-cN`, `-c=N` — short flag with attached value -/// `-N` — BSD-style line count -/// `-n`, `-c` — short flag (paired value comes next) -/// `-q`, `-v` — quiet/verbose -/// `--lines[=N]`, `--bytes[=N]` — long flag, optionally attached value -/// `--quiet`, `--verbose` -/// `N` — bare integer (the value half of `-n N`) -/// -/// `+N` offsets (skip-first semantics for `tail`), `-f`/`-F`/`--follow`, -/// `--help`, and any filename token are deliberately rejected — they would -/// change semantics if their host command were removed. -static SAFE_ARG_RE: LazyLock = LazyLock::new(|| { - Regex::new( - r"^(?:-[nc]=?\d+|-[nc]|-\d+|-[qv]|--lines(?:=\d+)?|--bytes(?:=\d+)?|--quiet|--verbose|\d+)$", - ) - .expect("static safe-arg regex compiles") -}); - -#[cfg(test)] -mod tests { - use super::*; - - fn run(cmd: &str) -> (String, Vec) { - let r = apply_bash_fixups(cmd); - (r.command, r.stripped) - } - - #[test] - fn strips_trailing_head_tail() { - let cases: &[(&str, &str, &[&str])] = &[ - ("ls | head", "ls", &["| head"]), - ("ls | head -5", "ls", &["| head -5"]), - ("ls | head -n 5", "ls", &["| head -n 5"]), - ("ls | head -n5", "ls", &["| head -n5"]), - ("ls | head -n=5", "ls", &["| head -n=5"]), - ("ls | head -c 100", "ls", &["| head -c 100"]), - ("ls | head --lines=20", "ls", &["| head --lines=20"]), - ("ls | head --lines 20", "ls", &["| head --lines 20"]), - ("ls | head --quiet -5", "ls", &["| head --quiet -5"]), - ("ls | tail -5", "ls", &["| tail -5"]), - ("ls | tail --bytes=200", "ls", &["| tail --bytes=200"]), - ("ls|head", "ls", &["|head"]), - ("ls | tail -20 ", "ls", &["| tail -20"]), - ("git log --oneline | head -20", "git log --oneline", &["| head -20"]), - ("echo a | tr a b | head -3", "echo a | tr a b", &["| head -3"]), - ("just build |& head -5", "just build", &["|& head -5"]), - ]; - for (input, want_cmd, want_stripped) in cases { - let (cmd, stripped) = run(input); - assert_eq!(cmd, *want_cmd, "input: {input:?}"); - assert_eq!(stripped, *want_stripped, "input: {input:?}"); - } - } - - #[test] - fn strips_with_multibyte_content() { - // brush-parser reports char-indexed positions; multi-byte UTF-8 before - // the trailing `| tail` must not shift the byte-level cut. Regression - // for a corrupted command that orphaned the grep pattern's closing - // quote (`… |✓|×-80`) and broke later re-parsing. - let cases: &[(&str, &str, &[&str])] = &[ - // Mirrors the real bug: `2>&1` sits mid-pipeline (grep becomes the - // effective tail) so only `| tail -80` is stripped, leaving the - // quoted grep pattern intact. - ( - "xcodebuild 2>&1 | grep -E \"a|✓|×|b\" | tail -80", - "xcodebuild 2>&1 | grep -E \"a|✓|×|b\"", - &["| tail -80"], - ), - ("echo ✓ | head -3", "echo ✓", &["| head -3"]), - ("printf '日本語' | tail -n 5", "printf '日本語'", &["| tail -n 5"]), - ]; - for (input, want_cmd, want_stripped) in cases { - let (cmd, stripped) = run(input); - assert_eq!(cmd, *want_cmd, "input: {input:?}"); - assert_eq!(stripped, *want_stripped, "input: {input:?}"); - // The rewrite must remain valid shell (no orphaned quote). - let options = ParserOptions::default(); - let source_info = SourceInfo::default(); - let mut reader = BufReader::new(cmd.as_bytes()); - let mut parser = Parser::new(&mut reader, &options, &source_info); - assert!(parser.parse_program().is_ok(), "rewrite not re-parseable: {cmd:?}"); - } - } - - #[test] - fn strips_2to1_after_multibyte() { - // Multi-byte before a trailing `2>&1` must not misplace the strip. - let (cmd, stripped) = run("echo ✓ × 2>&1"); - assert_eq!(cmd, "echo ✓ ×"); - assert_eq!(stripped, vec!["2>&1"]); - } - - #[test] - fn strips_redundant_2to1() { - let cases: &[(&str, &str, &[&str])] = &[ - ("cmd 2>&1", "cmd", &["2>&1"]), - ("just build 2>&1", "just build", &["2>&1"]), - ("just build 2>&1 | tail -3", "just build", &["| tail -3", "2>&1"]), - ("cargo build 2>&1 | head -50", "cargo build", &["| head -50", "2>&1"]), - ]; - for (input, want_cmd, want_stripped) in cases { - let (cmd, stripped) = run(input); - assert_eq!(cmd, *want_cmd, "input: {input:?}"); - assert_eq!(stripped, *want_stripped, "input: {input:?}"); - } - } - - #[test] - fn strips_across_compound_commands() { - let cases: &[(&str, &str, &[&str])] = &[ - ( - "just build 2>&1 | tail -3 && just up && sleep 4 && just healthz", - "just build && just up && sleep 4 && just healthz", - &["| tail -3", "2>&1"], - ), - ("cmd1 | head -5 && cmd2 && cmd3 | tail -3", "cmd1 && cmd2 && cmd3", &[ - "| head -5", - "| tail -3", - ]), - ("echo a; cmd | head -5; echo b", "echo a; cmd; echo b", &["| head -5"]), - ("cmd | head -5 || fallback | tail -3", "cmd || fallback", &["| head -5", "| tail -3"]), - ("cmd1 | head -5 && cmd2 2>&1 | grep err", "cmd1 && cmd2 2>&1 | grep err", &["| head -5"]), - ]; - for (input, want_cmd, want_stripped) in cases { - let (cmd, stripped) = run(input); - assert_eq!(cmd, *want_cmd, "input: {input:?}"); - assert_eq!(stripped, *want_stripped, "input: {input:?}"); - } - } - - #[test] - fn preserves_semantics_bearing_pipelines() { - let untouched: &[&str] = &[ - "tail -f /var/log/system.log", - "tail -F file.log", - "ls | tail -f -", - "ls | head -5 | sort", - "cat file | head -5 | wc -l", - "cat file | tail -n +2", - "cat file | tail +5", - "ls | head -5 > /tmp/out.txt", - "ls | head -5 2>/dev/null", - "echo \"ls | head -5\"", - "echo $(ls | head -5)", - "head -5 file.txt", - "head /etc/hosts", - "head -5", - "cmd 2>&1 | grep err", - "cmd > file 2>&1", - "cmd >& file", - "cmd 2>&1 > file", - "for f in *.txt; do\n echo $f\ndone | head -5", - "cat <&1`, - extracts a leading single-line `cd && ...` into `cwd` when `cwd` was not supplied, - rejects `async: true` when `async.enabled` is false. -There are no structured `head` or `tail` tool parameters in the current schema. Output limiting is handled by `OutputSink` truncation/artifacts, and the optional trailing-pipe fixup exists to avoid hiding output before the harness can capture it. +There are no structured `head` or `tail` tool parameters in the current schema, and commands run exactly as written — no pre-execution rewrites. Output limiting is handled by `OutputSink` truncation/artifacts. ## 2) Optional interception (blocked-command path) @@ -275,7 +274,6 @@ This component is wired by `CommandController.handleBashCommand()` and fed from - [`src/tools/bash.ts`](../packages/coding-agent/src/tools/bash.ts) — tool entrypoint, input handling/interception, async and PTY/non-PTY selection, result/error mapping, bash tool renderer. - [`src/tools/bash-pty-selection.ts`](../packages/coding-agent/src/tools/bash-pty-selection.ts) — `canUseInteractiveBashPty` predicate for choosing the local PTY overlay. -- [`src/tools/bash-command-fixup.ts`](../packages/coding-agent/src/tools/bash-command-fixup.ts) — native-backed conservative cleanup for trailing `head`/`tail` pipes and redundant `2>&1`. - [`src/tools/bash-interceptor.ts`](../packages/coding-agent/src/tools/bash-interceptor.ts) — interceptor rule matching and blocked-command messages. - [`src/exec/bash-executor.ts`](../packages/coding-agent/src/exec/bash-executor.ts) — non-PTY executor, shell session reuse, cancellation wiring, output sink integration. - [`src/exec/non-interactive-env.ts`](../packages/coding-agent/src/exec/non-interactive-env.ts) — non-interactive child-process env defaults (`buildNonInteractiveEnv`) used by the non-PTY executor. diff --git a/docs/settings.md b/docs/settings.md index a60a56daf..655c41ee7 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -431,7 +431,6 @@ Individual built-in tools are toggled by their own keys, e.g. `bash.enabled`, `e ```yaml bash: enabled: true - stripTrailingHeadTail: true autoBackground: enabled: false thresholdMs: 60000 @@ -455,7 +454,6 @@ lsp: | Key | Type | Default | Notes | |---|---|---|---| | `bash.enabled` | boolean | `true` | Enable the bash tool. | -| `bash.stripTrailingHeadTail` | boolean | `true` | Strip trailing head/tail noise from output. | | `bash.autoBackground.enabled` | boolean | `false` | Auto-background long-running commands. | | `bash.autoBackground.thresholdMs` | number | `60000` | Threshold before auto-backgrounding. | | `eval.py` | boolean | `true` | Python eval backend. `PI_PY=0` disables for the process. | diff --git a/docs/tools/bash.md b/docs/tools/bash.md index 5942af343..6adf40838 100644 --- a/docs/tools/bash.md +++ b/docs/tools/bash.md @@ -9,7 +9,6 @@ - `packages/coding-agent/src/tools/bash-interactive.ts` — PTY/TUI execution path. - `packages/coding-agent/src/tools/bash-interceptor.ts` — blocks tool-better shell patterns. - `packages/coding-agent/src/tools/bash-skill-urls.ts` — expands internal URLs to paths. - - `packages/coding-agent/src/tools/bash-command-fixup.ts` — strips trailing `| head`/`| tail` pipes and redundant `2>&1` (thin wrapper over native `pi_shell::fixup`). - `packages/coding-agent/src/tools/bash-pty-selection.ts` — `canUseInteractiveBashPty()` decides whether a call may use the local PTY overlay. - `packages/coding-agent/src/tools/gh-cache-invalidation.ts` — drops `github-cache` rows for mutating `gh issue`/`gh pr` subcommands. - `packages/coding-agent/src/exec/bash-executor.ts` — non-PTY shell execution. @@ -54,7 +53,7 @@ The tool returns a single `text` content block plus optional `details`. Stdout and stderr are merged before the model sees them. Definite non-zero exit codes are appended to the returned error result text as `Command exited with code `. ## Flow -1. `BashTool.execute()` in `packages/coding-agent/src/tools/bash.ts` reads `command`, normalizes `env`, and defaults `timeout` to `300`. When `bash.stripTrailingHeadTail` is enabled (default), `applyBashFixups()` from `packages/coding-agent/src/tools/bash-command-fixup.ts` first strips safe trailing `| head`/`| tail` pipes and redundant trailing `2>&1` from single-line commands. +1. `BashTool.execute()` in `packages/coding-agent/src/tools/bash.ts` reads `command`, normalizes `env`, and defaults `timeout` to `300`. Commands execute exactly as written — there is no pre-execution rewrite pass. 2. If `cwd` is absent, it rewrites a leading `cd && ...` into the structured `cwd` field and strips that prefix from `command`. 3. If `async: true` is requested while `async.enabled` is off, it throws `ToolError` before any execution. 4. If `bashInterceptor.enabled` is on, `checkBashInterception()` runs against both the original command and the `cd`-stripped command. A matching enabled rule throws before URL expansion or execution. diff --git a/packages/coding-agent/src/tools/bash-command-fixup.ts b/packages/coding-agent/src/tools/bash-command-fixup.ts deleted file mode 100644 index 53abb8187..000000000 --- a/packages/coding-agent/src/tools/bash-command-fixup.ts +++ /dev/null @@ -1,4 +0,0 @@ -/** Compatibility shim for extensions importing the historical bash fixup helper. */ - -export type { BashFixupResult } from "@oh-my-pi/pi-natives"; -export { applyBashFixups } from "@oh-my-pi/pi-natives";