diff --git a/CONFIG.md b/CONFIG.md index 5ed2a81b..27dde2c7 100644 --- a/CONFIG.md +++ b/CONFIG.md @@ -3,13 +3,13 @@ dirge reads an optional JSON config file named `config.json` from its config folder: -- If `ZS_CONFIG_DIR` is set: `$ZS_CONFIG_DIR/config.json` +- If `DIRGE_CONFIG_DIR` is set: `$DIRGE_CONFIG_DIR/config.json` - Otherwise: the platform config directory joined with `dirge/config.json` (for example `$XDG_CONFIG_HOME/dirge/config.json` on Linux) - Fallback: `$HOME/.config/dirge/config.json` All config keys are optional. CLI flags and their environment-backed values -(such as `ZS_PROVIDER` and `ZS_MODEL`) take precedence where both exist. +(such as `DIRGE_PROVIDER` and `DIRGE_MODEL`) take precedence where both exist. Example: @@ -59,7 +59,7 @@ Accepted top-level keys: | Key | Type | Description | | ------------------------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `provider` | string | Provider name. Built-ins are `openrouter`, `openai`, `anthropic`, `gemini`/`google`, and `ollama`; custom provider aliases are also accepted. Default: `openrouter`. | +| `provider` | string | Provider name. Built-ins are `openrouter`, `openai`, `anthropic`, `gemini`/`google`, `deepseek`, `glm`/`zhipu`, and `ollama`; custom provider aliases are also accepted. Default: `openrouter`. | | `model` | string | Model name. Default: `deepseek/deepseek-v4-flash`. | | `max_tokens` | integer | Maximum response tokens. Default: `8192`. | | `max_agent_turns` | integer | Maximum agent turns per response. Default: `100`. | diff --git a/README.md b/README.md index 23d1527c..eb54ed40 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ Minimal coding agent written in Rust, inspired by [pi](https://pi.dev/docs/lates ## Features - **Multi-provider**: OpenRouter, OpenAI, Anthropic, Gemini, DeepSeek, GLM, Ollama, plus custom providers -- **Standard tools**: read, write, edit, bash, grep, find_files, list_dir, write_todo_list +- **Standard tools**: read, write, edit, bash, grep, find_files, glob, list_dir, write_todo_list, apply_patch - **Line-numbered read output**: `read` tool prefixes each line with right-aligned line numbers (`123: content`) - **Environment-aware**: system prompt includes OS, shell, working directory, and git branch for context - **Semantic code tools** (tree-sitter): list_symbols, get_symbol_body, find_definition, find_callers, find_callees — supports TypeScript/TSX and Python @@ -36,7 +36,7 @@ _dirge_ is one of the smallest and most performant coding agents on the market. ### Tool result caching -Read-only tool calls (`read`, `grep`, `find_files`, `list_dir`) are cached per agent turn. Repeated calls with identical arguments within the same turn return cached results, avoiding redundant filesystem I/O. The cache clears automatically before each new prompt, and after `write`/`edit`/`bash` so a re-read sees fresh content. +Most tool calls (`read`, `write`, `edit`, `bash`, `grep`, `find_files`, `list_dir`) are cached per agent turn. Repeated calls with identical arguments within the same turn return cached results, avoiding redundant filesystem I/O. The cache clears automatically before each new prompt, and after `write`/`edit`/`bash` so a re-read sees fresh content. ### Error recovery @@ -107,11 +107,16 @@ dirge --provider glm # defaults to glm-4 | `/mode [mode]` | Set security mode (`standard`, `restrictive`, `accept`, `yolo`) | | `/reasoning` | Toggle reasoning visibility | | `/btw ` | Ask a quick question (no tools, doesn't affect session) | -| `/session` | List/save/load sessions | +| `/sessions` | List/save/load sessions | | `/loop [prompt]` | Start iterative coding loop | | `/worktree ` | Create a git worktree on branch | | `/wt-merge [branch]` | Merge worktree branch | | `/wt-exit` | Exit worktree | +| `/toggle` | Toggle features on/off (currently todo tools) | +| `/regen-prompts` | Restore built-in prompts | +| `/mcp` | List MCP servers and tools | +| `/quit` | Exit dirge | +| `/retry` | Retry last prompt | | `/help` | Show all commands | ### Key bindings @@ -179,10 +184,11 @@ Built-in prompts that change the agent's behavior and tone: | **`review-security`** | Security review mode — finds exploitable vulnerabilities | | **`simplify`** | Code simplification mode — refines for clarity without changing behavior | | **`write-prompt`** | Prompt writing mode — creates and optimizes agent prompts | +| **`default`** | Default system prompt — the base built-in prompt | Custom prompts can be placed in `$XDG_CONFIG_HOME/dirge/prompts/` as `.md` files. -The agent automatically loads `AGENTS.md` or `CLAUDE.md` from the project root or ancestor directories. Use `-n` / `--no-context-files` to disable. +The agent automatically loads `AGENTS.md` or `CLAUDE.md` from the project root, ancestor directories, and `~/.config/dirge/agent/AGENTS.md` as a global fallback. Use `-n` / `--no-context-files` to disable. ## Claude-compatible skills diff --git a/src/agent/builder.rs b/src/agent/builder.rs index 40b463d4..5ad7d3f2 100644 --- a/src/agent/builder.rs +++ b/src/agent/builder.rs @@ -109,25 +109,11 @@ pub async fn build_agent_inner( // Inject mode-specific reminders if let Some(prompt_name) = &context.current_prompt_name { - match prompt_name.as_str() { - "plan" => { - preamble.push_str("\n\n---\n\nYou are now in PLAN mode. Create a detailed implementation plan. Save it to PLAN.md in the current directory. Analyze the task, break it into concrete steps, consider edge cases and trade-offs. Do NOT write any code or run any commands until the user reviews and approves the plan."); - } - "review" | "review-security" => { - preamble.push_str("\n\n---\n\nYou are now in REVIEW mode. Review the code or plan carefully. Identify bugs, security issues, performance problems, and design flaws. Be thorough and specific. Provide actionable feedback."); - } - "code" => { - let plan_path = std::env::current_dir() - .unwrap_or_else(|_| ".".into()) - .join("PLAN.md"); - if plan_path.exists() { - preamble.push_str( - "\n\n---\n\nA plan file exists at PLAN.md. Execute the plan step by step. Write and test code following the plan. Report progress after each step. The plan is your guide — follow it closely." - ); - } - } - _ => {} - } + let plan_exists = std::env::current_dir() + .unwrap_or_else(|_| ".".into()) + .join("PLAN.md") + .exists(); + append_mode_reminder(&mut preamble, prompt_name, plan_exists); } let mut builder = AgentBuilder::new(model).preamble(&preamble); @@ -314,3 +300,80 @@ pub fn create_client(api_key: Option<&str>) -> anyhow::Result { + preamble.push_str("\n\n---\n\nYou are now in PLAN mode. Create a detailed implementation plan. Save it to PLAN.md in the current directory. Analyze the task, break it into concrete steps, consider edge cases and trade-offs. Do NOT write any code or run any commands until the user reviews and approves the plan."); + } + "review" | "review-security" => { + preamble.push_str("\n\n---\n\nYou are now in REVIEW mode. Review the code or plan carefully. Identify bugs, security issues, performance problems, and design flaws. Be thorough and specific. Provide actionable feedback."); + } + "code" if plan_exists => { + preamble.push_str( + "\n\n---\n\nA plan file exists at PLAN.md. Execute the plan step by step. Write and test code following the plan. Report progress after each step. The plan is your guide — follow it closely.", + ); + } + _ => {} + } +} + +#[cfg(test)] +mod reminder_tests { + use super::append_mode_reminder; + + #[test] + fn plan_mode_injects_plan_reminder() { + let mut p = String::from("base"); + append_mode_reminder(&mut p, "plan", false); + assert!(p.contains("PLAN mode")); + assert!(p.contains("PLAN.md")); + assert!(p.contains("Do NOT write any code")); + } + + #[test] + fn review_modes_inject_review_reminder() { + for mode in &["review", "review-security"] { + let mut p = String::from("base"); + append_mode_reminder(&mut p, mode, false); + assert!(p.contains("REVIEW mode"), "mode={mode}"); + assert!(p.contains("Identify bugs"), "mode={mode}"); + } + } + + // Regression: the `code` reminder must only appear when PLAN.md exists. + // Without that guard every code-mode session would have a stale "execute + // the plan" instruction even with no plan written. + #[test] + fn regression_code_mode_reminder_requires_plan_md() { + let mut p_with = String::from("base"); + append_mode_reminder(&mut p_with, "code", true); + assert!(p_with.contains("plan file exists")); + + let mut p_without = String::from("base"); + append_mode_reminder(&mut p_without, "code", false); + assert_eq!(p_without, "base", "no reminder must be added"); + } + + // Unknown prompts (custom user prompts) must produce no reminder so the + // plan/review semantics don't bleed into other modes. + #[test] + fn unknown_prompt_name_appends_nothing() { + let mut p = String::from("base"); + append_mode_reminder(&mut p, "my-custom-prompt", true); + assert_eq!(p, "base"); + } + + // Each reminder is prefixed by the section separator so it visually + // detaches from the prior prompt — regression-guards the leading "\n\n---". + #[test] + fn reminders_use_section_separator() { + let mut p = String::new(); + append_mode_reminder(&mut p, "plan", false); + assert!(p.starts_with("\n\n---\n\n"), "got: {p:?}"); + } +} diff --git a/src/agent/tools/apply_patch.rs b/src/agent/tools/apply_patch.rs index da977930..1d7e9ad2 100644 --- a/src/agent/tools/apply_patch.rs +++ b/src/agent/tools/apply_patch.rs @@ -294,4 +294,221 @@ mod tests { let def = tool.definition(String::new()).await; assert_eq!(def.name, "apply_patch"); } + + // Regression: update is documented as text-find-and-replace and must reject + // ambiguous matches rather than silently replacing the first one. Without + // this guard the agent could clobber wrong code in a file with repeated + // boilerplate (use statements, similar function bodies, etc.). + #[test] + fn regression_update_rejects_multiple_matches() { + let tf = TestFile::new("update-ambiguous.txt"); + std::fs::write(&tf.path, "foo bar foo baz foo").unwrap(); + let result = apply_update(&tf.path, "foo", "qux"); + assert!(result.is_err()); + let msg = result.unwrap_err(); + assert!(msg.contains("3 locations"), "got: {msg}"); + // File should be untouched. + assert_eq!( + std::fs::read_to_string(&tf.path).unwrap(), + "foo bar foo baz foo" + ); + } + + // Regression: prior to the fix, multi-op patches were documented as + // "atomic" but in fact left earlier successful ops applied when a later op + // failed. We now stop on first failure AND the prior ops MUST stay applied + // (no rollback). The error report must explicitly call out which op failed + // and ops after the failure must NOT execute. + #[tokio::test] + async fn regression_multi_op_stops_on_failure_prior_ops_remain() { + let a = TestFile::new("multi-op-a.txt"); + let b_existing = TestFile::new("multi-op-b.txt"); + let c_should_not_exist = TestFile::new("multi-op-c.txt"); + + // Pre-create B so the second op (create B) fails. + std::fs::write(&b_existing.path, "already here").unwrap(); + + let tool = ApplyPatchTool::new(None, None); + let result = tool + .call(ApplyPatchArgs { + operations: vec![ + PatchOp::Create { + path: a.path.clone(), + content: "A content".into(), + }, + PatchOp::Create { + path: b_existing.path.clone(), + content: "B content".into(), + }, + PatchOp::Create { + path: c_should_not_exist.path.clone(), + content: "C content".into(), + }, + ], + }) + .await + .unwrap(); + + // A was created. + assert!(Path::new(&a.path).exists(), "A must remain applied"); + assert_eq!(std::fs::read_to_string(&a.path).unwrap(), "A content"); + // B was not overwritten. + assert_eq!( + std::fs::read_to_string(&b_existing.path).unwrap(), + "already here" + ); + // C was never attempted. + assert!( + !Path::new(&c_should_not_exist.path).exists(), + "C must not run after failure" + ); + // Report names both the success and the failure. + assert!(result.contains("created"), "got: {result}"); + assert!(result.contains("FAILED"), "got: {result}"); + } + + // Regression: create previously had no size cap; the agent could write + // multi-GB files by accident. 1MB limit must be enforced before touching + // the filesystem, and the operation must not produce a partial write. + #[tokio::test] + async fn regression_create_rejects_oversized_content() { + let tf = TestFile::new("oversize.txt"); + let too_big = "x".repeat(1_048_577); // 1MB + 1 byte + + let tool = ApplyPatchTool::new(None, None); + let result = tool + .call(ApplyPatchArgs { + operations: vec![PatchOp::Create { + path: tf.path.clone(), + content: too_big, + }], + }) + .await + .unwrap(); + + assert!(result.contains("FAILED"), "got: {result}"); + assert!(result.contains("exceeds"), "got: {result}"); + assert!( + !Path::new(&tf.path).exists(), + "no file should exist after size-limit rejection" + ); + } + + // Right at the limit must succeed; off-by-one boundary check. + #[tokio::test] + async fn create_accepts_content_at_size_limit() { + let tf = TestFile::new("at-limit.txt"); + let at_limit = "x".repeat(1_048_576); // exactly 1MB + + let tool = ApplyPatchTool::new(None, None); + let result = tool + .call(ApplyPatchArgs { + operations: vec![PatchOp::Create { + path: tf.path.clone(), + content: at_limit, + }], + }) + .await + .unwrap(); + + assert!(!result.contains("FAILED"), "got: {result}"); + assert!(Path::new(&tf.path).exists()); + assert_eq!(std::fs::metadata(&tf.path).unwrap().len(), 1_048_576); + } + + // create_dir_all is called on the parent — confirms nested-path creates work. + #[test] + fn create_creates_parent_dirs() { + let dir = std::env::temp_dir().join(format!("dirge-test-nested-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let nested = dir.join("a/b/c/file.txt"); + let path_str = nested.to_str().unwrap(); + + let result = apply_create(path_str, "deep content"); + assert!(result.is_ok()); + assert_eq!(std::fs::read_to_string(&nested).unwrap(), "deep content"); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn delete_missing_file_returns_err() { + let path = format!("/tmp/dirge-test-delete-ghost-{}.txt", std::process::id()); + let _ = std::fs::remove_file(&path); + let result = apply_delete(&path); + assert!(result.is_err()); + } + + // Multi-op happy path: create + update + rename + delete in sequence, + // touching different files. Regression-tests that the loop applies each op + // in declaration order and the report lists each. + #[tokio::test] + async fn multi_op_happy_path_executes_in_order() { + let a = TestFile::new("multi-happy-a.txt"); + let b = TestFile::new("multi-happy-b.txt"); + let renamed = format!( + "/tmp/dirge-test-multi-happy-renamed-{}.txt", + std::process::id() + ); + let _ = std::fs::remove_file(&renamed); + + let tool = ApplyPatchTool::new(None, None); + let result = tool + .call(ApplyPatchArgs { + operations: vec![ + PatchOp::Create { + path: a.path.clone(), + content: "hello".into(), + }, + PatchOp::Update { + path: a.path.clone(), + old_text: "hello".into(), + new_text: "HELLO".into(), + }, + PatchOp::Create { + path: b.path.clone(), + content: "scratch".into(), + }, + PatchOp::Rename { + path: a.path.clone(), + new_path: renamed.clone(), + }, + PatchOp::Delete { + path: b.path.clone(), + }, + ], + }) + .await + .unwrap(); + + assert!(!result.contains("FAILED"), "got: {result}"); + assert!(!Path::new(&a.path).exists()); // renamed away + assert!(!Path::new(&b.path).exists()); // deleted + assert_eq!(std::fs::read_to_string(&renamed).unwrap(), "HELLO"); + let _ = std::fs::remove_file(&renamed); + + // Each successful op contributes a line to the report. + assert_eq!( + result.lines().filter(|l| !l.is_empty()).count(), + 5, + "report: {result}" + ); + } + + // Regression: PatchOp deserializes via internally-tagged `action` enum. + // Schema mismatch (e.g. missing `content` for create) must fail at deserialize. + #[test] + fn patch_op_deserializes_each_variant() { + let json = serde_json::json!([ + {"action": "create", "path": "/tmp/x", "content": "hi"}, + {"action": "update", "path": "/tmp/x", "old_text": "a", "new_text": "b"}, + {"action": "delete", "path": "/tmp/x"}, + {"action": "rename", "path": "/tmp/x", "new_path": "/tmp/y"}, + ]); + let ops: Vec = serde_json::from_value(json).unwrap(); + assert!(matches!(ops[0], PatchOp::Create { .. })); + assert!(matches!(ops[1], PatchOp::Update { .. })); + assert!(matches!(ops[2], PatchOp::Delete { .. })); + assert!(matches!(ops[3], PatchOp::Rename { .. })); + } } diff --git a/src/agent/tools/background.rs b/src/agent/tools/background.rs index ac0ac8fa..150ba86f 100644 --- a/src/agent/tools/background.rs +++ b/src/agent/tools/background.rs @@ -66,4 +66,160 @@ impl BackgroundStore { task.state = truncated; } } + + #[cfg(test)] + fn len(&self) -> usize { + self.0.lock().unwrap_or_else(|e| e.into_inner()).len() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn insert_then_get_returns_running() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + let task = store.get("t1").expect("task present"); + assert!(matches!(task.state, TaskState::Running)); + } + + #[test] + fn get_on_missing_returns_none() { + let store = BackgroundStore::new(); + assert!(store.get("nope").is_none()); + } + + // Regression: previously the store grew unbounded across a session because + // completed/failed tasks were never removed. get() now evicts on read. + #[test] + fn regression_get_removes_completed_task() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + store.update("t1", TaskState::Completed("done".into())); + assert_eq!(store.len(), 1); + + let first = store.get("t1"); + assert!(matches!( + first.unwrap().state, + TaskState::Completed(ref s) if s == "done" + )); + + // The first read evicts. + assert_eq!(store.len(), 0); + assert!(store.get("t1").is_none()); + } + + #[test] + fn regression_get_removes_failed_task() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + store.update("t1", TaskState::Failed("boom".into())); + + let first = store.get("t1"); + assert!(matches!( + first.unwrap().state, + TaskState::Failed(ref s) if s == "boom" + )); + assert_eq!(store.len(), 0); + } + + // Regression: get() must NOT evict tasks still running, otherwise polling + // wait=true would lose the task before completion. + #[test] + fn regression_get_keeps_running_task() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + + for _ in 0..5 { + let task = store.get("t1").expect("still present while running"); + assert!(matches!(task.state, TaskState::Running)); + } + assert_eq!(store.len(), 1); + } + + // Regression: subagent output was injected verbatim into parent context + // and blew the window. update() truncates to MAX_TASK_OUTPUT_CHARS. + #[test] + fn regression_update_truncates_completed_text() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + let huge = "x".repeat(MAX_TASK_OUTPUT_CHARS * 2); + store.update("t1", TaskState::Completed(huge)); + + let task = store.get("t1").unwrap(); + let TaskState::Completed(text) = task.state else { + panic!("expected Completed"); + }; + assert_eq!(text.chars().count(), MAX_TASK_OUTPUT_CHARS); + } + + #[test] + fn regression_update_truncates_failed_error() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + let huge = "e".repeat(MAX_TASK_OUTPUT_CHARS * 2); + store.update("t1", TaskState::Failed(huge)); + + let task = store.get("t1").unwrap(); + let TaskState::Failed(text) = task.state else { + panic!("expected Failed"); + }; + assert_eq!(text.chars().count(), MAX_TASK_OUTPUT_CHARS); + } + + #[test] + fn update_leaves_short_text_intact() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + store.update("t1", TaskState::Completed("hello".into())); + let TaskState::Completed(text) = store.get("t1").unwrap().state else { + panic!("expected Completed"); + }; + assert_eq!(text, "hello"); + } + + // Truncation uses chars().take(), so multibyte characters count as one each + // — guards against accidentally switching to bytes-based truncation that + // would split UTF-8 sequences. + #[test] + fn update_truncates_by_chars_not_bytes() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + // Each emoji is 4 bytes; producing MAX*2 chars = MAX*8 bytes. + let emojis = "🦀".repeat(MAX_TASK_OUTPUT_CHARS * 2); + store.update("t1", TaskState::Completed(emojis)); + let TaskState::Completed(text) = store.get("t1").unwrap().state else { + panic!("expected Completed"); + }; + assert_eq!(text.chars().count(), MAX_TASK_OUTPUT_CHARS); + // Verify no broken UTF-8: re-encoding round-trips. + assert_eq!(text.as_str(), &"🦀".repeat(MAX_TASK_OUTPUT_CHARS)); + } + + #[test] + fn update_on_missing_is_noop() { + let store = BackgroundStore::new(); + store.update("ghost", TaskState::Completed("never inserted".into())); + assert!(store.get("ghost").is_none()); + assert_eq!(store.len(), 0); + } + + // The store is Clone + thread-safe (Arc>). Clones must see each + // other's writes — guards against accidentally cloning the inner HashMap. + #[test] + fn clones_share_state() { + let a = BackgroundStore::new(); + let b = a.clone(); + + a.insert("t1".into()); + assert!(b.get("t1").is_some()); + // get() on `b` evicted via the shared mutex. + assert_eq!(a.len(), 1); // still running, still there + + b.update("t1", TaskState::Completed("via clone b".into())); + let from_a = a.get("t1").unwrap(); + assert!(matches!(from_a.state, TaskState::Completed(_))); + } } diff --git a/src/agent/tools/glob.rs b/src/agent/tools/glob.rs index 37308364..b312386b 100644 --- a/src/agent/tools/glob.rs +++ b/src/agent/tools/glob.rs @@ -204,4 +204,159 @@ mod tests { let def = tool.definition(String::new()).await; assert_eq!(def.name, "glob"); } + + // ---- Integration tests against a real temp directory ---- + + struct TempTree { + root: std::path::PathBuf, + } + + impl TempTree { + fn new(suffix: &str) -> Self { + let root = std::env::temp_dir().join(format!( + "dirge-glob-test-{}-{}", + std::process::id(), + suffix + )); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + Self { root } + } + + fn write(&self, rel: &str, content: &str) -> std::path::PathBuf { + let p = self.root.join(rel); + if let Some(parent) = p.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&p, content).unwrap(); + p + } + + fn root_str(&self) -> String { + self.root.to_string_lossy().into_owned() + } + } + + impl Drop for TempTree { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.root); + } + } + + #[tokio::test] + async fn glob_walks_files_under_path() { + let tree = TempTree::new("walks"); + tree.write("a.rs", ""); + tree.write("b.rs", ""); + tree.write("c.py", ""); + tree.write("sub/d.rs", ""); + + let tool = GlobTool::new(None, None); + let out = tool + .call(GlobArgs { + pattern: "**/*.rs".into(), + path: Some(tree.root_str()), + }) + .await + .unwrap(); + + let lines: Vec<&str> = out.lines().collect(); + assert_eq!(lines.len(), 3, "got: {out}"); + for f in ["a.rs", "b.rs", "sub/d.rs"] { + assert!(lines.contains(&f), "expected {f} in: {out}"); + } + // .py file should be excluded. + assert!(!out.contains("c.py")); + } + + // Regression: previously returned the literal string "no files matched", + // which the agent had to special-case. Now returns "" so the response is + // cleanly empty. + #[tokio::test] + async fn regression_empty_result_returns_empty_string() { + let tree = TempTree::new("empty"); + tree.write("a.txt", ""); + + let tool = GlobTool::new(None, None); + let out = tool + .call(GlobArgs { + pattern: "**/*.nonexistent".into(), + path: Some(tree.root_str()), + }) + .await + .unwrap(); + assert_eq!(out, ""); + } + + // Regression: mtime sort previously called metadata() on the relative path, + // looked up against CWD instead of the explicit root. When `path` was not + // CWD, all metadata calls failed silently and sort degraded to alphabetical. + // Now we keep absolute paths for metadata lookups. + #[tokio::test] + async fn regression_mtime_sort_with_explicit_path() { + let tree = TempTree::new("mtime"); + tree.write("oldest.rs", ""); + // Walk-clock-time is sufficient on every platform we run. + std::thread::sleep(std::time::Duration::from_millis(20)); + tree.write("middle.rs", ""); + std::thread::sleep(std::time::Duration::from_millis(20)); + tree.write("newest.rs", ""); + + let tool = GlobTool::new(None, None); + let out = tool + .call(GlobArgs { + pattern: "*.rs".into(), + path: Some(tree.root_str()), + }) + .await + .unwrap(); + + let lines: Vec<&str> = out.lines().collect(); + assert_eq!(lines, vec!["newest.rs", "middle.rs", "oldest.rs"]); + } + + // Regression: `ignore::WalkBuilder` is configured with `git_ignore(true)` + // so .gitignore'd files are skipped without needing an explicit deny list. + #[tokio::test] + async fn regression_respects_gitignore() { + let tree = TempTree::new("gitignore"); + // .gitignore is only honored inside a git repo, so we make one. + std::fs::create_dir_all(tree.root.join(".git")).unwrap(); + tree.write(".gitignore", "ignored.rs\n"); + tree.write("kept.rs", ""); + tree.write("ignored.rs", ""); + + let tool = GlobTool::new(None, None); + let out = tool + .call(GlobArgs { + pattern: "*.rs".into(), + path: Some(tree.root_str()), + }) + .await + .unwrap(); + + assert!(out.contains("kept.rs")); + assert!(!out.contains("ignored.rs"), "got: {out}"); + } + + // The glob→regex conversion escapes regex metachars so they can't be + // interpreted as regex. Without this, an agent passing `file.rs` would + // unexpectedly match `fileXrs` because `.` is regex metasyntax. + #[test] + fn glob_escapes_regex_metacharacters() { + let re = glob_to_regex("file.rs").unwrap(); + assert!(re.is_match("file.rs")); + assert!(!re.is_match("fileXrs")); + assert!(!re.is_match("filers")); + } + + // `*` is intentionally bounded to a single path segment — `*` does NOT + // descend into subdirs (only `**` does). Regression-guard against + // accidentally swapping `[^/]*` for `.*`. + #[test] + fn star_does_not_cross_directory_boundary() { + let re = glob_to_regex("*.rs").unwrap(); + assert!(re.is_match("main.rs")); + assert!(!re.is_match("src/main.rs")); + } } diff --git a/src/agent/tools/plan.rs b/src/agent/tools/plan.rs index f12ccfc6..c90ea575 100644 --- a/src/agent/tools/plan.rs +++ b/src/agent/tools/plan.rs @@ -217,4 +217,65 @@ mod tests { let exit = PlanExitTool::new(tx2).definition(String::new()).await; assert_eq!(exit.name, "plan_exit"); } + + // Regression: a prior version of plan_exit wrote a "Implementation Plan" + // placeholder to PLAN.md in CWD whenever the user accepted the mode + // switch. That side-effect bypassed the file-write permission system and + // surprised users whose CWD already contained an unrelated PLAN.md. The + // fix removed the write entirely — this test guards against + // re-introducing it by inspecting the source. + #[test] + fn regression_plan_exit_has_no_filesystem_side_effects() { + let src = include_str!("plan.rs"); + // The impl block for PlanExitTool. We don't want fs::write or PLAN.md + // string literals anywhere in the call() path. + let impl_start = src + .find("impl Tool for PlanExitTool") + .expect("PlanExitTool impl present"); + let impl_end = src[impl_start..] + .find("\n}\n") + .map(|i| impl_start + i) + .unwrap_or(src.len()); + let body = &src[impl_start..impl_end]; + assert!( + !body.contains("PLAN.md"), + "plan_exit must not reference PLAN.md (side-effect regression)" + ); + assert!( + !body.contains("fs::write"), + "plan_exit must not write files (side-effect regression)" + ); + } + + // Regression: dropping the receiver (UI not subscribed) must surface a + // clean error rather than panic or hang. + #[tokio::test] + async fn plan_enter_channel_unavailable() { + let (tx, rx) = mpsc::channel(1); + drop(rx); + let tool = PlanEnterTool::new(tx); + let result = tool.call(PlanEnterArgs {}).await; + assert!(result.is_err()); + assert!( + result.unwrap_err().to_string().contains("unavailable"), + "expected 'unavailable' error", + ); + } + + // Regression: if the UI accepts the request handle but drops the oneshot + // before replying, the tool must error cleanly (channel closed) rather + // than block forever. + #[tokio::test] + async fn plan_enter_reply_dropped() { + let (tx, mut rx) = mpsc::channel(1); + let tool = PlanEnterTool::new(tx); + let handle = tokio::spawn(async move { tool.call(PlanEnterArgs {}).await }); + + let req = rx.recv().await.unwrap(); + drop(req.reply); + + let result = handle.await.unwrap(); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("channel closed")); + } } diff --git a/src/agent/tools/question.rs b/src/agent/tools/question.rs index e70428dc..43e4c90a 100644 --- a/src/agent/tools/question.rs +++ b/src/agent/tools/question.rs @@ -319,4 +319,136 @@ mod tests { assert!(result.contains("A1")); assert!(result.contains("A2")); } + + // Header text must be emitted as a markdown `## …` heading so the agent + // sees structured output rather than a flat blob. + #[tokio::test] + async fn output_includes_header_as_markdown_heading() { + let (tx, mut rx) = mpsc::channel(1); + let tool = QuestionTool::new(tx); + let args = QuestionArgs { + questions: vec![QuestionItem { + question: "Which?".into(), + header: Some("Choice".into()), + options: vec![QuestionOption { + label: "A".into(), + description: "".into(), + }], + multi_select: None, + custom: false, + }], + }; + let handle = tokio::spawn(async move { tool.call(args).await }); + let req = rx.recv().await.unwrap(); + let _ = req + .reply + .send(QuestionResponse::Answered(vec![vec!["A".into()]])); + let out = handle.await.unwrap().unwrap(); + assert!(out.contains("## Choice"), "got: {out}"); + assert!(out.contains("**Q1:** Which?")); + assert!(out.contains("**A:** A")); + } + + // Regression: dropping the receiver (channel closed before tool call) + // must error cleanly. Don't panic, don't hang. + #[tokio::test] + async fn errors_when_channel_unavailable() { + let (tx, rx) = mpsc::channel(1); + drop(rx); + let tool = QuestionTool::new(tx); + let result = tool + .call(QuestionArgs { + questions: vec![QuestionItem { + question: "Q?".into(), + header: None, + options: vec![QuestionOption { + label: "A".into(), + description: "".into(), + }], + multi_select: None, + custom: false, + }], + }) + .await; + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("unavailable")); + } + + // Reply oneshot dropped without sending → tool surfaces channel-closed + // error rather than hanging. + #[tokio::test] + async fn errors_when_reply_dropped() { + let (tx, mut rx) = mpsc::channel(1); + let tool = QuestionTool::new(tx); + let handle = tokio::spawn(async move { + tool.call(QuestionArgs { + questions: vec![QuestionItem { + question: "Q?".into(), + header: None, + options: vec![QuestionOption { + label: "A".into(), + description: "".into(), + }], + multi_select: None, + custom: false, + }], + }) + .await + }); + let req = rx.recv().await.unwrap(); + drop(req.reply); + let result = handle.await.unwrap(); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("channel closed")); + } + + // Default for `custom` is true. Without serde reading the schema default + // an agent that omits the field would get `false`, which silently changes + // the picker UX. + #[test] + fn args_default_custom_is_true() { + let parsed: QuestionItem = serde_json::from_value(serde_json::json!({ + "question": "Q?", + "options": [{"label": "A", "description": ""}], + })) + .unwrap(); + assert!(parsed.custom); + } + + // Multi-select with multiple answers must comma-join. + #[tokio::test] + async fn multi_select_joins_answers_with_comma() { + let (tx, mut rx) = mpsc::channel(1); + let tool = QuestionTool::new(tx); + let args = QuestionArgs { + questions: vec![QuestionItem { + question: "Pick".into(), + header: None, + options: vec![ + QuestionOption { + label: "A".into(), + description: "".into(), + }, + QuestionOption { + label: "B".into(), + description: "".into(), + }, + QuestionOption { + label: "C".into(), + description: "".into(), + }, + ], + multi_select: Some(true), + custom: false, + }], + }; + let handle = tokio::spawn(async move { tool.call(args).await }); + let req = rx.recv().await.unwrap(); + let _ = req.reply.send(QuestionResponse::Answered(vec![vec![ + "A".into(), + "C".into(), + ]])); + let out = handle.await.unwrap().unwrap(); + assert!(out.contains("**A:** A, C"), "got: {out}"); + } } diff --git a/src/agent/tools/task_status.rs b/src/agent/tools/task_status.rs index 09e5ae12..7e384b0b 100644 --- a/src/agent/tools/task_status.rs +++ b/src/agent/tools/task_status.rs @@ -198,4 +198,82 @@ mod tests { let def = tool.definition(String::new()).await; assert_eq!(def.name, "task_status"); } + + // Regression: BackgroundStore::get() evicts on read. Once task_status + // returns a completed task to the agent, asking again must return + // "not found" rather than re-serving the same payload (which would let + // a bad agent loop on the same result and keep it in the store). + #[tokio::test] + async fn regression_completed_task_evicts_after_first_status_read() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + store.update("t1", TaskState::Completed("payload".into())); + + let tool = TaskStatusTool::new(store); + let first = tool + .call(TaskStatusArgs { + task_id: "t1".into(), + wait: None, + }) + .await + .unwrap(); + assert!(first.contains("state: completed")); + assert!(first.contains("payload")); + + let second = tool + .call(TaskStatusArgs { + task_id: "t1".into(), + wait: None, + }) + .await; + assert!(second.is_err()); + assert!(second.unwrap_err().to_string().contains("not found")); + } + + // wait=true must also return on failure (not just on completion), and the + // error text must be surfaced. + #[tokio::test] + async fn wait_returns_on_failure() { + let store = BackgroundStore::new(); + store.insert("t1".into()); + + let store_clone = store.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(50)).await; + store_clone.update("t1", TaskState::Failed("kaboom".into())); + }); + + let tool = TaskStatusTool::new(store); + let result = tool + .call(TaskStatusArgs { + task_id: "t1".into(), + wait: Some(true), + }) + .await + .unwrap(); + assert!(result.contains("state: failed")); + assert!(result.contains("kaboom")); + } + + // wait=true must surface a not-found error promptly rather than loop on + // an absent task. + #[tokio::test] + async fn wait_on_missing_task_errors_promptly() { + let store = BackgroundStore::new(); + let tool = TaskStatusTool::new(store); + + // Bound the call so a regression to infinite-loop fails the test. + let result = tokio::time::timeout( + Duration::from_secs(1), + tool.call(TaskStatusArgs { + task_id: "never-existed".into(), + wait: Some(true), + }), + ) + .await + .expect("must not loop on missing task"); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("not found")); + } } diff --git a/src/agent/tools/webfetch.rs b/src/agent/tools/webfetch.rs index 2f781d42..96909bf1 100644 --- a/src/agent/tools/webfetch.rs +++ b/src/agent/tools/webfetch.rs @@ -199,4 +199,87 @@ mod tests { let def = tool.definition(String::new()).await; assert_eq!(def.name, "webfetch"); } + + // Regression: prior bug passed `html.len()` as the second argument to + // html2text — that parameter is the *line-wrap width*, not buffer size. + // The result was effectively no wrapping at all. We now pass 100, which + // produces wrapped output for paragraphs that exceed that width. + #[test] + fn regression_html_to_markdown_wraps_at_reasonable_width() { + let long_word_count = 200; + // Build a paragraph that, without wrapping, would be ~one extremely + // long line. + let paragraph: String = std::iter::repeat("lorem") + .take(long_word_count) + .collect::>() + .join(" "); + let html = format!("

{}

", paragraph); + let md = html_to_markdown(&html); + + // The output must be split across multiple lines (wrap width=100). + let lines: Vec<&str> = md.lines().filter(|l| !l.is_empty()).collect(); + assert!( + lines.len() > 1, + "expected wrapped output, got single line of {} chars", + md.len() + ); + // No single line should be wildly wider than the wrap width. + for line in &lines { + assert!( + line.chars().count() < 200, + "line too long ({}): {line}", + line.chars().count() + ); + } + } + + #[tokio::test] + async fn rejects_empty_urls() { + let tool = WebFetchTool::new(None, None); + let result = tool + .call(WebFetchArgs { + urls: vec![], + max_chars: 3000, + }) + .await; + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("no URLs")); + } + + #[tokio::test] + async fn rejects_more_than_ten_urls() { + let tool = WebFetchTool::new(None, None); + let urls: Vec = (0..11) + .map(|i| format!("https://example.com/{i}")) + .collect(); + let result = tool + .call(WebFetchArgs { + urls, + max_chars: 3000, + }) + .await; + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("maximum 10")); + } + + // Regression: the WebFetchArgs default for max_chars must be 3000 — agents + // that omit the field should not get an unbounded fetch. + #[test] + fn webfetch_args_default_max_chars_is_3000() { + let parsed: WebFetchArgs = + serde_json::from_value(serde_json::json!({"urls": ["https://example.com"]})).unwrap(); + assert_eq!(parsed.max_chars, 3000); + } + + // html2text drops markup but keeps text content — guards against a + // dependency upgrade changing default behavior. + #[test] + fn html_to_markdown_strips_tags_but_keeps_text() { + let html = "
bold and emph
"; + let md = html_to_markdown(html); + assert!(md.contains("bold")); + assert!(md.contains("emph")); + assert!(!md.contains("")); + assert!(!md.contains("")); + } } diff --git a/src/agent/tools/websearch.rs b/src/agent/tools/websearch.rs index b2b8ed20..30afcc96 100644 --- a/src/agent/tools/websearch.rs +++ b/src/agent/tools/websearch.rs @@ -202,4 +202,58 @@ mod tests { let def = tool.definition(String::new()).await; assert_eq!(def.name, "websearch"); } + + // Each ExaResult field is optional from the API's perspective. Missing + // pieces should be skipped silently rather than rendering "**None**" or + // panicking — guards format_search_results against partial responses. + #[test] + fn format_handles_missing_fields() { + let results = vec![ + ExaResult { + title: None, + url: Some("https://no-title.example".into()), + text: Some("body".into()), + }, + ExaResult { + title: Some("No URL".into()), + url: None, + text: Some("body".into()), + }, + ExaResult { + title: Some("No text".into()), + url: Some("https://no-text.example".into()), + text: None, + }, + ]; + let out = format_search_results(&results); + assert!(out.contains("https://no-title.example")); + assert!(out.contains("**No URL**")); + assert!(out.contains("**No text**")); + assert!(!out.contains("None"), "got: {out}"); + } + + // Regression: WebSearchArgs default for num_results must be 10 to match + // the documented schema default. + #[test] + fn websearch_args_default_num_results_is_10() { + let parsed: WebSearchArgs = + serde_json::from_value(serde_json::json!({"query": "rust async"})).unwrap(); + assert_eq!(parsed.num_results, 10); + } + + // Text snippets in results are capped at 500 chars to prevent context + // blowout — long Exa results have been observed past 5K chars per item. + #[test] + fn format_truncates_long_text() { + let huge = "Z".repeat(2000); + let results = vec![ExaResult { + title: Some("t".into()), + url: Some("https://site.org".into()), + text: Some(huge), + }]; + let out = format_search_results(&results); + // Cap is 500 chars on the snippet; nothing else contributes 'Z' here. + let z_count = out.chars().filter(|c| *c == 'Z').count(); + assert_eq!(z_count, 500); + } } diff --git a/src/extras/memory.rs b/src/extras/memory.rs index 34d855aa..2d403c2a 100644 --- a/src/extras/memory.rs +++ b/src/extras/memory.rs @@ -1,11 +1,14 @@ use std::path::{Path, PathBuf}; +fn home_dir() -> PathBuf { + std::env::var("HOME") + .map(PathBuf::from) + .unwrap_or_else(|_| PathBuf::from(".")) +} + pub fn memory_dir(cwd: &Path) -> PathBuf { let project_id = project_id(cwd); - let base = dirs::data_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join("dirge") - .join("memories"); + let base = home_dir().join(".dirge").join("memories"); base.join(project_id) }