Merge branch 'main' into farm/dc17c3a2/omp-commit-doesn-t-exit

This commit is contained in:
Can Bölük
2026-05-15 05:04:24 +02:00
committed by GitHub
49 changed files with 1761 additions and 256 deletions
+63
View File
@@ -0,0 +1,63 @@
# Heavy build outputs — must never reach the build context. `target/` alone is
# >100 GB on a dev machine.
target/
node_modules/
dist/
runs/
# Per-host scratch the pi codebase uses for parallel agents / worktrees.
.fallow/
.worktrees/
.wt/
.opencode/
.pi_config/
.omp/plugins/
# VCS, editors, IDEs — irrelevant to the build, churn on every IDE keystroke.
.git/
.npm/
.vscode/
.zed/
.idea/
# OS + transient noise. Finder rewrites .DS_Store whenever you peek at a
# folder; profilers drop `CPU.*` blobs at random times. Letting any of these
# into the build context busts BuildKit's content hash and forces a full
# native rebuild for no good reason.
.DS_Store
*.swp
*.swo
*~
*.tmp
# Logs + profiling artifacts.
*.log
*.cpuprofile
*.heapprofile
*.heapsnapshot
CPU.*
# Build / test side outputs.
*.tsbuildinfo
coverage/
.nyc_output/
__pycache__/
compaction-results/
changes/
# Generated files (the in-image build regenerates them).
packages/coding-agent/src/internal-urls/docs-index.generated.ts
packages/natives/native/.build/
packages/natives/native/pi_natives.darwin-*.node
packages/natives/native/pi_natives.dev.node
packages/ai/test/.temp-images/
python/omp-rpc/src/omp_rpc.egg-info/
# Scratch files the repo creates ad-hoc.
syntax.jsonl
out.jsonl
out.html
pi-*.html
# Secrets. Should never be in the image regardless.
.env
+100
View File
@@ -0,0 +1,100 @@
# syntax=docker/dockerfile:1.7-labs
###############################################################################
# oh-my-pi — build-artifacts image
#
# Produces, in `/out/`, the cross-host build outputs that downstream consumers
# bake into their runtime images:
#
# - pi_natives.linux-<arch>.node — N-API addon compiled from `crates/pi-natives`
# - omp_rpc-<version>-py3-none-any.whl — Python RPC wheel from `python/omp-rpc`
#
# This image deliberately has no entrypoint and no apt-installed extras: it is
# meant to be referenced as a `COPY --from=` stage by other Dockerfiles.
#
# Build:
# docker build -t oh-my-pi/artifacts:dev .
#
# Consume from another Dockerfile:
# ARG PI_ARTIFACTS_IMAGE=oh-my-pi/artifacts:dev
# FROM ${PI_ARTIFACTS_IMAGE} AS pi-artifacts
# COPY --from=pi-artifacts /out/pi_natives.linux-*.node /opt/bun/bin/
# COPY --from=pi-artifacts /out/*.whl /tmp/wheels/
###############################################################################
############################
# 1) natives-builder — Rust + Bun → pi_natives.linux-<arch>.node
############################
FROM rust:1.86-slim-bookworm AS natives-builder
ARG BUN_VERSION=1.3.14
ENV BUN_INSTALL=/opt/bun \
PATH=/opt/bun/bin:/usr/local/cargo/bin:/usr/local/bin:/usr/bin:/bin \
CARGO_TERM_COLOR=never
RUN apt-get update \
&& apt-get install -y --no-install-recommends \
curl ca-certificates pkg-config libssl-dev unzip git \
&& rm -rf /var/lib/apt/lists/*
RUN curl -fsSL https://bun.sh/install | bash -s "bun-v${BUN_VERSION}" \
&& /opt/bun/bin/bun --version
WORKDIR /pi
# ─── Layer 1: workspace manifests + lockfiles only ───────────────────────────
# Editing source files (under `packages/<x>/src/…` or `crates/<x>/src/…`) won't
# bust the `bun install` layer below, because none of those globs match. Only
# touching a `package.json`, `Cargo.toml`, or a root lockfile invalidates this
# layer. `--parents` preserves the matched path under /pi/ (dockerfile 1.7-labs).
COPY --parents \
package.json bun.lock bunfig.toml \
tsconfig.base.json tsconfig.json \
Cargo.toml Cargo.lock rust-toolchain.toml \
packages/*/package.json \
packages/tsconfig.workspace.json \
crates/*/Cargo.toml \
/pi/
# ─── Layer 2: hydrate node_modules from the manifests above ──────────────────
RUN bun install --frozen-lockfile --ignore-scripts
# ─── Layer 3: full source ────────────────────────────────────────────────────
# `.dockerignore` keeps `target/`, `node_modules/`, `dist/`, `runs/`, editor /
# OS noise (`.DS_Store`, `CPU.*`, `*.cpuprofile`, …), and pre-built host-only
# natives output out of the build context. node_modules from Layer 2 is
# preserved across this COPY because it's never in the context to begin with.
COPY . /pi/
# ─── Layer 4: compile pi-natives to a Linux N-API addon ──────────────────────
# Persistent caches make repeat builds incremental even when the source layer
# invalidates: cargo's package index + git-deps + the workspace's target dir.
RUN --mount=type=cache,target=/root/.cargo/registry \
--mount=type=cache,target=/root/.cargo/git \
--mount=type=cache,target=/pi/target \
set -eux; \
rustup show; \
bun --cwd=packages/natives run build; \
mkdir -p /out; \
cp packages/natives/native/pi_natives.linux-*.node /out/
############################
# 2) python-builder — omp-rpc wheel
############################
FROM python:3.12-slim-bookworm AS python-builder
RUN apt-get update \
&& apt-get install -y --no-install-recommends git \
&& rm -rf /var/lib/apt/lists/*
RUN pip install --upgrade pip build
WORKDIR /src
COPY python/omp-rpc /src
RUN python -m build --wheel --outdir /out
############################
# 3) artifacts — final image, nothing but the two outputs.
############################
FROM scratch AS artifacts
COPY --from=natives-builder /out/pi_natives.linux-*.node /out/
COPY --from=python-builder /out/*.whl /out/
+29 -1
View File
@@ -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;
+453
View File
@@ -0,0 +1,453 @@
//! 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
View File
@@ -1,4 +1,5 @@
pub mod cancel;
pub mod fixup;
pub mod minimizer;
pub mod process;
pub mod shell;
+2
View File
@@ -2,6 +2,8 @@
> Execute Python or JavaScript code in persistent cell-based runtimes.
> **Notice:** Do not shell out to `python -c`/`python -e`, `bun -e`, or `node -e` via the `bash` tool for ad-hoc code execution. Use this tool instead — it gives you persistent state across cells, structured `display()` output, image/JSON capture, and proper cancellation/timeout handling that one-shot `-e`/`-c` invocations cannot provide.
## Source
- Entry: `packages/coding-agent/src/tools/eval.ts`
- Model-facing prompt: `packages/coding-agent/src/prompts/tools/eval.md`
+9 -1
View File
@@ -18,7 +18,7 @@
| Field | Type | Required | Description |
| --- | --- | --- | --- |
| `op` | `"repo_view" \| "pr_create" \| "pr_checkout" \| "pr_push" \| "search_issues" \| "search_prs" \| "search_code" \| "search_commits" \| "search_repos" \| "run_watch"` | Yes | Dispatch selector. `GithubTool.execute()` switches only on this field. |
| `repo` | `string` | No | `owner/repo` override. Ignored when the identifier argument is already a full GitHub URL. Required in practice when `gh` cannot infer repo context from the current checkout. |
| `repo` | `string` | No | `owner/repo` override. Ignored when the identifier argument is already a full GitHub URL. For `search_issues`/`search_prs`/`search_code`/`search_commits`, defaults to the current checkout's `owner/repo` when omitted (skipped when the query already contains a `repo:`/`org:`/`user:`/`owner:` qualifier or when current-repo resolution fails). Required in practice when `gh` cannot infer repo context from the current checkout. |
| `branch` | `string` | No | Used by `repo_view`, `pr_push`, and `run_watch`. `run_watch` falls back to current git branch when `run` is omitted; `pr_push` falls back to current branch. |
| `pr` | `string \| string[]` | No | Used by `pr_checkout`. Each item may be a PR number, branch name, or GitHub PR URL. Array form enables batching. Omitted means current branch PR. |
| `force` | `boolean` | No | Used only by `pr_checkout`. Defaults to `false`; allows resetting an existing `pr-<number>` local branch to the PR head commit. |
@@ -148,6 +148,8 @@ Push target resolution reads the `branch.<name>.ompPrHeadRef`, `pushRemote`/`rem
| Batching | None |
| Output | `# GitHub issues search`, echoed query, optional repo, result count, then one bullet per issue with repo/state/author/labels/timestamps/URL. |
`repo` defaults to the current checkout's `owner/repo` via `resolveSearchRepoScope()` when omitted. The default is suppressed when the query already contains a leading `repo:`/`org:`/`user:`/`owner:` qualifier or when `gh repo view` fails to resolve the current checkout (e.g. outside a github remote).
### `search_prs`
| Aspect | Value |
@@ -158,6 +160,8 @@ Push target resolution reads the `branch.<name>.ompPrHeadRef`, `pushRemote`/`rem
| Batching | None |
| Output | Same shape as `search_issues`, labeled as pull requests. |
`repo` defaults to the current checkout's `owner/repo` as in `search_issues`.
### `search_code`
| Aspect | Value |
@@ -168,6 +172,8 @@ Push target resolution reads the `branch.<name>.ompPrHeadRef`, `pushRemote`/`rem
| Batching | None |
| Output | `# GitHub code search`, result count, then one bullet per match with path, repo, short commit SHA, URL, and first normalized text-match fragment line when present. |
`repo` defaults to the current checkout's `owner/repo` as in `search_issues`.
### `search_commits`
| Aspect | Value |
@@ -178,6 +184,8 @@ Push target resolution reads the `branch.<name>.ompPrHeadRef`, `pushRemote`/`rem
| Batching | None |
| Output | `# GitHub commits search`, result count, then one bullet per commit: short SHA + first commit-message line, repo, author, date, URL. |
`repo` defaults to the current checkout's `owner/repo` as in `search_issues`.
### `search_repos`
| Aspect | Value |
+5
View File
@@ -25,6 +25,11 @@
- Fixed OAuth credentials being silently disabled when two omp processes (or any two `AuthStorage` instances sharing a `agent.db`) race on token refresh. Anthropic rotates refresh tokens on every use, so the loser's `invalid_grant` response previously soft-deleted the row that the winner just rotated, forcing the user to `/login` again. `#tryOAuthCredential` now re-reads the row from disk before declaring a definitive failure: if the persisted `refresh` differs from the snapshot it tried, the peer-rotated credential is reloaded and the request retries against the fresh token instead of disabling the live row.
- Closed a remaining race window in OAuth refresh-failure handling: between re-reading the credential row to check for peer rotation and the subsequent soft-delete, another process could still complete a refresh and rotate the row, leaving us to disable the freshly-rotated credential by `id`. The disable now runs as a single CAS update conditioned on the row's `data` still matching the snapshot we tried to refresh, and on `disabled_cause IS NULL`. If the CAS reports 0 rows changed (peer rotation, or row already disabled by a concurrent failure on the same snapshot), we reload from disk and retry instead of mutating the wrong row or emitting a spurious `credential_disabled` event.
### Changed
- Lowered the default steady-state stream idle timeout from 120s to 30s while preserving the existing environment overrides.
### Fixed
- Lazy built-in provider streams now enforce the shared idle watchdog and abort stalled provider requests, so session auto-retry can continue after transient network drops instead of remaining stuck. Caller aborts still terminate as aborted.
## [14.9.3] - 2026-05-10
@@ -96,6 +96,7 @@ export function detectOpenAICompat(model: Model<"openai-completions">, resolvedB
provider === "opencode-zen" ||
provider === "opencode-go" ||
baseUrl.includes("opencode.ai");
const isOpenCodeProvider = provider === "opencode-go" || provider === "opencode-zen";
const useMaxTokens =
provider === "mistral" ||
@@ -189,13 +190,15 @@ export function detectOpenAICompat(model: Model<"openai-completions">, resolvedB
: "openai",
reasoningContentField: "reasoning_content",
// Backends that 400 follow-up requests when prior assistant tool-call turns lack `reasoning_content`:
// - Kimi: documented invariant on its native API and via OpenCode-Go.
// - Kimi: documented invariant on its native API.
// - Any reasoning-capable model reached through OpenRouter: DeepSeek V4 Pro and similar enforce
// this server-side whenever the request is in thinking mode. We can't translate Anthropic's
// redacted/encrypted reasoning into DeepSeek's plaintext form, so cross-provider continuations
// rely on a placeholder — see `convertMessages` for the placeholder injection.
// - OpenCode-Go and OpenCode-Zen handle reasoning content internally and reject
// `reasoning_content` in client-sent messages — exclude them even for Kimi models.
requiresReasoningContentForToolCalls:
isKimiModel ||
(isKimiModel && !isOpenCodeProvider) ||
(isDeepseekFamily && Boolean(model.reasoning)) ||
((provider === "openrouter" || baseUrl.includes("openrouter.ai")) && Boolean(model.reasoning)),
// DeepSeek V4 rejects synthetic reasoning_content placeholders (".") on tool-call turns.
+35 -8
View File
@@ -19,7 +19,9 @@ import type {
Model,
OptionsForApi,
} from "../types";
import { type AbortSourceTracker, createAbortSourceTracker } from "../utils/abort";
import { AssistantMessageEventStream as EventStreamImpl } from "../utils/event-stream";
import { getStreamFirstEventTimeoutMs, getStreamIdleTimeoutMs, iterateWithIdleTimeout } from "../utils/idle-iterator";
import type { BedrockOptions } from "./amazon-bedrock";
import type { AnthropicOptions } from "./anthropic";
import type { AzureOpenAIResponsesOptions } from "./azure-openai-responses";
@@ -155,6 +157,9 @@ export function setBedrockProviderModule(module: BedrockProviderModule): void {
// Stream forwarding / error helpers
// ---------------------------------------------------------------------------
const LAZY_STREAM_IDLE_TIMEOUT_ERROR = "Provider stream stalled while waiting for the next event";
const LAZY_STREAM_FIRST_EVENT_TIMEOUT_ERROR = "Provider stream timed out while waiting for the first event";
function hasFinalResult(
source: AsyncIterable<AssistantMessageEvent>,
): source is AsyncIterable<AssistantMessageEvent> & { result(): Promise<AssistantMessage> } {
@@ -165,10 +170,23 @@ function forwardStream<TApi extends Api>(
target: EventStreamImpl,
source: AsyncIterable<AssistantMessageEvent>,
model: Model<TApi>,
options: OptionsForApi<TApi>,
abortTracker: AbortSourceTracker,
): void {
(async () => {
try {
for await (const event of source) {
const idleTimeoutMs = options.streamIdleTimeoutMs ?? getStreamIdleTimeoutMs();
const watchedSource = iterateWithIdleTimeout(source, {
idleTimeoutMs,
firstItemTimeoutMs: options.streamFirstEventTimeoutMs ?? getStreamFirstEventTimeoutMs(idleTimeoutMs),
errorMessage: LAZY_STREAM_IDLE_TIMEOUT_ERROR,
firstItemErrorMessage: LAZY_STREAM_FIRST_EVENT_TIMEOUT_ERROR,
onIdle: () => abortTracker.abortLocally(new Error(LAZY_STREAM_IDLE_TIMEOUT_ERROR)),
onFirstItemTimeout: () => abortTracker.abortLocally(new Error(LAZY_STREAM_FIRST_EVENT_TIMEOUT_ERROR)),
abortSignal: options.signal,
});
for await (const event of watchedSource) {
target.push(event);
}
if (hasFinalResult(source)) {
@@ -177,14 +195,19 @@ function forwardStream<TApi extends Api>(
target.end();
}
} catch (error) {
const message = createLazyLoadErrorMessage(model, error);
target.push({ type: "error", reason: "error", error: message });
const stopReason = abortTracker.wasCallerAbort() ? "aborted" : "error";
const message = createLazyLoadErrorMessage(model, error, stopReason);
target.push({ type: "error", reason: stopReason, error: message });
target.end(message);
}
})();
}
function createLazyLoadErrorMessage<TApi extends Api>(model: Model<TApi>, error: unknown): AssistantMessage {
function createLazyLoadErrorMessage<TApi extends Api>(
model: Model<TApi>,
error: unknown,
stopReason: Extract<AssistantMessage["stopReason"], "aborted" | "error"> = "error",
): AssistantMessage {
return {
role: "assistant",
content: [],
@@ -199,8 +222,9 @@ function createLazyLoadErrorMessage<TApi extends Api>(model: Model<TApi>, error:
totalTokens: 0,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
},
stopReason: "error",
errorMessage: error instanceof Error ? error.message : String(error),
stopReason,
errorMessage:
stopReason === "aborted" ? "Request was aborted" : error instanceof Error ? error.message : String(error),
timestamp: Date.now(),
};
}
@@ -214,11 +238,14 @@ function createLazyStream<TApi extends Api>(
): (model: Model<TApi>, context: Context, options: OptionsForApi<TApi>) => EventStreamImpl {
return (model, context, options) => {
const outer = new EventStreamImpl();
const streamOptions = (options ?? {}) as OptionsForApi<TApi>;
loadModule()
.then(module => {
const inner = module.stream(model, context, options);
forwardStream(outer, inner, model);
const abortTracker = createAbortSourceTracker(streamOptions.signal);
const providerOptions = { ...streamOptions, signal: abortTracker.requestSignal } as OptionsForApi<TApi>;
const inner = module.stream(model, context, providerOptions);
forwardStream(outer, inner, model, streamOptions, abortTracker);
})
.catch(error => {
const message = createLazyLoadErrorMessage(model, error);
+1 -1
View File
@@ -1,6 +1,6 @@
import { $env } from "@oh-my-pi/pi-utils";
const DEFAULT_STREAM_IDLE_TIMEOUT_MS = 120_000;
const DEFAULT_STREAM_IDLE_TIMEOUT_MS = 30_000;
const DEFAULT_STREAM_FIRST_EVENT_TIMEOUT_MS = 100_000;
function normalizeIdleTimeoutMs(value: string | undefined, fallback: number): number | undefined {
@@ -92,4 +92,84 @@ describe("register-builtins lazy streams", () => {
expect(result.stopReason).toBe("error");
expect(result.errorMessage).toContain("bedrock exploded");
});
it("turns idle lazy provider streams into retryable terminal errors", async () => {
const partialMessage = createAssistantMessage("stop");
let providerSignal: AbortSignal | undefined;
const source = {
async *[Symbol.asyncIterator]() {
yield { type: "start", partial: partialMessage } as const;
const { promise, reject } = Promise.withResolvers<never>();
if (providerSignal?.aborted) {
reject(new Error("Request was aborted"));
}
providerSignal?.addEventListener("abort", () => reject(new Error("Request was aborted")), {
once: true,
});
await promise;
},
} as unknown as AssistantMessageEventStream;
setBedrockProviderModule({
streamBedrock: (_model, _context, options) => {
providerSignal = options.signal;
return source;
},
});
const stream = streamBedrock(createModel(), baseContext, { streamIdleTimeoutMs: 10 });
const result = await Promise.race([stream.result(), Bun.sleep(500).then(() => "timeout" as const)]);
expect(result).not.toBe("timeout");
if (result === "timeout") {
throw new Error("Timed out waiting for forwarded stream stall result");
}
expect(providerSignal?.aborted).toBe(true);
expect(result.stopReason).toBe("error");
expect(result.errorMessage).toBe("Provider stream stalled while waiting for the next event");
});
it("preserves caller aborts while forwarding lazy provider streams", async () => {
const abortController = new AbortController();
const partialMessage = createAssistantMessage("stop");
let providerSignal: AbortSignal | undefined;
const source = {
async *[Symbol.asyncIterator]() {
yield { type: "start", partial: partialMessage } as const;
const { promise, reject } = Promise.withResolvers<never>();
if (providerSignal?.aborted) {
reject(new Error("Request was aborted"));
}
providerSignal?.addEventListener("abort", () => reject(new Error("Request was aborted")), {
once: true,
});
await promise;
},
} as unknown as AssistantMessageEventStream;
setBedrockProviderModule({
streamBedrock: (_model, _context, options) => {
providerSignal = options.signal;
return source;
},
});
const stream = streamBedrock(createModel(), baseContext, {
signal: abortController.signal,
streamIdleTimeoutMs: 500,
});
const iterator = stream[Symbol.asyncIterator]();
const firstEvent = await iterator.next();
expect(firstEvent.value?.type).toBe("start");
abortController.abort();
const result = await Promise.race([stream.result(), Bun.sleep(500).then(() => "timeout" as const)]);
expect(result).not.toBe("timeout");
if (result === "timeout") {
throw new Error("Timed out waiting for forwarded caller abort result");
}
expect(result.stopReason).toBe("aborted");
expect(result.errorMessage).toBe("Request was aborted");
});
});
+11
View File
@@ -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,15 +12,25 @@
### Changed
- Changed the `github` tool's search ops (`search_issues`, `search_prs`, `search_code`, `search_commits`) to default the `repo` scope to the current checkout's `owner/repo` when `repo` is omitted. The auto-scope is skipped when the query already carries an explicit `repo:`/`org:`/`user:`/`owner:` qualifier or when `gh repo view` cannot resolve a github remote (in which case the search proceeds across all of GitHub as before). `search_repos` is unchanged — repository-scoping there must live in the query.
- 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.
- Fixed `omp commit` hanging after a successful commit instead of returning to the shell. The command now mirrors the `runPrintMode` exit pattern and calls `postmortem.quit(0)` once the pipeline resolves so lingering HTTP/2 keep-alive sockets, the Settings autosave timer, and other AgentSession background handles don't keep the event loop pinned. ([#1041](https://github.com/can1357/oh-my-pi/issues/1041))
- Fixed hashline payload parsing to silently treat truly-blank lines as empty `~`-prefixed payload lines when more payload follows in the same run. The previous behavior broke at the blank ("payload line has no preceding +, <, or = operation.") even though the intent is obvious — the only ambiguity is between in-payload blanks and end-of-section blanks, and a one-line lookahead resolves it: blanks that precede a non-payload op still end the run cleanly as section separators. Recovers the common case of forgetting the leading separator on a blank inserted line without changing how trailing blanks between ops behave.
- Rewrote the hashline edit prompt examples to use an ASCII-only `TITLE = "Mr"` → `"Mrs"` / `"Dr"` motif instead of the previous `" • "` and `"·"` separators. Some agents had been copying the middle-dot literal characters into real edits as if they were format scaffolding (e.g. emitting payload lines like `~ ·`), since the demo inserts were near-twins of the existing string. The new example keeps every original op shape (single-line replace, multiline replace, insert AFTER/BEFORE, append, delete, blank, plus both anti-patterns) but uses content that is obviously domain-specific and clearly distinct from any payload separator. Pure prompt change; no parser, schema, or runtime behavior is affected.
- Fixed `discoverAgents()` ignoring `disabledProviders` for the `claude-plugins` provider. Plugin roots from `~/.claude/plugins/` were scanned unconditionally, so agents from Claude Code marketplace plugins continued to appear in `/agents` and the Agent Control Center even when `disabledProviders: [claude-plugins]` was set. The discovery path now checks `isProviderEnabled("claude-plugins")` before calling `listClaudePluginRoots()`, matching how every other capability respects the disabled-providers set. ([#1075](https://github.com/can1357/oh-my-pi/issues/1075))
## [15.0.1] - 2026-05-14
### Breaking Changes
@@ -31,6 +31,7 @@ const PRIORITY = 70; // Below claude.ts (80) so user .claude/ overrides win
interface ClaudePluginManifest {
skills?: string;
"slash-commands"?: string;
commands?: string;
}
interface ResolvedPluginDir {
@@ -59,24 +60,35 @@ function isWithinPluginRoot(rootPath: string, targetPath: string): boolean {
async function resolvePluginDir(
root: ClaudePluginRoot,
manifestKey: keyof ClaudePluginManifest,
manifestKeys: ReadonlyArray<keyof ClaudePluginManifest>,
fallback: string,
): Promise<ResolvedPluginDir> {
const manifest = await readPluginManifest(root);
const fallbackDir = path.join(root.path, fallback);
const configured = manifest?.[manifestKey];
if (typeof configured !== "string" || !configured.trim()) {
let configured: string | undefined;
let matchedKey: keyof ClaudePluginManifest | undefined;
for (const key of manifestKeys) {
const val = manifest?.[key];
if (typeof val === "string" && val.trim()) {
configured = val.trim();
matchedKey = key;
break;
}
}
if (configured === undefined) {
return { dir: fallbackDir };
}
const resolved = path.resolve(root.path, configured.trim());
const resolved = path.resolve(root.path, configured);
if (isWithinPluginRoot(root.path, resolved)) {
return { dir: resolved };
}
return {
dir: fallbackDir,
warning: `[claude-plugins] Ignoring ${String(manifestKey)} path outside plugin root for ${root.id}: ${configured}`,
warning: `[claude-plugins] Ignoring ${String(matchedKey)} path outside plugin root for ${root.id}: ${configured}`,
};
}
@@ -93,7 +105,7 @@ async function loadSkills(ctx: LoadContext): Promise<LoadResult<Skill>> {
const results = await Promise.all(
roots.map(async root => {
const { dir: skillsDir, warning } = await resolvePluginDir(root, "skills", "skills");
const { dir: skillsDir, warning } = await resolvePluginDir(root, ["skills"], "skills");
const result = await scanSkillsFromDir(ctx, {
dir: skillsDir,
providerId: PROVIDER_ID,
@@ -128,7 +140,7 @@ async function loadSlashCommands(ctx: LoadContext): Promise<LoadResult<SlashComm
const results = await Promise.all(
roots.map(async root => {
const { dir: commandsDir, warning } = await resolvePluginDir(root, "slash-commands", "commands");
const { dir: commandsDir, warning } = await resolvePluginDir(root, ["commands", "slash-commands"], "commands");
const result = await loadFilesFromDir<SlashCommand>(ctx, commandsDir, PROVIDER_ID, root.scope, {
extensions: ["md"],
transform: (name, content, filePath, source) => {
+42 -11
View File
@@ -25,9 +25,11 @@ when installed.
from __future__ import annotations
import asyncio
import ast
import base64
import builtins
import inspect
import io
import json
import os
@@ -120,6 +122,7 @@ class _RunnerState:
"__builtins__": builtins,
}
self.last_install_marker: int = 0
self.loop: asyncio.AbstractEventLoop | None = None
_STATE = _RunnerState()
@@ -688,13 +691,41 @@ _install_builtins(_STATE.user_ns)
# ---------------------------------------------------------------------------
_TLA_FLAG = getattr(ast, "PyCF_ALLOW_TOP_LEVEL_AWAIT", 0x2000)
def _get_event_loop() -> asyncio.AbstractEventLoop:
loop = _STATE.loop
if loop is None or loop.is_closed():
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
_STATE.loop = loop
return loop
def _run_compiled(code, ns: dict, *, want_value: bool) -> Any:
"""Execute a code object, awaiting it if compiled as a coroutine.
``want_value`` is True for the trailing expression — we return ``eval``'s
result (or the awaited coroutine's value). For statement blocks the
return is always ``None``.
"""
if code.co_flags & inspect.CO_COROUTINE:
coro = eval(code, ns)
result = _get_event_loop().run_until_complete(coro)
return result if want_value else None
if want_value:
return eval(code, ns)
exec(code, ns)
return None
def _exec_source(source: str, ns: dict) -> None:
"""Compile + execute ``source``; if the last node is an expression, route
its value through ``__omp_display`` so dataframes/figures render rich."""
try:
module = ast.parse(source, mode="exec")
except SyntaxError:
raise
its value through ``__omp_display`` so dataframes/figures render rich.
Top-level ``await`` / ``async for`` / ``async with`` is permitted; the
cell is driven through the runner's persistent event loop."""
module = ast.parse(source, mode="exec")
if not module.body:
return
@@ -704,16 +735,16 @@ def _exec_source(source: str, ns: dict) -> None:
body_module = ast.Module(body=module.body[:-1], type_ignores=[])
expr_module = ast.Expression(body=last.value)
ast.copy_location(expr_module, last)
body_code = compile(body_module, "<cell>", "exec")
expr_code = compile(expr_module, "<cell>", "eval")
exec(body_code, ns)
value = eval(expr_code, ns)
body_code = compile(body_module, "<cell>", "exec", flags=_TLA_FLAG)
expr_code = compile(expr_module, "<cell>", "eval", flags=_TLA_FLAG)
_run_compiled(body_code, ns, want_value=False)
value = _run_compiled(expr_code, ns, want_value=True)
if value is not None:
__omp_display(value, kind="result")
return
code = compile(module, "<cell>", "exec")
exec(code, ns)
code = compile(module, "<cell>", "exec", flags=_TLA_FLAG)
_run_compiled(code, ns, want_value=False)
# ---------------------------------------------------------------------------
+21 -3
View File
@@ -86,9 +86,27 @@ function collectPayload(
let index = startIndex;
while (index < lines.length) {
const line = stripTrailingCarriageReturn(lines[index]);
if (!line.startsWith(HL_EDIT_SEP)) break;
payload.push(line.slice(1));
index++;
if (line.startsWith(HL_EDIT_SEP)) {
payload.push(line.slice(1));
index++;
continue;
}
// Silently recover from a missing payload prefix on an otherwise blank
// line: if more payload follows (possibly past further blanks), treat
// each intervening blank as an empty `${HL_EDIT_SEP}` payload line.
// Trailing blanks before a non-payload op stay as section separators.
if (line.length === 0) {
let lookahead = index + 1;
while (lookahead < lines.length && stripTrailingCarriageReturn(lines[lookahead]).length === 0) {
lookahead++;
}
if (lookahead < lines.length && stripTrailingCarriageReturn(lines[lookahead]).startsWith(HL_EDIT_SEP)) {
for (let j = index; j < lookahead; j++) payload.push("");
index = lookahead;
continue;
}
}
break;
}
if (payload.length === 0 && requirePayload) {
throw new Error(`line ${opLineNum}: + and < operations require at least one ${HL_EDIT_SEP}TEXT payload line.`);
+5 -1
View File
@@ -5,7 +5,11 @@
"fileTypes": [".rs"],
"rootMarkers": ["Cargo.toml", "rust-analyzer.toml"],
"initOptions": {},
"settings": {},
"settings": {
"rust-analyzer": {
"checkOnSave": false
}
},
"capabilities": {
"flycheck": true,
"ssr": true,
@@ -6,10 +6,10 @@ Pick the operation via `op`. Each op uses a subset of the parameters:
- `pr_create` — Create a pull request. Either provide `title` (and optional `body`) or set `fill: true` to auto-fill from commits. Optional `base` (target, defaults to repo default), `head` (source, defaults to current branch), `draft`, `repo`, `reviewer[]`, `assignee[]`, `label[]`. Returns the new PR URL plus a summary.
- `pr_checkout` — Check one or more pull requests out into dedicated git worktrees. Optional `pr` (number, URL, branch, or array of any of those — pass an array to batch-check-out multiple PRs in one call), `repo`, `force` (reset existing local branch).
- `pr_push` — Push a checked-out PR branch back to its source branch. Requires the branch to have been checked out via `op: pr_checkout` (carries push metadata). Optional `branch`; defaults to the current checked-out git branch. Optional `forceWithLease`.
- `search_issues` — Search issues using normal GitHub issue search syntax. Optional `query` (required unless `since`/`until` is set), `repo`, `limit`, `since`, `until`, `dateField`.
- `search_prs` — Search pull requests using normal GitHub PR search syntax. Optional `query` (required unless `since`/`until` is set), `repo`, `limit`, `since`, `until`, `dateField`.
- `search_code` — Search code with GitHub code search syntax. Required `query`. Optional `repo`, `limit`. Returns matching paths with surrounding fragments. Date filtering (`since`/`until`) is **not** supported by GitHub code search.
- `search_commits` — Search commits across GitHub. Optional `query` (required unless `since`/`until` is set), `repo`, `limit`, `since`, `until`. `dateField` is ignored — always uses `committer-date`.
- `search_issues` — Search issues using normal GitHub issue search syntax. Optional `query` (required unless `since`/`until` is set), `repo`, `limit`, `since`, `until`, `dateField`. Defaults `repo` to the current checkout's `owner/repo` when omitted; pass an explicit `repo:`/`org:`/`user:` qualifier in `query` to search outside it.
- `search_prs` — Search pull requests using normal GitHub PR search syntax. Optional `query` (required unless `since`/`until` is set), `repo`, `limit`, `since`, `until`, `dateField`. Defaults `repo` to the current checkout's `owner/repo` when omitted; pass an explicit `repo:`/`org:`/`user:` qualifier in `query` to search outside it.
- `search_code` — Search code with GitHub code search syntax. Required `query`. Optional `repo`, `limit`. Returns matching paths with surrounding fragments. Defaults `repo` to the current checkout's `owner/repo` when omitted; pass an explicit `repo:`/`org:`/`user:` qualifier in `query` to search outside it. Date filtering (`since`/`until`) is **not** supported by GitHub code search.
- `search_commits` — Search commits across GitHub. Optional `query` (required unless `since`/`until` is set), `repo`, `limit`, `since`, `until`. `dateField` is ignored — always uses `committer-date`. Defaults `repo` to the current checkout's `owner/repo` when omitted; pass an explicit `repo:`/`org:`/`user:` qualifier in `query` to search outside it.
- `search_repos` — Search repositories across GitHub. Optional `query` (required unless `since`/`until` is set), `limit`, `since`, `until`, `dateField` (use query qualifiers like `org:`, `language:` instead of `repo`).
- Date filter format for `since` / `until`: relative duration `<n><unit>` (`m`/`h`/`d`/`w`/`mo`/`y`, e.g. `3d`, `12h`, `2w`), an ISO date `YYYY-MM-DD`, or an ISO datetime. Translated to a single GitHub-search qualifier (`created:≥…`, `created:≤…`, or `created:since..until`). `dateField: "updated"` maps to `updated:` for issues/prs and `pushed:` for repos. When you only want a date filter and no keywords, omit `query` entirely.
- `run_watch` — Watch a GitHub Actions workflow run. Optional `run` (id or URL). Omitting `run` watches all workflow runs for the current HEAD commit; `branch` falls back to the current branch. Optional `tail` (log lines per failed job). Streams snapshots, fast-fails on the first detected job failure (with a brief grace period to capture concurrent failures), then fetches tailed logs for the failed jobs. The full failed-job logs are saved as a session artifact for on-demand reads.
@@ -66,36 +66,35 @@ When braces bound your edit, you SHOULD prefer these shapes:
</common-failures>
<case file="mod.ts">
{{hline 1 "const DEF = \"guest\";"}}
{{hline 2 "export function label(name) {"}}
{{hline 1 "const TITLE = \"Mr\";"}}
{{hline 2 "export function greet(name) {"}}
{{hline 3 "\treturn ["}}
{{hline 4 "\t\tname?.trim() || DEF,"}}
{{hline 5 "\t\t\" • \","}}
{{hline 6 "\t].join(\"\");"}}
{{hline 4 "\t\tTITLE,"}}
{{hline 5 "\t\tname?.trim() || \"guest\","}}
{{hline 6 "\t].join(\" \");"}}
{{hline 7 "}"}}
</case>
<examples>
# Replace one line (the payload must re-emit the original indentation)
@@ mod.ts
= {{hrefr 4}}..{{hrefr 4}}
{{hsep}} name?.trim().toUpperCase() || DEF,
= {{hrefr 1}}..{{hrefr 1}}
{{hsep}}const TITLE = "Mrs";
# Replace a full multiline statement (widen to a self-contained boundary)
@@ mod.ts
= {{hrefr 3}}..{{hrefr 6}}
{{hsep}} return [
{{hsep}} name?.trim() || DEF,
{{hsep}} "·",
{{hsep}} " • ",
{{hsep}} ].join("");
{{hsep}} "Mrs",
{{hsep}} name?.trim() || "guest",
{{hsep}} ].join(" ");
# Insert AFTER/BEFORE a line
@@ mod.ts
+ {{hrefr 3}}
{{hsep}} "·",
+ {{hrefr 4}}
{{hsep}} "Dr",
< {{hrefr 5}}
{{hsep}} "·",
{{hsep}} "Dr",
# Append to file
@@ mod.ts
@@ -112,13 +111,12 @@ When braces bound your edit, you SHOULD prefer these shapes:
</examples>
<anti-pattern>
# WRONG — replaces 3 lines just to add one.
# WRONG — replaces 2 lines just to add one.
@@ mod.ts
= {{hrefr 1}}..{{hrefr 3}}
{{hsep}}const DEF = "guest";
= {{hrefr 1}}..{{hrefr 2}}
{{hsep}}const TITLE = "Mr";
{{hsep}}const DEBUG = false;
{{hsep}}export function label(name) {
{{hsep}} return [
{{hsep}}export function greet(name) {
# RIGHT — same effect, one-line insert
@@ mod.ts
+ {{hrefr 1}}
@@ -127,17 +125,15 @@ When braces bound your edit, you SHOULD prefer these shapes:
# WRONG — replace from the middle of a larger statement (error-prone)
@@ mod.ts
= {{hrefr 4}}..{{hrefr 5}}
{{hsep}} name?.trim() || DEF,
{{hsep}} "·",
{{hsep}} " • ",
{{hsep}} "Dr",
{{hsep}} name?.trim() || "guest",
# RIGHT — widen to the full statement
@@ mod.ts
= {{hrefr 3}}..{{hrefr 6}}
{{hsep}} return [
{{hsep}} name?.trim() || DEF,
{{hsep}} "·",
{{hsep}} " • ",
{{hsep}} ].join("");
{{hsep}} "Dr",
{{hsep}} name?.trim() || "guest",
{{hsep}} ].join(" ");
</anti-pattern>
<critical>
+5 -2
View File
@@ -15,6 +15,7 @@ import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { logger } from "@oh-my-pi/pi-utils";
import { isProviderEnabled } from "../capability";
import { findAllNearestProjectConfigDirs, getConfigDirs } from "../config";
import { listClaudePluginRoots } from "../discovery/helpers";
import { loadBundledAgents, parseAgent } from "./agents";
@@ -87,8 +88,10 @@ export async function discoverAgents(cwd: string, home: string = os.homedir()):
if (user) orderedDirs.push({ dir: user.path, source: "user" });
}
// Load agents from Claude Code marketplace plugins
const { roots: pluginRoots } = await listClaudePluginRoots(home, resolvedCwd);
// Load agents from Claude Code marketplace plugins (respects disabledProviders)
const { roots: pluginRoots } = isProviderEnabled("claude-plugins")
? await listClaudePluginRoots(home, resolvedCwd)
: { roots: [] };
const sortedPluginRoots = [...pluginRoots].sort((a, b) => {
if (a.scope === b.scope) return 0;
return a.scope === "project" ? -1 : 1;
@@ -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 `<system-warning>Stripped redundant ${quoted} — bash output is already truncated and stderr is already merged into stdout. NEVER use these patterns.</system-warning>`;
}
+20 -13
View File
@@ -17,11 +17,11 @@ 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";
import { formatStyledTruncationWarning, type OutputMeta } from "./output-meta";
import { formatStyledTruncationWarning, type OutputMeta, stripOutputNotice } from "./output-meta";
import { resolveToCwd } from "./path-utils";
import { formatToolWorkingDirectory, replaceTabs } from "./render-utils";
import { ToolAbortError, ToolError } from "./tool-errors";
@@ -246,6 +246,7 @@ export class BashTool implements AgentTool<BashToolSchema, BashToolDetails> {
readonly #asyncEnabled: boolean;
readonly #autoBackgroundEnabled: boolean;
readonly #autoBackgroundThresholdMs: number;
#bashFixupNoticeEmitted = false;
constructor(private readonly session: ToolSession) {
this.#asyncEnabled = this.session.settings.get("async.enabled");
@@ -484,15 +485,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 +575,11 @@ 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 = this.#bashFixupNoticeEmitted ? undefined : formatBashFixupNotice(bashFixups);
if (bashFixupNotice) {
pendingNotices.push(bashFixupNotice);
this.#bashFixupNoticeEmitted = true;
}
if (asyncRequested) {
if (!AsyncJobManager.instance()) {
@@ -977,8 +981,11 @@ export function createShellRenderer<TArgs>(config: ShellRendererConfig<TArgs>) {
const expanded = renderContext?.expanded ?? options.expanded;
const previewLines = renderContext?.previewLines ?? BASH_DEFAULT_PREVIEW_LINES;
// Get output from context (preferred) or fall back to result content
const output = renderContext?.output ?? result.content?.find(c => c.type === "text")?.text ?? "";
// Get output from context (preferred) or fall back to result content.
// Strip the LLM-facing notice appended by wrappedExecute so we don't
// double-print it alongside the styled warning line below.
const rawOutput = renderContext?.output ?? result.content?.find(c => c.type === "text")?.text ?? "";
const output = stripOutputNotice(rawOutput, details?.meta);
const displayOutput = output.trimEnd();
const showingFullOutput = expanded && renderContext?.isFullOutput === true;
@@ -11,7 +11,7 @@ import type { RenderResultOptions } from "../../extensibility/custom-tools/types
import type { Theme } from "../../modes/theme/theme";
import { Hasher, renderCodeCell, renderStatusLine } from "../../tui";
import type { BrowserToolDetails } from "../browser";
import { formatStyledTruncationWarning } from "../output-meta";
import { formatStyledTruncationWarning, stripOutputNotice } from "../output-meta";
import { replaceTabs, shortenPath } from "../render-utils";
const BROWSER_DEFAULT_PREVIEW_LINES = 10;
@@ -195,7 +195,7 @@ export const browserToolRenderer = {
const details = result.details;
const action = details?.action ?? argsObj.action;
const isError = result.isError === true;
const output = extractTextOutput(result.content);
const output = stripOutputNotice(extractTextOutput(result.content), details?.meta);
if (action === "run") {
let component = renderRunCell(argsObj, details, options, output, isError, theme);
+10 -2
View File
@@ -16,7 +16,12 @@ import evalDescription from "../prompts/tools/eval.md" with { type: "text" };
import { DEFAULT_MAX_BYTES, OutputSink, type OutputSummary, TailBuffer } from "../session/streaming-output";
import { getTreeBranch, getTreeContinuePrefix, renderCodeCell } from "../tui";
import { resolveEvalBackends, type ToolSession } from ".";
import { formatStyledTruncationWarning, resolveOutputMaxColumns, resolveOutputSinkHeadBytes } from "./output-meta";
import {
formatStyledTruncationWarning,
resolveOutputMaxColumns,
resolveOutputSinkHeadBytes,
stripOutputNotice,
} from "./output-meta";
import { formatTitle, replaceTabs, shortenPath, truncateToWidth, wrapBrackets } from "./render-utils";
import { ToolAbortError, ToolError } from "./tool-errors";
import { toolResult } from "./tool-result";
@@ -922,8 +927,11 @@ export const evalToolRenderer = {
): Component {
const details = result.details;
const output =
const rawOutput =
options.renderContext?.output ?? (result.content?.find(c => c.type === "text")?.text ?? "").trimEnd();
// Strip the LLM-facing notice (appended by wrappedExecute) before display;
// the styled `warningLine` below carries the same text in ⟨…⟩ form.
const output = stripOutputNotice(rawOutput, details?.meta).trimEnd();
const jsonOutputs = details?.jsonOutputs ?? [];
const jsonLines = jsonOutputs.flatMap((value, index) => {
+37 -4
View File
@@ -1774,6 +1774,39 @@ export async function resolveDefaultRepoMemoized(cwd: string, signal?: AbortSign
return untilAborted(signal, pending);
}
/**
* Matches search-query qualifiers that already scope to a repository, org, or
* user. When present, callers should avoid layering a default `repo:<current>`
* on top — the user has already expressed an explicit scope.
*
* Only the leading `repo:`/`org:`/`user:`/`owner:` token is treated as a
* scope marker; arbitrary substrings (e.g. inside quoted text) are ignored.
*/
const REPO_SCOPE_QUALIFIER_PATTERN = /(?:^|\s)-?(?:repo|org|user|owner):\S/i;
/**
* Resolve the effective `repo:` scope for a search op. Returns the explicit
* `repo` when set, `undefined` when the query already carries a scoping
* qualifier, and otherwise the current checkout's `owner/repo` via
* `resolveDefaultRepoMemoized`. Resolution failures (no git/gh context, no
* configured remote) silently fall back to `undefined` so the search proceeds
* across all of GitHub instead of throwing.
*/
async function resolveSearchRepoScope(
cwd: string,
repo: string | undefined,
query: string | undefined,
signal: AbortSignal | undefined,
): Promise<string | undefined> {
if (repo) return repo;
if (query && REPO_SCOPE_QUALIFIER_PATTERN.test(query)) return undefined;
try {
return await resolveDefaultRepoMemoized(cwd, signal);
} catch {
return undefined;
}
}
async function resolveGitHubBranchHead(
cwd: string,
repo: string,
@@ -3267,11 +3300,11 @@ async function executeSearchIssues(
params: GithubInput,
signal: AbortSignal | undefined,
): Promise<AgentToolResult<GhToolDetails>> {
const repo = normalizeOptionalString(params.repo);
const limit = resolveSearchLimit(params.limit);
const dateField = resolveSearchDateField("issues", params.dateField);
const dateQualifier = buildSearchDateQualifier(dateField, params.since, params.until);
const displayQuery = composeSearchQuery([params.query, dateQualifier]);
const repo = await resolveSearchRepoScope(session.cwd, normalizeOptionalString(params.repo), displayQuery, signal);
const apiQuery = composeSearchQuery([displayQuery, repo ? `repo:${repo}` : undefined, "is:issue"]);
const args = buildGhApiSearchArgs("issues", apiQuery, limit);
@@ -3285,11 +3318,11 @@ async function executeSearchPrs(
params: GithubInput,
signal: AbortSignal | undefined,
): Promise<AgentToolResult<GhToolDetails>> {
const repo = normalizeOptionalString(params.repo);
const limit = resolveSearchLimit(params.limit);
const dateField = resolveSearchDateField("prs", params.dateField);
const dateQualifier = buildSearchDateQualifier(dateField, params.since, params.until);
const displayQuery = composeSearchQuery([params.query, dateQualifier]);
const repo = await resolveSearchRepoScope(session.cwd, normalizeOptionalString(params.repo), displayQuery, signal);
const apiQuery = composeSearchQuery([displayQuery, repo ? `repo:${repo}` : undefined, "is:pr"]);
const args = buildGhApiSearchArgs("issues", apiQuery, limit);
@@ -3307,8 +3340,8 @@ async function executeSearchCode(
if (params.since !== undefined || params.until !== undefined) {
throw new ToolError("search_code does not support since/until; GitHub code search has no date qualifier.");
}
const repo = normalizeOptionalString(params.repo);
const limit = resolveSearchLimit(params.limit);
const repo = await resolveSearchRepoScope(session.cwd, normalizeOptionalString(params.repo), query, signal);
const apiQuery = composeSearchQuery([query, repo ? `repo:${repo}` : undefined]);
const args = buildGhApiSearchArgs("code", apiQuery, limit, ["Accept: application/vnd.github.text-match+json"]);
@@ -3322,11 +3355,11 @@ async function executeSearchCommits(
params: GithubInput,
signal: AbortSignal | undefined,
): Promise<AgentToolResult<GhToolDetails>> {
const repo = normalizeOptionalString(params.repo);
const limit = resolveSearchLimit(params.limit);
const dateField = resolveSearchDateField("commits", params.dateField);
const dateQualifier = buildSearchDateQualifier(dateField, params.since, params.until);
const displayQuery = composeSearchQuery([params.query, dateQualifier]);
const repo = await resolveSearchRepoScope(session.cwd, normalizeOptionalString(params.repo), displayQuery, signal);
const apiQuery = composeSearchQuery([displayQuery, repo ? `repo:${repo}` : undefined]);
const args = buildGhApiSearchArgs("commits", apiQuery, limit);
@@ -489,6 +489,32 @@ export function formatStyledTruncationWarning(meta: OutputMeta | undefined, them
return theme.fg("warning", wrapBrackets(message, theme));
}
/**
* Strip the trailing notice that {@link appendOutputNotice} bakes into the
* LLM-facing content body. Renderers should call this before printing
* `result.content` text in the TUI, because they emit a styled warning line of
* their own; without this, users see the same `[Showing lines …]` string twice
* (once verbatim from the body, once as the styled `⟨…⟩` warning).
*
* Safe to call eagerly: returns the input unchanged when no notice is present
* (e.g. during streaming, before {@link wrappedExecute} runs).
*/
export function stripOutputNotice(text: string, meta: OutputMeta | undefined): string {
const notice = formatOutputNotice(meta);
if (!notice) return text;
// Trim trailing whitespace from `text` and from the notice itself so we
// match regardless of whether: (a) the caller already trimEnd()'d, (b)
// extra blank lines slipped in after the notice (diagnostics blocks add
// `\n\n` between sections, OutputSink may pad), or (c) neither. Returns
// the prefix before the notice so the caller can re-trim as needed.
const trimmedText = text.trimEnd();
const trimmedNotice = notice.trimEnd();
if (trimmedText.endsWith(trimmedNotice)) {
return trimmedText.slice(0, -trimmedNotice.length);
}
return text;
}
// =============================================================================
// Tool wrapper
// =============================================================================
+4 -1
View File
@@ -60,6 +60,7 @@ import {
formatStyledTruncationWarning,
type OutputMeta,
resolveOutputMaxColumns,
stripOutputNotice,
} from "./output-meta";
import { expandPath, formatPathRelativeToCwd, resolveReadPath, splitPathAndSel } from "./path-utils";
import { formatBytes, replaceTabs, shortenPath, wrapBrackets } from "./render-utils";
@@ -2194,7 +2195,9 @@ export const readToolRenderer = {
const rawText = result.content?.find(c => c.type === "text")?.text ?? "";
// Prefer structured `displayContent` from details when available so the TUI
// shows clean file content (no model-only hashline anchors) without parsing the formatted text.
const contentText = details?.displayContent?.text ?? rawText;
// Fall back to the raw text, but strip the LLM-facing notice so it doesn't
// echo next to the styled warning line below.
const contentText = details?.displayContent?.text ?? stripOutputNotice(rawText, details?.meta);
const imageContent = result.content?.find(c => c.type === "image");
const rawPath = args?.file_path || args?.path || "";
const filePath = shortenPath(rawPath);
+3 -2
View File
@@ -16,7 +16,7 @@ import { executeSSH } from "../ssh/ssh-executor";
import { renderStatusLine } from "../tui";
import { CachedOutputBlock } from "../tui/output-block";
import type { ToolSession } from ".";
import { formatStyledTruncationWarning, type OutputMeta } from "./output-meta";
import { formatStyledTruncationWarning, type OutputMeta, stripOutputNotice } from "./output-meta";
import { ToolError } from "./tool-errors";
import { toolResult } from "./tool-result";
import { clampTimeout } from "./tool-timeouts";
@@ -253,7 +253,8 @@ export const sshToolRenderer = {
render: (width: number): string[] => {
// REACTIVE: read mutable options at render time
const { expanded, renderContext } = options;
const output = textContent.trimEnd();
// Strip LLM-facing notice so we don't echo it next to the styled warning.
const output = stripOutputNotice(textContent, details?.meta).trimEnd();
const outputLines: string[] = [];
if (output) {
@@ -38,6 +38,7 @@ export interface AnthropicSearchParams {
max_tokens?: number;
/** Sampling temperature (0–1). Lower = more focused/factual. */
temperature?: number;
signal?: AbortSignal;
}
/**
@@ -86,6 +87,7 @@ async function callSearch(
systemPrompt?: string,
maxTokens?: number,
temperature?: number,
signal?: AbortSignal,
): Promise<AnthropicApiResponse> {
const url = buildAnthropicUrl(auth);
const headers = buildAnthropicSearchHeaders(auth);
@@ -116,6 +118,7 @@ async function callSearch(
method: "POST",
headers,
body: JSON.stringify(body),
signal,
});
if (!response.ok) {
@@ -253,6 +256,7 @@ export async function searchAnthropic(params: AnthropicSearchParams): Promise<Se
params.system_prompt,
params.max_tokens,
params.temperature,
params.signal,
);
const result = parseResponse(response);
@@ -281,6 +285,7 @@ export class AnthropicProvider extends SearchProvider {
num_results: params.numSearchResults ?? params.limit,
max_tokens: params.maxOutputTokens,
temperature: params.temperature,
signal: params.signal,
});
}
}
@@ -29,6 +29,7 @@ export interface ExaSearchParams {
exclude_domains?: string[];
start_published_date?: string;
end_published_date?: string;
signal?: AbortSignal;
}
interface ExaSearchResult {
@@ -179,6 +180,7 @@ async function callExaSearch(apiKey: string, params: ExaSearchParams): Promise<E
"x-api-key": apiKey,
},
body: JSON.stringify(body),
signal: params.signal,
});
if (!response.ok) {
@@ -259,6 +261,7 @@ export class ExaProvider extends SearchProvider {
return searchExa({
query: params.query,
num_results: params.numSearchResults ?? params.limit,
signal: params.signal,
});
}
}
@@ -39,6 +39,7 @@ export interface GeminiSearchParams extends GeminiToolParams {
max_output_tokens?: number;
/** Sampling temperature (0–1). Lower = more focused/factual. */
temperature?: number;
signal?: AbortSignal;
}
export function buildGeminiRequestTools(params: GeminiToolParams): Array<Record<string, Record<string, unknown>>> {
@@ -235,6 +236,7 @@ async function callGeminiSearch(
maxOutputTokens?: number,
temperature?: number,
toolParams: GeminiToolParams = {},
signal?: AbortSignal,
): Promise<{
answer: string;
sources: SearchSource[];
@@ -308,6 +310,7 @@ async function callGeminiSearch(
...headers,
},
body: JSON.stringify(requestBody),
signal,
});
const urlFor = (attempt: number) =>
`${endpoints[Math.min(attempt, endpoints.length - 1)]}/v1internal:streamGenerateContent?alt=sse`;
@@ -500,6 +503,7 @@ export async function searchGemini(params: GeminiSearchParams): Promise<SearchRe
code_execution: params.code_execution,
url_context: params.url_context,
},
params.signal,
);
let sources = result.sources;
@@ -539,6 +543,7 @@ export class GeminiProvider extends SearchProvider {
google_search: params.googleSearch,
code_execution: params.codeExecution,
url_context: params.urlContext,
signal: params.signal,
});
}
}
@@ -17,6 +17,7 @@ const JINA_SEARCH_URL = "https://s.jina.ai";
export interface JinaSearchParams {
query: string;
num_results?: number;
signal?: AbortSignal;
}
interface JinaSearchResult {
@@ -33,13 +34,14 @@ export function findApiKey(): string | null {
}
/** Call Jina Reader search API. */
async function callJinaSearch(apiKey: string, query: string): Promise<JinaSearchResponse> {
async function callJinaSearch(apiKey: string, query: string, signal?: AbortSignal): Promise<JinaSearchResponse> {
const requestUrl = `${JINA_SEARCH_URL}/${encodeURIComponent(query)}`;
const response = await fetch(requestUrl, {
headers: {
Accept: "application/json",
Authorization: `Bearer ${apiKey}`,
},
signal,
});
if (!response.ok) {
@@ -58,7 +60,7 @@ export async function searchJina(params: JinaSearchParams): Promise<SearchRespon
throw new Error("JINA_API_KEY not found. Set it in environment or .env file.");
}
const response = await callJinaSearch(apiKey, params.query);
const response = await callJinaSearch(apiKey, params.query, params.signal);
const sources: SearchSource[] = [];
for (const result of response) {
@@ -91,6 +93,7 @@ export class JinaProvider extends SearchProvider {
return searchJina({
query: params.query,
num_results: params.numSearchResults ?? params.limit,
signal: params.signal,
});
}
}
@@ -20,6 +20,7 @@ const DEFAULT_NUM_RESULTS = 10;
export interface ZaiSearchParams {
query: string;
num_results?: number;
signal?: AbortSignal;
}
interface ZaiSearchResult {
@@ -55,7 +56,7 @@ export async function findApiKey(): Promise<string | null> {
return findCredential(getEnvApiKey("zai"), "zai");
}
async function callZaiTool(apiKey: string, args: Record<string, unknown>): Promise<unknown> {
async function callZaiTool(apiKey: string, args: Record<string, unknown>, signal?: AbortSignal): Promise<unknown> {
const response = await fetch(ZAI_MCP_URL, {
method: "POST",
headers: {
@@ -72,6 +73,7 @@ async function callZaiTool(apiKey: string, args: Record<string, unknown>): Promi
arguments: args,
},
}),
signal,
});
if (!response.ok) {
@@ -157,7 +159,7 @@ async function callZaiSearch(apiKey: string, params: ZaiSearchParams): Promise<u
let lastError: unknown;
for (let i = 0; i < attempts.length; i++) {
try {
return await callZaiTool(apiKey, attempts[i]);
return await callZaiTool(apiKey, attempts[i], params.signal);
} catch (error) {
lastError = error;
const isLastAttempt = i === attempts.length - 1;
@@ -302,6 +304,7 @@ export class ZaiProvider extends SearchProvider {
return searchZai({
query: params.query,
num_results: params.numSearchResults ?? params.limit,
signal: params.signal,
});
}
}
@@ -337,6 +337,88 @@ describe("AgentSession retry fallback", () => {
expect(lastAssistant.content).toContainEqual({ type: "text", text: "Recovered after OpenAI timeout" });
});
it("auto-retries stream stall errors", async () => {
const model = getBundledModel("openai", "gpt-4o-mini");
if (!model) {
throw new Error("Expected bundled OpenAI test model to exist");
}
const stallMessage = "Provider stream stalled while waiting for the next event";
const requestedModels: string[] = [];
let attemptCount = 0;
const agent = new Agent({
getApiKey: provider => `${provider}-test-key`,
initialState: {
model,
systemPrompt: ["Test"],
tools: [],
messages: [],
},
streamFn: requestedModel => {
requestedModels.push(`${requestedModel.provider}/${requestedModel.id}`);
const stream = new MockAssistantStream();
queueMicrotask(() => {
attemptCount += 1;
if (attemptCount === 1) {
const message = createAssistantMessage(requestedModel, {
stopReason: "error",
errorMessage: stallMessage,
});
stream.push({ type: "start", partial: message });
stream.push({ type: "error", reason: "error", error: message });
return;
}
if (attemptCount === 2) {
const message = createAssistantMessage(requestedModel, {
text: "Recovered after stream stall",
stopReason: "stop",
});
stream.push({
type: "start",
partial: createAssistantMessage(requestedModel, { text: "", stopReason: "stop" }),
});
stream.push({ type: "done", reason: "stop", message });
return;
}
throw new Error(`Unexpected retry attempt in stream stall test: ${attemptCount}`);
});
return stream;
},
});
const settings = Settings.isolated({
"compaction.enabled": false,
"retry.baseDelayMs": 5,
"retry.maxRetries": 1,
});
settings.setModelRole("default", `${model.provider}/${model.id}`);
session = new AgentSession({
agent,
sessionManager: SessionManager.inMemory(),
settings,
modelRegistry,
});
const { retryStartEvents, retryEndEvents } = trackRetryEvents(session);
await session.prompt("Retry stream stall");
await session.waitForIdle();
expect(requestedModels).toEqual([`${model.provider}/${model.id}`, `${model.provider}/${model.id}`]);
expect(retryStartEvents).toHaveLength(1);
expect(retryStartEvents[0]).toMatchObject({
attempt: 1,
maxAttempts: 1,
errorMessage: stallMessage,
});
expect(retryEndEvents).toHaveLength(1);
expect(retryEndEvents[0]).toMatchObject({ success: true, attempt: 1 });
const lastAssistant = getLastAssistantMessage(session);
expect(lastAssistant.stopReason).toBe("stop");
expect(lastAssistant.content).toContainEqual({ type: "text", text: "Recovered after stream stall" });
});
it("auto-retries OpenAI processing-request transient errors", async () => {
const model = getBundledModel("openai", "gpt-4o-mini");
if (!model) {
@@ -313,6 +313,29 @@ describe("hashline parser — block op syntax", () => {
expect(applyDiff(content, diff)).toBe("aaa\n\n# not a header\n+ not an op\n spaced\nccc");
});
it("treats blank lines inside a payload run as empty payload lines", () => {
// Truly blank lines (no leading separator) inside an active payload run
// are silently rewritten to empty payload lines as long as more payload
// follows. This recovers from a common typo where the model forgets the
// separator on what should be a blank inserted line.
const diff = [`= ${sameLineRange(tag(2, "bbb"))}`, pl("first"), "", "", pl("after")].join("\n");
expect(applyDiff(content, diff)).toBe("aaa\nfirst\n\n\nafter\nccc");
});
it("does not consume trailing blank lines between sections as payload", () => {
// Blanks that precede a non-payload op (here, another `=`) end the
// payload run cleanly — they're section separators, not payload.
const diff = [
`= ${sameLineRange(tag(1, "aaa"))}`,
pl("AAA"),
"",
"",
`= ${sameLineRange(tag(3, "ccc"))}`,
pl("CCC"),
].join("\n");
expect(applyDiff(content, diff)).toBe("AAA\nbbb\nCCC");
});
it("rejects missing payloads and orphan payload lines", () => {
expect(() => parseHashline(`+ ${tag(1, "aaa")}`)).toThrow(/require at least one/);
expect(() => parseHashline(pl("orphan"))).toThrow(/payload line has no preceding/);
@@ -90,6 +90,24 @@ describe.skipIf(!SHOULD_RUN)("python runner subprocess", () => {
}
});
it("supports top-level await across cells", async () => {
using tempDir = TempDir.createSync("@python-runner-await-");
const kernel = await PythonKernel.start({ cwd: tempDir.path() });
try {
const first = await executePythonWithKernel(
kernel,
["import asyncio", "x = await asyncio.sleep(0, result=21)", "x * 2"].join("\n"),
);
expect(first.exitCode).toBe(0);
expect(first.output).toContain("42");
const second = await executePythonWithKernel(kernel, "x + 1");
expect(second.exitCode).toBe(0);
expect(second.output).toContain("22");
} finally {
await kernel.shutdown();
}
});
it("translates %pwd magic to the user namespace", async () => {
using tempDir = TempDir.createSync("@python-runner-magic-");
const kernel = await PythonKernel.start({ cwd: tempDir.path() });
@@ -0,0 +1,80 @@
/**
* Regression test for #1075:
* discoverAgents() must skip Claude plugin roots when claude-plugins is disabled.
*/
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { disableProvider, enableProvider } from "../../src/capability";
import { clearCache as clearFsCache } from "../../src/capability/fs";
import { clearClaudePluginRootsCache } from "../../src/discovery/helpers";
import { discoverAgents } from "../../src/task/discovery";
const PLUGIN_AGENT_MD = [
"---",
"name: simplifier",
"description: A code simplifier agent from a Claude plugin",
"---",
"Simplify code.",
].join("\n");
describe("discoverAgents — claude-plugins disabled provider", () => {
let tempHome: string;
beforeEach(() => {
tempHome = fs.mkdtempSync(path.join(os.tmpdir(), "pi-agent-disco-home-"));
// Build a fake Claude plugin install with an agents/ subdirectory.
const pluginInstallPath = path.join(tempHome, "plugin-cache", "code-simplifier");
const agentsDir = path.join(pluginInstallPath, "agents");
fs.mkdirSync(agentsDir, { recursive: true });
fs.writeFileSync(path.join(agentsDir, "simplifier.md"), PLUGIN_AGENT_MD);
// Register the plugin in the Claude registry so listClaudePluginRoots picks it up.
const claudePluginsDir = path.join(tempHome, ".claude", "plugins");
fs.mkdirSync(claudePluginsDir, { recursive: true });
fs.writeFileSync(
path.join(claudePluginsDir, "installed_plugins.json"),
JSON.stringify({
version: 2,
plugins: {
"code-simplifier@claude-plugins-official": [
{
installPath: pluginInstallPath,
version: "1.0.0",
scope: "user",
installedAt: "2025-01-01T00:00:00Z",
lastUpdated: "2025-01-01T00:00:00Z",
},
],
},
}),
);
// Start each test with a clean provider + cache state.
enableProvider("claude-plugins");
clearFsCache();
clearClaudePluginRootsCache();
});
afterEach(() => {
fs.rmSync(tempHome, { recursive: true, force: true });
// Restore global state so other tests in the suite are not affected.
enableProvider("claude-plugins");
clearFsCache();
clearClaudePluginRootsCache();
});
test("includes plugin agents when claude-plugins is enabled", async () => {
const { agents } = await discoverAgents(tempHome, tempHome);
expect(agents.map(a => a.name)).toContain("simplifier");
});
test("excludes plugin agents when claude-plugins is disabled", async () => {
disableProvider("claude-plugins");
clearClaudePluginRootsCache();
const { agents } = await discoverAgents(tempHome, tempHome);
expect(agents.map(a => a.name)).not.toContain("simplifier");
});
});
@@ -393,6 +393,85 @@ describe("listClaudePluginRoots", () => {
expect(found).toBeDefined();
expect(found?.path).toContain(path.join(".claude", "commands", "ship.md"));
});
test("reads slash commands directory from plugin manifest commands field (standard Claude plugin format)", async () => {
const pluginsDir = path.join(tempDir, ".claude", "plugins");
const pluginPath = path.join(tempDir, "plugins", "manifest-commands-key");
await fs.mkdir(path.join(pluginsDir), { recursive: true });
await fs.mkdir(path.join(pluginPath, ".claude-plugin"), { recursive: true });
await fs.mkdir(path.join(pluginPath, ".claude", "commands"), { recursive: true });
const registry = {
version: 2,
plugins: {
"manifest-commands-key@market": [
{
scope: "user",
installPath: pluginPath,
version: "1.0.0",
installedAt: "2025-01-01T00:00:00Z",
lastUpdated: "2025-01-01T00:00:00Z",
},
],
},
};
await fs.writeFile(path.join(pluginsDir, "installed_plugins.json"), JSON.stringify(registry));
await fs.writeFile(
path.join(pluginPath, ".claude-plugin", "plugin.json"),
JSON.stringify({ commands: "./.claude/commands" }),
);
await fs.writeFile(path.join(pluginPath, ".claude", "commands", "plan.md"), "Plan it\n");
const result = await loadCapability<SlashCommand>("slash-commands", { cwd: tempDir });
expect(result.warnings).toEqual([]);
const found = result.all.find(command => command.name === "manifest-commands-key:plan");
expect(found).toBeDefined();
expect(found?.path).toContain(path.join(".claude", "commands", "plan.md"));
});
test("commands field takes precedence over slash-commands field when both are present", async () => {
const pluginsDir = path.join(tempDir, ".claude", "plugins");
const pluginPath = path.join(tempDir, "plugins", "manifest-commands-precedence");
await fs.mkdir(path.join(pluginsDir), { recursive: true });
await fs.mkdir(path.join(pluginPath, ".claude-plugin"), { recursive: true });
// commands points to .claude/commands, slash-commands points to a different dir
await fs.mkdir(path.join(pluginPath, ".claude", "commands"), { recursive: true });
await fs.mkdir(path.join(pluginPath, "legacy-commands"), { recursive: true });
const registry = {
version: 2,
plugins: {
"manifest-commands-precedence@market": [
{
scope: "user",
installPath: pluginPath,
version: "1.0.0",
installedAt: "2025-01-01T00:00:00Z",
lastUpdated: "2025-01-01T00:00:00Z",
},
],
},
};
await fs.writeFile(path.join(pluginsDir, "installed_plugins.json"), JSON.stringify(registry));
await fs.writeFile(
path.join(pluginPath, ".claude-plugin", "plugin.json"),
JSON.stringify({ commands: "./.claude/commands", "slash-commands": "./legacy-commands" }),
);
await fs.writeFile(path.join(pluginPath, ".claude", "commands", "ship.md"), "Ship it\n");
// This file exists only under the legacy dir — should NOT be found
await fs.writeFile(path.join(pluginPath, "legacy-commands", "old.md"), "Old\n");
const result = await loadCapability<SlashCommand>("slash-commands", { cwd: tempDir });
expect(result.warnings).toEqual([]);
const found = result.all.find(command => command.name === "manifest-commands-precedence:ship");
const notFound = result.all.find(command => command.name === "manifest-commands-precedence:old");
expect(found).toBeDefined();
expect(notFound).toBeUndefined();
});
test("ignores manifest skills directory that resolves outside plugin root", async () => {
const pluginsDir = path.join(tempDir, ".claude", "plugins");
const pluginPath = path.join(tempDir, "plugins", "manifest-skills-outside");
@@ -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,8 @@ 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("<system-warning>");
expect(text).toContain("Stripped redundant `| head -3`");
});
it("does not strip when the setting is disabled", async () => {
@@ -604,6 +604,80 @@ describe("github tool", () => {
expect(reposArgs.some(arg => typeof arg === "string" && arg.includes("repo:ignored/value"))).toBe(false);
});
it("search_prs: defaults `repo:` to the current checkout when `repo` is omitted", async () => {
const textSpy = vi.spyOn(git.github, "text").mockResolvedValue("acme/widgets\n");
const jsonSpy = vi.spyOn(git.github, "json").mockResolvedValue({ items: [] });
const tool = new GithubTool(createSession("/tmp/gh-default-prs"));
await tool.execute("search-prs", {
op: "search_prs",
query: "is:open",
limit: 1,
});
// `gh repo view --json nameWithOwner` runs against the session cwd to fetch the
// default scope; the resolved owner/repo gets layered onto the API query.
expect(textSpy).toHaveBeenCalled();
const repoViewArgs = textSpy.mock.calls[0]?.[1] ?? [];
expect(repoViewArgs.slice(0, 2)).toEqual(["repo", "view"]);
expect(repoViewArgs).toContain("nameWithOwner");
const apiArgs = jsonSpy.mock.calls[0]?.[1] ?? [];
expect(apiArgs).toContain("q=is:open repo:acme/widgets is:pr");
});
it("search_issues: skips the current-repo default when the query already carries a scope qualifier", async () => {
const textSpy = vi.spyOn(git.github, "text").mockResolvedValue("acme/widgets\n");
const jsonSpy = vi.spyOn(git.github, "json").mockResolvedValue({ items: [] });
const tool = new GithubTool(createSession("/tmp/gh-default-skip-qualifier"));
await tool.execute("search-issues", {
op: "search_issues",
query: "is:open org:torvalds",
limit: 1,
});
// Explicit `org:` qualifier suppresses the auto-resolved `repo:` injection.
expect(textSpy).not.toHaveBeenCalled();
const apiArgs = jsonSpy.mock.calls[0]?.[1] ?? [];
expect(apiArgs).toContain("q=is:open org:torvalds is:issue");
expect(apiArgs.some(a => typeof a === "string" && a.startsWith("q=") && a.includes("repo:acme/widgets"))).toBe(
false,
);
});
it("search_code: falls back to global search when `gh repo view` cannot resolve the current checkout", async () => {
const textSpy = vi.spyOn(git.github, "text").mockRejectedValue(new Error("not a git repository"));
const jsonSpy = vi.spyOn(git.github, "json").mockResolvedValue({ items: [] });
const tool = new GithubTool(createSession("/tmp/gh-default-no-remote"));
await tool.execute("search-code", {
op: "search_code",
query: "findThing",
limit: 1,
});
expect(textSpy).toHaveBeenCalled();
const apiArgs = jsonSpy.mock.calls[0]?.[1] ?? [];
// No `repo:` should be injected — resolution failed, so the search proceeds globally.
expect(apiArgs).toContain("q=findThing");
expect(apiArgs.some(a => typeof a === "string" && a.startsWith("q=") && a.includes("repo:"))).toBe(false);
});
it("search_commits: honors an explicit `repo` override over the current-checkout default", async () => {
const textSpy = vi.spyOn(git.github, "text").mockResolvedValue("acme/widgets\n");
const jsonSpy = vi.spyOn(git.github, "json").mockResolvedValue({ items: [] });
const tool = new GithubTool(createSession("/tmp/gh-default-explicit-override"));
await tool.execute("search-commits", {
op: "search_commits",
query: "fix",
repo: "other/project",
limit: 1,
});
// Explicit `repo` short-circuits resolution — no `gh repo view` invocation.
expect(textSpy).not.toHaveBeenCalled();
const apiArgs = jsonSpy.mock.calls[0]?.[1] ?? [];
expect(apiArgs).toContain("q=fix repo:other/project");
});
it("checks out a pull request into a worktree and configures contributor push metadata", async () => {
const fixture = await createPrFixture();
const tempHome = await setupTempHome();
@@ -0,0 +1,112 @@
/**
* Round-trip contract between `appendOutputNotice` (via `formatOutputNotice`)
* and `stripOutputNotice`: anything the tool wrapper bakes into the LLM-facing
* content body, the TUI renderer must be able to peel off so the styled
* `⟨…⟩` warning line doesn't double-print next to the verbatim body text.
*
* Regression: bash/eval/ssh/browser/read all printed the same `[Showing …]`
* string twice — once from the body content, once as the styled warning line.
*/
import { describe, expect, it } from "bun:test";
import { formatOutputNotice, type OutputMeta, stripOutputNotice } from "../../src/tools/output-meta";
const truncation: OutputMeta = {
truncation: {
direction: "middle",
truncatedBy: "middle",
totalLines: 8,
totalBytes: 320,
outputLines: 4,
outputBytes: 105,
headRange: { start: 1, end: 2 },
tailRange: { start: 7, end: 8 },
elidedLines: 4,
elidedBytes: 215,
},
};
const tailTruncation: OutputMeta = {
truncation: {
direction: "tail",
truncatedBy: "bytes",
totalLines: 100,
totalBytes: 10_000,
outputLines: 40,
outputBytes: 4_000,
maxBytes: 4_000,
shownRange: { start: 61, end: 100 },
artifactId: "abc123",
},
};
const limitsOnly: OutputMeta = {
limits: { matchLimit: { reached: 50, suggestion: 100 } },
};
describe("stripOutputNotice", () => {
it("removes the exact notice appended by the wrapper for middle elision", () => {
const body = "line1\nline2\n[… 4 lines elided (215B) …]\nline7\nline8";
const notice = formatOutputNotice(truncation);
const combined = body + notice;
// Round-trip: the wrapper appends, the renderer peels off exactly.
expect(stripOutputNotice(combined, truncation)).toBe(body);
});
it("removes the notice for tail truncation including artifact reference", () => {
const body = "long output…";
const notice = formatOutputNotice(tailTruncation);
expect(notice).toContain("artifact://abc123");
expect(stripOutputNotice(body + notice, tailTruncation)).toBe(body);
});
it("removes the notice for limit-only meta (no truncation)", () => {
const body = "results…";
const notice = formatOutputNotice(limitsOnly);
expect(notice).toContain("matches limit reached");
expect(stripOutputNotice(body + notice, limitsOnly)).toBe(body);
});
it("matches the trimEnd()'d body the renderer actually sees", () => {
// bash.ts/eval.ts call `.trimEnd()` on the body before passing it in.
// The notice itself ends with `]`, so trimEnd is a no-op on its tail;
// confirm the strip still succeeds when the renderer hands us either
// the trimmed or untrimmed form.
const body = "the output";
const combined = `${body}${formatOutputNotice(truncation)}\n\n`;
// renderer trims, then strips
expect(stripOutputNotice(combined.trimEnd(), truncation)).toBe(body);
// renderer strips first
expect(stripOutputNotice(combined, truncation).trimEnd()).toBe(body);
});
it("returns input unchanged when meta is undefined", () => {
expect(stripOutputNotice("plain text", undefined)).toBe("plain text");
});
it("returns input unchanged when meta has no notice-emitting fields", () => {
// e.g. meta carries only `source` info; formatOutputNotice yields "".
const sourceOnly: OutputMeta = { source: { type: "path", value: "/tmp/x" } };
expect(formatOutputNotice(sourceOnly)).toBe("");
expect(stripOutputNotice("plain text", sourceOnly)).toBe("plain text");
});
it("returns input unchanged when body does not actually carry the notice (streaming case)", () => {
// During streaming, `renderContext.output` is the live sink content
// before wrappedExecute has appended anything. Calling stripOutputNotice
// eagerly must not corrupt that prefix.
const streaming = "partial output so far…";
expect(stripOutputNotice(streaming, truncation)).toBe(streaming);
});
it("only strips the trailing occurrence, not a coincidental earlier match", () => {
const noticeText = formatOutputNotice(truncation);
// The same notice text appearing mid-body (unlikely but possible if the
// command literally printed it) must be preserved when not at the tail.
const body = `prefix${noticeText} middle suffix`;
expect(stripOutputNotice(body, truncation)).toBe(body);
});
});
+1
View File
@@ -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
+20
View File
@@ -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. */
+1
View File
@@ -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;
+5
View File
@@ -31,6 +31,10 @@ function extractFolderFromPath(sessionPath: string): string {
function isAssistantMessage(entry: SessionEntry): entry is SessionMessageEntry {
if (entry.type !== "message") return false;
const msgEntry = entry as SessionMessageEntry;
// Legacy sessions (pre-id tracking) recorded message entries without an `id`.
// They're not linkable and would violate the messages.entry_id NOT NULL
// constraint, so skip them at the parser boundary.
if (typeof msgEntry.id !== "string" || msgEntry.id.length === 0) return false;
return msgEntry.message?.role === "assistant";
}
@@ -40,6 +44,7 @@ function isAssistantMessage(entry: SessionEntry): entry is SessionMessageEntry {
function isUserMessage(entry: SessionEntry): entry is SessionMessageEntry {
if (entry.type !== "message") return false;
const msgEntry = entry as SessionMessageEntry;
if (typeof msgEntry.id !== "string" || msgEntry.id.length === 0) return false;
return msgEntry.message?.role === "user";
}
+9
View File
@@ -278,6 +278,9 @@ class RpcClient:
session_dir: str | Path | None = None,
cwd: str | Path | None = None,
env: Mapping[str, str] | None = None,
user: int | str | None = None,
group: int | str | None = None,
extra_groups: Sequence[int | str] | None = None,
thinking: ThinkingLevel | None = None,
append_system_prompt: str | None = None,
provider_session_id: str | None = None,
@@ -302,6 +305,9 @@ class RpcClient:
self._session_dir = Path(session_dir) if session_dir is not None else None
self._cwd = Path(cwd) if cwd is not None else None
self._env = dict(env or {})
self._user = user
self._group = group
self._extra_groups = list(extra_groups) if extra_groups is not None else None
self._thinking = thinking
self._append_system_prompt = append_system_prompt
self._provider_session_id = provider_session_id
@@ -403,6 +409,9 @@ class RpcClient:
list(self._build_command()),
cwd=str(self._cwd) if self._cwd is not None else None,
env={**os.environ, **self._env},
user=self._user,
group=self._group,
extra_groups=self._extra_groups,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
+45
View File
@@ -0,0 +1,45 @@
from __future__ import annotations
from unittest.mock import patch
import pytest
from omp_rpc import RpcClient
class _Sentinel(Exception):
pass
def _start_and_capture(**kwargs):
client = RpcClient(**kwargs)
with patch("omp_rpc.client.subprocess.Popen", side_effect=_Sentinel("aborted")) as mock_popen:
with pytest.raises(_Sentinel):
client.start()
assert mock_popen.call_count == 1
return mock_popen.call_args
def test_no_user_group_defaults_to_none():
call = _start_and_capture(executable="omp")
assert call.kwargs["user"] is None
assert call.kwargs["group"] is None
assert call.kwargs["extra_groups"] is None
def test_user_and_group_kwargs_threaded():
call = _start_and_capture(
executable="omp",
user=2001,
group="omp",
extra_groups=[2000, "docker"],
)
assert call.kwargs["user"] == 2001
assert call.kwargs["group"] == "omp"
assert call.kwargs["extra_groups"] == [2000, "docker"]
def test_extra_groups_none_distinct_from_empty():
call = _start_and_capture(executable="omp", extra_groups=[])
# [] means an empty supplementary group list and differs from None.
assert call.kwargs["extra_groups"] == []