fix(coding-agent/tools): resolved bash fixup parsing for head/tail chunks
- Added top-level parsing and segment splitting to apply bash fixups only on safe command chunks. - Replaced `stripTrailingHeadTail` usage with `applyBashFixups` and array-based notice formatting. - Fixed terminal `| head`/`| tail` and redundant `2>&1` stripping while preserving command semantics. - Updated fixup tests for cross-command cases and removed superseded head-tail-only test coverage.
This commit is contained in:
@@ -13,7 +13,9 @@ use pi_shell::{
|
||||
MinimizerResult as CoreMinimizerResult, Shell as CoreShell,
|
||||
ShellExecuteOptions as CoreShellExecuteOptions, ShellOptions as CoreShellOptions,
|
||||
ShellRunOptions as CoreShellRunOptions, ShellRunResult as CoreShellRunResult,
|
||||
execute_shell as core_execute_shell, minimizer,
|
||||
execute_shell as core_execute_shell,
|
||||
fixup::{BashFixupResult as CoreBashFixupResult, apply_bash_fixups as core_apply_bash_fixups},
|
||||
minimizer,
|
||||
};
|
||||
|
||||
use crate::task;
|
||||
@@ -285,6 +287,32 @@ fn bridge_chunks(
|
||||
(Some(tx), Some(handle))
|
||||
}
|
||||
|
||||
/// 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<String>,
|
||||
}
|
||||
|
||||
impl From<CoreBashFixupResult> 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;
|
||||
|
||||
@@ -0,0 +1,448 @@
|
||||
//! 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::*};
|
||||
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<String>,
|
||||
}
|
||||
|
||||
/// Apply the bash fixups to `cmd`. See module docs for full rules.
|
||||
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<String> = 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<String>,
|
||||
) {
|
||||
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<String>,
|
||||
) {
|
||||
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<String>,
|
||||
) -> 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 last_start = last_loc.start.index;
|
||||
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_loc.end.index].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_loc.end.index));
|
||||
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<String>,
|
||||
) {
|
||||
// `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 mut anchor = name_loc.end.index;
|
||||
for item in &suffix.0 {
|
||||
if let Some(loc) = item.location() {
|
||||
anchor = anchor.max(loc.end.index);
|
||||
}
|
||||
}
|
||||
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());
|
||||
}
|
||||
|
||||
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<Regex> = 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<String>) {
|
||||
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_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 <<EOF | head -5\ncontent\nEOF",
|
||||
"ls\nls | head -5",
|
||||
"echo \"unterminated | head -5",
|
||||
];
|
||||
for input in untouched {
|
||||
let (cmd, stripped) = run(input);
|
||||
assert_eq!(cmd, *input, "input: {input:?}");
|
||||
assert!(stripped.is_empty(), "input: {input:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
pub mod cancel;
|
||||
pub mod minimizer;
|
||||
pub mod fixup;
|
||||
pub mod process;
|
||||
pub mod shell;
|
||||
#[cfg(windows)]
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added the `set_host_uri_schemes` RPC command so hosts can register and replace writable/read-only internal URI schemes with scheme metadata (`writable`, `immutable`) at runtime
|
||||
@@ -11,10 +12,15 @@
|
||||
|
||||
### Changed
|
||||
|
||||
- Changed bash command preprocessing to strip trailing `| head` and `| tail` pipelines (including `|&`) from each top-level segment in command chains separated by `;`, `&&`, `||`, or `&`
|
||||
- Changed bash fixup notices to state that stderr is already merged into stdout and to reflect that fixes were applied for multiple stripped segments when several transforms fire
|
||||
- Changed shell-minimizer per-line truncation marker from a bare `…` to `…[+N]`, where `N` is the count of dropped Unicode scalars. The bracketed tally disambiguates minimizer-driven cuts from genuine `…` characters in the source (paths, JSON, stack traces, etc.) and gives the agent an exact count so it can decide whether the missing tail is recoverable inline or warrants reading the `[raw output: artifact://<id>]` footer the bash wrapper already emits when the minimizer rewrites output. Affects pipeline Stage 5 (`truncate_lines_at` in `defs/*.toml`) and the internal callers in `filters/git.rs`, `filters/listing.rs`, and `filters/lint.rs`. ([#1046](https://github.com/can1357/oh-my-pi/issues/1046))
|
||||
- Changed bash command preprocessing to use the real `brush-parser` AST via `pi-natives` `applyBashFixups` instead of a hand-rolled top-level mask scanner. The previous regex/character-walking implementation reimplemented quote/heredoc/`$(...)` tracking with conservative bail-outs (notably refusing to fixup commands containing here-strings); the AST-driven version inherits the full shell parser, so semantics-preserving rewrites like stripping `| head -5` off `cat <<<'content' | head -5` now succeed instead of being skipped. No public API change — `applyBashFixups(command)` returns the same `{ command, stripped }` shape.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed bash command fixups to remove a redundant standalone trailing `2>&1` redirect when no other pipe or redirection remains
|
||||
- Fixed command-fixup notices to list all stripped segments instead of reporting only one
|
||||
- Fixed summarized `read` output stalling agents on elided regions by appending an explicit footer like `[NN lines across MM elided regions; read <path>:raw or a line range like <path>:1-9999 for verbatim content]`. The footer fires whenever the structural summarizer elided at least one span, so the model gets a concrete recovery selector instead of having to guess from a bare `...` / `{ .. }` marker. Surfaces `elidedLines` on `ReadToolDetails.summary` alongside the existing `elidedSpans`. ([#1046](https://github.com/can1357/oh-my-pi/issues/1046))
|
||||
- Updated the `read` tool prompt to describe the new elision footer and instruct the model to follow `:raw` (or an explicit line range) when the elided body is actually needed, rather than guessing.
|
||||
- Fixed plugin extensions failing to load when their `peerDependencies` reference internal `pi-*` packages under any scope other than `@mariozechner` (e.g. `Cannot find module '@earendil-works/pi-tui'` from `@juicesharp/rpiv-ask-user-question`, or `Cannot find module '@oh-my-pi/pi-utils'` from `@oh-my-pi/swarm-extension`). The legacy-pi specifier shim now treats `@mariozechner`, `@earendil-works`, **and** the canonical `@oh-my-pi` itself as aliases for the same set of bundled in-process packages (`pi-agent-core`, `pi-ai`, `pi-coding-agent`, `pi-natives`, `pi-tui`, `pi-utils`), and additionally rewrites the upstream-only `pi-ai/oauth` subpath onto our `pi-ai/utils/oauth` layout. Restored the `Key` runtime helper export on `@oh-my-pi/pi-tui` to match upstream — plugins using `Key.enter` / `Key.ctrl("c")` (e.g. `@plannotator/pi-extension`, `@juicesharp/rpiv-ask-user-question`) no longer fail with `Export named 'Key' not found`. End-to-end verified against `@juicesharp/rpiv-ask-user-question`, `@oh-my-pi/swarm-extension`, and `@plannotator/pi-extension` — each now loads cleanly with all of its tools/commands/handlers registered. Plugins importing any of those scopes are remapped to the omp binary's own copy at load time, so peer deps are no longer dragged in from npm and there is exactly one module instance per package regardless of which scope name the plugin's manifest happened to declare.
|
||||
|
||||
@@ -1,78 +1,47 @@
|
||||
/**
|
||||
* Conservative transforms applied to a bash command before execution.
|
||||
*
|
||||
* Currently strips trailing `| head [args]` / `| tail [args]` pipelines that
|
||||
* exist purely to limit output length: the harness already truncates bash
|
||||
* output and exposes the full result via an artifact, so these pipes only
|
||||
* hide content the agent wanted. We refuse to strip in any case where the
|
||||
* pipe could carry real semantics (multi-line scripts, follow flags, file
|
||||
* arguments, downstream commands, redirects, subshells, etc.).
|
||||
* Two fixups are applied, each anchored to the end of a top-level segment
|
||||
* (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 they only
|
||||
* hide content the agent wanted.
|
||||
*
|
||||
* 2. A redundant trailing `2>&1` left 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 heavy lifting (tokenization, quoting, heredoc handling, command
|
||||
* substitution, nested compound commands) lives in Rust under
|
||||
* `pi_shell::fixup`, driven by the real `brush-parser` AST. This module is a
|
||||
* thin sync wrapper plus user-facing notice formatting.
|
||||
*/
|
||||
import { applyBashFixups as nativeApplyBashFixups } from "@oh-my-pi/pi-natives";
|
||||
|
||||
export interface BashFixupResult {
|
||||
/** Possibly-rewritten command. */
|
||||
command: string;
|
||||
/** Original substring that was removed, if any (verbatim, including the leading `|`). */
|
||||
stripped?: string;
|
||||
/** Substrings that were stripped, in the order they were removed. */
|
||||
stripped: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Token shapes for `head`/`tail` that we recognize as pure "limit output" flags.
|
||||
*
|
||||
* We deliberately reject `-f`, `-F`, `--follow`, `+N` line offsets, filenames,
|
||||
* and anything else that could change semantics when removed.
|
||||
*
|
||||
* -nN, -n N, -n=N, -cN, -c N, -c=N
|
||||
* -N (BSD-style `head -5`)
|
||||
* -q, -v, --quiet, --verbose
|
||||
* --lines[=N| N], --bytes[=N| N]
|
||||
* bare integer (the value half of `--lines 5` / `-n 5`)
|
||||
* Apply both fixups to a bash command. On any parse failure, multi-line input,
|
||||
* or no-op transform, returns the input verbatim with `stripped: []`.
|
||||
*/
|
||||
const SAFE_HEAD_TAIL_ARG = String.raw`(?:-[nc]=?\s*\d+|-\d+|-[qv]|--lines(?:=?\s*\d+)?|--bytes(?:=?\s*\d+)?|--quiet|--verbose|\d+)`;
|
||||
|
||||
/**
|
||||
* Matches a trailing `| head|tail [safe-args]` segment anchored to the end of
|
||||
* the command. The leading `\s*` is bounded to inline whitespace (no newline)
|
||||
* so a `|` on its own line never gets swallowed.
|
||||
*/
|
||||
const TRAILING_HEAD_TAIL_RE = new RegExp(
|
||||
String.raw`[ \t]*\|[ \t]*(?:head|tail)(?:[ \t]+${SAFE_HEAD_TAIL_ARG})*[ \t]*$`,
|
||||
);
|
||||
|
||||
/**
|
||||
* Strip a trailing `| head` / `| tail` from a single-line bash command.
|
||||
*
|
||||
* Bail-out conditions (all preserve the original verbatim):
|
||||
* - command contains any newline (multi-line scripts may legitimately end a
|
||||
* pipeline with `head`/`tail` to bound a generator);
|
||||
* - the matched segment is not the entire command (we never reduce a command
|
||||
* to an empty string);
|
||||
* - the `head`/`tail` carries any flag we don't recognize (e.g. `-f`, `-F`,
|
||||
* `+N`, filenames, redirects) — the regex simply won't match.
|
||||
*/
|
||||
export function stripTrailingHeadTail(command: string): BashFixupResult {
|
||||
// Single-line guard. We check the raw string for any newline anywhere, not
|
||||
// just at the boundary, because shell continuations, heredocs, function
|
||||
// bodies, and `for ... done | head` blocks all live behind a newline.
|
||||
if (command.includes("\n")) return { command };
|
||||
|
||||
const match = TRAILING_HEAD_TAIL_RE.exec(command);
|
||||
if (!match || match.index === undefined) return { command };
|
||||
|
||||
const remainder = command.slice(0, match.index).replace(/[ \t]+$/, "");
|
||||
// Never reduce the command to nothing — that would execute as a no-op and
|
||||
// almost certainly indicates a false-positive match (e.g. the LLM wrote
|
||||
// `head -5` standalone hoping to read its own stdin).
|
||||
if (remainder === "") return { command };
|
||||
|
||||
return { command: remainder, stripped: match[0].trim() };
|
||||
export function applyBashFixups(command: string): BashFixupResult {
|
||||
return nativeApplyBashFixups(command);
|
||||
}
|
||||
|
||||
/**
|
||||
* Human-readable notice for the stripped segment. Mirrors the shape of
|
||||
* Human-readable notice for the fixups that fired. Mirrors the shape of
|
||||
* `formatTimeoutClampNotice` so it can ride alongside the other bash notices.
|
||||
*/
|
||||
export function formatHeadTailStripNotice(stripped: string | undefined): string | undefined {
|
||||
if (!stripped) return undefined;
|
||||
return `Stripped trailing \`${stripped}\` — bash output is truncated automatically and the full result is available via \`artifact://<id>\`.`;
|
||||
export function formatBashFixupNotice(stripped: readonly string[]): string | undefined {
|
||||
if (!stripped.length) return undefined;
|
||||
const quoted = stripped.map(s => `\`${s}\``).join(", ");
|
||||
return `Stripped redundant ${quoted} — bash output is truncated automatically and stderr is already merged into stdout.`;
|
||||
}
|
||||
|
||||
@@ -17,7 +17,7 @@ import { renderStatusLine } from "../tui";
|
||||
import { CachedOutputBlock } from "../tui/output-block";
|
||||
import { getSixelLineMask } from "../utils/sixel";
|
||||
import type { ToolSession } from ".";
|
||||
import { formatHeadTailStripNotice, stripTrailingHeadTail } from "./bash-command-fixup";
|
||||
import { applyBashFixups, formatBashFixupNotice } from "./bash-command-fixup";
|
||||
import { type BashInteractiveResult, runInteractiveBashPty } from "./bash-interactive";
|
||||
import { checkBashInterception } from "./bash-interceptor";
|
||||
import { expandInternalUrls, type InternalUrlExpansionOptions } from "./bash-skill-urls";
|
||||
@@ -292,7 +292,7 @@ export class BashTool implements AgentTool<BashToolSchema, BashToolDetails> {
|
||||
#buildCompletedResult(
|
||||
result: BashResult | BashInteractiveResult,
|
||||
timeoutSec: number,
|
||||
options: { requestedTimeoutSec?: number; notices?: string[]; terminalId?: string } = {},
|
||||
options: { requestedTimeoutSec?: number; notices?: readonly string[]; terminalId?: string } = {},
|
||||
): AgentToolResult<BashToolDetails> {
|
||||
const outputLines = [this.#formatResultOutput(result)];
|
||||
const notices = options.notices?.filter(Boolean) ?? [];
|
||||
@@ -315,7 +315,7 @@ export class BashTool implements AgentTool<BashToolSchema, BashToolDetails> {
|
||||
label: string,
|
||||
previewText: string,
|
||||
timeoutSec: number,
|
||||
options: { requestedTimeoutSec?: number; notices?: string[] } = {},
|
||||
options: { requestedTimeoutSec?: number; notices?: readonly string[] } = {},
|
||||
): AgentToolResult<BashToolDetails> {
|
||||
const details: BashToolDetails = {
|
||||
timeoutSeconds: timeoutSec,
|
||||
@@ -484,15 +484,15 @@ export class BashTool implements AgentTool<BashToolSchema, BashToolDetails> {
|
||||
let command = rawCommand;
|
||||
const env = normalizeBashEnv(rawEnv);
|
||||
|
||||
// Drop trailing `| head|tail` pipes that exist purely to limit output —
|
||||
// the harness already truncates bash output. Single-line only; the helper
|
||||
// refuses anything that could change semantics.
|
||||
let headTailStripped: string | undefined;
|
||||
// Apply conservative bash fixups (strip trailing `| head|tail` and redundant
|
||||
// `2>&1`). The helper is single-line only and refuses anything that could
|
||||
// change semantics.
|
||||
let bashFixups: string[] = [];
|
||||
if (this.session.settings.get("bash.stripTrailingHeadTail")) {
|
||||
const fixup = stripTrailingHeadTail(command);
|
||||
if (fixup.stripped) {
|
||||
const fixup = applyBashFixups(command);
|
||||
if (fixup.stripped.length > 0) {
|
||||
command = fixup.command;
|
||||
headTailStripped = fixup.stripped;
|
||||
bashFixups = fixup.stripped;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -574,8 +574,8 @@ export class BashTool implements AgentTool<BashToolSchema, BashToolDetails> {
|
||||
const pendingNotices: string[] = [];
|
||||
const timeoutClampNotice = formatTimeoutClampNotice(requestedTimeoutSec, timeoutSec);
|
||||
if (timeoutClampNotice) pendingNotices.push(timeoutClampNotice);
|
||||
const headTailStripNotice = formatHeadTailStripNotice(headTailStripped);
|
||||
if (headTailStripNotice) pendingNotices.push(headTailStripNotice);
|
||||
const bashFixupNotice = formatBashFixupNotice(bashFixups);
|
||||
if (bashFixupNotice) pendingNotices.push(bashFixupNotice);
|
||||
|
||||
if (asyncRequested) {
|
||||
if (!AsyncJobManager.instance()) {
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { applyBashFixups, type BashFixupResult, formatBashFixupNotice } from "../../src/tools/bash-command-fixup";
|
||||
|
||||
function fixup(command: string): BashFixupResult {
|
||||
return applyBashFixups(command);
|
||||
}
|
||||
|
||||
describe("applyBashFixups — strips harmless trailing head/tail", () => {
|
||||
const cases: Array<[string, string, string[]]> = [
|
||||
// [input, expected command, expected stripped list]
|
||||
["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", "ls", ["| tail"]],
|
||||
["ls | tail -5", "ls", ["| tail -5"]],
|
||||
["ls | tail -n 5", "ls", ["| tail -n 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"]],
|
||||
// `|&` (pipe stdout+stderr) is recognized as a pipe too.
|
||||
["just build |& head -5", "just build", ["|& head -5"]],
|
||||
];
|
||||
|
||||
for (const [input, expectedCommand, expectedStripped] of cases) {
|
||||
it(`strips: ${input}`, () => {
|
||||
const out = fixup(input);
|
||||
expect(out.command).toBe(expectedCommand);
|
||||
expect(out.stripped).toEqual(expectedStripped);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe("applyBashFixups — strips redundant 2>&1", () => {
|
||||
const cases: Array<[string, string, string[]]> = [
|
||||
["cmd 2>&1", "cmd", ["2>&1"]],
|
||||
["just build 2>&1", "just build", ["2>&1"]],
|
||||
// Combined: trailing `| tail -3` then leftover `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 (const [input, expectedCommand, expectedStripped] of cases) {
|
||||
it(`strips: ${input}`, () => {
|
||||
const out = fixup(input);
|
||||
expect(out.command).toBe(expectedCommand);
|
||||
expect(out.stripped).toEqual(expectedStripped);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe("applyBashFixups — strips across compound commands", () => {
|
||||
const cases: Array<[string, string, string[]]> = [
|
||||
[
|
||||
"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"]],
|
||||
// Only the head/tail-bearing segment gets touched; cmd2's stderr merge survives.
|
||||
["cmd1 | head -5 && cmd2 2>&1 | grep err", "cmd1 && cmd2 2>&1 | grep err", ["| head -5"]],
|
||||
];
|
||||
|
||||
for (const [input, expectedCommand, expectedStripped] of cases) {
|
||||
it(`strips: ${input}`, () => {
|
||||
const out = fixup(input);
|
||||
expect(out.command).toBe(expectedCommand);
|
||||
expect(out.stripped).toEqual(expectedStripped);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe("applyBashFixups — preserves semantics-bearing pipelines", () => {
|
||||
const untouched: string[] = [
|
||||
// follow-mode and file readers
|
||||
"tail -f /var/log/system.log",
|
||||
"tail -F file.log",
|
||||
"ls | tail -f -",
|
||||
// non-trailing head/tail
|
||||
"ls | head -5 | sort",
|
||||
"cat file | head -5 | wc -l",
|
||||
// +N offset (skip-first semantics, not a limit)
|
||||
"cat file | tail -n +2",
|
||||
"cat file | tail +5",
|
||||
// redirects on head's output
|
||||
"ls | head -5 > /tmp/out.txt",
|
||||
"ls | head -5 2>/dev/null",
|
||||
// inside a string / subshell — top-level end is `"` or `)`
|
||||
'echo "ls | head -5"',
|
||||
"echo $(ls | head -5)",
|
||||
// no `|` at all
|
||||
"head -5 file.txt",
|
||||
"head /etc/hosts",
|
||||
// would reduce to empty
|
||||
"| head -5",
|
||||
"head -5",
|
||||
// 2>&1 with other redirects or piped consumer — must stay
|
||||
"cmd 2>&1 | grep err",
|
||||
"cmd > file 2>&1",
|
||||
"cmd >& file",
|
||||
"cmd 2>&1 > file",
|
||||
// bail-outs: multi-line / heredoc / unbalanced quotes
|
||||
"for f in *.txt; do\n echo $f\ndone | head -5",
|
||||
"cat <<EOF | head -5\ncontent\nEOF",
|
||||
"ls\nls | head -5",
|
||||
'echo "unterminated | head -5',
|
||||
];
|
||||
|
||||
for (const input of untouched) {
|
||||
it(`leaves alone: ${JSON.stringify(input)}`, () => {
|
||||
const out = fixup(input);
|
||||
expect(out.command).toBe(input);
|
||||
expect(out.stripped).toEqual([]);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe("formatBashFixupNotice", () => {
|
||||
it("returns undefined when nothing was stripped", () => {
|
||||
expect(formatBashFixupNotice([])).toBeUndefined();
|
||||
});
|
||||
|
||||
it("embeds a single stripped segment in the notice", () => {
|
||||
const notice = formatBashFixupNotice(["| head -5"]);
|
||||
expect(notice).toContain("`| head -5`");
|
||||
expect(notice).toContain("stderr is already merged");
|
||||
});
|
||||
|
||||
it("joins multiple stripped segments with commas", () => {
|
||||
const notice = formatBashFixupNotice(["| tail -3", "2>&1"]);
|
||||
expect(notice).toContain("`| tail -3`");
|
||||
expect(notice).toContain("`2>&1`");
|
||||
expect(notice).toMatch(/`\| tail -3`,\s*`2>&1`/);
|
||||
});
|
||||
});
|
||||
@@ -1,100 +0,0 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import {
|
||||
type BashFixupResult,
|
||||
formatHeadTailStripNotice,
|
||||
stripTrailingHeadTail,
|
||||
} from "../../src/tools/bash-command-fixup";
|
||||
|
||||
function strip(command: string): BashFixupResult {
|
||||
return stripTrailingHeadTail(command);
|
||||
}
|
||||
|
||||
describe("stripTrailingHeadTail — strips harmless trailing limits", () => {
|
||||
const cases: Array<[string, string, string]> = [
|
||||
// [input, expected command, expected stripped suffix]
|
||||
["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", "ls", "| tail"],
|
||||
["ls | tail -5", "ls", "| tail -5"],
|
||||
["ls | tail -n 5", "ls", "| tail -n 5"],
|
||||
["ls | tail --bytes=200", "ls", "| tail --bytes=200"],
|
||||
["ls|head", "ls", "|head"],
|
||||
["ls | tail -20 ", "ls", "| tail -20"],
|
||||
// cd/sub-pipeline preserved; only the trailing limit goes
|
||||
["git log --oneline | head -20", "git log --oneline", "| head -20"],
|
||||
["echo a | tr a b | head -3", "echo a | tr a b", "| head -3"],
|
||||
// command with stderr redirect before the limit stays intact
|
||||
["cargo build 2>&1 | head -50", "cargo build 2>&1", "| head -50"],
|
||||
];
|
||||
|
||||
for (const [input, expectedCommand, expectedStripped] of cases) {
|
||||
it(`strips: ${input}`, () => {
|
||||
const out = strip(input);
|
||||
expect(out.command).toBe(expectedCommand);
|
||||
expect(out.stripped).toBe(expectedStripped);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe("stripTrailingHeadTail — preserves semantics-bearing pipelines", () => {
|
||||
const untouched: string[] = [
|
||||
// follow-mode and file readers
|
||||
"tail -f /var/log/system.log",
|
||||
"tail -F file.log",
|
||||
"ls | tail -f -",
|
||||
// non-trailing head/tail
|
||||
"ls | head -5 | sort",
|
||||
"cat file | head -5 | wc -l",
|
||||
// +N offset (skip-first semantics, not a limit)
|
||||
"cat file | tail -n +2",
|
||||
"cat file | tail +5",
|
||||
// downstream commands / operators
|
||||
"ls | head -5 && echo done",
|
||||
"ls | head -5 || echo failed",
|
||||
"ls | head -5 ; echo done",
|
||||
"ls | head -5 &",
|
||||
// redirects on head's output
|
||||
"ls | head -5 > /tmp/out.txt",
|
||||
"ls | head -5 2>/dev/null",
|
||||
// inside a string / subshell — anchored end is `"` or `)`
|
||||
'echo "ls | head -5"',
|
||||
"echo $(ls | head -5)",
|
||||
// no `|` at all
|
||||
"head -5 file.txt",
|
||||
"head /etc/hosts",
|
||||
// would reduce to empty
|
||||
"| head -5",
|
||||
"head -5",
|
||||
// multiline scripts: head bounds a loop body, must stay
|
||||
"for f in *.txt; do\n echo $f\ndone | head -5",
|
||||
"cat <<EOF | head -5\ncontent\nEOF",
|
||||
"ls\nls | head -5",
|
||||
];
|
||||
|
||||
for (const input of untouched) {
|
||||
it(`leaves alone: ${JSON.stringify(input)}`, () => {
|
||||
const out = strip(input);
|
||||
expect(out.command).toBe(input);
|
||||
expect(out.stripped).toBeUndefined();
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe("formatHeadTailStripNotice", () => {
|
||||
it("returns undefined when nothing was stripped", () => {
|
||||
expect(formatHeadTailStripNotice(undefined)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("embeds the stripped segment in the notice", () => {
|
||||
const notice = formatHeadTailStripNotice("| head -5");
|
||||
expect(notice).toContain("| head -5");
|
||||
expect(notice).toContain("artifact://");
|
||||
});
|
||||
});
|
||||
@@ -88,7 +88,7 @@ describe("BashTool head/tail stripping", () => {
|
||||
} as AgentToolContext);
|
||||
const text = result.content.find(b => b.type === "text")?.text ?? "";
|
||||
expect(text).toContain("100");
|
||||
expect(text).toContain("Stripped trailing `| head -3`");
|
||||
expect(text).toContain("Stripped redundant `| head -3`");
|
||||
});
|
||||
|
||||
it("does not strip when the setting is disabled", async () => {
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
### Added
|
||||
|
||||
- Added a per-release version sentinel napi export (`__piNativesV{major}_{minor}_{patch}`). The Rust `js_name` is bumped in lock-step with the package version by `scripts/release.ts`; the JS loader computes the expected name from `package.json#version` and throws an actionable error when the on-disk `.node` doesn't expose it. This converts the silent `<sym> is not a function` crash from a stale addon into a load-time failure pointing at the real fix.
|
||||
- Added `applyBashFixups(command)` — a synchronous brush-parser-driven rewrite that strips trailing `| head|tail …`, redundant `2>&1`, and the `|&` shorthand from top-level pipelines, returning `{ command, stripped }`. Replaces the hand-rolled top-level mask scanner in `pi-coding-agent`; tokenization, quoting, heredocs, command substitution, and nested compound commands are now handled by the real shell AST instead of regex/character-walking. Lives in `pi_shell::fixup` on the Rust side.
|
||||
|
||||
### Fixed
|
||||
|
||||
|
||||
Vendored
+20
@@ -138,6 +138,15 @@ export declare class Shell {
|
||||
*/
|
||||
export declare function __piNativesV15_0_1(): void
|
||||
|
||||
/**
|
||||
* 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).
|
||||
*/
|
||||
export declare function applyBashFixups(command: string): BashFixupResult
|
||||
|
||||
/**
|
||||
* Apply ast-grep rewrite rules to matching files; honors `dryRun` and returns
|
||||
* a promise.
|
||||
@@ -324,6 +333,17 @@ export interface AstReplaceResult {
|
||||
parseErrors?: Array<string>
|
||||
}
|
||||
|
||||
/**
|
||||
* Result of [`apply_bash_fixups`]: a possibly-rewritten command plus the
|
||||
* substrings that were removed (in source order).
|
||||
*/
|
||||
export interface BashFixupResult {
|
||||
/** Possibly-rewritten command. Equal to the input when no fixup fired. */
|
||||
command: string
|
||||
/** Substrings removed, in source order — suitable for a user-facing notice. */
|
||||
stripped: Array<string>
|
||||
}
|
||||
|
||||
/** Clipboard image payload encoded as PNG bytes. */
|
||||
export interface ClipboardImage {
|
||||
/** PNG-encoded image bytes. */
|
||||
|
||||
@@ -24,6 +24,7 @@ export const Shell = nativeBindings.Shell;
|
||||
|
||||
// functions
|
||||
export const __piNativesV15_0_1 = nativeBindings.__piNativesV15_0_1;
|
||||
export const applyBashFixups = nativeBindings.applyBashFixups;
|
||||
export const astEdit = nativeBindings.astEdit;
|
||||
export const astGrep = nativeBindings.astGrep;
|
||||
export const copyToClipboard = nativeBindings.copyToClipboard;
|
||||
|
||||
Reference in New Issue
Block a user