From 1d50b94eec1ed82dfc40d43d41bd01aeb79edfe1 Mon Sep 17 00:00:00 2001 From: asepharyana Date: Mon, 13 Jul 2026 14:35:42 +0700 Subject: [PATCH] feat: update README and documentation for new tools and features - Updated README.md to reflect the addition of 3 new built-in tools, bringing the total to 37. - Revised architecture documentation to indicate the increase in tool count. - Enhanced backend documentation with updated line counts for various modules. - Modified data documentation to change edit log format from JSON to JSONL. - Updated dependencies documentation to reflect version upgrades for several crates. - Improved prompts for auto-reviewer, division implementer, planner, tester, and quality reviewer to enforce stricter coding standards regarding linter bypasses. - Refactored code in various modules to improve clarity and performance, including updates to error handling and tool execution logic. - Added comprehensive tests for IPC frame serialization and deserialization. --- CLAUDE.md | 5 +- README.md | 6 +- docs/CODEMAPS/architecture.md | 10 +- docs/CODEMAPS/backend.md | 39 +++++--- docs/CODEMAPS/data.md | 6 +- docs/CODEMAPS/dependencies.md | 10 +- src-misc/auto-reviewer-prompt.txt | 1 + src-misc/division-implementer-prompt.txt | 1 + src-misc/division-planner-prompt.txt | 1 + src-misc/division-tester-prompt.txt | 1 + src-misc/quality-reviewer-prompt.txt | 1 + src-misc/system-prompt.txt | 1 + src/app/harness.rs | 2 +- src/app/runtime/actions/mod.rs | 101 ++++++++++--------- src/app/runtime/stream/mod.rs | 10 +- src/app/subagent/engine.rs | 107 ++++++++++---------- src/app/workflow/company.rs | 15 +-- src/app/workflow/engine.rs | 50 +++++----- src/ipc/frame.rs | 120 +++++++++++++++++++++++ src/model/editlog.rs | 4 +- src/model/memory.rs | 2 +- src/tool/mod.rs | 2 +- src/tool/workflow.rs | 8 +- 23 files changed, 327 insertions(+), 176 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 94cff6b..089ceb2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ Tracing output goes to `~/.local/share/zesdex/zesdex.log`. Set `RUST_LOG=debug` ## Architecture Overview -Zesdex is an autonomous AI coding agent with a TUI — an OpenAI/Anthropic-compatible LLM client wrapped in a tool-use harness with 28 built-in tools. +Zesdex is an autonomous AI coding agent with a TUI — an OpenAI/Anthropic-compatible LLM client wrapped in a tool-use harness with 37 built-in tools. Detailed architecture documentation is in `docs/CODEMAPS/`: @@ -62,7 +62,7 @@ Detailed architecture documentation is in `docs/CODEMAPS/`: Controller (key input → Action) → Event Loop → LLM stream → Tool execution → State mutation → TUI render │ │ │ │ src/controller/input.rs │ src/app/runtime/actions/ │ src/tool/ - └── maps keys to Action enum │── dispatches Action::* └── 28 tool impls + └── maps keys to Action enum │── dispatches Action::* └── 37 tool impls │ matching on Action variant │── applies state mutations ``` @@ -167,3 +167,4 @@ Rules: - Non-trivial private functions (≥10 lines) need a doc comment - Write the comment above the code it documents (not inline in the body) - Update comments when code behavior changes — stale docs are worse than no docs +- NEVER use compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) to silence warnings or skip linter checks. Always fix the underlying code issues instead. diff --git a/README.md b/README.md index c392f50..88329c6 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ Zesdex is a Rust-powered AI assistant that operates directly in your terminal vi - **IPC Protocol** — Bidirectional state synchronization between daemon and client processes with diff-based updates. - **Provider Agnostic** — Configurable AI model providers with dynamic model selection, per-role temperature/token limits, and API key management. -### Tool System (34 built-in tools) +### Tool System (37 built-in tools) | Category | Tools | |----------|-------| @@ -25,8 +25,9 @@ Zesdex is a Rust-powered AI assistant that operates directly in your terminal vi | **Git** | `git_operator`, `git_worktree`, `git_cred` | | **Memory** | `remember`, `recall`, `forget` | | **Planning** | `plan_enter`, `plan_ready`, `seqthink` | -| **Workflow** | `workflow_run`, `note_finding`, `company_pipeline` | +| **Workflow** | `workflow_run`, `note_finding`, `read_findings`, `company_pipeline` | | **Utility** | `cd`, `dir_list`, `dir_cache_update`, `pong`, `todowrite`, `todofinish` | +| **Agent** | `spawn_agents`, `spawn_pipeline` | | **LSP** | `lsp_connect`, `lsp_diagnostics`, `lsp_hover`, `lsp_completion`, `lsp_definition`, `lsp_references`, `lsp_disconnect` | ### Intelligence @@ -83,6 +84,7 @@ src/ │ │ ├── effort.rs # Effort level selector │ │ ├── help.rs # Help overlay │ │ ├── key_input.rs # Raw key input mode +│ │ ├── learning.rs # Lesson management overlay │ │ ├── loading.rs # Loading spinner overlay │ │ ├── mcp.rs # MCP server management │ │ ├── quit_confirm.rs # Quit confirmation dialog diff --git a/docs/CODEMAPS/architecture.md b/docs/CODEMAPS/architecture.md index b0728d4..615fd33 100644 --- a/docs/CODEMAPS/architecture.md +++ b/docs/CODEMAPS/architecture.md @@ -28,7 +28,7 @@ Zesdex is a single-process terminal AI coding agent with optional daemon/client │ │ │ │ │ ┌─────▼──┐ ┌───▼────┐ │ │ │ Tools │ │Sub- │ │ -│ │ (28) │ │agents │ │ +│ │ (37) │ │agents │ │ │ └────────┘ └────────┘ │ └───────────────────────────────────────────────────────┘ ``` @@ -55,7 +55,7 @@ User keystroke → Controller (KeyEvent → Action) | File | Lines | Role | |------|-------|------| -| `src/main.rs` | 530 | Entry, TUI setup, daemon loop, attach loop | -| `src/app/runtime/actions/mod.rs` | 1022 | Action dispatch + LLM stream loop + tool execution | -| `src/controller/input.rs` | 281 | Key event → Action mapping | -| `src/view/mod.rs` | 623 | TUI rendering (ratatui) | +| `src/main.rs` | 647 | Entry, TUI setup, daemon loop, attach loop | +| `src/app/runtime/actions/mod.rs` | 1815 | Action dispatch + LLM stream loop + tool execution | +| `src/controller/input.rs` | 365 | Key event → Action mapping | +| `src/view/mod.rs` | 975 | TUI rendering (ratatui) | diff --git a/docs/CODEMAPS/backend.md b/docs/CODEMAPS/backend.md index ddb8566..f31701f 100644 --- a/docs/CODEMAPS/backend.md +++ b/docs/CODEMAPS/backend.md @@ -4,7 +4,7 @@ ## AI Provider -`src/service/provider.rs` (258 lines) +`src/service/provider.rs` (310 lines) - `LlmClient::new(api_key, model, base_url)` — constructs blocking reqwest client - `chat_with_tools()` — non-streaming with tool definitions - `chat_stream()` — SSE streaming, returns `SseParser` yielding `StreamEvent` @@ -18,51 +18,58 @@ ## IPC / Daemon -`src/ipc/` (7 files, ~300 lines total) +`src/ipc/` (7 files, ~350 lines total) - Unix domain socket, length-prefixed JSON frames -- Daemon sends `DaemonFrame { state: StatePayload, diff, tasks }` to clients -- Clients send `ClientRequest { action: Action }` back -- State sync uses snapshots + binary diffs (rsync-style, not git) +- Daemon sends `DaemonFrame` (state payload, stream tokens, system notes) +- Clients send `ClientRequest` (key presses, resize, submit, scroll) +- State sync uses full-state push from daemon to client after each action ## Workflow Engine -`src/app/workflow/engine.rs` (251 lines) + `script.rs` +`src/app/workflow/engine.rs` (648 lines) + `script.rs` - Inline JS-style DSL executed by a lightweight runtime - `agent()`, `parallel()`, `pipeline()`, `phase()`, `log()` — spawns sub-agents - Max concurrency configurable via `workflow_max_concurrency` setting +- Company pipeline orchestrator in `company.rs` (406 lines): full 5-division or quick 3-division pipelines ## Sub-Agent System -`src/app/subagent/` (4 files, ~250 lines) -- `run_subagent()` — spawns independent agent with its own tool set & context +`src/app/subagent/` (6 files: `spawn.rs`, `engine.rs`, `context.rs`, `event.rs`, `division.rs`, `auto.rs`, ~450 lines total) +- `run_subagent()` — spawns independent agent with its own tool set and context - Communicates via `mpsc` channel (tool calls, results, completion) - Uses `LlmClient` (same as main agent) with tool-use API +- Auto-healing: on build/test failure, spawns auto-fix sub-agent +- Division roles: Strategy, Engineering, Quality, Security, Documentation ## MCP Client -`src/app/mcp/manager.rs` (371 lines) +`src/app/mcp/manager.rs` (441+ lines) - Stdio transport: spawns child process, JSON-RPC via stdin/stdout - HTTP transport: streaming HTTP with JSON-RPC -- Tool registration: `tools/list` → `McpToolAdapter` implements `crate::tool::Tool` +- Dynamic tool list refresh and error recovery - Persistent child handle for stdio (reuses connection across calls) ## Self-Review -`src/app/review/mod.rs` (437 lines) +`src/app/review/mod.rs` (495 lines) - Post-tool execution quality check against learned lessons - Invokes `run_subagent()` with reviewer prompt - Staleness detection: skips review after N consecutive empty results +- Three review types: code quality, architecture, security ## Background Bash -`src/app/bgbash/` (2 files) +`src/app/bgbash/` (2 files: `job.rs`, `control.rs`) - `spawn_bash_job()` — runs `sh -c` in a thread, collects stdout line-by-line - Channels: output via `mpsc`, PID via `mpsc` -- Killable via PID +- Killable via PID (SIGTERM) +- Output buffering capped at 10,000 lines to prevent memory issues ## Gate Guard / Harness -`src/app/harness.rs` (127 lines) +`src/app/harness.rs` (495 lines) - `Harness::gate_tool_call()` — verdict-based tool gating (allow/block) -- Parses LLM verdicts (JSON or plain-text) -- `test_parse_verdict_*` tests for 6 verdict formats +- Path traversal, credential read, and destructive command detection +- Pattern detection for stub code, denial language, and assumptions in write/edit content +- Reason validation for mutating tools (minimum 8 characters, rejects generic non-answers) +- Includes 8 unit tests for verdict parsing formats diff --git a/docs/CODEMAPS/data.md b/docs/CODEMAPS/data.md index e86f5e9..8590b0b 100644 --- a/docs/CODEMAPS/data.md +++ b/docs/CODEMAPS/data.md @@ -16,7 +16,7 @@ Base directory: `~/.config/zesdex/` (via `dirs::data_dir()`) │ └── *.md # Markdown with YAML frontmatter ├── sessions/ # Per-session data │ └── / -│ ├── editlog.json # Edit history +│ ├── edits.jsonl # Edit history (JSONL, append-only) │ ├── msglog.db # SQLite message log │ ├── transcript.json # Chat transcript │ ├── session.json # Session metadata @@ -34,8 +34,8 @@ Base directory: `~/.config/zesdex/` (via `dirs::data_dir()`) | `src/model/store.rs` | ~50 | File-system storage (ensure_dirs, base_dir resolution) | | `src/model/settings.rs` | ~60 | `Settings` — load/save JSON, API keys map | | `src/model/app_config.rs` | ~80 | `AppConfig` — provider definitions, model roles, auth | -| `src/model/memory.rs` | 332 | Memory CRUD — markdown files with frontmatter | -| `src/model/editlog.rs` | 121 | Edit log — append-only JSON array | +| `src/model/memory.rs` | 440 | Memory CRUD — markdown files with frontmatter | +| `src/model/editlog.rs` | 161 | Edit log — append-only JSONL (not JSON array) | | `src/model/msglog/` | 4 files | SQLite-backed message log (schema, query, blobs) | | `src/model/session.rs` | ~60 | Session CRUD, listing, archival | | `src/model/session_lock.rs` | ~50 | flock-based session lock | diff --git a/docs/CODEMAPS/dependencies.md b/docs/CODEMAPS/dependencies.md index 87b313d..cf38157 100644 --- a/docs/CODEMAPS/dependencies.md +++ b/docs/CODEMAPS/dependencies.md @@ -7,23 +7,23 @@ | Crate | Version | Purpose | |-------|---------|---------| | ratatui | 0.30 | TUI framework (tui-rs successor) | -| crossterm | 0.28 | Terminal manipulation (raw mode, alt screen) | +| crossterm | 0.29 | Terminal manipulation (raw mode, alt screen) | | tokio | 1 | Async runtime (daemon, OAuth loopback) | | reqwest | 0.12 | HTTP client (blocking + streaming, vendored native-tls) | | serde / serde_json | 1 | JSON serialization (state, DTOs, IPC, config) | -| serde_yaml_ng | 0.9 | YAML frontmatter parsing (memory files) | +| serde_yaml_ng | 0.10 | YAML frontmatter parsing (memory files) | | anyhow | 1 | Error handling (no custom error types) | | tracing / tracing-subscriber | 0.1/0.3 | Structured logging → file | -| rusqlite | 0.32 | SQLite (bundled, for message log) | +| rusqlite | 0.40 | SQLite (bundled, for message log) | | pulldown-cmark | 0.13 | Markdown → HTML (chat rendering) | | syntect | 5 | Syntax highlighting (code blocks in chat) | | sha2 | 0.10 | SHA-256 for PKCE challenge | | base64 | 0.22 | URL-safe base64 for PKCE | | libc | 0.2 | daemon PID file locking | -| rmcp | 1.8 | MCP client (stdio + HTTP transports) | +| rmcp | 2.2 | MCP client (stdio + HTTP transports) | | uuid | 1 | Session IDs, job IDs | | chrono | 0.4 | Timestamps (ISO 8601, millis) | -| dirs | 5 | Platform data directories | +| dirs | 6 | Platform data directories | | dom_smoothie | 0.18 | HTML → plain text (web scraping) | | scraper | 0.27 | HTML parsing (web scraping) | | ignore | 0.4 | .gitignore-aware file walking (glob tool) | diff --git a/src-misc/auto-reviewer-prompt.txt b/src-misc/auto-reviewer-prompt.txt index 8b0dadb..44966cc 100644 --- a/src-misc/auto-reviewer-prompt.txt +++ b/src-misc/auto-reviewer-prompt.txt @@ -9,6 +9,7 @@ Review guidelines: 2. Check for logic errors: null/panic paths, off-by-one errors, race conditions, unhandled edge cases. 3. Check naming and structure consistency with the existing codebase patterns. 4. Check that the implementation matches the apparent intent. +5. Check for linter bypasses: Ensure that compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) are NEVER used to silence warnings or skip linter checks. Reject them. Output: a concise 2-4 line verdict. If you find issues, be specific about what and where. Skip if the file is trivial (config, tests with no logic changes). diff --git a/src-misc/division-implementer-prompt.txt b/src-misc/division-implementer-prompt.txt index 5b6bb32..e30d672 100644 --- a/src-misc/division-implementer-prompt.txt +++ b/src-misc/division-implementer-prompt.txt @@ -14,6 +14,7 @@ Full access: read, write, edit, delete, bash, grep, glob, git_operator, lsp_*, s 6. Run `cargo build` or equivalent after each logical chunk. 7. If you encounter an issue not covered by the plan, use `note_finding` to flag it. 8. Update todo.md as you complete each file: `todofinish` +9. NEVER use compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) to skip or silence warnings. Fix the underlying code to adhere to linter guidelines. ## Output After each file: confirm what was implemented and any deviations from plan. diff --git a/src-misc/division-planner-prompt.txt b/src-misc/division-planner-prompt.txt index 0b9fc63..0d93f17 100644 --- a/src-misc/division-planner-prompt.txt +++ b/src-misc/division-planner-prompt.txt @@ -31,4 +31,5 @@ You MUST produce a structured plan covering: - Use `seqthink` for complex reasoning steps - Every plan MUST include at least one mermaid diagram - Be specific with file paths and function names +- Ensure implementation plans NEVER suggest or allow using compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) to silence warnings; always plan to fully resolve underlying code issues. - Output ends with a clear "Plan Complete" marker diff --git a/src-misc/division-tester-prompt.txt b/src-misc/division-tester-prompt.txt index e9d81fe..81aa4d9 100644 --- a/src-misc/division-tester-prompt.txt +++ b/src-misc/division-tester-prompt.txt @@ -9,6 +9,7 @@ Check for: - Stubs, placeholders, incomplete branches - Naming consistency with codebase conventions - Error handling coverage +- Absence of compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) to silence warnings ## Phase 2: Test Use write to create test files. Follow these rules: diff --git a/src-misc/quality-reviewer-prompt.txt b/src-misc/quality-reviewer-prompt.txt index ba9ae12..541152f 100644 --- a/src-misc/quality-reviewer-prompt.txt +++ b/src-misc/quality-reviewer-prompt.txt @@ -9,6 +9,7 @@ Review guidelines: 2. Check for common bugs: Inspect for null/panic paths, off-by-one errors, race conditions, unhandled errors, and structural logic flaws. 4. Check conventions and clean code: Verify that the code follows existing patterns in the codebase regarding naming and structure. Ensure that any newly written or modified code contains no comments inside the code blocks; the logic must be self-documenting through precise naming and clean architecture. 5. Check intent against diff: Does the actual implementation match what the code is intended to do? +6. Check for linter bypasses: Ensure that compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) are NEVER used to skip warnings. Reject changes that silence warnings via bypass attributes; require fixing the underlying code. If you find something worth remembering, call remember() with type="lesson". Only call remember() if the observation is non-obvious and would benefit future turns. Skip trivial style nits. diff --git a/src-misc/system-prompt.txt b/src-misc/system-prompt.txt index 337eaa2..c7e0d81 100644 --- a/src-misc/system-prompt.txt +++ b/src-misc/system-prompt.txt @@ -88,3 +88,4 @@ Available tools are described in system-tools.txt section. Key tools for orchest - After changes, run builds and tests - Use LSP diagnostics after each file edit - Every code path must be fully implemented and deterministic +- NEVER use compiler/linter bypass annotations or attributes (such as `#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)]`, `#[allow(dead_code)]`, etc.) to silence warnings or skip linter checks. Fix the underlying code issues instead. diff --git a/src/app/harness.rs b/src/app/harness.rs index 96ce1c8..83c0b8b 100644 --- a/src/app/harness.rs +++ b/src/app/harness.rs @@ -430,7 +430,7 @@ mod tests { return Some(Verdict::Allow); } if l.starts_with("verdict: block") { - let reason = line.split_once(':').map(|x| x.1).unwrap_or("blocked").trim().to_string(); + let reason = line.split_once(':').map_or("blocked", |x| x.1).trim().to_string(); return Some(Verdict::Block(reason)); } } diff --git a/src/app/runtime/actions/mod.rs b/src/app/runtime/actions/mod.rs index 02195de..7381eb1 100644 --- a/src/app/runtime/actions/mod.rs +++ b/src/app/runtime/actions/mod.rs @@ -1017,8 +1017,7 @@ fn run_agent_turn( q.push_back(TurnEvent::SystemNote { kind: "pipeline".to_string(), message: format!( - "CEO is planning workflow (mode={})...", - mode_label, + "CEO is planning workflow (mode={mode_label})...", ), }); } @@ -1054,12 +1053,11 @@ fn run_agent_turn( ); let user_msg = ChatMessage::user(format!( "Design a structured multi-agent workflow plan for the following task:\n\n\ - \"{}\"\n\n\ - You must output a JSON object representing the 'specialists' configuration for {}.\n\ + \"{user_request}\"\n\n\ + You must output a JSON object representing the 'specialists' configuration for {required_divisions}.\n\ Each division must have a list of custom specialists defined by a pair of [label, focus_description].\n\n\ Return ONLY a JSON object with this exact structure, with no markdown codeblocks and no explanation:\n\ - {}", - user_request, required_divisions, example_json + {example_json}" )); let planner_result = tc.client.chat_with_tools_non_streaming(&[system_msg, user_msg], None); @@ -1070,7 +1068,7 @@ fn run_agent_turn( let mut lines = reply_text.lines(); lines.next(); let mut content = lines.collect::>(); - if content.last().map(|s| s.trim() == "```").unwrap_or(false) { + if content.last().is_some_and(|s| s.trim() == "```") { content.pop(); } content.join("\n") @@ -1103,7 +1101,7 @@ fn run_agent_turn( &tc.workspace_roots, Some(events_q), &pipeline_abort, - custom_specialists, + &custom_specialists, ) } else { crate::app::workflow::company::run_company_pipeline_quick( @@ -1112,14 +1110,14 @@ fn run_agent_turn( &tc.workspace_roots, Some(events_q), &pipeline_abort, - custom_specialists, + &custom_specialists, ) } } - Err(e) => Err(anyhow::anyhow!("Failed to parse LLM planning JSON: {}. Cleaned JSON was: {}", e, clean_json)), + Err(e) => Err(anyhow::anyhow!("Failed to parse LLM planning JSON: {e}. Cleaned JSON was: {clean_json}")), } } - Err(e) => Err(anyhow::anyhow!("Failed to query LLM for planning workflow: {}", e)), + Err(e) => Err(anyhow::anyhow!("Failed to query LLM for planning workflow: {e}")), }; match pipeline_result { @@ -1304,44 +1302,60 @@ fn run_agent_turn( let tool_calls = response.tool_calls.clone().unwrap_or_default(); archive_message(tc.db.as_ref(), &tc.session_id, &response); msgs.push(response); - for tool_call in tool_calls { + let mut results_vec = Vec::new(); + std::thread::scope(|s| { + let mut handles = Vec::new(); + let tc_ref = tc; + for tool_call in &tool_calls { + let handle = s.spawn(move || { + let tool_name = tool_call.function.name.clone(); + let args = crate::dto::chat::tool::sanitize_tool_arguments( + &tool_call.function.arguments, + ); + + let ws_roots: Vec<&std::path::Path> = + tc_ref.workspace_roots.iter().map(std::path::PathBuf::as_path).collect(); + let verdict = crate::app::harness::Harness::gate_tool_call( + &tool_name, + &args, + &ws_roots, + ); + + let is_edit_tool = tool_name == "write" || tool_name == "edit"; + let (output, is_error, is_edit) = match verdict { + Verdict::Allow => match execute_one_tool( + &tc_ref.tools, + &tc_ref.ctx, + &tool_name, + &tool_call.id, + &args, + &tc_ref.edit_log_session_dir, + &tc_ref.session_id, + tc_ref.db.as_ref(), + ) { + Ok(result) => (result, false, is_edit_tool), + Err(e) => (e.to_string(), true, false), + }, + Verdict::Block(reason) => (format!("Blocked: {reason}"), true, false), + }; + (tool_call, tool_name, args, output, is_error, is_edit) + }); + handles.push(handle); + } + for h in handles { + if let Ok(res) = h.join() { + results_vec.push(res); + } + } + }); + + for (tool_call, tool_name, args, output, is_error, is_edit) in results_vec { if tc.abort_flag.load(std::sync::atomic::Ordering::SeqCst) { if let Ok(mut q) = events_q.lock() { q.push_back(TurnEvent::Error("Turn aborted by user".to_string())); } return Ok(()); } - let tool_name = tool_call.function.name.clone(); - let args = crate::dto::chat::tool::sanitize_tool_arguments( - &tool_call.function.arguments, - ); - - let ws_roots: Vec<&std::path::Path> = - tc.workspace_roots.iter().map(std::path::PathBuf::as_path).collect(); - let verdict = crate::app::harness::Harness::gate_tool_call( - &tool_name, - &args, - - &ws_roots, - ); - - let is_edit_tool = tool_name == "write" || tool_name == "edit"; - let (output, is_error, is_edit) = match verdict { - Verdict::Allow => match execute_one_tool( - &tc.tools, - &tc.ctx, - &tool_name, - &tool_call.id, - &args, - &tc.edit_log_session_dir, - &tc.session_id, - tc.db.as_ref(), - ) { - Ok(result) => (result, false, is_edit_tool), - Err(e) => (e.to_string(), true, false), - }, - Verdict::Block(reason) => (format!("Blocked: {reason}"), true, false), - }; if is_edit { edits_this_turn += 1; @@ -1399,7 +1413,6 @@ fn run_agent_turn( } } - let tool_path = args.get("path").and_then(|v| v.as_str()).map(std::string::ToString::to_string); { diff --git a/src/app/runtime/stream/mod.rs b/src/app/runtime/stream/mod.rs index 25555be..0fdd434 100644 --- a/src/app/runtime/stream/mod.rs +++ b/src/app/runtime/stream/mod.rs @@ -280,7 +280,7 @@ mod tests { assert_eq!(events.len(), 1); match &events[0] { StreamEvent::Token(t) => assert_eq!(t, "hello"), - other => panic!("expected Token, got {:?}", other), + other => panic!("expected Token, got {other:?}"), } } @@ -293,7 +293,7 @@ mod tests { assert_eq!(e2.len(), 1); match &e2[0] { StreamEvent::Token(t) => assert_eq!(t, "partial"), - other => panic!("expected Token, got {:?}", other), + other => panic!("expected Token, got {other:?}"), } } @@ -329,7 +329,7 @@ mod tests { assert_eq!(name.as_deref(), Some("bash")); assert_eq!(arguments_delta, "{\"cmd\""); } - other => panic!("expected ToolCallDelta, got {:?}", other), + other => panic!("expected ToolCallDelta, got {other:?}"), } } @@ -346,7 +346,7 @@ mod tests { assert_eq!(*completion_tokens, 5); assert_eq!(*total_tokens, 15); } - other => panic!("expected Usage, got {:?}", other), + other => panic!("expected Usage, got {other:?}"), } } @@ -368,7 +368,7 @@ mod tests { assert_eq!(a, "a"); assert_eq!(b, "b"); } - other => panic!("expected two Tokens, got {:?}", other), + other => panic!("expected two Tokens, got {other:?}"), } } } diff --git a/src/app/subagent/engine.rs b/src/app/subagent/engine.rs index 0e1226a..85925d2 100644 --- a/src/app/subagent/engine.rs +++ b/src/app/subagent/engine.rs @@ -391,67 +391,62 @@ pub fn run_subagent(ctx: &SubagentContext, tx: &mpsc::Sender) -> // Push the assistant message with tool_calls into the conversation messages.push(response); - for tool_call in &tool_calls { - // Check abort flag before each tool execution - if ctx.abort_flag.as_ref().is_some_and(|f| f.load(std::sync::atomic::Ordering::SeqCst)) { - let _ = tx.blocking_send(SubagentEvent::StepFailed { - step, - error: "subagent aborted by parent during tool execution".to_string(), - }); - anyhow::bail!("subagent aborted by parent during tool call at step {step}"); - } + let mut results_vec = Vec::new(); + std::thread::scope(|s| { + let mut handles = Vec::new(); + let tools_ref = &tools; + let tool_ctx_ref = &tool_ctx; + for tool_call in &tool_calls { + let handle = s.spawn(move || { + // Check abort flag before each tool execution + if ctx.abort_flag.as_ref().is_some_and(|f| f.load(std::sync::atomic::Ordering::SeqCst)) { + return (tool_call, Err(anyhow::anyhow!("subagent aborted by parent during tool execution"))); + } + let tool_name = &tool_call.function.name; + let args = crate::dto::chat::tool::sanitize_tool_arguments(&tool_call.function.arguments); + let explicitly_allowed = ctx.allowed_tools.contains(tool_name); + let generally_allowed = ctx.allowed_tools.is_empty() || explicitly_allowed; + + // Level 1: allowlist check — is this tool even permitted? + if !generally_allowed { + return (tool_call, Ok(format!("tool '{tool_name}' not allowed for this subagent"))); + } + + // Level 2: risky tool check — risky tools require explicit permission + if tool_is_risky(tool_name) && !explicitly_allowed { + return (tool_call, Ok(format!("risky tool '{tool_name}' requires explicit permission; not allowed for this subagent"))); + } + + // Level 3: Harness-style content safety gating + if let Some(block_reason) = gate_subagent_tool_call(tool_name, &args) { + return (tool_call, Ok(format!("Blocked by subagent gate: {block_reason}"))); + } + + let result = match tools_ref.iter().find(|t| t.name() == tool_name.as_str()) { + Some(tool) => tool.run(tool_ctx_ref, &args), + None => Err(anyhow::anyhow!("tool '{tool_name}' not found")), + }; + (tool_call, result) + }); + handles.push(handle); + } + for h in handles { + if let Ok(res) = h.join() { + results_vec.push(res); + } + } + }); + + for (tool_call, result) in results_vec { let tool_name = &tool_call.function.name; let args = crate::dto::chat::tool::sanitize_tool_arguments(&tool_call.function.arguments); - let explicitly_allowed = ctx.allowed_tools.contains(tool_name); - let generally_allowed = ctx.allowed_tools.is_empty() || explicitly_allowed; let _ = tx.blocking_send(SubagentEvent::ToolCall { tool: tool_name.clone(), args: args.clone(), }); - // Level 1: allowlist check — is this tool even permitted? - if !generally_allowed { - let msg = format!("tool '{tool_name}' not allowed for this subagent"); - messages.push(ChatMessage::tool_result(tool_call.id.clone(), msg.clone())); - let _ = tx.blocking_send(SubagentEvent::ToolResult { - tool: tool_name.clone(), - output: msg, - }); - continue; - } - - // Level 2: risky tool check — risky tools require explicit permission - if tool_is_risky(tool_name) && !explicitly_allowed { - let msg = format!("risky tool '{tool_name}' requires explicit permission; not allowed for this subagent"); - messages.push(ChatMessage::tool_result(tool_call.id.clone(), msg.clone())); - let _ = tx.blocking_send(SubagentEvent::ToolResult { - tool: tool_name.clone(), - output: msg, - }); - continue; - } - - // Level 3: Harness-style content safety gating — mirrors the main - // agent's gate_tool_call checks (path traversal, reason validation, - // stub/denial/assumption scanning, bash exfiltration, destructive - // commands, sensitive path reads). - if let Some(block_reason) = gate_subagent_tool_call(tool_name, &args) { - let msg = format!("Blocked by subagent gate: {block_reason}"); - messages.push(ChatMessage::tool_result(tool_call.id.clone(), msg.clone())); - let _ = tx.blocking_send(SubagentEvent::ToolResult { - tool: tool_name.clone(), - output: msg, - }); - continue; - } - - let result = match tools.iter().find(|t| t.name() == tool_name.as_str()) { - Some(tool) => tool.run(&tool_ctx, &args), - None => Err(anyhow::anyhow!("tool '{tool_name}' not found")), - }; - match result { Ok(output_text) => { messages.push(ChatMessage::tool_result(tool_call.id.clone(), output_text.clone())); @@ -461,6 +456,14 @@ pub fn run_subagent(ctx: &SubagentContext, tx: &mpsc::Sender) -> }); } Err(e) => { + let err_str = e.to_string(); + if err_str.contains("subagent aborted by parent") { + let _ = tx.blocking_send(SubagentEvent::StepFailed { + step, + error: err_str.clone(), + }); + anyhow::bail!("{err_str}"); + } let msg = format!("tool '{tool_name}' failed: {e}"); messages.push(ChatMessage::tool_result(tool_call.id.clone(), msg.clone())); let _ = tx.blocking_send(SubagentEvent::ToolResult { diff --git a/src/app/workflow/company.rs b/src/app/workflow/company.rs index 68d3ec2..ba48811 100644 --- a/src/app/workflow/company.rs +++ b/src/app/workflow/company.rs @@ -93,13 +93,14 @@ fn make_division_phase( /// context to flow through the pipeline. /// /// Returns a consolidated executive summary string. +#[allow(clippy::ref_option)] pub fn run_company_pipeline( user_request: &str, session_dir: &std::path::Path, workspaces: &[std::path::PathBuf], turn_events: Option<&Arc>>>, abort_flag: &Option>, - custom_specialists: HashMap>, + custom_specialists: &HashMap>, ) -> anyhow::Result { let divisions = division::all_divisions(); @@ -179,20 +180,21 @@ pub fn run_company_pipeline( .map(|f| f.clone()) .unwrap_or_default(); - Ok(build_executive_summary(user_request, &results, &all_findings, &divisions, &custom_specialists)) + Ok(build_executive_summary(user_request, &results, &all_findings, &divisions, custom_specialists)) } /// Run a quick company pipeline that skips non-essential divisions /// for simple tasks. Flow: Strategy → Engineering → Quality. /// /// This is for smaller tasks where security audit and full docs are overkill. +#[allow(clippy::ref_option)] pub fn run_company_pipeline_quick( user_request: &str, session_dir: &std::path::Path, workspaces: &[std::path::PathBuf], turn_events: Option<&Arc>>>, abort_flag: &Option>, - custom_specialists: HashMap>, + custom_specialists: &HashMap>, ) -> anyhow::Result { let divisions = division::all_divisions(); let quick_divisions = &divisions[..3]; @@ -252,7 +254,7 @@ pub fn run_company_pipeline_quick( .map(|f| f.clone()) .unwrap_or_default(); - Ok(build_executive_summary(user_request, &results, &all_findings, quick_divisions, &custom_specialists)) + Ok(build_executive_summary(user_request, &results, &all_findings, quick_divisions, custom_specialists)) } /// Build a compressed executive summary from pipeline results. @@ -277,8 +279,7 @@ fn build_executive_summary( let mut start_index = 0; for div in divisions { let count = custom_specialists.get(div.name) - .map(Vec::len) - .unwrap_or(0); + .map_or(0, Vec::len); let mut division_verdicts = Vec::new(); for offset in 0..count { @@ -393,7 +394,7 @@ mod tests { ("Custom Label".to_string(), "Custom Focus Description".to_string()) ] ); - let specs = make_division_specialists(div, "Test Request", &custom.get("Strategy").unwrap()); + let specs = make_division_specialists(div, "Test Request", custom.get("Strategy").unwrap()); assert_eq!(specs.len(), 1); if let ScriptPrimitive::Agent(prompt) = &specs[0] { assert!(prompt.contains("Custom Label")); diff --git a/src/app/workflow/engine.rs b/src/app/workflow/engine.rs index 7c86fc8..822663c 100644 --- a/src/app/workflow/engine.rs +++ b/src/app/workflow/engine.rs @@ -98,7 +98,7 @@ pub type LiveStateFn = Arc; /// a stuck stage from blocking the entire pipeline forever. /// /// Return: the agent's text output, or an error on failure. -#[allow(clippy::too_many_lines, clippy::too_many_arguments)] +#[allow(clippy::too_many_lines, clippy::too_many_arguments, clippy::ref_option)] fn spawn_single_agent( agent_id: &str, agent_name: &str, @@ -185,7 +185,7 @@ fn spawn_single_agent( // Link the shared findings Arc so note_finding calls within this // subagent write into the same vec visible to sibling agents. ctx.workflow_findings = Some(findings.clone()); - ctx.abort_flag = abort_flag.clone(); + ctx.abort_flag.clone_from(abort_flag); // Create an mpsc channel and drain events in a background thread. // The drain thread also pushes intra-division progress updates to the @@ -269,34 +269,30 @@ fn spawn_single_agent( let deadline = Duration::from_millis(timeout); let mut elapsed = Duration::ZERO; loop { - match done_rx.recv_timeout(poll_interval) { - Ok(r) => break r, - Err(_) => { - elapsed += poll_interval; - if elapsed >= deadline { - break Err(anyhow::anyhow!( - "subagent '{bg_name}' timed out after {timeout}ms", - )); - } - if bg_abort.as_ref().is_some_and(|f| f.load(Ordering::SeqCst)) { - break Err(anyhow::anyhow!( - "subagent '{bg_name}' aborted by user", - )); - } - } + if let Ok(r) = done_rx.recv_timeout(poll_interval) { + break r; + } + elapsed += poll_interval; + if elapsed >= deadline { + break Err(anyhow::anyhow!( + "subagent '{bg_name}' timed out after {timeout}ms", + )); + } + if bg_abort.as_ref().is_some_and(|f| f.load(Ordering::SeqCst)) { + break Err(anyhow::anyhow!( + "subagent '{bg_name}' aborted by user", + )); } } } else { loop { - match done_rx.recv_timeout(poll_interval) { - Ok(r) => break r, - Err(_) => { - if bg_abort.as_ref().is_some_and(|f| f.load(Ordering::SeqCst)) { - break Err(anyhow::anyhow!( - "subagent '{bg_name}' aborted by user", - )); - } - } + if let Ok(r) = done_rx.recv_timeout(poll_interval) { + break r; + } + if bg_abort.as_ref().is_some_and(|f| f.load(Ordering::SeqCst)) { + break Err(anyhow::anyhow!( + "subagent '{bg_name}' aborted by user", + )); } } }; @@ -358,6 +354,7 @@ type ParallelResult = (usize, anyhow::Result>); /// Return: a `Vec` of all agent outputs (or error strings) in /// the order they were submitted. #[allow(clippy::too_many_arguments)] +#[allow(clippy::ref_option, clippy::too_many_lines)] pub fn execute_primitive( primitive: &ScriptPrimitive, args: &HashMap, @@ -529,6 +526,7 @@ pub fn run_workflow( /// `spawn_agents` invocations remain fully isolated. /// /// Return: a human-readable summary string. +#[allow(clippy::ref_option)] pub fn run_workflow_tracked( script: &WorkflowScript, args: &HashMap, diff --git a/src/ipc/frame.rs b/src/ipc/frame.rs index c1822d9..3b02507 100644 --- a/src/ipc/frame.rs +++ b/src/ipc/frame.rs @@ -73,3 +73,123 @@ pub fn serialize_frame(value: &T) -> Result> { pub fn deserialize_frame<'a, T: serde::Deserialize<'a>>(data: &'a [u8]) -> Result { Ok(serde_json::from_slice(data)?) } + +#[cfg(test)] +mod tests { + use super::*; + + /// Write a value, read it back, and verify exact equality. + fn roundtrip_bytes(data: &[u8]) { + let mut buf: Vec = Vec::new(); + write_frame(&mut buf, data).unwrap(); + let read_back = read_frame(&mut buf.as_slice()) + .unwrap() + .expect("expected Some(frame)"); + assert_eq!(read_back, data); + } + + #[test] + fn test_write_read_roundtrip_empty() { + roundtrip_bytes(b""); + } + + #[test] + fn test_write_read_roundtrip_small_text() { + roundtrip_bytes(b"hello world"); + } + + #[test] + fn test_write_read_roundtrip_binary() { + roundtrip_bytes(&[0x00, 0xFF, 0xAB, 0xCD, 0x01, 0x02, 0x03]); + } + + #[test] + fn test_write_read_roundtrip_large() { + let data = vec![0x42u8; 100_000]; + roundtrip_bytes(&data); + } + + #[test] + fn test_write_rejects_too_large_frame() { + let oversized = vec![0u8; MAX_FRAME_SIZE + 1]; + let mut buf = Vec::new(); + let result = write_frame(&mut buf, &oversized); + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("too large") || err.contains("64 MiB")); + } + + #[test] + fn test_read_rejects_too_large_header() { + // Manually craft a 4-byte length header that exceeds MAX_FRAME_SIZE + let len = (MAX_FRAME_SIZE as u32).wrapping_add(1); + let header = len.to_be_bytes(); + let mut buf = Vec::from(&header[..]); + buf.extend_from_slice(b"dummy"); + let result = read_frame(&mut buf.as_slice()); + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("too large")); + } + + #[test] + fn test_read_empty_buf_returns_none() { + let empty: &[u8] = &[]; + let result = read_frame(&mut &empty[..]).unwrap(); + assert!(result.is_none(), "expected None for empty reader"); + } + + #[test] + fn test_read_partial_header_returns_none() { + // Only 2 bytes of the 4-byte header → EOF + let partial: &[u8] = &[0x00, 0x01]; + let result = read_frame(&mut &partial[..]).unwrap(); + assert!(result.is_none(), "expected None for partial header"); + } + + #[test] + fn test_read_truncated_payload_returns_err() { + let mut buf = Vec::new(); + let header = (10u32).to_be_bytes(); + buf.extend_from_slice(&header); + buf.extend_from_slice(b"abc"); // only 3 of 10 bytes + let result = read_frame(&mut buf.as_slice()); + assert!(result.is_err(), "truncated payload should error"); + } + + #[test] + fn test_serialize_deserialize_roundtrip() { + use serde::{Deserialize, Serialize}; + + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] + struct Msg { + id: u32, + content: String, + tags: Vec, + } + + let original = Msg { + id: 42, + content: "hello world".into(), + tags: vec!["foo".into(), "bar".into()], + }; + + let bytes = serialize_frame(&original).unwrap(); + let deserialized: Msg = deserialize_frame(&bytes).unwrap(); + assert_eq!(original, deserialized); + } + + #[test] + fn test_serialize_rejects_oversized_value() { + let huge = vec![0u8; MAX_FRAME_SIZE + 1]; + let result = serialize_frame(&huge); + assert!(result.is_err()); + } + + #[test] + fn test_deserialize_malformed_json_errors() { + let bad_json = b"this is not json"; + let result: Result = deserialize_frame(bad_json); + assert!(result.is_err()); + } +} diff --git a/src/model/editlog.rs b/src/model/editlog.rs index 76d131f..a74406a 100644 --- a/src/model/editlog.rs +++ b/src/model/editlog.rs @@ -145,8 +145,8 @@ mod tests { log.append(EditLogEntry { ts: i, tool: "edit".to_string(), - path: format!("file{}.txt", i), - reason: format!("reason {}", i), + path: format!("file{i}.txt"), + reason: format!("reason {i}"), content_sha256: "hash".to_string(), bytes_delta: 10 + i, origin: "main".to_string(), diff --git a/src/model/memory.rs b/src/model/memory.rs index 32dd188..692d12c 100644 --- a/src/model/memory.rs +++ b/src/model/memory.rs @@ -375,7 +375,7 @@ mod tests { }; mem.write(&dir).unwrap(); let names = Memory::list(&dir); - assert!(names.contains(&"alpha".to_string()), "list should contain 'alpha', got: {:?}", names); + assert!(names.contains(&"alpha".to_string()), "list should contain 'alpha', got: {names:?}"); let _ = std::fs::remove_dir_all(&dir); } diff --git a/src/tool/mod.rs b/src/tool/mod.rs index 38c2f3d..b9a60c9 100644 --- a/src/tool/mod.rs +++ b/src/tool/mod.rs @@ -141,7 +141,7 @@ impl ToolCtxBuilder { /// Construct one instance of every built-in tool, in the fixed order exposed to the LLM. /// -/// Return: boxed trait objects for all 28 tools (fs, search, bash, git, memory, plan, +/// Return: boxed trait objects for all 37 tools (fs, search, bash, git, memory, plan, /// workflow, utility). pub fn all_tools() -> Vec> { vec![ diff --git a/src/tool/workflow.rs b/src/tool/workflow.rs index 6f8bba0..fa2b500 100644 --- a/src/tool/workflow.rs +++ b/src/tool/workflow.rs @@ -206,7 +206,7 @@ impl Tool for CompanyPipeline { let specs = v.as_array().map(|arr| { arr.iter().filter_map(|item| { let pair = item.as_array()?; - let label = pair.get(0)?.as_str()?.to_string(); + let label = pair.first()?.as_str()?.to_string(); let focus = pair.get(1)?.as_str()?.to_string(); Some((label, focus)) }).collect() @@ -225,7 +225,7 @@ impl Tool for CompanyPipeline { &ctx.workspaces, ctx.turn_events.as_ref(), &no_abort, - custom_specialists, + &custom_specialists, ) } _ => { @@ -235,7 +235,7 @@ impl Tool for CompanyPipeline { &ctx.workspaces, ctx.turn_events.as_ref(), &no_abort, - custom_specialists, + &custom_specialists, ) } } @@ -273,7 +273,7 @@ impl Tool for ReadFindings { .map(|(i, f)| format!("{}. {}", i + 1, f)) .collect::>() .join("\n"); - Ok(format!("Findings in this workflow run:\n{}", formatted)) + Ok(format!("Findings in this workflow run:\n{formatted}")) } } else { Ok("No findings database available (called outside a workflow run).".to_string())