diff --git a/README.md b/README.md index 7c6f681..0188550 100644 --- a/README.md +++ b/README.md @@ -203,6 +203,8 @@ Resolution order: `--config ` → `$RA_CONFIG` → `./ra.toml` → | `gh` | Runs native GitHub CLI (`gh`) with argv-safe arguments; ACP hosts use the terminal reverse-call with shell quoting. This path does not use RTK. | | `jq` | Runs jq filters against inline JSON or a JSON file with argv-safe stdin and a bounded JSON envelope. | | `mergiraf` | Runs mergiraf merge, solve, and languages actions with argv-safe arguments and bounded JSON output. | +| `sd` | Runs sd regex/literal find-replace across explicit file paths with argv-safe arguments and bounded JSON output. | +| `comby` | Runs comby structural check, diff, and rewrite actions with argv-safe arguments and bounded JSON output. | | `mise` | Runs mise tasks/tests with argv-safe arguments and bounded JSON output. | | `just` | Runs just recipes with argv-safe arguments and bounded JSON output. | | `wrkflw` | Runs wrkflw local GitHub Actions validation/execution with argv-safe arguments and bounded JSON output. | @@ -231,13 +233,13 @@ native because it is structured code search rather than a plain shell command. `bash` remains the fallback for project scripts, tests, and one-off command pipelines. -Native `git` / `gh` / `jq` / `mergiraf` prioritize argv safety over RTK rewriting. If a +Native `git` / `gh` / `jq` / `mergiraf` / `sd` / `comby` prioritize argv safety over RTK rewriting. If a high-volume native CLI command needs RTK output compression, run it through `bash` instead so the existing RTK rewrite path can apply. `jq` requires the system `jq` binary on `PATH`; missing jq returns structured install guidance instead of an opaque spawn error. -`mergiraf` requires the system `mergiraf` binary on `PATH`; missing -mergiraf returns structured install guidance. Native `mise` / `just` / +`mergiraf`, `sd`, and `comby` require their system binaries on `PATH`; +missing binaries return structured install guidance. Native `mise` / `just` / `wrkflw` are intended for test-first task and workflow loops such as `mise run test`, `just test`, and local GitHub Actions validation. They run local binaries with argv-safe arguments, optional `cwd`/timeout diff --git a/openspec/changes/add-sd-comby-tools/.openspec.yaml b/openspec/changes/add-sd-comby-tools/.openspec.yaml new file mode 100644 index 0000000..db47328 --- /dev/null +++ b/openspec/changes/add-sd-comby-tools/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-02 diff --git a/openspec/changes/add-sd-comby-tools/design.md b/openspec/changes/add-sd-comby-tools/design.md new file mode 100644 index 0000000..0ef0d28 --- /dev/null +++ b/openspec/changes/add-sd-comby-tools/design.md @@ -0,0 +1,125 @@ +# Design + +## Context + +Ra already has native tool wrappers for common CLIs. `git` and `gh` preserve +argv boundaries, while `jq`, task workflow tools, webfetch, tmux, and openspec +return bounded JSON envelopes with structured missing-binary and timeout +handling. The `sd` and `comby` tools should follow the JSON-envelope pattern +because they can fail independently, emit large diffs, and need deterministic +fake-binary integration tests. + +The user-facing need is split across two rewriting classes: + +- `sd`: fast regex or literal replacement across explicit file path arguments. +- `comby`: structural template check, diff, and rewrite actions across a + directory using Comby's `:[hole]` syntax. + +## Goals / Non-Goals + +**Goals:** + +- Provide default built-in tools named `sd` and `comby`. +- Preserve exact allow-list semantics with `[tools].builtin = ["sd"]` and + `[tools].builtin = ["comby"]`. +- Spawn both host binaries through `Command::arg` without shell interpolation. +- Validate unsafe or unsupported request shapes before spawning a binary. +- Return bounded JSON envelopes for success, non-zero exits, invalid requests, + timeouts, and missing binaries. +- Cover behavior with fake-binary integration tests that do not require real + `sd` or `comby` installs. + +**Non-Goals:** + +- Do not vendor, install, or version-probe `sd` or `comby`. +- Do not parse or reinterpret `sd`/`comby` stdout and stderr beyond bounding + the returned envelope. +- Do not support stdin mode for either tool. +- Do not change `edit`, `apply_patch`, `ast_grep`, or `bash` semantics. + +## Decisions + +### Use dedicated tool modules + +Add `src/tools/sd.rs` and `src/tools/comby.rs` rather than folding the tools +into the generic task workflow wrapper. The schemas and validation rules are +tool-specific: `sd` requires `find`, `replace`, and non-empty `paths`; `comby` +has action-dependent requirements and CLI mappings. + +Alternative considered: expose both as generic `args` wrappers. That would +preserve argv boundaries but would not give the model discoverable fields or +pre-spawn validation. + +### Keep execution local and transparent + +Both tools resolve `cwd` against the session cwd, validate that it is a +directory, find the binary with `which`, then spawn the host command locally +with stdin set to null. The returned command metadata records `program`, +`args`, and `cwd`. Ra does not use ACP terminal wrapping for these tools so +argv boundaries and bounded JSON envelopes remain consistent. + +Alternative considered: route through the host terminal like `git`/`gh`. That +would weaken deterministic envelope handling and expose these rewrite tools to +shell rendering. + +### Map `sd` directly to its CLI + +`sd` maps to: + +```text +sd [--fixed-strings] [extra_args...] -- +``` + +`paths` must be non-empty because the agent tool call has no interactive stdin +stream to rewrite. `extra_args` is appended before the find/replace positionals +so callers can pass flags such as `--flags i` without shell strings. Ra inserts +`--` before `find`/`replace` so leading-dash patterns and replacements are not +interpreted as sd flags. + +Alternative considered: support empty `paths` as stdin mode. That would make +tool calls hang or do nothing in unattended agent contexts, so validation +rejects it before spawning `sd`. + +### Map `comby` to explicit actions + +`comby` uses `CombyAction`: + +- `rewrite`: `comby [extensions...] [-d directory] + [-matcher matcher] [-include-files regex] [-exclude-files regex] -in-place + [extra_args...]` +- `check`: `comby "" [extensions...] [-d directory] [-matcher matcher] + [-include-files regex] [-exclude-files regex] -match-only [extra_args...]` +- `diff`: `comby [extensions...] [-d directory] + [-matcher matcher] [-include-files regex] [-exclude-files regex] -diff + [extra_args...]` + +`rewrite` and `diff` require `rewrite_template`; `check` does not. `rewrite` +defaults to in-place mutation as requested. `check` and `diff` do not pass +`-in-place`, preserving dry-run behavior. + +Alternative considered: a `dry_run` boolean on a single rewrite action. The +enum keeps the wire shape clearer for the model and matches existing +enum-action tool patterns. + +### Bound output with PRD defaults + +`sd` defaults to a 30,000 ms timeout and 32,768 byte result envelope. +`comby` defaults to a 60,000 ms timeout and 65,536 byte result envelope. +Both trim stderr first to a fixed safety budget, then clip stdout as needed to +preserve valid JSON and set `truncated: true`. + +Alternative considered: no default timeout. These commands can traverse many +files, so bounded defaults are safer for unattended agent runs. + +## Risks / Trade-offs + +- Missing host binaries -> Return structured install guidance and avoid + opaque spawn errors. +- `extra_args` can still request surprising CLI behavior -> Keep argv-safe + execution, document that it is an advanced escape hatch, and rely on tool + allow-lists/hooks for policy. +- `comby` CLI flags may vary across versions -> Do not version-probe in this + change; surface non-zero exits transparently in the envelope. +- Very small `max_output_bytes` values may still exceed the requested budget + because valid JSON must be preserved -> Match existing envelope behavior and + document the budget as best-effort. diff --git a/openspec/changes/add-sd-comby-tools/proposal.md b/openspec/changes/add-sd-comby-tools/proposal.md new file mode 100644 index 0000000..c476f6d --- /dev/null +++ b/openspec/changes/add-sd-comby-tools/proposal.md @@ -0,0 +1,43 @@ +# Add Native sd and comby Tools + +## Why + +Ra agents need safer first-class options for bulk text rewriting and structural +code rewriting. Today those workflows fall back to shell-assembled `sed` or +`comby` commands, which hides argv shape from the tool catalog, makes quoting +fragile, and provides no consistent result envelope or missing-binary guidance. + +## What Changes + +- Add a native built-in `sd` tool for regex or literal find/replace across + explicit file path arguments. +- Add a native built-in `comby` tool for structural code check, diff, and + in-place rewrite actions. +- Execute both tools through argv-safe process spawning with no shell + interpolation and no stdin mode. +- Return bounded JSON envelopes with command metadata, exit status, stdout, + stderr, truncation state, validation errors, timeout errors, and structured + missing-binary guidance. +- Register both tools in the default built-in catalog and preserve exact + `[tools].builtin` allow-list behavior using the tool names `sd` and `comby`. +- Add fake-binary integration tests and document both tools in `spec/tools.md`. + +## Capabilities + +### New Capabilities + +### Modified Capabilities + +- `tools`: add native `sd` and `comby` built-in tool contracts to the existing + tool catalog requirements. + +## Impact + +- Affected code: `src/tools/`, tool registration in `src/tools/mod.rs`, and + focused integration tests under `tests/`. +- Affected docs: `spec/tools.md` and any tool catalog comments that enumerate + built-ins. +- Runtime dependencies: `sd` and `comby` must be discoverable on `PATH`; Ra + does not install, vendor, or version-probe either binary. +- No breaking changes. Existing `edit`, `apply_patch`, and `bash` semantics are + unchanged. diff --git a/openspec/changes/add-sd-comby-tools/specs/tools/spec.md b/openspec/changes/add-sd-comby-tools/specs/tools/spec.md new file mode 100644 index 0000000..04544db --- /dev/null +++ b/openspec/changes/add-sd-comby-tools/specs/tools/spec.md @@ -0,0 +1,133 @@ +# Tools Delta + +## ADDED Requirements + +### Requirement: Native sd and comby Tool Catalog + +Ra SHALL include `sd` and `comby` in the default built-in catalog when +`[tools].builtin` is empty. + +#### Scenario: Empty allow-list exposes sd and comby + +- **WHEN** Ra builds the default built-in tool catalog with an empty + `[tools].builtin` allow-list +- **THEN** the catalog includes `sd` and `comby` + +#### Scenario: Non-empty allow-list can select sd + +- **WHEN** Ra builds the default built-in tool catalog with `[tools].builtin` + containing only `sd` +- **THEN** the catalog contains `sd` and omits unspecified tools + +#### Scenario: Non-empty allow-list can select comby + +- **WHEN** Ra builds the default built-in tool catalog with `[tools].builtin` + containing only `comby` +- **THEN** the catalog contains `comby` and omits unspecified tools + +### Requirement: sd Rewrite Execution + +Ra SHALL provide an `sd` tool that runs the host `sd` binary for regex or +literal find/replace using argv-safe process spawning and explicit path +arguments. + +#### Scenario: sd regex replacement preserves argv boundaries + +- **WHEN** the caller provides `find`, `replace`, and one or more `paths` +- **THEN** Ra invokes `sd` with `find`, `replace`, and each path as separate + argv entries + +#### Scenario: sd string mode is passed as a flag + +- **WHEN** the caller sets `string_mode: true` +- **THEN** Ra invokes `sd` with `--fixed-strings` before the find and replace + positionals + +#### Scenario: sd leading-dash positionals are protected + +- **WHEN** the caller provides `find` or `replace` values beginning with `-` +- **THEN** Ra inserts `--` before the find and replace positionals + +#### Scenario: sd extra args are argv-safe + +- **WHEN** the caller provides `extra_args` +- **THEN** Ra passes each extra argument as a separate argv entry without shell + interpolation + +#### Scenario: sd requires explicit paths + +- **WHEN** the caller provides an empty `paths` array +- **THEN** Ra returns `error.kind: "invalid_request"` before invoking `sd` + +### Requirement: comby Structural Rewrite Execution + +Ra SHALL provide a `comby` tool that runs the host `comby` binary for +structural check, diff, and rewrite actions using argv-safe process spawning. + +#### Scenario: comby rewrite mutates in place + +- **WHEN** the caller sets `action: "rewrite"` with a rewrite template +- **THEN** Ra invokes `comby` with `-in-place` + +#### Scenario: comby check does not require rewrite template + +- **WHEN** the caller sets `action: "check"` without `rewrite_template` +- **THEN** Ra invokes `comby` with an empty rewrite positional and + `-match-only` + +#### Scenario: comby diff requires rewrite template + +- **WHEN** the caller sets `action: "diff"` without `rewrite_template` +- **THEN** Ra returns `error.kind: "invalid_request"` before invoking `comby` + +#### Scenario: comby rewrite requires rewrite template + +- **WHEN** the caller sets `action: "rewrite"` without `rewrite_template` +- **THEN** Ra returns `error.kind: "invalid_request"` before invoking `comby` + +#### Scenario: comby filters map to argv + +- **WHEN** the caller provides extensions, directory, matcher, include_files, + exclude_files, or extra_args +- **THEN** Ra passes each requested value as separate argv entries without + shell interpolation + +### Requirement: sd and comby Result Envelopes + +Ra SHALL return bounded, valid JSON envelopes for `sd` and `comby` execution +results, including failures. + +#### Scenario: successful rewrite returns structured output + +- **WHEN** `sd` or `comby` exits with status 0 +- **THEN** Ra returns JSON with `ok: true`, the tool name, command metadata, + `exit_code: 0`, stdout, stderr, and `truncated` + +#### Scenario: non-zero exit is structured + +- **WHEN** `sd` or `comby` exits with a non-zero status +- **THEN** Ra returns JSON with `ok: false`, the exit code, stdout, stderr, and + `error.kind: "command_failed"` + +#### Scenario: missing sd binary is structured + +- **WHEN** `sd` is not found on `PATH` +- **THEN** Ra returns JSON with `ok: false`, no exit code, + `error.kind: "missing_sd"`, and installation guidance + +#### Scenario: missing comby binary is structured + +- **WHEN** `comby` is not found on `PATH` +- **THEN** Ra returns JSON with `ok: false`, no exit code, + `error.kind: "missing_comby"`, and installation guidance + +#### Scenario: timeout is structured + +- **WHEN** `sd` or `comby` exceeds `timeout_ms` +- **THEN** Ra returns JSON with `ok: false`, no exit code, and + `error.kind: "timeout"` + +#### Scenario: output is bounded + +- **WHEN** stdout or stderr exceeds `max_output_bytes` +- **THEN** Ra returns valid JSON with `truncated: true` diff --git a/openspec/changes/add-sd-comby-tools/tasks.md b/openspec/changes/add-sd-comby-tools/tasks.md new file mode 100644 index 0000000..b52393e --- /dev/null +++ b/openspec/changes/add-sd-comby-tools/tasks.md @@ -0,0 +1,23 @@ +# Tasks + +## 1. Tool Contracts + +- [x] 1.1 Define the `sd` input schema, CLI argv mapping, validation rules, defaults, and result envelope. +- [x] 1.2 Define the `comby` input schema, enum actions, CLI argv mapping, validation rules, defaults, and result envelope. + +## 2. Implementation + +- [x] 2.1 Add `SdTool` under `src/tools/` using argv-safe process spawning, explicit paths, cwd resolution, timeout handling, output budgeting, and missing-binary guidance. +- [x] 2.2 Add `CombyTool` under `src/tools/` using argv-safe process spawning, action-dependent validation, cwd resolution, timeout handling, output budgeting, and missing-binary guidance. +- [x] 2.3 Register `sd` and `comby` in `tools::default_builtins` and preserve exact allow-list behavior by tool name. + +## 3. Documentation + +- [x] 3.1 Document `sd` in `spec/tools.md` with schema, examples, output envelope, and error cases. +- [x] 3.2 Document `comby` in `spec/tools.md` with schema, action mappings, examples, output envelope, and error cases. + +## 4. Tests + +- [x] 4.1 Add fake-binary integration tests for `sd` covering argv mapping, string mode, path validation, missing binary, non-zero exit, truncation, and catalog registration. +- [x] 4.2 Add fake-binary integration tests for `comby` covering rewrite/check/diff argv mapping, validation, missing binary, non-zero exit, truncation, and catalog registration. +- [x] 4.3 Run formatting, focused tool tests, OpenSpec validation, and the full Rust test suite when feasible. diff --git a/spec/ra.toml.example b/spec/ra.toml.example index f7d2797..bac973c 100644 --- a/spec/ra.toml.example +++ b/spec/ra.toml.example @@ -41,6 +41,8 @@ api_key_env = "ANTHROPIC_API_KEY" # gh — run native GitHub CLI with argv-safe arguments # jq — run jq filters against inline JSON or a JSON file # mergiraf — syntax-aware merge conflict resolution +# sd — regex/literal find-replace across explicit file paths +# comby — structural code check/diff/rewrite # grep — structured text search # glob — structured file discovery # ls — structured directory listing @@ -57,7 +59,7 @@ api_key_env = "ANTHROPIC_API_KEY" # tmux_wait — block until a tmux event, hook expression, program result, or sleep timeout resolves # An empty list (or omitted section) ships every built-in tool. [tools] -# builtin = ["read", "write", "edit", "bash", "ast_grep", "git", "gh", "jq", "mergiraf", "grep", "glob", "ls", "fuzzy", "apply_patch", "webfetch_fetch", "webfetch_crawl", "openspec", "tmux_run", "tmux_send", "tmux_capture", "tmux_kill", "tmux_listen", "tmux_wait"] +# builtin = ["read", "write", "edit", "bash", "ast_grep", "git", "gh", "jq", "mergiraf", "sd", "comby", "grep", "glob", "ls", "fuzzy", "apply_patch", "webfetch_fetch", "webfetch_crawl", "openspec", "tmux_run", "tmux_send", "tmux_capture", "tmux_kill", "tmux_listen", "tmux_wait"] builtin = [] # ─── Skills (Claude Code / agentskills.io) ────────────────────────── diff --git a/spec/tools.md b/spec/tools.md index d602bf4..05514ac 100644 --- a/spec/tools.md +++ b/spec/tools.md @@ -117,6 +117,12 @@ Prefer `jq` over `bash` pipelines for JSON filtering. It preserves argv boundaries, feeds input through stdin, returns bounded JSON, and surfaces missing-`jq` installation guidance in a structured response. +Prefer `sd` over `bash` + `sed` for regex or literal find/replace across +files. Prefer `comby` over shell-composed structural rewrite commands for +template-based code checks, diffs, and in-place rewrites. Both preserve argv +boundaries, return bounded JSON, and surface missing-binary guidance in a +structured response. + Prefer `tmux_run`, `tmux_send`, `tmux_capture`, `tmux_kill`, `tmux_listen`, and `tmux_wait` over `bash` when the task needs persistent terminal state, interactive input, later output inspection, @@ -131,11 +137,11 @@ compose a shell string. In local CLI / A2A / TUI mode Ra spawns the binary directly with `Command::args`, preserving argv boundaries. With an ACP host attached, `git` and `gh` reuse the same permission-gated `terminal/*` reverse-call path as `bash`, rendering the argv array as a shell-quoted command -line for the host terminal. `jq`, `mergiraf`, `mise`, `just`, and `wrkflw` -spawn local binaries directly so they can preserve stdin/output-envelope -behavior; ACP terminal permission prompts do not wrap those local spawns. The -central `[tools].builtin` allow-list and PreToolUse/PostToolUse hooks still -apply. +line for the host terminal. `jq`, `mergiraf`, `sd`, `comby`, `mise`, `just`, +and `wrkflw` spawn local binaries directly so they can preserve +stdin/output-envelope behavior; ACP terminal permission prompts do not wrap +those local spawns. The central `[tools].builtin` allow-list and +PreToolUse/PostToolUse hooks still apply. These native wrappers do not route through RTK. That tradeoff preserves argv semantics in the local process path instead of converting the call @@ -143,9 +149,9 @@ back into a shell command for compression. Use the `bash` tool for verbose commands when RTK compression is more important than argv-safe process execution. -`git` and `gh` return combined stdout+stderr. `jq`, `mergiraf`, `mise`, -`just`, and `wrkflw` return bounded JSON envelopes. Tool calls emit progress -and `[exit=N]` event chunks on the broadcast bus. +`git` and `gh` return combined stdout+stderr. `jq`, `mergiraf`, `sd`, +`comby`, `mise`, `just`, and `wrkflw` return bounded JSON envelopes. Tool +calls emit progress and `[exit=N]` event chunks on the broadcast bus. ### `git` @@ -305,6 +311,164 @@ stdout, stderr, and exit code. Missing mergiraf uses `cargo install mergiraf`. Output exceeding `max_output_bytes` is clipped with `truncated:true`. +### `sd` + +Run native [`sd`](https://github.com/chmln/sd) for regex or literal +find/replace across explicit file paths. Ra invokes `sd` with an argv array and +never uses stdin mode, so `paths` must contain at least one entry. Because no +shell expands arguments, `paths` are passed literally; use the `glob` tool or +another file-discovery step first when you need glob expansion. + +CLI mapping: + +```text +sd [--fixed-strings] [extra_args...] -- +``` + +```json +{ + "find": "foo_(\\w+)", + "replace": "bar_$1", + "paths": ["src/main.rs", "src/lib.rs"], + "timeout_ms": 30000, + "max_output_bytes": 32768 +} +``` + +```json +{ + "find": "a.b", + "replace": "x", + "paths": ["README.md"], + "string_mode": true, + "extra_args": ["--flags", "i"] +} +``` + +| Field | Type | Required | Default | Notes | +|-------|------|----------|---------|-------| +| find | string | yes | | Regex pattern, or literal text when `string_mode:true` | +| replace | string | yes | | Replacement string; sd handles capture references such as `$1` | +| paths | array of strings | yes | | Explicit file paths; must not be empty | +| string_mode | boolean | no | `false` | Maps to `--fixed-strings` | +| extra_args | array of strings | no | `[]` | Advanced sd flags passed before find/replace | +| cwd | string | no | session cwd | Working directory; relative paths resolve there | +| timeout_ms | number | no | `30000` | Process timeout | +| max_output_bytes | number | no | `32768` | Bounds Ra's returned JSON envelope; `0` means unbounded | + +Returned envelope: + +```json +{ + "ok": true, + "tool": "sd", + "command": { + "program": "sd", + "args": ["--", "foo", "bar", "src/main.rs"], + "cwd": "/repo" + }, + "exit_code": 0, + "stdout": "", + "stderr": null, + "truncated": false +} +``` + +Error cases are returned as valid JSON with `ok:false`. Empty `paths` or an +invalid `cwd` use `error.kind:"invalid_request"` before spawning `sd`. +Non-zero exits use `error.kind:"command_failed"` with stdout, stderr, and exit +code. Timeouts use `error.kind:"timeout"`. Missing sd uses +`error.kind:"missing_sd"` with installation guidance including `cargo install +sd`. Output exceeding `max_output_bytes` is clipped with `truncated:true`. + +### `comby` + +Run native [`comby`](https://comby.dev/) for structural template matching and +rewriting. The `action` field selects one of three safe command shapes: + +| Action | CLI mapping | +|--------|-------------| +| `rewrite` | `comby [extensions...] [-d directory] [-matcher matcher] [-include-files regex] [-exclude-files regex] -in-place [extra_args...]` | +| `check` | `comby "" [extensions...] [-d directory] [-matcher matcher] [-include-files regex] [-exclude-files regex] -match-only [extra_args...]` | +| `diff` | `comby [extensions...] [-d directory] [-matcher matcher] [-include-files regex] [-exclude-files regex] -diff [extra_args...]` | + +`rewrite` mutates in place by default. `check` and `diff` do not pass +`-in-place`. + +```json +{ + "action": "rewrite", + "match_template": "foo(:[arg])", + "rewrite_template": "bar(:[arg])", + "extensions": [".rs"], + "directory": "src", + "matcher": "rust", + "timeout_ms": 60000, + "max_output_bytes": 65536 +} +``` + +```json +{ + "action": "diff", + "match_template": "old_api(:[x])", + "rewrite_template": "new_api(:[x])", + "extensions": [".py"], + "matcher": "python" +} +``` + +```json +{ + "action": "check", + "match_template": "deprecated(:[x])", + "extensions": [".js"], + "include_files": "src/.*" +} +``` + +| Field | Type | Required | Default | Notes | +|-------|------|----------|---------|-------| +| action | string | yes | | `rewrite`, `check`, or `diff` | +| match_template | string | yes | | Comby template using hole syntax such as `:[arg]` | +| rewrite_template | string | for `rewrite`/`diff` | | Comby rewrite template | +| extensions | array of strings | no | `[]` | File extensions passed as positional filters | +| directory | string | no | | Passed to `-d` | +| matcher | string | no | | Passed to `-matcher`, e.g. `rust`, `python`, `generic` | +| include_files | string | no | | Regex passed to `-include-files` | +| exclude_files | string | no | | Regex passed to `-exclude-files` | +| extra_args | array of strings | no | `[]` | Advanced comby flags passed after Ra's action flag | +| cwd | string | no | session cwd | Working directory for the process | +| timeout_ms | number | no | `60000` | Process timeout | +| max_output_bytes | number | no | `65536` | Bounds Ra's returned JSON envelope; `0` means unbounded | + +Returned envelope: + +```json +{ + "ok": true, + "tool": "comby", + "action": "diff", + "command": { + "program": "comby", + "args": ["foo(:[arg])", "bar(:[arg])", ".rs", "-diff"], + "cwd": "/repo" + }, + "exit_code": 0, + "stdout": "--- a/src/lib.rs\n+++ b/src/lib.rs\n...", + "stderr": null, + "truncated": false +} +``` + +Error cases are returned as valid JSON with `ok:false`. Missing or empty +`rewrite_template` for `rewrite`/`diff`, empty `match_template`, or invalid +`cwd` use `error.kind:"invalid_request"` before spawning `comby`. Non-zero +exits use `error.kind:"command_failed"` with stdout, stderr, and exit code. +Timeouts use `error.kind:"timeout"`. Missing comby uses +`error.kind:"missing_comby"` with installation guidance. Output exceeding +`max_output_bytes` is clipped with `truncated:true`. + ### `mise` Run native `mise` for project task/test loops. Pass only arguments after the diff --git a/src/init.rs b/src/init.rs index 2021d37..64223c3 100644 --- a/src/init.rs +++ b/src/init.rs @@ -35,12 +35,12 @@ banner = true # Empty means: enable every built-in tool. # Basic tools: read, write, edit, bash. # Structured search tools: ast_grep. -# Native CLI tools: git, gh, jq, mergiraf. +# Native CLI tools: git, gh, jq, mergiraf, sd, comby. # Extended tools: grep, glob, ls, fuzzy, apply_patch. # Web docs tools: webfetch_fetch, webfetch_crawl. # OpenSpec tool: openspec. # Tmux tools: tmux_run, tmux_send, tmux_capture, tmux_kill, tmux_listen, tmux_wait. -# builtin = ["read", "write", "edit", "bash", "ast_grep", "git", "gh", "jq", "mergiraf", "grep", "glob", "ls", "fuzzy", "apply_patch", "webfetch_fetch", "webfetch_crawl", "openspec", "tmux_run", "tmux_send", "tmux_capture", "tmux_kill", "tmux_listen", "tmux_wait"] +# builtin = ["read", "write", "edit", "bash", "ast_grep", "git", "gh", "jq", "mergiraf", "sd", "comby", "grep", "glob", "ls", "fuzzy", "apply_patch", "webfetch_fetch", "webfetch_crawl", "openspec", "tmux_run", "tmux_send", "tmux_capture", "tmux_kill", "tmux_listen", "tmux_wait"] builtin = [] [skills] diff --git a/src/lib.rs b/src/lib.rs index 9471dd7..db78fd5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,9 +33,9 @@ pub use tool_ctx::{ TerminalRunResult, ToolCtx, }; pub use tools::{ - default_builtins, default_builtins_with_cfg, ApplyPatchTool, AstGrepTool, BashTool, EditTool, - FuzzyTool, GhTool, GitTool, GlobTool, GrepTool, JqTool, JustTool, LsTool, LspTool, - MergirafTool, MiseTool, ReadTool, RtkRewriter, TmuxCaptureTool, TmuxKillTool, TmuxListenTool, - TmuxRunTool, TmuxSendTool, TmuxWaitTool, Tool, WebfetchCrawlTool, WebfetchFetchTool, WriteTool, - WrkflwTool, + default_builtins, default_builtins_with_cfg, ApplyPatchTool, AstGrepTool, BashTool, + CombyAction, CombyTool, EditTool, FuzzyTool, GhTool, GitTool, GlobTool, GrepTool, JqTool, + JustTool, LsTool, LspTool, MergirafTool, MiseTool, ReadTool, RtkRewriter, SdTool, + TmuxCaptureTool, TmuxKillTool, TmuxListenTool, TmuxRunTool, TmuxSendTool, TmuxWaitTool, Tool, + WebfetchCrawlTool, WebfetchFetchTool, WriteTool, WrkflwTool, }; diff --git a/src/tools/comby.rs b/src/tools/comby.rs new file mode 100644 index 0000000..5f4475d --- /dev/null +++ b/src/tools/comby.rs @@ -0,0 +1,637 @@ +//! Native comby wrapper. +//! +//! This tool exposes common structural rewrite workflows as enum actions while +//! delegating matching and rewriting semantics to the system `comby` binary. + +use crate::events::Event; +use crate::tool_ctx::ToolCtx; +use crate::tools::core::Tool; +use anyhow::{anyhow, Context, Result}; +use async_trait::async_trait; +use schemars::{schema_for, JsonSchema}; +use serde::Deserialize; +use serde_json::json; +use std::path::{Path, PathBuf}; +use std::process::Stdio; +use std::time::Duration; +use tokio::process::Command; + +const COMBY_BINARY: &str = "comby"; +const DEFAULT_TIMEOUT_MS: u64 = 60_000; +const DEFAULT_MAX_OUTPUT_BYTES: usize = 65_536; +const DEFAULT_STDERR_BYTES: usize = 32_000; + +#[derive(Debug, Clone, Copy, Deserialize, JsonSchema, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum CombyAction { + /// Apply match_template -> rewrite_template in place across matching files. + Rewrite, + /// Report matches without modifying files. + Check, + /// Show unified diff for the requested rewrite without modifying files. + Diff, +} + +impl CombyAction { + fn as_str(self) -> &'static str { + match self { + Self::Rewrite => "rewrite", + Self::Check => "check", + Self::Diff => "diff", + } + } +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct CombyParams { + /// Which comby operation to run. + pub action: CombyAction, + /// Comby match template, e.g. `foo(:[arg])`. + pub match_template: String, + /// Comby rewrite template. Required for `rewrite` and `diff`. + #[serde(default)] + pub rewrite_template: Option, + /// File extensions to match, e.g. [".rs", ".py"]. Empty lets comby decide. + #[serde(default)] + pub extensions: Vec, + /// Root directory passed to comby with `-d`. + #[serde(default)] + pub directory: Option, + /// Language matcher passed with `-matcher`, e.g. "rust", "python", "generic". + #[serde(default)] + pub matcher: Option, + /// Only match files whose path matches this regex. + #[serde(default)] + pub include_files: Option, + /// Exclude files whose path matches this regex. + #[serde(default)] + pub exclude_files: Option, + /// Additional comby flags passed after Ra's action flags. + #[serde(default)] + pub extra_args: Vec, + /// Working directory for the command. Relative `directory` is left as a comby argument. + #[serde(default)] + pub cwd: Option, + /// Process timeout in milliseconds. + #[serde(default = "default_timeout_ms")] + pub timeout_ms: u64, + /// Maximum bytes in Ra's returned JSON envelope. Set to 0 for unbounded. + #[serde(default = "default_max_output_bytes")] + pub max_output_bytes: usize, +} + +pub struct CombyTool; + +#[async_trait] +impl Tool for CombyTool { + fn name(&self) -> &str { + COMBY_BINARY + } + + fn description(&self) -> &str { + "Run the native comby CLI for structural code rewriting. Supports \ + `rewrite` (in-place), `check` (match-only), and `diff` actions with \ + argv-safe process spawning. Returns a bounded JSON envelope with \ + stdout, stderr, exit status, truncation state, validation errors, \ + timeouts, and missing-comby guidance." + } + + fn schema(&self) -> serde_json::Value { + serde_json::to_value(schema_for!(CombyParams)).unwrap() + } + + async fn execute( + &self, + call_id: &str, + input: serde_json::Value, + ctx: &ToolCtx, + ) -> Result { + let _scope = crate::nemo_obs::tool_scope(COMBY_BINARY); + let params: CombyParams = + serde_json::from_value(input).context("invalid params for comby")?; + let action = params.action; + let invocation = match CombyInvocation::from_params(params, &ctx.cwd).await { + Ok(invocation) => invocation, + Err(error) => return Ok(invalid_request_json(action, error)), + }; + + execute_comby(call_id, invocation, ctx).await + } +} + +#[derive(Debug, Clone)] +struct CombyInvocation { + action: CombyAction, + args: Vec, + cwd: PathBuf, + timeout_ms: u64, + max_output_bytes: usize, +} + +impl CombyInvocation { + async fn from_params(params: CombyParams, session_cwd: &Path) -> Result { + if params.match_template.is_empty() { + return Err(anyhow!("comby requires a non-empty match_template")); + } + + let rewrite_template = non_empty(params.rewrite_template); + if matches!(params.action, CombyAction::Rewrite | CombyAction::Diff) + && rewrite_template.is_none() + { + return Err(anyhow!( + "comby action `{}` requires rewrite_template", + params.action.as_str() + )); + } + + let cwd = resolve_cwd(params.cwd, session_cwd); + validate_cwd(&cwd).await?; + + let mut args = Vec::new(); + args.push(params.match_template); + args.push(match params.action { + CombyAction::Check => String::new(), + CombyAction::Rewrite | CombyAction::Diff => { + rewrite_template.expect("validated rewrite template") + } + }); + args.extend(params.extensions); + + if let Some(directory) = non_empty(params.directory) { + args.push("-d".to_string()); + args.push(directory); + } + if let Some(matcher) = non_empty(params.matcher) { + args.push("-matcher".to_string()); + args.push(matcher); + } + if let Some(include_files) = non_empty(params.include_files) { + args.push("-include-files".to_string()); + args.push(include_files); + } + if let Some(exclude_files) = non_empty(params.exclude_files) { + args.push("-exclude-files".to_string()); + args.push(exclude_files); + } + + match params.action { + CombyAction::Rewrite => args.push("-in-place".to_string()), + CombyAction::Check => args.push("-match-only".to_string()), + CombyAction::Diff => args.push("-diff".to_string()), + } + args.extend(params.extra_args); + + Ok(Self { + action: params.action, + args, + cwd, + timeout_ms: params.timeout_ms, + max_output_bytes: params.max_output_bytes, + }) + } + + fn command_json(&self) -> serde_json::Value { + json!({ + "program": COMBY_BINARY, + "args": self.args, + "cwd": self.cwd, + }) + } + + fn summary(&self) -> String { + shell_words(COMBY_BINARY, &self.args) + } +} + +async fn execute_comby( + call_id: &str, + invocation: CombyInvocation, + ctx: &ToolCtx, +) -> Result { + let comby = match which::which(COMBY_BINARY) { + Ok(path) => path, + Err(_) => return Ok(missing_comby_json(&invocation)), + }; + + let _ = ctx.events.send(Event::ToolCallUpdate { + id: call_id.to_string(), + chunk: format!("[comby] {}", invocation.summary()), + }); + + let output = match run_comby(&comby, &invocation).await { + Ok(output) => output, + Err(CombyRunError::Timeout) => return Ok(timeout_json(&invocation)), + Err(CombyRunError::Other(error)) => return Err(error), + }; + + let exit_code = output.exit_code; + let _ = ctx.events.send(Event::ToolCallUpdate { + id: call_id.to_string(), + chunk: format!("[exit={exit_code}]"), + }); + + format_comby_output(&invocation, output) +} + +#[derive(Debug)] +enum CombyRunError { + Timeout, + Other(anyhow::Error), +} + +impl From for CombyRunError { + fn from(error: anyhow::Error) -> Self { + Self::Other(error) + } +} + +#[derive(Debug, Clone)] +struct CombyOutput { + exit_code: i32, + stdout: String, + stderr: String, +} + +async fn run_comby( + binary_path: &Path, + invocation: &CombyInvocation, +) -> std::result::Result { + let child = Command::new(binary_path) + .args(&invocation.args) + .current_dir(&invocation.cwd) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .with_context(|| { + format!( + "spawn {} in {}", + invocation.summary(), + invocation.cwd.display() + ) + })?; + + let output_future = child.wait_with_output(); + let output = + match tokio::time::timeout(Duration::from_millis(invocation.timeout_ms), output_future) + .await + { + Ok(output) => output, + Err(_) => return Err(CombyRunError::Timeout), + } + .context("wait comby")?; + + Ok(CombyOutput { + exit_code: output.status.code().unwrap_or(-1), + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + stderr: String::from_utf8_lossy(&output.stderr).into_owned(), + }) +} + +fn format_comby_output(invocation: &CombyInvocation, output: CombyOutput) -> Result { + let stdout = output.stdout; + let mut stderr = output.stderr; + let ok = output.exit_code == 0; + let stderr_truncated = trim_to_char_budget(&mut stderr, DEFAULT_STDERR_BYTES); + + let mut base = json!({ + "ok": ok, + "tool": COMBY_BINARY, + "action": invocation.action.as_str(), + "command": invocation.command_json(), + "exit_code": output.exit_code, + "stdout": stdout.clone(), + "stderr": nullable_string(&stderr), + "truncated": stderr_truncated, + }); + + if !ok { + base["error"] = json!({ + "kind": "command_failed", + "message": "comby exited with a non-zero status" + }); + } + + bounded_output_json(base, &stdout, stderr, invocation.max_output_bytes) +} + +fn invalid_request_json(action: CombyAction, error: anyhow::Error) -> String { + serde_json::to_string_pretty(&json!({ + "ok": false, + "tool": COMBY_BINARY, + "action": action.as_str(), + "command": { + "program": COMBY_BINARY, + "args": [], + "cwd": null + }, + "exit_code": null, + "stdout": "", + "stderr": null, + "truncated": false, + "error": { + "kind": "invalid_request", + "message": error.to_string() + } + })) + .expect("invalid request JSON is serializable") +} + +fn missing_comby_json(invocation: &CombyInvocation) -> String { + serde_json::to_string_pretty(&json!({ + "ok": false, + "tool": COMBY_BINARY, + "action": invocation.action.as_str(), + "command": invocation.command_json(), + "exit_code": null, + "stdout": "", + "stderr": null, + "truncated": false, + "error": { + "kind": "missing_comby", + "message": "comby was not found on PATH, so the comby tool could not be run.", + "install": [ + "Install comby from https://comby.dev or your system package manager.", + "After comby is available on PATH, rerun this tool." + ] + } + })) + .expect("missing comby JSON is serializable") +} + +fn timeout_json(invocation: &CombyInvocation) -> String { + serde_json::to_string_pretty(&json!({ + "ok": false, + "tool": COMBY_BINARY, + "action": invocation.action.as_str(), + "command": invocation.command_json(), + "exit_code": null, + "stdout": "", + "stderr": null, + "truncated": false, + "error": { + "kind": "timeout", + "message": format!( + "comby exceeded the configured timeout of {} ms", + invocation.timeout_ms + ) + } + })) + .expect("timeout JSON is serializable") +} + +fn bounded_output_json( + mut value: serde_json::Value, + stdout: &str, + stderr: String, + max_output_bytes: usize, +) -> Result { + let mut rendered = serde_json::to_string_pretty(&value)?; + if max_output_bytes == 0 || rendered.len() <= max_output_bytes { + return Ok(rendered); + } + + let mut low = 0; + let mut high = stdout.chars().count(); + let mut best = 0; + + while low <= high { + let mid = low + (high - low) / 2; + value["stdout"] = json!(with_truncation_marker(&take_chars(stdout, mid))); + value["stderr"] = nullable_string(&stderr); + value["truncated"] = json!(true); + let probe = serde_json::to_string_pretty(&value)?; + if probe.len() <= max_output_bytes { + best = mid; + low = mid + 1; + } else if mid == 0 { + break; + } else { + high = mid - 1; + } + } + + value["stdout"] = json!(with_truncation_marker(&take_chars(stdout, best))); + rendered = serde_json::to_string_pretty(&value)?; + if rendered.len() <= max_output_bytes || stderr.is_empty() { + return Ok(rendered); + } + + let mut stderr_trimmed = stderr; + trim_to_char_budget(&mut stderr_trimmed, DEFAULT_STDERR_BYTES.min(1024)); + value["stderr"] = json!(stderr_trimmed); + rendered = serde_json::to_string_pretty(&value)?; + if rendered.len() <= max_output_bytes || best == 0 { + return Ok(rendered); + } + + value["stdout"] = json!(with_truncation_marker("")); + value["truncated"] = json!(true); + serde_json::to_string_pretty(&value).map_err(Into::into) +} + +fn nullable_string(text: &str) -> serde_json::Value { + if text.is_empty() { + serde_json::Value::Null + } else { + json!(text) + } +} + +fn resolve_cwd(cwd: Option, session_cwd: &Path) -> PathBuf { + match non_empty(cwd) { + Some(cwd) => { + let path = PathBuf::from(cwd); + if path.is_absolute() { + path + } else { + session_cwd.join(path) + } + } + None => session_cwd.to_path_buf(), + } +} + +async fn validate_cwd(cwd: &Path) -> Result<()> { + let metadata = tokio::fs::metadata(cwd) + .await + .with_context(|| format!("read cwd {}", cwd.display()))?; + if !metadata.is_dir() { + return Err(anyhow!("cwd is not a directory: {}", cwd.display())); + } + Ok(()) +} + +fn non_empty(value: Option) -> Option { + value.and_then(|value| { + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }) +} + +fn trim_to_char_budget(text: &mut String, max_bytes: usize) -> bool { + if text.len() <= max_bytes { + return false; + } + + let mut end = 0; + for (idx, ch) in text.char_indices() { + let next = idx + ch.len_utf8(); + if next > max_bytes { + break; + } + end = next; + } + + text.truncate(end); + if !text.ends_with('\n') { + text.push('\n'); + } + text.push_str("[truncated]\n"); + true +} + +fn take_chars(text: &str, count: usize) -> String { + text.chars().take(count).collect() +} + +fn with_truncation_marker(text: &str) -> String { + if text.is_empty() { + "[truncated]\n".to_string() + } else if text.ends_with('\n') { + format!("{text}[truncated]\n") + } else { + format!("{text}\n[truncated]\n") + } +} + +fn shell_words(binary: &str, args: &[String]) -> String { + std::iter::once(shell_word(binary)) + .chain(args.iter().map(|arg| shell_word(arg))) + .collect::>() + .join(" ") +} + +fn shell_word(s: &str) -> String { + if s.chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '/' | '_' | '-' | '=' | ':')) + { + s.to_string() + } else { + format!("'{}'", s.replace('\'', "'\\''")) + } +} + +fn default_timeout_ms() -> u64 { + DEFAULT_TIMEOUT_MS +} + +fn default_max_output_bytes() -> usize { + DEFAULT_MAX_OUTPUT_BYTES +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn invocation_builds_rewrite_argv() { + let dir = tempfile::tempdir().unwrap(); + let invocation = CombyInvocation::from_params( + CombyParams { + action: CombyAction::Rewrite, + match_template: "foo(:[x])".into(), + rewrite_template: Some("bar(:[x])".into()), + extensions: vec![".rs".into()], + directory: Some("src".into()), + matcher: Some("rust".into()), + include_files: Some(".*\\.rs".into()), + exclude_files: Some("target".into()), + extra_args: vec!["-jobs".into(), "1".into()], + cwd: None, + timeout_ms: DEFAULT_TIMEOUT_MS, + max_output_bytes: DEFAULT_MAX_OUTPUT_BYTES, + }, + dir.path(), + ) + .await + .unwrap(); + + assert_eq!( + invocation.args, + vec![ + "foo(:[x])", + "bar(:[x])", + ".rs", + "-d", + "src", + "-matcher", + "rust", + "-include-files", + ".*\\.rs", + "-exclude-files", + "target", + "-in-place", + "-jobs", + "1" + ] + ); + } + + #[tokio::test] + async fn check_does_not_require_rewrite_template() { + let dir = tempfile::tempdir().unwrap(); + let invocation = CombyInvocation::from_params( + CombyParams { + action: CombyAction::Check, + match_template: "foo(:[x])".into(), + rewrite_template: None, + extensions: vec![], + directory: None, + matcher: None, + include_files: None, + exclude_files: None, + extra_args: vec![], + cwd: None, + timeout_ms: DEFAULT_TIMEOUT_MS, + max_output_bytes: DEFAULT_MAX_OUTPUT_BYTES, + }, + dir.path(), + ) + .await + .unwrap(); + + assert_eq!(invocation.args, vec!["foo(:[x])", "", "-match-only"]); + } + + #[tokio::test] + async fn diff_requires_rewrite_template() { + let dir = tempfile::tempdir().unwrap(); + let err = CombyInvocation::from_params( + CombyParams { + action: CombyAction::Diff, + match_template: "foo(:[x])".into(), + rewrite_template: None, + extensions: vec![], + directory: None, + matcher: None, + include_files: None, + exclude_files: None, + extra_args: vec![], + cwd: None, + timeout_ms: DEFAULT_TIMEOUT_MS, + max_output_bytes: DEFAULT_MAX_OUTPUT_BYTES, + }, + dir.path(), + ) + .await + .unwrap_err(); + + assert!(err.to_string().contains("requires rewrite_template")); + } +} diff --git a/src/tools/mod.rs b/src/tools/mod.rs index 3d4f633..f1802d5 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -12,6 +12,8 @@ //! - `git` — run native git with argv-safe arguments //! - `gh` — run native GitHub CLI with argv-safe arguments //! - `jq` — run jq filters with argv-safe stdin +//! - `sd` — regex/literal find-replace across explicit paths +//! - `comby` — structural code check/diff/rewrite //! - `mise` — run mise tasks and tests with argv-safe arguments //! - `just` — run just recipes with argv-safe arguments //! - `wrkflw` — validate/run GitHub Actions workflows locally @@ -33,6 +35,7 @@ //! - `tmux_wait` — block until a tmux event or timeout mod cli; +mod comby; mod core; mod extended; mod fs; @@ -41,12 +44,14 @@ mod lsp; mod mergiraf; mod openspec; mod rtk; +mod sd; mod search; mod task_workflow; mod tmux; mod webfetch; pub use cli::{GhTool, GitTool}; +pub use comby::{CombyAction, CombyTool}; pub use core::{BashTool, ReadTool}; pub use extended::{ApplyPatchTool, FuzzyTool, GlobTool, GrepTool, LsTool}; pub use fs::{EditTool, WriteTool}; @@ -55,6 +60,7 @@ pub use lsp::{resolve_openlsp_binary, LspTool}; pub use mergiraf::MergirafTool; pub use openspec::OpenSpecTool; pub use rtk::RtkRewriter; +pub use sd::SdTool; pub use search::AstGrepTool; pub use task_workflow::{JustTool, MiseTool, WrkflwTool}; pub use tmux::{ @@ -109,6 +115,12 @@ pub fn default_builtins_with_cfg( if want("jq") { out.push(Arc::new(JqTool)); } + if want("sd") { + out.push(Arc::new(SdTool)); + } + if want("comby") { + out.push(Arc::new(CombyTool)); + } if want("mise") { out.push(Arc::new(MiseTool)); } @@ -222,6 +234,19 @@ mod tests { assert_eq!(builtin_names(&["jq"]), vec!["jq"]); } + #[test] + fn default_catalog_includes_sd_and_comby_tools() { + let names = builtin_names(&[]); + assert!(names.contains(&"sd".to_string())); + assert!(names.contains(&"comby".to_string())); + } + + #[test] + fn allowlist_can_select_sd_and_comby_exactly() { + assert_eq!(builtin_names(&["sd"]), vec!["sd"]); + assert_eq!(builtin_names(&["comby"]), vec!["comby"]); + } + #[test] fn default_catalog_includes_task_workflow_tools() { let names = builtin_names(&[]); diff --git a/src/tools/sd.rs b/src/tools/sd.rs new file mode 100644 index 0000000..db87318 --- /dev/null +++ b/src/tools/sd.rs @@ -0,0 +1,519 @@ +//! Native sd wrapper. +//! +//! This tool keeps fast regex/literal replacements out of shell strings while +//! delegating replacement semantics to the system `sd` binary. + +use crate::events::Event; +use crate::tool_ctx::ToolCtx; +use crate::tools::core::Tool; +use anyhow::{anyhow, Context, Result}; +use async_trait::async_trait; +use schemars::{schema_for, JsonSchema}; +use serde::Deserialize; +use serde_json::json; +use std::path::{Path, PathBuf}; +use std::process::Stdio; +use std::time::Duration; +use tokio::process::Command; + +const SD_BINARY: &str = "sd"; +const DEFAULT_TIMEOUT_MS: u64 = 30_000; +const DEFAULT_MAX_OUTPUT_BYTES: usize = 32_768; +const DEFAULT_STDERR_BYTES: usize = 32_000; + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct SdParams { + /// Find pattern, interpreted as a regex unless `string_mode` is true. + pub find: String, + /// Replacement string. Capture references such as `$1` are interpreted by sd. + pub replace: String, + /// Explicit file paths to rewrite. Must not be empty; stdin mode is not supported. + #[serde(default)] + pub paths: Vec, + /// Treat `find` as a literal string, not a regex. + #[serde(default)] + pub string_mode: bool, + /// Additional sd flags passed before the find/replace positionals. + #[serde(default)] + pub extra_args: Vec, + /// Working directory for resolving relative paths. + #[serde(default)] + pub cwd: Option, + /// Process timeout in milliseconds. + #[serde(default = "default_timeout_ms")] + pub timeout_ms: u64, + /// Maximum bytes in Ra's returned JSON envelope. Set to 0 for unbounded. + #[serde(default = "default_max_output_bytes")] + pub max_output_bytes: usize, +} + +pub struct SdTool; + +#[async_trait] +impl Tool for SdTool { + fn name(&self) -> &str { + SD_BINARY + } + + fn description(&self) -> &str { + "Run the native sd CLI for regex or literal find/replace across \ + explicit file paths with argv-safe process spawning. Stdin mode is \ + intentionally unsupported: provide at least one path. Returns a \ + bounded JSON envelope with stdout, stderr, exit status, truncation \ + state, validation errors, timeouts, and missing-sd guidance." + } + + fn schema(&self) -> serde_json::Value { + serde_json::to_value(schema_for!(SdParams)).unwrap() + } + + async fn execute( + &self, + call_id: &str, + input: serde_json::Value, + ctx: &ToolCtx, + ) -> Result { + let _scope = crate::nemo_obs::tool_scope(SD_BINARY); + let params: SdParams = serde_json::from_value(input).context("invalid params for sd")?; + let invocation = match SdInvocation::from_params(params, &ctx.cwd).await { + Ok(invocation) => invocation, + Err(error) => return Ok(invalid_request_json(error)), + }; + + execute_sd(call_id, invocation, ctx).await + } +} + +#[derive(Debug, Clone)] +struct SdInvocation { + args: Vec, + cwd: PathBuf, + timeout_ms: u64, + max_output_bytes: usize, +} + +impl SdInvocation { + async fn from_params(params: SdParams, session_cwd: &Path) -> Result { + if params.find.is_empty() { + return Err(anyhow!("sd requires a non-empty find pattern")); + } + if params.paths.is_empty() { + return Err(anyhow!( + "sd requires at least one path; stdin mode is not supported" + )); + } + + let cwd = resolve_cwd(params.cwd, session_cwd); + validate_cwd(&cwd).await?; + + let mut args = Vec::new(); + if params.string_mode { + args.push("--fixed-strings".to_string()); + } + args.extend(params.extra_args); + args.push("--".to_string()); + args.push(params.find); + args.push(params.replace); + args.extend(params.paths); + + Ok(Self { + args, + cwd, + timeout_ms: params.timeout_ms, + max_output_bytes: params.max_output_bytes, + }) + } + + fn command_json(&self) -> serde_json::Value { + json!({ + "program": SD_BINARY, + "args": self.args, + "cwd": self.cwd, + }) + } + + fn summary(&self) -> String { + shell_words(SD_BINARY, &self.args) + } +} + +async fn execute_sd(call_id: &str, invocation: SdInvocation, ctx: &ToolCtx) -> Result { + let sd = match which::which(SD_BINARY) { + Ok(path) => path, + Err(_) => return Ok(missing_sd_json(&invocation)), + }; + + let _ = ctx.events.send(Event::ToolCallUpdate { + id: call_id.to_string(), + chunk: format!("[sd] {}", invocation.summary()), + }); + + let output = match run_sd(&sd, &invocation).await { + Ok(output) => output, + Err(SdRunError::Timeout) => return Ok(timeout_json(&invocation)), + Err(SdRunError::Other(error)) => return Err(error), + }; + + let exit_code = output.exit_code; + let _ = ctx.events.send(Event::ToolCallUpdate { + id: call_id.to_string(), + chunk: format!("[exit={exit_code}]"), + }); + + format_sd_output(&invocation, output) +} + +#[derive(Debug)] +enum SdRunError { + Timeout, + Other(anyhow::Error), +} + +impl From for SdRunError { + fn from(error: anyhow::Error) -> Self { + Self::Other(error) + } +} + +#[derive(Debug, Clone)] +struct SdOutput { + exit_code: i32, + stdout: String, + stderr: String, +} + +async fn run_sd( + binary_path: &Path, + invocation: &SdInvocation, +) -> std::result::Result { + let child = Command::new(binary_path) + .args(&invocation.args) + .current_dir(&invocation.cwd) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .with_context(|| { + format!( + "spawn {} in {}", + invocation.summary(), + invocation.cwd.display() + ) + })?; + + let output_future = child.wait_with_output(); + let output = + match tokio::time::timeout(Duration::from_millis(invocation.timeout_ms), output_future) + .await + { + Ok(output) => output, + Err(_) => return Err(SdRunError::Timeout), + } + .context("wait sd")?; + + Ok(SdOutput { + exit_code: output.status.code().unwrap_or(-1), + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + stderr: String::from_utf8_lossy(&output.stderr).into_owned(), + }) +} + +fn format_sd_output(invocation: &SdInvocation, output: SdOutput) -> Result { + let stdout = output.stdout; + let mut stderr = output.stderr; + let ok = output.exit_code == 0; + let stderr_truncated = trim_to_char_budget(&mut stderr, DEFAULT_STDERR_BYTES); + + let mut base = json!({ + "ok": ok, + "tool": SD_BINARY, + "command": invocation.command_json(), + "exit_code": output.exit_code, + "stdout": stdout.clone(), + "stderr": nullable_string(&stderr), + "truncated": stderr_truncated, + }); + + if !ok { + base["error"] = json!({ + "kind": "command_failed", + "message": "sd exited with a non-zero status" + }); + } + + bounded_output_json(base, &stdout, stderr, invocation.max_output_bytes) +} + +fn invalid_request_json(error: anyhow::Error) -> String { + serde_json::to_string_pretty(&json!({ + "ok": false, + "tool": SD_BINARY, + "command": { + "program": SD_BINARY, + "args": [], + "cwd": null + }, + "exit_code": null, + "stdout": "", + "stderr": null, + "truncated": false, + "error": { + "kind": "invalid_request", + "message": error.to_string() + } + })) + .expect("invalid request JSON is serializable") +} + +fn missing_sd_json(invocation: &SdInvocation) -> String { + serde_json::to_string_pretty(&json!({ + "ok": false, + "tool": SD_BINARY, + "command": invocation.command_json(), + "exit_code": null, + "stdout": "", + "stderr": null, + "truncated": false, + "error": { + "kind": "missing_sd", + "message": "sd was not found on PATH, so the sd tool could not be run.", + "install": [ + "Install sd using your system package manager or `cargo install sd`.", + "After sd is available on PATH, rerun this tool." + ] + } + })) + .expect("missing sd JSON is serializable") +} + +fn timeout_json(invocation: &SdInvocation) -> String { + serde_json::to_string_pretty(&json!({ + "ok": false, + "tool": SD_BINARY, + "command": invocation.command_json(), + "exit_code": null, + "stdout": "", + "stderr": null, + "truncated": false, + "error": { + "kind": "timeout", + "message": format!("sd exceeded the configured timeout of {} ms", invocation.timeout_ms) + } + })) + .expect("timeout JSON is serializable") +} + +fn bounded_output_json( + mut value: serde_json::Value, + stdout: &str, + stderr: String, + max_output_bytes: usize, +) -> Result { + let mut rendered = serde_json::to_string_pretty(&value)?; + if max_output_bytes == 0 || rendered.len() <= max_output_bytes { + return Ok(rendered); + } + + let mut low = 0; + let mut high = stdout.chars().count(); + let mut best = 0; + + while low <= high { + let mid = low + (high - low) / 2; + value["stdout"] = json!(with_truncation_marker(&take_chars(stdout, mid))); + value["stderr"] = nullable_string(&stderr); + value["truncated"] = json!(true); + let probe = serde_json::to_string_pretty(&value)?; + if probe.len() <= max_output_bytes { + best = mid; + low = mid + 1; + } else if mid == 0 { + break; + } else { + high = mid - 1; + } + } + + value["stdout"] = json!(with_truncation_marker(&take_chars(stdout, best))); + rendered = serde_json::to_string_pretty(&value)?; + if rendered.len() <= max_output_bytes || stderr.is_empty() { + return Ok(rendered); + } + + let mut stderr_trimmed = stderr; + trim_to_char_budget(&mut stderr_trimmed, DEFAULT_STDERR_BYTES.min(1024)); + value["stderr"] = json!(stderr_trimmed); + rendered = serde_json::to_string_pretty(&value)?; + if rendered.len() <= max_output_bytes || best == 0 { + return Ok(rendered); + } + + value["stdout"] = json!(with_truncation_marker("")); + value["truncated"] = json!(true); + serde_json::to_string_pretty(&value).map_err(Into::into) +} + +fn nullable_string(text: &str) -> serde_json::Value { + if text.is_empty() { + serde_json::Value::Null + } else { + json!(text) + } +} + +fn resolve_cwd(cwd: Option, session_cwd: &Path) -> PathBuf { + match non_empty(cwd) { + Some(cwd) => { + let path = PathBuf::from(cwd); + if path.is_absolute() { + path + } else { + session_cwd.join(path) + } + } + None => session_cwd.to_path_buf(), + } +} + +async fn validate_cwd(cwd: &Path) -> Result<()> { + let metadata = tokio::fs::metadata(cwd) + .await + .with_context(|| format!("read cwd {}", cwd.display()))?; + if !metadata.is_dir() { + return Err(anyhow!("cwd is not a directory: {}", cwd.display())); + } + Ok(()) +} + +fn non_empty(value: Option) -> Option { + value.and_then(|value| { + let trimmed = value.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }) +} + +fn trim_to_char_budget(text: &mut String, max_bytes: usize) -> bool { + if text.len() <= max_bytes { + return false; + } + + let mut end = 0; + for (idx, ch) in text.char_indices() { + let next = idx + ch.len_utf8(); + if next > max_bytes { + break; + } + end = next; + } + + text.truncate(end); + if !text.ends_with('\n') { + text.push('\n'); + } + text.push_str("[truncated]\n"); + true +} + +fn take_chars(text: &str, count: usize) -> String { + text.chars().take(count).collect() +} + +fn with_truncation_marker(text: &str) -> String { + if text.is_empty() { + "[truncated]\n".to_string() + } else if text.ends_with('\n') { + format!("{text}[truncated]\n") + } else { + format!("{text}\n[truncated]\n") + } +} + +fn shell_words(binary: &str, args: &[String]) -> String { + std::iter::once(shell_word(binary)) + .chain(args.iter().map(|arg| shell_word(arg))) + .collect::>() + .join(" ") +} + +fn shell_word(s: &str) -> String { + if s.chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '/' | '_' | '-' | '=' | ':')) + { + s.to_string() + } else { + format!("'{}'", s.replace('\'', "'\\''")) + } +} + +fn default_timeout_ms() -> u64 { + DEFAULT_TIMEOUT_MS +} + +fn default_max_output_bytes() -> usize { + DEFAULT_MAX_OUTPUT_BYTES +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn invocation_builds_sd_argv() { + let dir = tempfile::tempdir().unwrap(); + let invocation = SdInvocation::from_params( + SdParams { + find: "foo".into(), + replace: "bar".into(), + paths: vec!["src/main.rs".into()], + string_mode: true, + extra_args: vec!["--flags".into(), "i".into()], + cwd: None, + timeout_ms: DEFAULT_TIMEOUT_MS, + max_output_bytes: DEFAULT_MAX_OUTPUT_BYTES, + }, + dir.path(), + ) + .await + .unwrap(); + + assert_eq!( + invocation.args, + vec![ + "--fixed-strings", + "--flags", + "i", + "--", + "foo", + "bar", + "src/main.rs" + ] + ); + } + + #[tokio::test] + async fn invocation_rejects_empty_paths() { + let dir = tempfile::tempdir().unwrap(); + let err = SdInvocation::from_params( + SdParams { + find: "foo".into(), + replace: "bar".into(), + paths: vec![], + string_mode: false, + extra_args: vec![], + cwd: None, + timeout_ms: DEFAULT_TIMEOUT_MS, + max_output_bytes: DEFAULT_MAX_OUTPUT_BYTES, + }, + dir.path(), + ) + .await + .unwrap_err(); + + assert!(err.to_string().contains("at least one path")); + } +} diff --git a/tests/comby_tool.rs b/tests/comby_tool.rs new file mode 100644 index 0000000..7711044 --- /dev/null +++ b/tests/comby_tool.rs @@ -0,0 +1,356 @@ +//! Integration tests for the native comby tool. These use a fake `comby` +//! binary so the tests verify Ra's argv/cwd/envelope behavior without +//! depending on the host comby installation. + +use ra::{ + tools::{CombyTool, Tool}, + ToolCtx, +}; +use serde_json::Value; +use std::{ffi::OsString, fs, os::unix::fs::PermissionsExt, path::Path, sync::OnceLock}; +use tokio::sync::Mutex; + +struct EnvRestore { + key: &'static str, + old_value: Option, +} + +impl EnvRestore { + fn set>(key: &'static str, value: K) -> Self { + let old_value = std::env::var_os(key); + std::env::set_var(key, value.into()); + Self { key, old_value } + } +} + +impl Drop for EnvRestore { + fn drop(&mut self) { + if let Some(value) = &self.old_value { + std::env::set_var(self.key, value); + } else { + std::env::remove_var(self.key); + } + } +} + +fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) +} + +fn make_ctx() -> ToolCtx { + let (events, _) = tokio::sync::broadcast::channel(16); + ToolCtx::local(events) +} + +fn json_output(output: &str) -> Value { + serde_json::from_str(output).unwrap_or_else(|err| panic!("invalid json: {err}: {output}")) +} + +fn write_fake_comby(dir: &Path, body: &str) { + let path = dir.join("comby"); + fs::write( + &path, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$COMBY_ARGS_FILE\"\npwd > \"$COMBY_CWD_FILE\"\n{body}\n" + ), + ) + .unwrap(); + let mut perms = fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(path, perms).unwrap(); +} + +#[tokio::test] +async fn default_catalog_contains_comby_and_allowlist_is_exact() { + let names = ra::default_builtins(&[]) + .into_iter() + .map(|tool| tool.name().to_string()) + .collect::>(); + + assert!( + names.contains(&"comby".to_string()), + "missing comby: {names:?}" + ); + + let filtered = ra::default_builtins(&["comby".to_string()]) + .into_iter() + .map(|tool| tool.name().to_string()) + .collect::>(); + assert_eq!(filtered, vec!["comby"]); +} + +#[tokio::test] +async fn rewrite_uses_in_place_and_preserves_filters() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let work_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + let source = work_dir.path().join("lib.rs"); + fs::write(&source, "fn old() {}\n").unwrap(); + write_fake_comby( + bin_dir.path(), + "for arg in \"$@\"; do if [ \"$arg\" = \"-in-place\" ]; then printf 'fn new() {}\\n' > lib.rs; fi; done\nprintf 'rewrote\\n'\n", + ); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("COMBY_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("COMBY_CWD_FILE", cwd_file.as_os_str()); + + let mut ctx = make_ctx(); + ctx.cwd = work_dir.path().to_path_buf(); + let output = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "rewrite", + "match_template": "old(:[x])", + "rewrite_template": "new(:[x])", + "extensions": [".rs"], + "directory": ".", + "matcher": "rust", + "include_files": ".*\\.rs", + "exclude_files": "target", + "extra_args": ["-jobs", "1"] + }), + &ctx, + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec![ + "old(:[x])", + "new(:[x])", + ".rs", + "-d", + ".", + "-matcher", + "rust", + "-include-files", + ".*\\.rs", + "-exclude-files", + "target", + "-in-place", + "-jobs", + "1" + ] + ); + assert_eq!( + fs::read_to_string(cwd_file).unwrap().trim(), + work_dir.path().to_str().unwrap() + ); + assert_eq!(fs::read_to_string(source).unwrap(), "fn new() {}\n"); + + let output = json_output(&output); + assert_eq!(output["ok"], true); + assert_eq!(output["tool"], "comby"); + assert_eq!(output["action"], "rewrite"); + assert_eq!(output["exit_code"], 0); + assert_eq!(output["stdout"], "rewrote\n"); +} + +#[tokio::test] +async fn check_uses_match_only_without_rewrite_template() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_comby(bin_dir.path(), "printf 'match\\n'\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("COMBY_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("COMBY_CWD_FILE", cwd_file.as_os_str()); + + let output = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "check", + "match_template": "old(:[x])", + "extensions": [".rs"] + }), + &make_ctx(), + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec!["old(:[x])", "", ".rs", "-match-only"] + ); + let output = json_output(&output); + assert_eq!(output["ok"], true); + assert_eq!(output["action"], "check"); +} + +#[tokio::test] +async fn diff_uses_diff_and_returns_output() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_comby( + bin_dir.path(), + "printf '%s\\n' '--- a/lib.rs' '+++ b/lib.rs'\n", + ); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("COMBY_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("COMBY_CWD_FILE", cwd_file.as_os_str()); + + let output = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "diff", + "match_template": "old(:[x])", + "rewrite_template": "new(:[x])" + }), + &make_ctx(), + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec!["old(:[x])", "new(:[x])", "-diff"] + ); + let output = json_output(&output); + assert_eq!(output["ok"], true); + assert_eq!(output["action"], "diff"); + assert!(output["stdout"].as_str().unwrap().contains("--- a/lib.rs")); + assert!(output["stdout"].as_str().unwrap().contains("+++ b/lib.rs")); +} + +#[tokio::test] +async fn rewrite_without_template_is_rejected_before_spawning_comby() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_comby(bin_dir.path(), "printf 'should-not-run\\n'\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("COMBY_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("COMBY_CWD_FILE", cwd_file.as_os_str()); + + let output = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "rewrite", + "match_template": "old(:[x])" + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&output); + + assert_eq!(output["ok"], false); + assert_eq!(output["action"], "rewrite"); + assert_eq!(output["error"]["kind"], "invalid_request"); + assert!( + !args_file.exists(), + "comby should not run when request validation fails" + ); +} + +#[tokio::test] +async fn missing_comby_returns_structured_guidance() { + let _guard = env_lock().lock().await; + let empty_path = tempfile::tempdir().unwrap(); + let _path = EnvRestore::set("PATH", empty_path.path().as_os_str()); + + let output = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "check", + "match_template": "old(:[x])" + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&output); + + assert_eq!(output["ok"], false); + assert_eq!(output["exit_code"], Value::Null); + assert_eq!(output["error"]["kind"], "missing_comby"); + assert!(output["error"]["install"][0] + .as_str() + .unwrap() + .contains("comby")); +} + +#[tokio::test] +async fn non_zero_comby_exit_is_structured() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_comby( + bin_dir.path(), + "printf 'partial\\n'\nprintf 'bad template\\n' >&2\nexit 3\n", + ); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("COMBY_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("COMBY_CWD_FILE", cwd_file.as_os_str()); + + let output = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "check", + "match_template": "old(:[x])" + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&output); + + assert_eq!(output["ok"], false); + assert_eq!(output["exit_code"], 3); + assert_eq!(output["stdout"], "partial\n"); + assert_eq!(output["stderr"], "bad template\n"); + assert_eq!(output["error"]["kind"], "command_failed"); +} + +#[tokio::test] +async fn output_truncation_preserves_valid_json() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_comby(bin_dir.path(), "printf '%02000d\\n' 0\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("COMBY_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("COMBY_CWD_FILE", cwd_file.as_os_str()); + + let rendered = CombyTool + .execute( + "comby", + serde_json::json!({ + "action": "check", + "match_template": "old(:[x])", + "max_output_bytes": 700 + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&rendered); + + assert_eq!(output["truncated"], true); + assert!(output["stdout"].as_str().unwrap().contains("[truncated]")); +} diff --git a/tests/sd_tool.rs b/tests/sd_tool.rs new file mode 100644 index 0000000..b2a9623 --- /dev/null +++ b/tests/sd_tool.rs @@ -0,0 +1,365 @@ +//! Integration tests for the native sd tool. These use a fake `sd` binary so +//! the tests verify Ra's argv/cwd/envelope behavior without depending on the +//! host sd installation. + +use ra::{ + tools::{SdTool, Tool}, + ToolCtx, +}; +use serde_json::Value; +use std::{ffi::OsString, fs, os::unix::fs::PermissionsExt, path::Path, sync::OnceLock}; +use tokio::sync::Mutex; + +struct EnvRestore { + key: &'static str, + old_value: Option, +} + +impl EnvRestore { + fn set>(key: &'static str, value: K) -> Self { + let old_value = std::env::var_os(key); + std::env::set_var(key, value.into()); + Self { key, old_value } + } +} + +impl Drop for EnvRestore { + fn drop(&mut self) { + if let Some(value) = &self.old_value { + std::env::set_var(self.key, value); + } else { + std::env::remove_var(self.key); + } + } +} + +fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) +} + +fn make_ctx() -> ToolCtx { + let (events, _) = tokio::sync::broadcast::channel(16); + ToolCtx::local(events) +} + +fn json_output(output: &str) -> Value { + serde_json::from_str(output).unwrap_or_else(|err| panic!("invalid json: {err}: {output}")) +} + +fn write_fake_sd(dir: &Path, body: &str) { + let path = dir.join("sd"); + fs::write( + &path, + format!( + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$SD_ARGS_FILE\"\npwd > \"$SD_CWD_FILE\"\n{body}\n" + ), + ) + .unwrap(); + let mut perms = fs::metadata(&path).unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(path, perms).unwrap(); +} + +#[tokio::test] +async fn default_catalog_contains_sd_and_allowlist_is_exact() { + let names = ra::default_builtins(&[]) + .into_iter() + .map(|tool| tool.name().to_string()) + .collect::>(); + + assert!(names.contains(&"sd".to_string()), "missing sd: {names:?}"); + + let filtered = ra::default_builtins(&["sd".to_string()]) + .into_iter() + .map(|tool| tool.name().to_string()) + .collect::>(); + assert_eq!(filtered, vec!["sd"]); +} + +#[tokio::test] +async fn basic_regex_replacement_preserves_argv_and_cwd() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let work_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + let source = work_dir.path().join("data.txt"); + fs::write(&source, "alpha beta\n").unwrap(); + write_fake_sd( + bin_dir.path(), + "if [ \"$1\" = \"--\" ] && [ \"$2\" = \"alpha\" ] && [ \"$3\" = \"gamma\" ]; then printf 'gamma beta\\n' > \"$4\"; fi\nprintf 'rewrote\\n'\n", + ); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + let mut ctx = make_ctx(); + ctx.cwd = work_dir.path().to_path_buf(); + let output = SdTool + .execute( + "sd", + serde_json::json!({ + "find": "alpha", + "replace": "gamma", + "paths": ["data.txt"] + }), + &ctx, + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec!["--", "alpha", "gamma", "data.txt"] + ); + assert_eq!( + fs::read_to_string(cwd_file).unwrap().trim(), + work_dir.path().to_str().unwrap() + ); + assert_eq!(fs::read_to_string(source).unwrap(), "gamma beta\n"); + + let output = json_output(&output); + assert_eq!(output["ok"], true); + assert_eq!(output["tool"], "sd"); + assert_eq!(output["command"]["program"], "sd"); + assert_eq!(output["exit_code"], 0); + assert_eq!(output["stdout"], "rewrote\n"); + assert_eq!(output["stderr"], Value::Null); + assert_eq!(output["truncated"], false); +} + +#[tokio::test] +async fn capture_group_replacement_is_passed_verbatim() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_sd(bin_dir.path(), "printf 'ok\\n'\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + SdTool + .execute( + "sd", + serde_json::json!({ + "find": "(foo)-(bar)", + "replace": "$2-$1", + "paths": ["data.txt"] + }), + &make_ctx(), + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec!["--", "(foo)-(bar)", "$2-$1", "data.txt"] + ); +} + +#[tokio::test] +async fn string_mode_and_extra_args_are_passed_before_positionals() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_sd(bin_dir.path(), "printf 'ok\\n'\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + SdTool + .execute( + "sd", + serde_json::json!({ + "find": "a.b", + "replace": "x", + "paths": ["data.txt"], + "string_mode": true, + "extra_args": ["--flags", "i"] + }), + &make_ctx(), + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec![ + "--fixed-strings", + "--flags", + "i", + "--", + "a.b", + "x", + "data.txt" + ] + ); +} + +#[tokio::test] +async fn leading_dash_find_and_replace_are_not_parsed_as_flags() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_sd(bin_dir.path(), "printf 'ok\\n'\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + SdTool + .execute( + "sd", + serde_json::json!({ + "find": "-old", + "replace": "-new", + "paths": ["data.txt"] + }), + &make_ctx(), + ) + .await + .unwrap(); + + assert_eq!( + fs::read_to_string(args_file) + .unwrap() + .lines() + .collect::>(), + vec!["--", "-old", "-new", "data.txt"] + ); +} + +#[tokio::test] +async fn empty_paths_are_rejected_before_spawning_sd() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_sd(bin_dir.path(), "printf 'should-not-run\\n'\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + let output = SdTool + .execute( + "sd", + serde_json::json!({ "find": "a", "replace": "b", "paths": [] }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&output); + + assert_eq!(output["ok"], false); + assert_eq!(output["error"]["kind"], "invalid_request"); + assert!( + !args_file.exists(), + "sd should not run when request validation fails" + ); +} + +#[tokio::test] +async fn missing_sd_returns_structured_guidance() { + let _guard = env_lock().lock().await; + let empty_path = tempfile::tempdir().unwrap(); + let _path = EnvRestore::set("PATH", empty_path.path().as_os_str()); + + let output = SdTool + .execute( + "sd", + serde_json::json!({ + "find": "a", + "replace": "b", + "paths": ["data.txt"] + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&output); + + assert_eq!(output["ok"], false); + assert_eq!(output["exit_code"], Value::Null); + assert_eq!(output["error"]["kind"], "missing_sd"); + assert!(output["error"]["install"][0] + .as_str() + .unwrap() + .contains("cargo install sd")); +} + +#[tokio::test] +async fn non_zero_sd_exit_is_structured() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_sd( + bin_dir.path(), + "printf 'partial\\n'\nprintf 'bad pattern\\n' >&2\nexit 2\n", + ); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + let output = SdTool + .execute( + "sd", + serde_json::json!({ + "find": "[", + "replace": "b", + "paths": ["data.txt"] + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&output); + + assert_eq!(output["ok"], false); + assert_eq!(output["exit_code"], 2); + assert_eq!(output["stdout"], "partial\n"); + assert_eq!(output["stderr"], "bad pattern\n"); + assert_eq!(output["error"]["kind"], "command_failed"); +} + +#[tokio::test] +async fn output_truncation_preserves_valid_json() { + let _guard = env_lock().lock().await; + let bin_dir = tempfile::tempdir().unwrap(); + let args_file = bin_dir.path().join("args.txt"); + let cwd_file = bin_dir.path().join("cwd.txt"); + write_fake_sd(bin_dir.path(), "printf '%02000d\\n' 0\n"); + let _path = EnvRestore::set("PATH", bin_dir.path().as_os_str()); + let _args = EnvRestore::set("SD_ARGS_FILE", args_file.as_os_str()); + let _cwd = EnvRestore::set("SD_CWD_FILE", cwd_file.as_os_str()); + + let rendered = SdTool + .execute( + "sd", + serde_json::json!({ + "find": "a", + "replace": "b", + "paths": ["data.txt"], + "max_output_bytes": 600 + }), + &make_ctx(), + ) + .await + .unwrap(); + let output = json_output(&rendered); + + assert_eq!(output["truncated"], true); + assert!(output["stdout"].as_str().unwrap().contains("[truncated]")); +}