diff --git a/README.md b/README.md index 8ce7522..c4b8ddf 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,8 @@ crash-recoverable and inspectable from the CLI the whole way. | `read_file` | One path for files (hashline `line#hash` anchors), directories (sorted listings), and http(s) URLs (streamed cap; refused when the sandbox denies network) | | `write_file` / `edit_file` | Writes under sandbox confinement; edits by exact string or by anchored hashline patch with stale-anchor recovery | | `grep` / `glob` | Regex content search and path patterns, `.gitignore`-aware | +| `ast_grep` / `ast_edit` | Structural search and rewrite over the syntax tree via [ast-grep](https://ast-grep.github.io) (when installed); rewrites preview by default and write only on `apply: true` | +| `github` | GitHub CLI operations with your own `gh` login (when installed) — reads run in parallel, mutations stay serial | | `agent` | The pen: delegate to durable child agents (above) | | `job` | The coordination plane: list children with live state, wait for a result, cancel a background child | | `todo` | A durable session plan in the store — survives crashes and resumes; one item in progress at a time, enforced by the runtime | diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index ebf15ac..4cf4f98 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -598,6 +598,9 @@ async fn run( Store::default_path(), &session.id, ))); + // GitHub, with the user's own gh login. Top-level runs only — pen + // children keep to the workspace unless the coordinator acts itself. + registry.register(std::sync::Arc::new(bullpen_tools::GitHub::new())); // Follow-up questions reach the terminal only when someone is on it; // otherwise the detached variant answers with the reason instead. let interactive = !json && std::io::IsTerminal::is_terminal(&std::io::stdin()); diff --git a/crates/harness/src/pen.rs b/crates/harness/src/pen.rs index 41fff81..6a38962 100644 --- a/crates/harness/src/pen.rs +++ b/crates/harness/src/pen.rs @@ -32,7 +32,7 @@ use std::time::Duration; use bullpen_agent::{Agent, AgentConfig}; use bullpen_llm::{Provider, Role, ToolSpec}; use bullpen_store::{Session, SessionWorker, Store}; -use bullpen_tools::{Glob, Grep, ReadFile, Registry, Tool, ToolCtx, ToolError}; +use bullpen_tools::{AstGrep, Glob, Grep, ReadFile, Registry, Tool, ToolCtx, ToolError}; use serde_json::{Value, json}; use crate::{StoreJournal, prepare_session, worktree}; @@ -187,6 +187,7 @@ fn registry_for_mode(mode: &str) -> Result { r.register(Arc::new(ReadFile)); r.register(Arc::new(Grep)); r.register(Arc::new(Glob)); + r.register(Arc::new(AstGrep::new())); Ok(r) } // Full workspace tools — still no nested pen. diff --git a/crates/tools/src/ast.rs b/crates/tools/src/ast.rs new file mode 100644 index 0000000..97f1994 --- /dev/null +++ b/crates/tools/src/ast.rs @@ -0,0 +1,514 @@ +//! Structural code search and rewrite, by shelling out to [ast-grep]. +//! +//! Two tools share one adapter: `ast_grep` finds pattern matches as +//! `file:line` results, `ast_edit` rewrites them — previewing by default +//! and applying only on `apply: true`, so the model sees the diff before +//! anything changes on disk. The binary is discovered at call time +//! (`ast-grep`, then `sg`, verified by `--version`), and its absence is a +//! clear error with an install hint, never a degraded fallback to text +//! search. +//! +//! Applying under a sandbox runs the binary through the same shell +//! wrapper as `bash`, so on macOS the rewrite is Seatbelt-confined with +//! everything else; elsewhere it carries bash's documented caveat. +//! +//! [ast-grep]: https://ast-grep.github.io + +use std::path::PathBuf; +use std::process::Stdio; +use std::time::Duration; + +use bullpen_llm::ToolSpec; +use serde_json::{Value, json}; + +use crate::{Tool, ToolCtx, ToolError, required_str, truncate_middle}; + +const TIMEOUT_SECS: u64 = 120; +const MAX_MATCHES: usize = 200; +const MAX_OUTPUT_BYTES: usize = 100_000; + +/// Locate a working ast-grep binary: an explicit override, else `ast-grep` +/// or `sg` on PATH — accepted only if `--version` says it is ast-grep, +/// because `sg` is also the Unix shell-group utility. +async fn find_binary(explicit: Option<&PathBuf>) -> Result { + let candidates: Vec = match explicit { + Some(path) => vec![path.display().to_string()], + None => vec!["ast-grep".into(), "sg".into()], + }; + for candidate in &candidates { + let probe = tokio::process::Command::new(candidate) + .arg("--version") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .kill_on_drop(true) + .output() + .await; + if let Ok(out) = probe + && out.status.success() + && String::from_utf8_lossy(&out.stdout).starts_with("ast-grep") + { + return Ok(candidate.clone()); + } + } + Err(ToolError::Failed(format!( + "no ast-grep binary found (tried {}) — install it from \ + https://ast-grep.github.io (e.g. `cargo install ast-grep`) to use \ + structural search", + candidates.join(", ") + ))) +} + +/// Shared inputs: the pattern, an optional language, and a path (default +/// the workspace root). +fn common_args(input: &Value) -> Result<(String, Vec), ToolError> { + let pattern = required_str(input, "pattern")?.to_string(); + let mut args = vec!["run".to_string(), "--pattern".to_string(), pattern]; + if let Some(lang) = input.get("lang").and_then(Value::as_str) { + args.push("--lang".into()); + args.push(lang.into()); + } + let path = input.get("path").and_then(Value::as_str).unwrap_or("."); + Ok((path.to_string(), args)) +} + +async fn run_binary( + ctx: &ToolCtx, + program: &str, + args: &[String], +) -> Result { + let child = tokio::process::Command::new(program) + .args(args) + .current_dir(&ctx.workspace) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .map_err(|e| ToolError::Failed(format!("failed to run {program}: {e}")))?; + tokio::time::timeout(Duration::from_secs(TIMEOUT_SECS), child.wait_with_output()) + .await + .map_err(|_| ToolError::Timeout(TIMEOUT_SECS))? + .map_err(|e| ToolError::Failed(format!("failed to collect output: {e}"))) +} + +fn failed(program: &str, out: &std::process::Output) -> ToolError { + ToolError::Failed(format!( + "{program} exited with {}:\n{}", + out.status.code().unwrap_or(-1), + truncate_middle(String::from_utf8_lossy(&out.stderr).into_owned(), 2_000) + )) +} + +/// `ast_grep`: structural search over the workspace. +pub struct AstGrep { + binary: Option, +} + +impl AstGrep { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + Self { binary: None } + } + + pub fn with_binary(path: impl Into) -> Self { + Self { + binary: Some(path.into()), + } + } +} + +#[async_trait::async_trait] +impl Tool for AstGrep { + fn name(&self) -> &'static str { + "ast_grep" + } + + fn spec(&self) -> ToolSpec { + ToolSpec { + name: "ast_grep".into(), + description: "Structural code search via ast-grep: `pattern` is \ + matched against the syntax tree, not text, with \ + metavariables like $NAME and $$$ARGS (e.g. \ + `foo($$$ARGS)` finds every call to foo). Results \ + are `file:line` plus the matched text. `lang` \ + forces a language; `path` narrows the search. \ + Needs the ast-grep binary installed." + .into(), + input_schema: json!({ + "type": "object", + "properties": { + "pattern": {"type": "string", "description": "Structural pattern, e.g. `foo($$$ARGS)`"}, + "lang": {"type": "string", "description": "Language to parse as (e.g. rust, ts, python); inferred from extensions when omitted"}, + "path": {"type": "string", "description": "File or directory to search (default: workspace root)"} + }, + "required": ["pattern"] + }), + } + } + + fn parallel_safe(&self, _input: &Value) -> bool { + true + } + + fn replay_safe(&self) -> bool { + true + } + + async fn run(&self, ctx: &ToolCtx, _call_id: &str, input: Value) -> Result { + let program = find_binary(self.binary.as_ref()).await?; + let (path, mut args) = common_args(&input)?; + args.push("--json=stream".into()); + args.push(path); + + let out = run_binary(ctx, &program, &args).await?; + if !out.status.success() { + return Err(failed(&program, &out)); + } + + let stdout = String::from_utf8_lossy(&out.stdout); + let matches: Vec = stdout + .lines() + .filter_map(|line| serde_json::from_str::(line).ok()) + .map(|m| { + let file = m["file"].as_str().unwrap_or("?").to_string(); + // ast-grep reports 0-based lines; the workspace convention + // (read_file, grep) is 1-based. + let line = m["range"]["start"]["line"].as_u64().unwrap_or(0) + 1; + let text = m["text"].as_str().unwrap_or("").to_string(); + let (first, rest) = match text.split_once('\n') { + Some((first, _)) => (first, " …"), + None => (text.as_str(), ""), + }; + format!("{file}:{line}\t{first}{rest}") + }) + .collect(); + + if matches.is_empty() { + return Ok("no matches".into()); + } + let shown = matches + .iter() + .take(MAX_MATCHES) + .cloned() + .collect::>(); + let mut result = format!("{} match(es):\n{}", matches.len(), shown.join("\n")); + if matches.len() > MAX_MATCHES { + result.push_str(&format!("\n[stopped at {MAX_MATCHES} matches]")); + } + Ok(truncate_middle(result, MAX_OUTPUT_BYTES)) + } +} + +/// `ast_edit`: structural rewrite, previewed by default. +pub struct AstEdit { + binary: Option, +} + +impl AstEdit { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + Self { binary: None } + } + + pub fn with_binary(path: impl Into) -> Self { + Self { + binary: Some(path.into()), + } + } +} + +/// Quote one argument for `bash -c`, for the sandbox-wrapped apply path. +fn shell_quote(arg: &str) -> String { + format!("'{}'", arg.replace('\'', "'\\''")) +} + +#[async_trait::async_trait] +impl Tool for AstEdit { + fn name(&self) -> &'static str { + "ast_edit" + } + + fn spec(&self) -> ToolSpec { + ToolSpec { + name: "ast_edit".into(), + description: "Structural rewrite via ast-grep: every match of \ + `pattern` becomes `rewrite`, with metavariables \ + carried over (e.g. `foo($A)` → `bar($A)`). By \ + default nothing is written — the call returns the \ + diff to review; call again with `apply: true` to \ + write it. `lang` and `path` as in ast_grep. Needs \ + the ast-grep binary installed." + .into(), + input_schema: json!({ + "type": "object", + "properties": { + "pattern": {"type": "string", "description": "Structural pattern to match"}, + "rewrite": {"type": "string", "description": "Replacement, using the pattern's metavariables"}, + "lang": {"type": "string", "description": "Language to parse as; inferred from extensions when omitted"}, + "path": {"type": "string", "description": "File or directory to rewrite (default: workspace root)"}, + "apply": {"type": "boolean", "description": "Write the changes (default: false — preview the diff)"} + }, + "required": ["pattern", "rewrite"] + }), + } + } + + fn parallel_safe(&self, input: &Value) -> bool { + // A preview is a read; an apply mutates the workspace. + !input.get("apply").and_then(Value::as_bool).unwrap_or(false) + } + + async fn run(&self, ctx: &ToolCtx, _call_id: &str, input: Value) -> Result { + let program = find_binary(self.binary.as_ref()).await?; + let rewrite = required_str(&input, "rewrite")?.to_string(); + let apply = input.get("apply").and_then(Value::as_bool).unwrap_or(false); + let (path, mut args) = common_args(&input)?; + args.push("--rewrite".into()); + args.push(rewrite); + if apply { + args.push("--update-all".into()); + } + args.push(path); + + let out = if apply && let Some(sandbox) = &ctx.sandbox { + // The same posture as `bash`: the rewrite runs through the + // sandbox's shell wrapper, Seatbelt-confined where the OS + // enforces it. + let command = std::iter::once(program.as_str()) + .chain(args.iter().map(String::as_str)) + .map(shell_quote) + .collect::>() + .join(" "); + let (wrapped, wrapped_args) = sandbox.wrap_bash(&command); + run_binary(ctx, &wrapped, &wrapped_args).await? + } else { + run_binary(ctx, &program, &args).await? + }; + if !out.status.success() { + return Err(failed(&program, &out)); + } + + let stdout = String::from_utf8_lossy(&out.stdout); + if apply { + // ast-grep reports "Applied N changes" on stderr, not stdout. + let stderr = String::from_utf8_lossy(&out.stderr); + let summary = format!("{}\n{}", stdout.trim(), stderr.trim()); + let summary = summary.trim(); + return Ok(if summary.is_empty() { + "no matches — nothing changed".into() + } else { + truncate_middle(summary.to_string(), MAX_OUTPUT_BYTES) + }); + } + if stdout.trim().is_empty() { + return Ok("no matches — nothing to rewrite".into()); + } + Ok(truncate_middle( + format!( + "preview only — nothing written; call again with `apply: true` \ + to write this:\n\n{stdout}" + ), + MAX_OUTPUT_BYTES, + )) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn ctx(dir: &tempfile::TempDir) -> ToolCtx { + ToolCtx::new(dir.path().to_path_buf()) + } + + /// A stand-in binary: passes the version probe, records its arguments, + /// and prints a canned payload. + fn stub(dir: &tempfile::TempDir, payload: &str) -> PathBuf { + let path = dir.path().join("fake-ast-grep"); + let script = format!( + "#!/bin/sh\nif [ \"$1\" = --version ]; then echo ast-grep 0.0.0-stub; exit 0; fi\n\ + printf '%s ' \"$@\" > {}/args.txt\ncat <<'PAYLOAD'\n{payload}\nPAYLOAD\n", + dir.path().display() + ); + std::fs::write(&path, script).unwrap(); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + // Exec'ing a just-written script races concurrent tests' forks on + // Linux (ETXTBSY: a forked child still holds the write fd until its + // own exec completes). Probe until the script is spawnable. + for _ in 0..200 { + if std::process::Command::new(&path) + .arg("--version") + .output() + .is_ok() + { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + path + } + + fn recorded_args(dir: &tempfile::TempDir) -> String { + std::fs::read_to_string(dir.path().join("args.txt")).unwrap() + } + + #[tokio::test] + async fn a_missing_binary_is_a_clear_error_with_an_install_hint() { + let dir = tempfile::tempdir().unwrap(); + let err = AstGrep::with_binary("/nonexistent/ast-grep") + .run(&ctx(&dir), "t", json!({"pattern": "foo($A)"})) + .await + .unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("no ast-grep binary"), "{msg}"); + assert!(msg.contains("install"), "{msg}"); + } + + #[tokio::test] + async fn search_renders_matches_one_indexed_and_passes_the_right_flags() { + let dir = tempfile::tempdir().unwrap(); + let payload = r#"{"text":"foo(1)","file":"a.rs","range":{"start":{"line":1}}} +{"text":"foo(2)\nmore","file":"b/c.rs","range":{"start":{"line":41}}}"#; + let bin = stub(&dir, payload); + + let out = AstGrep::with_binary(&bin) + .run( + &ctx(&dir), + "t", + json!({"pattern": "foo($A)", "lang": "rust", "path": "src"}), + ) + .await + .unwrap(); + assert!(out.contains("2 match(es):"), "{out}"); + assert!(out.contains("a.rs:2\tfoo(1)"), "{out}"); + // Multi-line match text keeps only its first line. + assert!(out.contains("b/c.rs:42\tfoo(2) …"), "{out}"); + + let args = recorded_args(&dir); + assert!(args.contains("run --pattern foo($A) --lang rust"), "{args}"); + assert!(args.contains("--json=stream src"), "{args}"); + } + + #[tokio::test] + async fn edit_previews_by_default_and_applies_only_on_request() { + let dir = tempfile::tempdir().unwrap(); + let bin = stub(&dir, "a.rs\n1│-foo(1)\n1│+bar(1)"); + + let out = AstEdit::with_binary(&bin) + .run( + &ctx(&dir), + "t", + json!({"pattern": "foo($A)", "rewrite": "bar($A)"}), + ) + .await + .unwrap(); + assert!(out.contains("preview only"), "{out}"); + assert!(out.contains("apply: true"), "{out}"); + let args = recorded_args(&dir); + assert!(args.contains("--rewrite bar($A)"), "{args}"); + assert!(!args.contains("--update-all"), "{args}"); + + let bin = stub(&dir, "Applied 2 changes"); + let out = AstEdit::with_binary(&bin) + .run( + &ctx(&dir), + "t", + json!({"pattern": "foo($A)", "rewrite": "bar($A)", "apply": true}), + ) + .await + .unwrap(); + assert_eq!(out, "Applied 2 changes"); + assert!(recorded_args(&dir).contains("--update-all"), "{}", ""); + } + + #[tokio::test] + async fn missing_required_fields_are_invalid_input() { + let dir = tempfile::tempdir().unwrap(); + let bin = stub(&dir, ""); + let err = AstGrep::with_binary(&bin) + .run(&ctx(&dir), "t", json!({})) + .await + .unwrap_err(); + assert!(matches!(err, ToolError::InvalidInput(_))); + let err = AstEdit::with_binary(&bin) + .run(&ctx(&dir), "t", json!({"pattern": "x"})) + .await + .unwrap_err(); + assert!(matches!(err, ToolError::InvalidInput(_))); + } + + #[test] + fn previews_and_searches_are_parallel_safe_applies_are_not() { + let grep = AstGrep::new(); + let edit = AstEdit::new(); + assert!(grep.parallel_safe(&json!({"pattern": "x"}))); + assert!(grep.replay_safe()); + assert!(edit.parallel_safe(&json!({"pattern": "x", "rewrite": "y"}))); + assert!(!edit.parallel_safe(&json!({"pattern": "x", "rewrite": "y", "apply": true}))); + assert!(!edit.replay_safe()); + } + + #[test] + fn shell_quoting_survives_single_quotes() { + assert_eq!(shell_quote("fo'o"), "'fo'\\''o'"); + } + + /// End-to-end against a real binary, when one is reachable: honored in + /// local runs with ast-grep installed (or `AST_GREP_BIN` set); skipped + /// where it is absent, like CI. + #[tokio::test] + async fn real_binary_roundtrip_when_available() { + let explicit = std::env::var("AST_GREP_BIN").ok().map(PathBuf::from); + if find_binary(explicit.as_ref()).await.is_err() { + eprintln!("skipping: no real ast-grep binary available"); + return; + } + let make = |dir: &tempfile::TempDir| { + std::fs::write( + dir.path().join("a.rs"), + "fn main() {\n let x = foo(1);\n}\n", + ) + .unwrap(); + }; + let tool_pair = || match &explicit { + Some(bin) => (AstGrep::with_binary(bin), AstEdit::with_binary(bin)), + None => (AstGrep::new(), AstEdit::new()), + }; + + let dir = tempfile::tempdir().unwrap(); + make(&dir); + let (grep, edit) = tool_pair(); + let out = grep + .run(&ctx(&dir), "t", json!({"pattern": "foo($A)"})) + .await + .unwrap(); + assert!(out.contains("a.rs:2\tfoo(1)"), "{out}"); + + let out = edit + .run( + &ctx(&dir), + "t", + json!({"pattern": "foo($A)", "rewrite": "bar($A)"}), + ) + .await + .unwrap(); + assert!(out.contains("preview only"), "{out}"); + // The preview wrote nothing. + let content = std::fs::read_to_string(dir.path().join("a.rs")).unwrap(); + assert!(content.contains("foo(1)"), "{content}"); + + let out = edit + .run( + &ctx(&dir), + "t", + json!({"pattern": "foo($A)", "rewrite": "bar($A)", "apply": true}), + ) + .await + .unwrap(); + assert!(out.contains("Applied"), "{out}"); + let content = std::fs::read_to_string(dir.path().join("a.rs")).unwrap(); + assert!(content.contains("bar(1)"), "{content}"); + } +} diff --git a/crates/tools/src/github.rs b/crates/tools/src/github.rs new file mode 100644 index 0000000..3fdc8f8 --- /dev/null +++ b/crates/tools/src/github.rs @@ -0,0 +1,301 @@ +//! GitHub operations, by shelling out to the [gh] CLI. +//! +//! One tool, raw `gh` arguments as an array — no shell in between, so +//! nothing needs quoting and nothing can be injected. The tool adds what +//! gh cannot know: the runtime decides parallel safety from the verb (a +//! `view` can ride alongside anything; a `merge` cannot), a sandbox that +//! denies network refuses the call outright, and a missing or logged-out +//! binary is a clear error with the setup hint. +//! +//! gh runs with the user's own login and acts with their authority on +//! remote state — which the workspace sandbox's *write* confinement +//! deliberately does not govern: it fences the filesystem, not GitHub. +//! +//! [gh]: https://cli.github.com + +use std::process::Stdio; +use std::time::Duration; + +use bullpen_llm::ToolSpec; +use serde_json::{Value, json}; + +use crate::{Tool, ToolCtx, ToolError, truncate_middle}; + +const TIMEOUT_SECS: u64 = 120; +const MAX_OUTPUT_BYTES: usize = 100_000; + +/// Second-position verbs that only read remote state. `api` is absent on +/// purpose: it can carry any method, so it stays serial. Defaults closed. +const READ_VERBS: &[&str] = &["view", "list", "diff", "checks", "status"]; + +pub struct GitHub { + binary: String, +} + +impl GitHub { + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + Self { + binary: "gh".into(), + } + } + + pub fn with_binary(binary: impl Into) -> Self { + Self { + binary: binary.into(), + } + } +} + +fn parse_args(input: &Value) -> Result, ToolError> { + let args: Vec = input + .get("args") + .and_then(Value::as_array) + .map(|a| { + a.iter() + .filter_map(Value::as_str) + .map(str::to_string) + .collect() + }) + .unwrap_or_default(); + if args.is_empty() { + return Err(ToolError::InvalidInput( + "`args` must be a non-empty array of gh arguments, e.g. \ + [\"pr\", \"view\", \"18\"]" + .into(), + )); + } + Ok(args) +} + +#[async_trait::async_trait] +impl Tool for GitHub { + fn name(&self) -> &'static str { + "github" + } + + fn spec(&self) -> ToolSpec { + ToolSpec { + name: "github".into(), + description: "Run a GitHub CLI (gh) command with the user's login: \ + `args` is the argument array, e.g. [\"pr\", \"list\"], \ + [\"pr\", \"view\", \"18\", \"--comments\"], [\"run\", \ + \"view\", \"--log-failed\"], [\"issue\", \"create\", \ + \"--title\", ...]. No shell is involved — pass each \ + argument as its own array element. Acts as the user on \ + GitHub, so mutations (create, merge, close) are real; \ + prefer read commands unless the task calls for a \ + change. Needs gh installed and authenticated." + .into(), + input_schema: json!({ + "type": "object", + "properties": { + "args": { + "type": "array", + "items": {"type": "string"}, + "description": "gh arguments, one element each" + } + }, + "required": ["args"] + }), + } + } + + fn parallel_safe(&self, input: &Value) -> bool { + // A read (`pr view`, `run list`, `search …`) can run alongside + // anything; everything else — mutations and `api` — stays serial. + match parse_args(input) { + Ok(args) => { + args.first().is_some_and(|first| first == "search") + || args + .get(1) + .is_some_and(|verb| READ_VERBS.contains(&verb.as_str())) + } + Err(_) => false, + } + } + + async fn run(&self, ctx: &ToolCtx, _call_id: &str, input: Value) -> Result { + if let Some(sandbox) = &ctx.sandbox + && !sandbox.capabilities().allow_network + { + return Err(ToolError::Failed( + "sandbox: network access is disabled, so GitHub is out of reach".into(), + )); + } + let args = parse_args(&input)?; + + let child = tokio::process::Command::new(&self.binary) + .args(&args) + .current_dir(&ctx.workspace) + // Never fall into an interactive prompt a headless run cannot + // answer. + .env("GH_PROMPT_DISABLED", "1") + .env("NO_COLOR", "1") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .map_err(|e| { + ToolError::Failed(format!( + "failed to run {}: {e} — install the GitHub CLI from \ + https://cli.github.com and run `gh auth login`", + self.binary + )) + })?; + let out = tokio::time::timeout(Duration::from_secs(TIMEOUT_SECS), child.wait_with_output()) + .await + .map_err(|_| ToolError::Timeout(TIMEOUT_SECS))? + .map_err(|e| ToolError::Failed(format!("failed to collect output: {e}")))?; + + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + if !out.status.success() { + return Err(ToolError::Failed(format!( + "gh {} exited with {}:\n{}", + args.join(" "), + out.status.code().unwrap_or(-1), + truncate_middle(format!("{}{}", stderr.trim(), stdout.trim()), 4_000) + ))); + } + let text = if stdout.trim().is_empty() { + // Some gh commands (e.g. `run watch`) narrate on stderr. + stderr.into_owned() + } else { + stdout.into_owned() + }; + Ok(if text.trim().is_empty() { + "(no output)".into() + } else { + truncate_middle(text, MAX_OUTPUT_BYTES) + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn ctx(dir: &tempfile::TempDir) -> ToolCtx { + ToolCtx::new(dir.path().to_path_buf()) + } + + /// A stand-in gh: records its arguments, prints a canned payload. + fn stub(dir: &tempfile::TempDir, payload: &str, exit: i32) -> PathBuf { + let path = dir.path().join("fake-gh"); + let script = format!( + "#!/bin/sh\nprintf '%s ' \"$@\" > {}/args.txt\n\ + printf '%s\\n' \"{payload}\"\nexit {exit}\n", + dir.path().display() + ); + std::fs::write(&path, script).unwrap(); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + // See ast.rs: exec'ing a just-written script can hit ETXTBSY under + // parallel test load; probe until spawnable. + for _ in 0..200 { + if std::process::Command::new(&path).arg("-h").output().is_ok() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + path + } + + fn tool(path: &std::path::Path) -> GitHub { + GitHub::with_binary(path.display().to_string()) + } + + #[tokio::test] + async fn runs_gh_with_the_args_verbatim() { + let dir = tempfile::tempdir().unwrap(); + let bin = stub(&dir, "18 OPEN feat: things", 0); + let out = tool(&bin) + .run( + &ctx(&dir), + "t", + json!({"args": ["pr", "view", "18", "--comments"]}), + ) + .await + .unwrap(); + assert_eq!(out.trim(), "18 OPEN feat: things"); + let args = std::fs::read_to_string(dir.path().join("args.txt")).unwrap(); + // The -h element is the warm-up probe's leftover only if the real + // call never overwrote it; the real call must have. + assert_eq!(args.trim(), "pr view 18 --comments"); + } + + #[tokio::test] + async fn failures_carry_the_exit_and_stderr() { + let dir = tempfile::tempdir().unwrap(); + let bin = stub(&dir, "auth required", 4); + let err = tool(&bin) + .run(&ctx(&dir), "t", json!({"args": ["pr", "list"]})) + .await + .unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("exited with 4"), "{msg}"); + assert!(msg.contains("auth required"), "{msg}"); + } + + #[tokio::test] + async fn a_missing_binary_hints_at_setup() { + let dir = tempfile::tempdir().unwrap(); + let err = GitHub::with_binary("/nonexistent/gh") + .run(&ctx(&dir), "t", json!({"args": ["pr", "list"]})) + .await + .unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("cli.github.com"), "{msg}"); + assert!(msg.contains("gh auth login"), "{msg}"); + } + + #[tokio::test] + async fn a_network_denying_sandbox_refuses() { + let dir = tempfile::tempdir().unwrap(); + let sandbox = std::sync::Arc::new(bullpen_sandbox::Sandbox::strict(dir.path())); + let c = ToolCtx::new(dir.path()).with_sandbox(sandbox); + let err = GitHub::new() + .run(&c, "t", json!({"args": ["pr", "list"]})) + .await + .unwrap_err(); + assert!(err.to_string().contains("network"), "{err}"); + } + + #[tokio::test] + async fn empty_args_are_invalid() { + let dir = tempfile::tempdir().unwrap(); + for input in [json!({}), json!({"args": []})] { + let err = GitHub::new().run(&ctx(&dir), "t", input).await.unwrap_err(); + assert!(matches!(err, ToolError::InvalidInput(_)), "{err}"); + } + } + + #[test] + fn only_reads_are_parallel_safe() { + let gh = GitHub::new(); + for read in [ + json!({"args": ["pr", "view", "18"]}), + json!({"args": ["pr", "list"]}), + json!({"args": ["run", "list"]}), + json!({"args": ["pr", "checks", "18"]}), + json!({"args": ["pr", "diff", "18"]}), + json!({"args": ["repo", "view"]}), + json!({"args": ["search", "code", "foo"]}), + ] { + assert!(gh.parallel_safe(&read), "{read}"); + } + for write in [ + json!({"args": ["pr", "create"]}), + json!({"args": ["pr", "merge", "18"]}), + json!({"args": ["issue", "close", "3"]}), + json!({"args": ["api", "repos/o/r/pulls"]}), + json!({"args": []}), + ] { + assert!(!gh.parallel_safe(&write), "{write}"); + } + assert!(!gh.replay_safe()); + } +} diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index ad01f62..696242c 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -5,13 +5,17 @@ //! the parallel-safety decision via [`Tool::parallel_safe`]. mod ask; +mod ast; mod bash; mod fs; +mod github; mod search; pub use ask::{Ask, Asker}; +pub use ast::{AstEdit, AstGrep}; pub use bash::Bash; pub use fs::{EditFile, ReadFile, WriteFile}; +pub use github::GitHub; pub use search::{Glob, Grep}; use std::collections::BTreeMap; @@ -112,7 +116,8 @@ impl Registry { self.tools.values().map(|t| t.spec()).collect() } - /// The standard workspace registry: shell, file I/O, and search. + /// The standard workspace registry: shell, file I/O, and search — both + /// textual and structural. pub fn standard() -> Self { let mut r = Self::new(); r.register(Arc::new(Bash)); @@ -121,6 +126,8 @@ impl Registry { r.register(Arc::new(EditFile)); r.register(Arc::new(Grep)); r.register(Arc::new(Glob)); + r.register(Arc::new(AstGrep::new())); + r.register(Arc::new(AstEdit::new())); r } } @@ -179,6 +186,8 @@ mod tests { assert_eq!( names, vec![ + "ast_edit", + "ast_grep", "bash", "edit_file", "glob", diff --git a/docs/TOOLS.md b/docs/TOOLS.md index 9356ff1..8513673 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -22,6 +22,8 @@ what bullpen ships today, and in what order the rest should land. | `job` | background coordination | The coordination plane, exposed to the model: `list` children with store-derived state (status + pid liveness), `wait` polls to a terminal state and returns the child's answer, `cancel` signals a background child — which finishes as failed and stays resumable. | | `todo` | session plan | Durable todo list in the store; replay-safe via deterministic item ids; the store enforces one item in progress at a time. | | `ask` | follow-up questions | Transport-agnostic: interactive runs answer from the terminal, detached runs (background, `--json`, piped) fail the call with the reason instead of blocking forever. Numbered options resolve to their text. | +| `ast_grep` / `ast_edit` | structural search / rewrite | Shell out to the [ast-grep] binary, discovered at call time (`ast-grep`, then `sg`, verified by `--version`); absence is a clear error with an install hint. `ast_edit` previews the diff by default and writes only on `apply: true` — the catalog's preview-then-resolve, folded into one tool. Applies run through the sandbox's shell wrapper, like `bash`. | +| `github` | GitHub CLI ops | Raw `gh` arguments as an array, no shell in between. The runtime derives parallel safety from the verb (views/lists/diffs/checks ride together, mutations and `api` stay serial), a network-denying sandbox refuses the call, and a missing binary errors with the install + `gh auth login` hint. Registered on top-level runs only — pen children keep to the workspace. | Two catalog entries cost nothing because they already exist under other names: `find` is `glob`, `search` is `grep`, and `task` is the pen's @@ -37,10 +39,6 @@ names: `find` is `glob`, `search` is `grep`, and `task` is the pen's ## Then: files & search, deepened -- **`ast_grep` / `ast_edit`** — structural queries and previewed rewrites - by shelling out to [ast-grep]. Preview-then-apply maps onto intent - records: the preview is durable, the apply is a separate confirmed step - (the catalog's `resolve`). - **richer `read`** — directories and URLs are in; archives, SQLite, and PDFs remain, each an incremental, independently testable decoder behind the existing tool. @@ -50,7 +48,6 @@ names: `find` is `glob`, `search` is `grep`, and `task` is the pen's Each of these wraps a proven external surface; the work is inputs, sandbox policy, and output discipline, not invention. -- **`github`** — `gh` CLI operations (repo, PR, issues, run-watch). - **`web_search`** — provider-backed search (page retrieval already ships as URL reads). `--sandbox-strict` (network cut) must disable it cleanly, as it already does for URL reads.