Adds the Tool trait, config loading/schema, the Provider trait plus an Llm event surface, a permission service, and the core engine loop with a doom-loop guard and retry scaffolding. The processor drives a text/tool-call/final-text turn against a Provider, laying the groundwork for the MockProvider integration test and the real Anthropic path.
Implements the first five harness-tools: read, write, bash (with timeout/output truncation), glob, and grep, plus shared path-resolution helpers. Wires them into the core tool registry and processor.
Ports opencode's multi-strategy string-replacer chain (and its test table) into harness-tools::edit, giving the edit tool the same fuzzy-match fallback behavior opencode relies on.
Adds the Anthropic SSE codec (codec/anthropic.rs) translating Anthropic's event stream into LlmEvents, the AnthropicProvider, and registry wiring so the engine loop can run against the real API instead of just MockProvider.
Adds the harness-app composition root wiring config, providers, tools, and the engine loop together, plus the harness run -p "<prompt>" debug command in harness-tui::main for driving a one-shot prompt end-to-end from the CLI.
darman
added this to the AI Harness project 2026-07-10 17:08:15 +02:00
darman
requested review from claude-reviewer 2026-07-11 14:41:48 +02:00
Solid milestone. The engine loop / processor split is clean, the streaming state machine in processor.rs is careful about part idx preservation on re-flush, and test coverage is genuinely good (doom-loop, retry, permission ask flow, codec SSE decode all covered). A few things worth addressing:
Blocking I/O on the async runtime (medium)
Several tool paths do synchronous, potentially long-running work directly inside async fn execute, blocking a runtime worker for the whole walk/search:
grep.rs:5598 — WalkBuilder::build() + searcher.search_path() loop over the entire tree, synchronously.
glob.rs:5398 — same synchronous WalkBuilder walk.
engine/system.rs:2085 — env_header() shells out to git status with the blocking std::process::Command, and it runs once per step in the async run_session loop.
Headless this is fine, but once the TUI (M2) shares the runtime a large-repo grep/glob (or a slow git status) will stall other tasks. Suggest wrapping the walk/search in tokio::task::spawn_blocking and using tokio::process::Command for git status. Also worth caching the env header rather than recomputing git status every step.
edit/mod.rs:4327 reads with String::from_utf8_lossy and later writes the result back (4357). Any invalid UTF-8 byte becomes U+FFFD and is then persisted — so an edit to a file containing non-UTF8 bytes silently corrupts the untouched parts. Consider erroring on non-UTF8 input instead of lossily rewriting (opencode rejects it).
Minor
glob.rs: literal_separator(true) means *.rs won't match nested files (src/main.rs needs **/*.rs). Matches standard glob semantics but is a common user surprise — worth a note in the tool description.
on_tool_call (processor.rs:1668): pending_tools.remove(&call_id).unwrap_or_default() falls back to PartId::default(). Safe today because the Anthropic codec always emits ToolInputStart first, but two default-id parts would collide if a provider ever skips it. A fresh PartId::new() fallback would be safer.
anthropic.rs:3257: retry-after is parsed as integer seconds only; the HTTP-date form is silently dropped to None.
Nothing blocking. Nice work.
— automated review (Claude)
## Review: M1 — Headless core loop + Anthropic
Solid milestone. The engine loop / processor split is clean, the streaming state machine in `processor.rs` is careful about part idx preservation on re-flush, and test coverage is genuinely good (doom-loop, retry, permission ask flow, codec SSE decode all covered). A few things worth addressing:
### Blocking I/O on the async runtime (medium)
Several tool paths do synchronous, potentially long-running work directly inside `async fn execute`, blocking a runtime worker for the whole walk/search:
- `grep.rs:5598` — `WalkBuilder::build()` + `searcher.search_path()` loop over the entire tree, synchronously.
- `glob.rs:5398` — same synchronous `WalkBuilder` walk.
- `engine/system.rs:2085` — `env_header()` shells out to `git status` with the blocking `std::process::Command`, and it runs **once per step** in the async `run_session` loop.
Headless this is fine, but once the TUI (M2) shares the runtime a large-repo grep/glob (or a slow git status) will stall other tasks. Suggest wrapping the walk/search in `tokio::task::spawn_blocking` and using `tokio::process::Command` for git status. Also worth caching the env header rather than recomputing git status every step.
### edit/read silently corrupt non-UTF8 files (low-medium)
`edit/mod.rs:4327` reads with `String::from_utf8_lossy` and later writes the result back (`4357`). Any invalid UTF-8 byte becomes U+FFFD and is then persisted — so an edit to a file containing non-UTF8 bytes silently corrupts the untouched parts. Consider erroring on non-UTF8 input instead of lossily rewriting (opencode rejects it).
### Minor
- `glob.rs`: `literal_separator(true)` means `*.rs` won't match nested files (`src/main.rs` needs `**/*.rs`). Matches standard glob semantics but is a common user surprise — worth a note in the tool description.
- `on_tool_call` (`processor.rs:1668`): `pending_tools.remove(&call_id).unwrap_or_default()` falls back to `PartId::default()`. Safe today because the Anthropic codec always emits `ToolInputStart` first, but two default-id parts would collide if a provider ever skips it. A fresh `PartId::new()` fallback would be safer.
- `anthropic.rs:3257`: `retry-after` is parsed as integer seconds only; the HTTP-date form is silently dropped to `None`.
Nothing blocking. Nice work.
— automated review (Claude)
darman
self-assigned this 2026-07-11 14:44:31 +02:00
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
claude-reviewer
Tooltrait, config loading, permission service, and the engine loop (text → tool call → final text), exercised viaMockProviderharness-appcomposition root and theharness run -p "<prompt>"debug commandharness run -pdebug command d83f8c84b7Review: M1 — Headless core loop + Anthropic
Solid milestone. The engine loop / processor split is clean, the streaming state machine in
processor.rsis careful about part idx preservation on re-flush, and test coverage is genuinely good (doom-loop, retry, permission ask flow, codec SSE decode all covered). A few things worth addressing:Blocking I/O on the async runtime (medium)
Several tool paths do synchronous, potentially long-running work directly inside
async fn execute, blocking a runtime worker for the whole walk/search:grep.rs:5598—WalkBuilder::build()+searcher.search_path()loop over the entire tree, synchronously.glob.rs:5398— same synchronousWalkBuilderwalk.engine/system.rs:2085—env_header()shells out togit statuswith the blockingstd::process::Command, and it runs once per step in the asyncrun_sessionloop.Headless this is fine, but once the TUI (M2) shares the runtime a large-repo grep/glob (or a slow git status) will stall other tasks. Suggest wrapping the walk/search in
tokio::task::spawn_blockingand usingtokio::process::Commandfor git status. Also worth caching the env header rather than recomputing git status every step.edit/read silently corrupt non-UTF8 files (low-medium)
edit/mod.rs:4327reads withString::from_utf8_lossyand later writes the result back (4357). Any invalid UTF-8 byte becomes U+FFFD and is then persisted — so an edit to a file containing non-UTF8 bytes silently corrupts the untouched parts. Consider erroring on non-UTF8 input instead of lossily rewriting (opencode rejects it).Minor
glob.rs:literal_separator(true)means*.rswon't match nested files (src/main.rsneeds**/*.rs). Matches standard glob semantics but is a common user surprise — worth a note in the tool description.on_tool_call(processor.rs:1668):pending_tools.remove(&call_id).unwrap_or_default()falls back toPartId::default(). Safe today because the Anthropic codec always emitsToolInputStartfirst, but two default-id parts would collide if a provider ever skips it. A freshPartId::new()fallback would be safer.anthropic.rs:3257:retry-afteris parsed as integer seconds only; the HTTP-date form is silently dropped toNone.Nothing blocking. Nice work.
— automated review (Claude)
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.