diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5640ee6f..0363e6a2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,9 @@
## Unreleased
+- Improve planning mode prompt and tool docs; clarify absolute-path usage and preview rules.
+- Centralize path approval for tools and always list all missing required params in INVALID_ARGS.
+
- Updated instructions for `/login` command and invalid input handling.
- Fix server name on `chat/contentReceived` when preparing tool call.
- Fix variable replacing in some tool prompts.
diff --git a/docs/features.md b/docs/features.md
index 95babd2b..29f2b293 100644
--- a/docs/features.md
+++ b/docs/features.md
@@ -38,7 +38,7 @@ ECA support built-in tools to avoid user extra installation and configuration, t
=== "Filesystem"
- Provides access to filesystem under workspace root, listing, reading and writing files, important for agentic operations.
+ Provides access to the filesystem for listing, reading, writing, editing and moving files. Operates primarily on workspace files; paths outside the workspace require approval.
- `eca_directory_tree`: list a directory as a tree (can be recursive).
- `eca_read_file`: read a file content.
diff --git a/resources/prompts/plan_behavior.md b/resources/prompts/plan_behavior.md
index 10f776e0..51ea125c 100644
--- a/resources/prompts/plan_behavior.md
+++ b/resources/prompts/plan_behavior.md
@@ -1,70 +1,127 @@
-You are ECA (Editor Code Assistant), an AI coding assistant.
+# ECA — Editor Code Assistant (Planning Mode)
-## Plan Mode
+## Guiding Principles & Rules
-You are in planning mode. Analyze the user's request and create a detailed implementation plan that can be executed later.
-
-### Your Task
-Whatever the user asks for, you must:
-1. Analyze the request thoroughly
-2. Create a concrete plan showing exactly what would be done
-3. Present this as a plan for review (not as completed work)
+### Role & Scope
+- You are ECA, an AI coding assistant.
+- **Planning mode only**: analyze the request thoroughly and produce an implementation plan.
+- You **do not modify files**.
### Core Principle
-You're in read-only mode. Nothing you do will modify files. Your job is to show WHAT would be changed and HOW, so it can be implemented after approval.
-NEVER print codeblocks for file changes unless explicitly requested - use the appropriate tool.
-
-### Tools for Planning
-- `eca_read_file`, `eca_grep`, `eca_directory_tree`: Explore codebase
-- `eca_shell_command`: Read-only commands ONLY (forbidden: >, >>, rm, mv, cp, touch, git add/commit/push)
-- `eca_preview_file_change`: Show exact file changes in your present plan step.
-
-### Workflow
-1. **Understand** - Analyze what the user wants
-2. **Explore** - Work through different approaches. During exploration:
- - Show code possibilities in markdown blocks with language names
- - Take your time reasoning for the best solution possible.
- - Think through multiple options freely
-3. **Decide** - Choose the best solution. If multiple good approaches exist and user preference would help, present the options and ask for guidance before continuing.
-4. **Present Plan** - Write comprehensive plan with:
- - Clear summary and step-by-step approach
- - Call preview tool for file changes when exist
- - Use FUTURE/CONDITIONAL language: "will change", "would modify", "the plan includes"
- - NEVER use past tense: don't say "I've changed", "I modified", "Updated"
- - Descriptions of other actions (tests, analysis, etc.)
-
-### When to Use What for Code
-**During Exploration (Step 2):**
-- Use markdown code blocks to show code possibilities
-- This is for thinking through approaches and iterations
-- Use full language names like 'javascript', not 'js'
-
-**In Final Plan (Step 4):**
-- Use `eca_preview_file_change` to show your decided changes
-- Actually CALL the tool - don't write fake tool syntax in markdown
-- The tool call should appear in your plan narrative, not as standalone items
-
-### Preview Tool (eca_preview_file_change) Guidelines
-- Use ONLY for final implementation, not during exploration
-- Break large changes into focused pieces
-- For new files: original_content = ""
-- If preview fails: re-read file and match content exactly
-
-### Remember
-Plans can involve many activities beyond code changes (running tests, analysis, etc.).
-When presenting file modifications, call the preview tool naturally within your plan narrative - don't list tool calls as separate standalone items.
-The tool will generate diffs automatically; focus your explanation on WHAT will change and WHY.
-
-
-The chat is markdown mode.
-When using markdown in assistant messages, use backticks to format file, directory, function, and class names.
-Pay attention to the language name after the code block backticks start, use the full language name like 'javascript' instead of 'js'.
-
-
-
-You have tools at your disposal to solve the coding task. Follow these rules regarding tool calls:
-1. ALWAYS follow the tool call schema exactly as specified and make sure to provide all necessary parameters.
-2. If you need additional information that you can get via tool calls, prefer that over asking the user.
-3. If you are not sure about file content or codebase structure pertaining to the user's request, use your tools to read files and gather the relevant information: do NOT guess or make up an answer.
-4. You have the capability to call multiple tools in a single response, batch your tool calls together for optimal performance.
-
+- Show **what** would change and **how**.
+- Do **not** include file diffs or full new-file contents in code blocks.
+- Diffs and new files are shown **only** via the tool `eca_preview_file_change`.
+
+### Language & Tone
+- **Never claim completed actions:** created, built, implemented, fixed, edited, updated, changed, ran, executed, published, shipped, merged, committed, applied.
+- **Always use conditional/neutral framing:** “would create”, “would modify”, “if applied”, “the preview shows”.
+- Keep narration minimal and focused on decisions and rationale.
+
+### CRITICAL: The First Response Rule
+- Your **very first response** to the user's request **must always** be the complete Phase 1 plan, starting with the `## Understand` section.
+- **Never** begin your first response with a tool call.
+
+### Technical Requirements
+- **Paths & Repository:**
+ - Project root = current `pwd`.
+ - **All paths must be absolute** (prefix with cached `pwd` result).
+ - Call `pwd` **at most once per plan** and cache the result.
+ - Empty directories are valid context.
+ - Never read non-existent files before preview creation.
+- **Tool Call Parameters:**
+ - **Required parameters:** Provide every parameter with **non-empty, concrete values**.
+ - **Concrete targets** must be either:
+ 1) Absolute path (starts with `/`), or
+ 2) Anchored glob rooted at project root.
+ - For `grep`: Pattern must include a **specific identifier** (class/function/file stem) likely to match uniquely.
+
+---
+
+## Core Workflow: The 3 Phases
+
+### Phase 1: Initial Plan Creation
+When creating a **new** plan, output **all four sections in this exact order**.
+
+#### 1) ## Understand
+- **Goal:** One sentence stating the user's goal.
+- **Tools:** **NO TOOLS ALLOWED.**
+
+#### 2) ## Explore
+- **Goal:** Thorough analysis of options and reasoning. Short code snippets allowed (examples/specs only; not diffs).
+- **Tools & Rules for Exploration:**
+ - **Allowed Tools:** `eca_read_file`, `eca_grep`, `eca_directory_tree`, `eca_shell_command` (read-only; no destructive ops like `>`, `>>`, `rm`, `mv`, etc.).
+ - **Availability:** Exploration tools are allowed **ONLY HERE** during initial plan creation. They can be used freely in Phase 2.
+ - **Execution Rules:**
+ - **Before each call:** Write 1–3 bullets explaining what you’re investigating and why.
+ - **Start narrow:** Most specific scope first (single file > directory > repo).
+ - **Follow the evidence:** Only read files your grep/tree calls actually found.
+ - **Validate feasibility:** Check interfaces, dependencies, patterns, conflicts.
+ - **Use all available tools** as needed to verify the approach.
+ - **Cache results:** Never repeat the same tool call within one response.
+ - **Exit criteria:** Stop once you can answer:
+ 1) what exists, 2) what needs changing, 3) that the plan is implementable.
+
+#### 3) ## Decide
+- **Goal:** State the chosen approach with rationale based on Explore.
+- **If multiple viable approaches exist:** include a comparison (markdown table or bullets) with trade-offs.
+- **If only one approach is viable:** briefly explain why alternatives won’t work.
+- **Focus:** technical fit, complexity, maintainability, alignment with existing patterns.
+- **Tools:** **NO TOOLS ALLOWED.**
+
+#### 4) ## Present Plan
+- **Goal:** Step-by-step plan with **conditional/future wording** (e.g., “would add”, “would modify”, “if applied”, “the preview shows”).
+- **Required: File Summary** — absolute paths the plan would touch:
+ - **Would modify:** `/abs/path1`, `/abs/path2`
+ - **Would create:** `/abs/new1`, `/abs/new2`
+ - **Would delete (if any):_** `/abs/old1`
+- **Required closing line:** **"Would you like me to preview these changes now?"**
+- **Tools:** **NO TOOLS ALLOWED.**
+
+---
+
+### Phase 2: Plan Discussion & Refinement
+This phase is the central loop for iterating on the plan. The process returns here whenever the user requests a change, either after the initial plan (Phase 1) or after seeing a preview (Phase 3).
+
+**Rules for this phase:**
+- **Tool Usage:** Exploration tools (`eca_read_file`, `eca_grep`, etc.) **CAN** be used freely to answer questions or investigate alternatives.
+- **Outputting Updates:** If exploration reveals needed changes, you must output a dedicated **`### Plan Updates`** section with the following structure:
+ - **Summary:** Briefly summarize what changes from the original plan.
+ - **Updated File Summary:** Provide the complete, updated list of files.
+ - **Would modify:** `/abs/path1`, `/abs/updated_path`
+ - **Would create:** `/abs/new1`
+ - **Would delete:** `/abs/old1`
+ - **Closing Line:** End with the required closing line: **"Would you like me to preview these changes now?"**
+- **Loop Behavior:** This cycle of discussion, exploration, and presenting `### Plan Updates` can repeat as many times as needed until the user is ready to see a new preview.
+
+---
+
+### Phase 3: Preview Implementation
+Execute this phase **ONLY** when the user explicitly opts in (e.g., “preview”, “show diff”, “go ahead”).
+
+**Tool for Previews:**
+- `eca_preview_file_change` — Show file changes as diffs.
+
+**Preview Protocol:**
+- **Narration:** Use conditional phrasing (“would add/modify”, “the preview would show”).
+- **New Files:** For new files, the tool call must use `original_content = ""`.
+- **Modifications:** For modifications, provide an **exact, unique anchor** from the current file content.
+- **One Call Per Path:** Use only one `eca_preview_file_change` call per file path. Multiple calls for the same file are only allowed if they use different, non-overlapping anchors.
+- **Minimal Reads:** Only read a file (`eca_read_file`) when you need a stable anchor for a modification. Perform a maximum of **one read per file per response**.
+- **Retry on Failure:** If an anchor fails, re-read the file once, choose a different, more stable anchor, and retry the `eca_preview_file_change` call once.
+
+---
+
+## Self-Audit Checklist (run before sending)
+- [ ] Phase 1 present and in correct order (Understand, Explore, Decide, Present Plan)
+- [ ] No tool calls before **## Understand**
+- [ ] Exploration calls **only** in **## Explore** during Phase 1
+- [ ] `pwd` called at most once; result cached
+- [ ] All paths are absolute
+- [ ] No duplicate tool calls in the same response
+- [ ] For preview: one call per path (unless different anchors required)
+- [ ] New files use `original_content = ""`
+- [ ] Language is conditional/neutral throughout
+
+## Long Conversations
+- Briefly restate the section format every 3–5 user messages.
+- Re-offer preview **only** if discussion led to plan changes or file updates.
diff --git a/resources/prompts/tools/eca_directory_tree.md b/resources/prompts/tools/eca_directory_tree.md
index b39ea2fa..af0a8222 100644
--- a/resources/prompts/tools/eca_directory_tree.md
+++ b/resources/prompts/tools/eca_directory_tree.md
@@ -1,3 +1,6 @@
-Returns a recursive tree view of files and directories starting from the specified path.
-The path parameter must be an absolute path, not a relative path.
-**Only works within the directories: {workspaceRoots}.**
+Shows a recursive tree of directories and files under the given path.
+
+Usage:
+- `path` must be an absolute path.
+- Optional: `max_depth` to limit traversal.
+- Skips hidden entries (dotfiles and dot-directories).
diff --git a/resources/prompts/tools/eca_edit_file.md b/resources/prompts/tools/eca_edit_file.md
index ee5bb816..1297c634 100644
--- a/resources/prompts/tools/eca_edit_file.md
+++ b/resources/prompts/tools/eca_edit_file.md
@@ -1,15 +1,12 @@
-You must use `eca_read_file` to get the file's exact contents before attempting an edit.
-
-## Best Practices
-
-- Prefer small, targeted edits over replacing entire functions
-- Match content from `eca_read_file` as closely as possible
-- For single occurrence (default): include enough surrounding context to make the match unique
-- For multiple occurrences (`all_occurrences: true`): provide the exact literal content to replace.
-
-## Common Issues
-
-- "content not found": read the file again to verify the actual formatting
-- "ambiguous match" (single occurrence only): include more surrounding context
-- To delete content: use empty string as `new_content`
-- To prepend/append: `new_content` must contain both the new and the original content
+Apply precise content replacement in a file.
+
+Usage:
+- `path` must be an absolute path; `original_content` and `new_content` are required.
+- You must use your `eca_read_file` tool at least once in the conversation before editing.
+- `original_content` must be copied verbatim from the file you read (use `eca_read_file` output) — do not invent or modify it.
+- `original_content` must be exact (including whitespace); include enough surrounding context for a unique match.
+- Prefer small, targeted edits over replacing entire functions.
+- Requires exactly one match; otherwise it fails. Add context to disambiguate, or set `all_occurrences` to true to update all matches.
+- If the edit fails, you must reread the file and call eca_edit_file again.
+- To delete content, set `new_content` to an empty string.
+- To prepend/append content, `new_content` must include both the new and the original content.
diff --git a/resources/prompts/tools/eca_editor_diagnostics.md b/resources/prompts/tools/eca_editor_diagnostics.md
index 20ecbfb1..99024d00 100644
--- a/resources/prompts/tools/eca_editor_diagnostics.md
+++ b/resources/prompts/tools/eca_editor_diagnostics.md
@@ -1,2 +1,5 @@
-Return editor diagnostics/findings (Ex: LSP diagnostics) for workspaces.
-Only provide the path if you want to get diagnostics for a specific file.
+Return editor diagnostics for a file or the current workspaces.
+
+Usage:
+- Optional: `path` absolute file path to scope diagnostics.
+- Uses editor/LSP diagnostics; read-only.
diff --git a/resources/prompts/tools/eca_grep.md b/resources/prompts/tools/eca_grep.md
index abccd883..dc7f12c3 100644
--- a/resources/prompts/tools/eca_grep.md
+++ b/resources/prompts/tools/eca_grep.md
@@ -1,3 +1,8 @@
-Fast content search tool that works with any codebase size. Finds the paths to files that have matching contents using regular expressions.
-Supports full regex syntax (eg. "log.*Error", "function\\s+\\w+", etc.). Filter files by pattern with the include parameter (eg. "*.js", "*.{ts,tsx}").
-Returns matching file paths sorted by modification time. Use this tool when you need to find files containing specific patterns.
+Find files whose contents match a regular expression.
+
+Usage:
+- `path` must be an absolute path.
+- `pattern` is a regular expression to match file contents.
+- Optional: `include` file-glob filter (e.g., "*.clj", "*.{clj,cljs}").
+- Optional: `max_results` limits the number of returned paths.
+- Returns matching file paths, one per line.
diff --git a/resources/prompts/tools/eca_move_file.md b/resources/prompts/tools/eca_move_file.md
index 2d638d3c..9f9c554a 100644
--- a/resources/prompts/tools/eca_move_file.md
+++ b/resources/prompts/tools/eca_move_file.md
@@ -1,4 +1,7 @@
-Move or rename files and directories.
-Can move files between directories and rename them in a single operation.
-If the destination exists, the operation will fail. Works across different directories and can be used for simple renaming within the same directory.
-Both source and destination must be within the directories: {workspaceRoots}.
+Move or rename a file or directory.
+
+Usage:
+- `source` and `destination` must be absolute paths.
+- Fails if `destination` already exists; choose a non-existing target.
+- Works across directories or for simple renames within a directory.
+- Create the destination parent directory first if needed.
diff --git a/resources/prompts/tools/eca_preview_file_change.md b/resources/prompts/tools/eca_preview_file_change.md
index 1212d29f..0c47e184 100644
--- a/resources/prompts/tools/eca_preview_file_change.md
+++ b/resources/prompts/tools/eca_preview_file_change.md
@@ -1,8 +1,8 @@
-Preview file changes without making any actual modifications. Shows the exact diff that would be applied when the plan is implemented.
+Simulate an edit or new file creation; returns only the proposed content (no changes applied).
-**IMPORTANT: The `path` parameter must be an absolute path** (e.g., `/Users/name/project/file.js`, not `./file.js` or relative paths).
-
-Use only for showing definitive implementation, not for iterative exploration.
-When crafting the `orginal_content`, you must match the original content from the `eca_read_file` tool output exactly, including all indentation (spaces/tabs) and newlines.
-ALWAYS prefer making small, targeted content changes.
-For new files, original_content must be empty string.
+Usage:
+- `path` must be an absolute path.
+- Existing files: provide `original_content` and `new_content`; match exactly, including whitespace.
+- New files: set `original_content = ""` and provide full `new_content`.
+- Prefer small, targeted edits over replacing entire functions.
+- Read-only; to apply, use `eca_edit_file` or `eca_write_file`. To preview deletion, set `new_content` to an empty string.
diff --git a/resources/prompts/tools/eca_read_file.md b/resources/prompts/tools/eca_read_file.md
index f80e85b0..99796568 100644
--- a/resources/prompts/tools/eca_read_file.md
+++ b/resources/prompts/tools/eca_read_file.md
@@ -1,3 +1,6 @@
-Read the contents of a file from the file system. Use this tool when you need to examine the contents of a single file.
-Optionally use the 'line_offset' and/or 'limit' parameters to read specific contents of the file when you know the range.
-Prefer call once this tool over multiple calls passing small offsets. **Only works within the directories: {workspaceRoots}.**
+Read a file’s current content.
+
+Usage:
+- `path` must be an absolute path.
+- Optional: `line_offset` (0-based start line) and `limit` (max lines).
+- UTF-8 text only. Prefer one well-scoped read over many tiny reads.
diff --git a/resources/prompts/tools/eca_shell_command.md b/resources/prompts/tools/eca_shell_command.md
index 68283f7f..e8bb86c7 100644
--- a/resources/prompts/tools/eca_shell_command.md
+++ b/resources/prompts/tools/eca_shell_command.md
@@ -1,5 +1,12 @@
Executes an arbitrary shell command ensuring proper handling and security measures.
-1. Command Execution:
+
+Before executing the command, please follow these steps:
+
+1. Directory Verification:
+ - If the command will create new directories or files, first use the List tool to verify the parent directory exists and is the correct location
+ - For example, before running "mkdir foo/bar", first use List to check that "foo" exists and is the intended parent directory
+
+2. Command Execution:
- Always quote file paths that contain spaces with double quotes (e.g., cd \" path with spaces/file.txt \")
- Examples of proper quoting:
- cd \"/Users/name/My Documents\" (correct)
@@ -8,10 +15,13 @@ Executes an arbitrary shell command ensuring proper handling and security measur
- python /path/with spaces/script.py (incorrect - will fail)
- After ensuring proper quoting, execute the command.
- Capture the output of the command.
- - VERY IMPORTANT: You MUST avoid using search command `grep`. Instead use eca_grep to search. You MUST avoid read tools like `cat`, `head`, `tail`, and `ls`, and use eca_read_file or eca_directory_tree.
+
+Usage notes:
+ - The `command` argument is required.
- It is very helpful if you write a clear, concise description of what this command does in 5-10 words.
- When issuing multiple commands, use the ';' or '&&' operator to separate them. DO NOT use newlines (newlines are ok in quoted strings).
- - Try to maintain your current working directory throughout the session by using absolute paths and avoiding usage of `cd`. You may use `cd` if the User explicitly requests it.
+ - VERY IMPORTANT: You MUST avoid using search command `grep`. Instead use eca_grep to search. You MUST avoid read tools like `cat`, `head`, `tail`, and `ls`, and use eca_read_file or eca_directory_tree.
+ - Try to maintain your current working directory throughout the session by using absolute paths and avoiding usage of `cd`. You my use `cd` if the User explicitly requests it.
pytest /foo/bar/tests
diff --git a/resources/prompts/tools/eca_write_file.md b/resources/prompts/tools/eca_write_file.md
index 7f7a503c..81f604ba 100644
--- a/resources/prompts/tools/eca_write_file.md
+++ b/resources/prompts/tools/eca_write_file.md
@@ -1,5 +1,7 @@
-Create a new file or completely overwrite an existing file with new content.
-This tool will automatically create any necessary parent directories if they don't exist.
-Use this tool when you want to create a new file from scratch or completely replace the entire content of an existing file.
-For partial edits or content replacement within existing files, use eca_edit_file instead.
-**Only works within the directories: {workspaceRoots}.**
+Create a new file or completely overwrite an existing one.
+
+Usage:
+- `path` must be an absolute path.
+- `content` is the full file content to write.
+- Automatically creates parent directories if they do not exist.
+- For partial edits, prefer `eca_edit_file`.
diff --git a/src/eca/config.clj b/src/eca/config.clj
index e684df80..2c500ed9 100644
--- a/src/eca/config.clj
+++ b/src/eca/config.clj
@@ -80,8 +80,15 @@
:disabledTools ["eca_preview_file_change"]}
"plan" {:systemPromptFile "prompts/plan_behavior.md"
:disabledTools ["eca_edit_file" "eca_write_file" "eca_move_file"]
- :toolCall {:approval {:deny {"eca_shell_command"
- {:argsMatchers {"command" [".*>.*",
+ :toolCall {:approval {:allow {"eca_shell_command"
+ {:argsMatchers {"command" ["pwd"]}}
+ "eca_preview_file_change" {}
+ "eca_grep" {}
+ "eca_read_file" {}
+ "eca_directory_tree" {}}
+ :deny {"eca_shell_command"
+ {:argsMatchers {"command" ["[12&]?>>?\\s*(?!/dev/null($|\\s))\\S+"
+ ".*>.*",
".*\\|\\s*(tee|dd|xargs).*",
".*\\b(sed|awk|perl)\\s+.*-i.*",
".*\\b(rm|mv|cp|touch|mkdir)\\b.*",
diff --git a/src/eca/features/tools.clj b/src/eca/features/tools.clj
index c913ece7..d3ba3a71 100644
--- a/src/eca/features/tools.clj
+++ b/src/eca/features/tools.clj
@@ -80,19 +80,25 @@
]
(logger/info logger-tag (format "Calling tool '%s' with args '%s'" name arguments))
(let [arguments (update-keys arguments clojure.core/name)
- db @db*]
+ db @db*
+ tool-meta (some #(when (= name (:name %)) %)
+ (all-tools chat-id behavior db config))
+ required-args-error (when-let [parameters (:parameters tool-meta)]
+ (tools.util/required-params-error parameters arguments))]
(try
- (let [result (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
- (native-tool-handler arguments {:db db
- :db* db*
- :config config
- :messenger messenger
- :behavior behavior
- :chat-id chat-id
- :tool-call-id tool-call-id
- :call-state-fn call-state-fn
- :state-transition-fn state-transition-fn})
- (f.mcp/call-tool! name arguments {:db db}))]
+ (let [result (-> (if required-args-error
+ required-args-error
+ (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
+ (native-tool-handler arguments {:db db
+ :db* db*
+ :config config
+ :messenger messenger
+ :behavior behavior
+ :chat-id chat-id
+ :tool-call-id tool-call-id
+ :call-state-fn call-state-fn
+ :state-transition-fn state-transition-fn})
+ (f.mcp/call-tool! name arguments {:db db}))))]
(logger/debug logger-tag "Tool call result: " result)
(metrics/count-up! "tool-called" {:name name :error (:error result)} metrics)
result)
diff --git a/src/eca/features/tools/filesystem.clj b/src/eca/features/tools/filesystem.clj
index a96e0a8c..0edb1618 100644
--- a/src/eca/features/tools/filesystem.clj
+++ b/src/eca/features/tools/filesystem.clj
@@ -14,13 +14,8 @@
(set! *warn-on-reflection* true)
-(defn ^:private allowed-path? [db path]
- (some #(fs/starts-with? path (shared/uri->filename (:uri %)))
- (:workspace-folders db)))
-
-(defn ^:private path-validations [db]
- [["path" fs/exists? "$path is not a valid path"]
- ["path" (partial allowed-path? db) (str "Access denied - path $path outside allowed directories: " (tools.util/workspace-roots-strs db))]])
+(defn ^:private path-validations []
+ [["path" fs/exists? "$path is not a valid path"]])
(def ^:private directory-tree-max-depth 10)
@@ -35,7 +30,7 @@
(defn ^:private directory-tree [arguments {:keys [db config]}]
(let [path (delay (fs/canonicalize (get arguments "path")))]
- (or (tools.util/invalid-arguments arguments (path-validations db))
+ (or (tools.util/invalid-arguments arguments (path-validations))
(let [max-depth (or (get arguments "max_depth") directory-tree-max-depth)
dir-count* (atom 0)
file-count* (atom 0)
@@ -69,8 +64,8 @@
summary (format "%d directories, %d files" @dir-count* @file-count*)]
(tools.util/single-text-content (str body "\n\n" summary)))))))
-(defn ^:private read-file [arguments {:keys [db config]}]
- (or (tools.util/invalid-arguments arguments (concat (path-validations db)
+(defn ^:private read-file [arguments {:keys [config]}]
+ (or (tools.util/invalid-arguments arguments (concat (path-validations)
[["path" fs/readable? "File $path is not readable"]
["path" (complement fs/directory?) "$path is a directory, not a file"]]))
(let [line-offset (or (get arguments "line_offset") 0)
@@ -99,13 +94,12 @@
(str "Reading file " (fs/file-name (fs/file path)))
"Reading file"))
-(defn ^:private write-file [arguments {:keys [db]}]
- (or (tools.util/invalid-arguments arguments [["path" (partial allowed-path? db) (str "Access denied - path $path outside allowed directories: " (tools.util/workspace-roots-strs db))]])
- (let [path (get arguments "path")
- content (get arguments "content")]
- (fs/create-dirs (fs/parent (fs/path path)))
- (spit path content)
- (tools.util/single-text-content (format "Successfully wrote to %s" path)))))
+(defn ^:private write-file [arguments _]
+ (let [path (get arguments "path")
+ content (get arguments "content")]
+ (fs/create-dirs (fs/parent (fs/path path)))
+ (spit path content)
+ (tools.util/single-text-content (format "Successfully wrote to %s" path))))
(defn ^:private write-file-summary [{:keys [args]}]
(if-let [path (get args "path")]
@@ -178,8 +172,8 @@
Returns matching file paths, prioritizing by modification time when possible.
Validates that the search path is within allowed workspace directories."
- [arguments {:keys [db]}]
- (or (tools.util/invalid-arguments arguments (concat (path-validations db)
+ [arguments _]
+ (or (tools.util/invalid-arguments arguments (concat (path-validations)
[["path" fs/readable? "File $path is not readable"]
["pattern" #(and % (not (string/blank? %))) "Invalid content regex pattern '$pattern'"]
["include" #(or (nil? %) (not (string/blank? %))) "Invalid file pattern '$include'"]
@@ -245,8 +239,9 @@
(text-match/apply-content-change-to-string file-content original-content new-content all? path)
(smart-edit/apply-smart-edit file-content original-content new-content path)))
+
(defn ^:private edit-file [arguments {:keys [db]}]
- (or (tools.util/invalid-arguments arguments (concat (path-validations db)
+ (or (tools.util/invalid-arguments arguments (concat (path-validations)
[["path" fs/readable? "File $path is not readable"]]))
(let [path (get arguments "path")
original-content (get arguments "original_content")
@@ -268,38 +263,34 @@
(handle-file-change-result {:error :conflict} path nil)))))
(handle-file-change-result result path nil)))))
-(defn ^:private preview-file-change [arguments {:keys [db]}]
- (or (tools.util/invalid-arguments arguments [["path" (partial allowed-path? db) (str "Access denied - path $path outside allowed directories: " (tools.util/workspace-roots-strs db))]])
- (let [path (get arguments "path")
- original-content (get arguments "original_content")
- new-content (get arguments "new_content")
- all? (boolean (get arguments "all_occurrences"))
- file-exists? (fs/exists? path)]
- (cond
- file-exists?
- (let [result (apply-file-edit-strategy (slurp path) original-content new-content all? path)]
- (handle-file-change-result result path
- (format "Change simulation completed for %s. Original file unchanged - preview only." path)))
-
- (and (not file-exists?) (= "" original-content))
- (tools.util/single-text-content (format "New file creation simulation completed for %s. File will be created - preview only." path))
-
- :else
- (tools.util/single-text-content
- (format "Preview error for %s: For new files, original_content must be empty string (\"\"). Use markdown blocks during exploration, then eca_preview_file_change for final implementation only."
- path)
- :error)))))
-
-(defn ^:private move-file [arguments {:keys [db]}]
- (let [workspace-dirs (tools.util/workspace-roots-strs db)]
- (or (tools.util/invalid-arguments arguments [["source" fs/exists? "$source is not a valid path"]
- ["source" (partial allowed-path? db) (str "Access denied - path $source outside allowed directories: " workspace-dirs)]
- ["destination" (partial allowed-path? db) (str "Access denied - path $destination outside allowed directories: " workspace-dirs)]
- ["destination" (complement fs/exists?) "Path $destination already exists"]])
- (let [source (get arguments "source")
- destination (get arguments "destination")]
- (fs/move source destination {:replace-existing false})
- (tools.util/single-text-content (format "Successfully moved %s to %s" source destination))))))
+(defn ^:private preview-file-change [arguments _]
+ (let [path (get arguments "path")
+ original-content (get arguments "original_content")
+ new-content (get arguments "new_content")
+ all? (boolean (get arguments "all_occurrences"))
+ file-exists? (fs/exists? path)]
+ (cond
+ file-exists?
+ (let [result (apply-file-edit-strategy (slurp path) original-content new-content all? path)]
+ (handle-file-change-result result path
+ (format "Change simulation completed for %s. Original file unchanged - preview only." path)))
+
+ (and (not file-exists?) (= "" original-content))
+ (tools.util/single-text-content (format "New file creation simulation completed for %s. File will be created - preview only." path))
+
+ :else
+ (tools.util/single-text-content
+ (format "Preview error for %s: For new files, original_content must be empty string (\"\")."
+ path)
+ :error))))
+
+(defn ^:private move-file [arguments _]
+ (or (tools.util/invalid-arguments arguments [["source" fs/exists? "$source is not a valid path"]
+ ["destination" (complement fs/exists?) "Path $destination already exists"]])
+ (let [source (get arguments "source")
+ destination (get arguments "destination")]
+ (fs/move source destination {:replace-existing false})
+ (tools.util/single-text-content (format "Successfully moved %s to %s" source destination)))))
(def definitions
{"eca_directory_tree"
@@ -311,6 +302,7 @@
:description (format "Maximum depth to traverse (default: %s)" directory-tree-max-depth)}}
:required ["path"]}
:handler #'directory-tree
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
:summary-fn (constantly "Listing file tree")}
"eca_read_file"
{:description (tools.util/read-tool-description "eca_read_file")
@@ -323,6 +315,7 @@
:description "Maximum lines to read (default: configured in tools.readFile.maxLines, defaults to 2000)"}}
:required ["path"]}
:handler #'read-file
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
:summary-fn #'read-file-summary}
"eca_write_file"
{:description (tools.util/read-tool-description "eca_write_file")
@@ -333,6 +326,7 @@
:description "The complete content to write to the file"}}
:required ["path" "content"]}
:handler #'write-file
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
:summary-fn #'write-file-summary}
"eca_edit_file"
{:description (tools.util/read-tool-description "eca_edit_file")
@@ -347,6 +341,7 @@
:description "Whether to replace all occurrences of the file or just the first one (default)"}}
:required ["path" "original_content" "new_content"]}
:handler #'edit-file
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
:summary-fn (constantly "Editing file")}
"eca_preview_file_change"
{:description (tools.util/read-tool-description "eca_preview_file_change")
@@ -361,6 +356,7 @@
:description "Whether to preview replacing all occurrences or just the first one (default)"}}
:required ["path" "original_content" "new_content"]}
:handler #'preview-file-change
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
:summary-fn (constantly "Previewing change")}
"eca_move_file"
{:description (tools.util/read-tool-description "eca_move_file")
@@ -371,6 +367,7 @@
:description "The new absolute file path to move to."}}
:required ["source" "destination"]}
:handler #'move-file
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["source" "destination"])
:summary-fn (constantly "Moving file")}
"eca_grep"
{:description (tools.util/read-tool-description "eca_grep")
@@ -385,6 +382,7 @@
:description "Maximum number of results to return (default: 1000)"}}
:required ["path" "pattern"]}
:handler #'grep
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["path"])
:summary-fn #'grep-summary}})
(defmethod tools.util/tool-call-details-before-invocation :eca_edit_file [_name arguments _server _ctx]
diff --git a/src/eca/features/tools/shell.clj b/src/eca/features/tools/shell.clj
index 84484413..1b29dc71 100644
--- a/src/eca/features/tools/shell.clj
+++ b/src/eca/features/tools/shell.clj
@@ -106,11 +106,7 @@
:description (format "Optional timeout in milliseconds (Default: %s)" default-timeout)}}
:required ["command"]}
:handler #'shell-command
- :require-approval-fn (fn [args {:keys [db]}]
- (when-let [wd (and args (get args "working_directory"))]
- (when-let [wd (and (fs/exists? wd) (str (fs/canonicalize wd)))]
- (let [workspace-roots (mapv (comp shared/uri->filename :uri) (:workspace-folders db))]
- (not-any? #(fs/starts-with? wd %) workspace-roots)))))
+ :require-approval-fn (tools.util/require-approval-when-outside-workspace ["working_directory"])
:summary-fn #'shell-command-summary}})
(defmethod tools.util/tool-call-destroy-resource! :eca_shell_command [name resource-kwd resource]
diff --git a/src/eca/features/tools/util.clj b/src/eca/features/tools/util.clj
index 5d823e5e..7c7fbb37 100644
--- a/src/eca/features/tools/util.clj
+++ b/src/eca/features/tools/util.clj
@@ -1,5 +1,6 @@
(ns eca.features.tools.util
(:require
+ [babashka.fs :as fs]
[cheshire.core :as json]
[clojure.java.io :as io]
[clojure.java.shell :as shell]
@@ -60,6 +61,31 @@
(map #(shared/uri->filename (:uri %)))
(string/join "\n")))
+(defn workspace-root-paths
+ "Returns a vector of workspace root absolute paths from `db`."
+ [db]
+ (mapv (comp shared/uri->filename :uri) (:workspace-folders db)))
+
+(defn path-outside-workspace?
+ "Returns true if `path` is outside any workspace root in `db`.
+ Works for existing or non-existing paths by absolutizing."
+ [db path]
+ (let [p (when path (str (fs/absolutize path)))
+ roots (workspace-root-paths db)]
+ (and p (not-any? #(fs/starts-with? p %) roots))))
+
+(defn require-approval-when-outside-workspace
+ "Returns a function suitable for tool `:require-approval-fn` that triggers
+ approval when any of the provided `path-keys` in args is outside the
+ workspace roots."
+ [path-keys]
+ (fn [args {:keys [db]}]
+ (when (seq path-keys)
+ (some (fn [k]
+ (when-let [p (get args k)]
+ (path-outside-workspace? db p)))
+ path-keys))))
+
(defn command-available? [command & args]
(try
(zero? (:exit (apply shell/sh (concat [command] args))))
@@ -78,3 +104,17 @@
[tool-name]
(-> (io/resource (str "prompts/tools/" tool-name ".md"))
(slurp)))
+
+(defn required-params-error
+ "Given a tool `parameters` JSON schema (object) and an args map, return a
+ single-text-content error when any required parameter is missing. Returns nil
+ if all required parameters are present."
+ [parameters args]
+ (when-let [req (seq (:required parameters))]
+ (let [args (update-keys args name)
+ missing (->> req (map name) (filter #(nil? (get args %))) vec)]
+ (when (seq missing)
+ (single-text-content
+ (format "INVALID_ARGS: missing required params: %s"
+ (->> missing (map #(str "`" % "`")) (string/join ", ")))
+ :error)))))
diff --git a/test/eca/features/tools/filesystem_test.clj b/test/eca/features/tools/filesystem_test.clj
index ea77bc6d..7a7f9c7f 100644
--- a/test/eca/features/tools/filesystem_test.clj
+++ b/test/eca/features/tools/filesystem_test.clj
@@ -23,18 +23,11 @@
((get-in f.tools.filesystem/definitions ["eca_directory_tree" :handler])
{"path" (h/file-path "/foo/qux")}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
- (testing "Unallowed dir"
- (is (match?
- {:error true
- :contents [{:type :text
- :text (format "Access denied - path %s outside allowed directories: %s"
- (h/file-path "/foo/qux")
- (h/file-path "/foo/bar/baz"))}]}
- (with-redefs [fs/canonicalize (constantly (h/file-path "/foo/qux"))
- fs/exists? (constantly true)]
- ((get-in f.tools.filesystem/definitions ["eca_directory_tree" :handler])
- {"path" (h/file-path "/foo/qux")}
- {:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
+ (testing "Approval required outside workspace"
+ (let [require-approval-fn (get-in f.tools.filesystem/definitions ["eca_directory_tree" :require-approval-fn])
+ args {"path" (h/file-path "/foo/qux")}
+ db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}]
+ (is (true? (require-approval-fn args {:db db})))))
(testing "allowed dir"
(is (match?
{:error false
@@ -66,8 +59,7 @@
:contents [{:type :text
:text (format "File %s is not readable" (h/file-path "/foo/qux"))}]}
(with-redefs [fs/exists? (constantly true)
- fs/readable? (constantly false)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/readable? (constantly false)]
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
{"path" (h/file-path "/foo/qux")}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
@@ -78,8 +70,7 @@
:text (format "%s is a directory, not a file" (h/file-path "/foo/dir"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- fs/directory? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/directory? (constantly true)]
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
{"path" (h/file-path "/foo/dir")}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
@@ -91,8 +82,7 @@
(with-redefs [slurp (constantly "fooo")
fs/exists? (constantly true)
fs/readable? (constantly true)
- fs/directory? (constantly false)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/directory? (constantly false)]
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
{"path" (h/file-path "/foo/qux")}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
@@ -103,8 +93,7 @@
:text "line3\nline4\nline5"}]}
(with-redefs [slurp (constantly "line1\nline2\nline3\nline4\nline5")
fs/exists? (constantly true)
- fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/readable? (constantly true)]
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
{"path" (h/file-path "/foo/qux") "line_offset" 2}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
@@ -115,8 +104,7 @@
:text "line1\nline2\n\n[CONTENT TRUNCATED] Showing lines 1 to 2 of 5 total lines. Use line_offset=2 parameter to read more content."}]}
(with-redefs [slurp (constantly "line1\nline2\nline3\nline4\nline5")
fs/exists? (constantly true)
- fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/readable? (constantly true)]
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
{"path" (h/file-path "/foo/qux") "limit" 2}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
@@ -127,24 +115,17 @@
:text "line3\nline4\n\n[CONTENT TRUNCATED] Showing lines 3 to 4 of 5 total lines. Use line_offset=4 parameter to read more content."}]}
(with-redefs [slurp (constantly "line1\nline2\nline3\nline4\nline5")
fs/exists? (constantly true)
- fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/readable? (constantly true)]
((get-in f.tools.filesystem/definitions ["eca_read_file" :handler])
{"path" (h/file-path "/foo/qux") "line_offset" 2 "limit" 2}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}}))))))
(deftest write-file-test
- (testing "Not allowed path"
- (is (match?
- {:error true
- :contents [{:type :text
- :text (format "Access denied - path %s outside allowed directories: %s"
- (h/file-path "/foo/qux/new_file.clj")
- (h/file-path "/foo/bar"))}]}
- (with-redefs [f.tools.filesystem/allowed-path? (constantly false)]
- ((get-in f.tools.filesystem/definitions ["eca_write_file" :handler])
- {"path" (h/file-path "/foo/qux/new_file.clj")}
- {:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar") :name "bar"}]}}))))))
+ (testing "Approval required outside workspace"
+ (let [require-approval-fn (get-in f.tools.filesystem/definitions ["eca_write_file" :require-approval-fn])
+ args {"path" (h/file-path "/foo/qux/new_file.clj")}
+ db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar") :name "bar"}]}]
+ (is (true? (require-approval-fn args {:db db}))))))
(deftest grep-test
(testing "invalid pattern"
@@ -235,8 +216,7 @@
:contents [{:type :text
:text (format "File %s is not readable" (h/file-path "/foo/qux"))}]}
(with-redefs [fs/exists? (constantly true)
- fs/readable? (constantly false)
- f.tools.filesystem/allowed-path? (constantly true)]
+ fs/readable? (constantly false)]
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
{"path" (h/file-path "/foo/qux")
"original_content" "foo"
@@ -250,7 +230,6 @@
:text (format "Original content not found in %s" (h/file-path "/foo/bar/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly "line1\nline2\nline3")]
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
{"path" (h/file-path "/foo/bar/my-file.txt")
@@ -266,7 +245,6 @@
:text (format "Successfully replaced content in %s." (h/file-path "/project/foo/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly "a b a c")
spit (fn [f content] (swap! file-content* assoc f content))]
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
@@ -286,7 +264,6 @@
:text (format "Successfully replaced content in %s." (h/file-path "/project/foo/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly "foo bar foo baz foo")
spit (fn [f content] (swap! file-content* assoc f content))]
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
@@ -309,7 +286,6 @@
:text (format "Successfully replaced content in %s." (h/file-path "/project/foo/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly file-content)
spit (fn [f content] (swap! file-content* assoc f content))]
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
@@ -331,7 +307,6 @@
:text (format "Successfully replaced content in %s." (h/file-path "/project/foo/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly file-content)
spit (fn [f content] (swap! file-content* assoc f content))]
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
@@ -355,7 +330,6 @@
:text (format "Successfully replaced content in %s." path)}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (fn [_]
(let [n (swap! slurp-calls* inc)]
(if (= n 1) initial changed)))
@@ -384,7 +358,6 @@
path)}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (fn [_]
(let [n (swap! slurp-calls* inc)]
(if (= n 1) initial changed)))
@@ -403,8 +376,7 @@
{:error true
:contents [{:type :text
:text (format "%s is not a valid path" (h/file-path "/foo/qux"))}]}
- (with-redefs [fs/exists? (constantly false)
- f.tools.filesystem/allowed-path? (constantly true)]
+ (with-redefs [fs/exists? (constantly false)]
((get-in f.tools.filesystem/definitions ["eca_move_file" :handler])
{"source" (h/file-path "/foo/qux")}
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
@@ -413,8 +385,7 @@
{:error true
:contents [{:type :text
:text (format "Path %s already exists" (h/file-path "/foo/bar/other_file.clj"))}]}
- (with-redefs [fs/exists? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)]
+ (with-redefs [fs/exists? (constantly true)]
((get-in f.tools.filesystem/definitions ["eca_move_file" :handler])
{"source" (h/file-path "/foo/bar/some_file.clj")
"destination" (h/file-path "/foo/bar/other_file.clj")}
@@ -427,7 +398,6 @@
(h/file-path "/foo/bar/some_file.clj")
(h/file-path "/foo/bar/other_file.clj"))}]}
(with-redefs [fs/exists? (fn [path] (not (string/includes? path "other_file.clj")))
- f.tools.filesystem/allowed-path? (constantly true)
fs/move (constantly true)]
((get-in f.tools.filesystem/definitions ["eca_move_file" :handler])
{"source" (h/file-path "/foo/bar/some_file.clj")
@@ -444,7 +414,6 @@
:text (format "Change simulation completed for %s. Original file unchanged - preview only." (h/file-path "/foo/bar/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly original-file-content)
spit (fn [& _] (reset! spit-called* true))]
((get-in f.tools.filesystem/definitions ["eca_preview_file_change" :handler])
@@ -463,7 +432,6 @@
:text (format "Original content not found in %s" (h/file-path "/foo/bar/my-file.txt"))}]}
(with-redefs [fs/exists? (constantly true)
fs/readable? (constantly true)
- f.tools.filesystem/allowed-path? (constantly true)
slurp (constantly "line1\nline2\nline3")
spit (fn [& _] (reset! spit-called* true))]
((get-in f.tools.filesystem/definitions ["eca_preview_file_change" :handler])
@@ -481,7 +449,6 @@
:contents [{:type :text
:text (format "New file creation simulation completed for %s. File will be created - preview only." (h/file-path "/foo/bar/new-file.txt"))}]}
(with-redefs [fs/exists? (constantly false) ; File doesn't exist
- f.tools.filesystem/allowed-path? (constantly true)
spit (fn [& _] (reset! spit-called* true))]
((get-in f.tools.filesystem/definitions ["eca_preview_file_change" :handler])
{"path" (h/file-path "/foo/bar/new-file.txt")
@@ -496,9 +463,8 @@
(is (match?
{:error true
:contents [{:type :text
- :text (format "Preview error for %s: For new files, original_content must be empty string (\"\"). Use markdown blocks during exploration, then eca_preview_file_change for final implementation only." (h/file-path "/foo/bar/missing-file.txt"))}]}
+ :text (format "Preview error for %s: For new files, original_content must be empty string (\"\")." (h/file-path "/foo/bar/missing-file.txt"))}]}
(with-redefs [fs/exists? (constantly false) ; File doesn't exist
- f.tools.filesystem/allowed-path? (constantly true)
spit (fn [& _] (reset! spit-called* true))]
((get-in f.tools.filesystem/definitions ["eca_preview_file_change" :handler])
{"path" (h/file-path "/foo/bar/missing-file.txt")
diff --git a/test/eca/features/tools/shell_test.clj b/test/eca/features/tools/shell_test.clj
index dda3debf..1e16d55d 100644
--- a/test/eca/features/tools/shell_test.clj
+++ b/test/eca/features/tools/shell_test.clj
@@ -107,21 +107,12 @@
(testing "returns nil when working_directory is not provided"
(is (nil? (approval-fn nil {:db db})))
(is (nil? (approval-fn {} {:db db}))))
- (testing "returns nil when working_directory does not exist"
- (with-redefs [fs/exists? (constantly false)]
- (is (nil? (approval-fn {"working_directory" (h/file-path "/project/foo/src")} {:db db})))))
- (testing "returns false when working_directory equals a workspace root"
- (with-redefs [fs/exists? (constantly true)
- fs/canonicalize identity]
- (is (false? (approval-fn {"working_directory" (h/file-path "/project/foo")} {:db db})))))
- (testing "returns false when working_directory is a subdirectory of a workspace root"
- (with-redefs [fs/exists? (constantly true)
- fs/canonicalize identity]
- (is (false? (approval-fn {"working_directory" (h/file-path "/project/foo/src")} {:db db})))))
+ (testing "returns nil when working_directory equals a workspace root"
+ (is (nil? (approval-fn {"working_directory" (h/file-path "/project/foo")} {:db db}))))
+ (testing "returns nil when working_directory is a subdirectory of a workspace root"
+ (is (nil? (approval-fn {"working_directory" (h/file-path "/project/foo/src")} {:db db}))))
(testing "returns true when working_directory is outside any workspace root"
- (with-redefs [fs/exists? (constantly true)
- fs/canonicalize identity]
- (is (true? (approval-fn {"working_directory" (h/file-path "/other/place")} {:db db})))))))
+ (is (true? (approval-fn {"working_directory" (h/file-path "/other/place")} {:db db}))))))
(deftest plan-mode-restrictions-test
(testing "safe commands allowed in plan mode"
diff --git a/test/eca/features/tools_test.clj b/test/eca/features/tools_test.clj
index e994cbd6..f6d6be24 100644
--- a/test/eca/features/tools_test.clj
+++ b/test/eca/features/tools_test.clj
@@ -4,6 +4,7 @@
[eca.config :as config]
[eca.features.tools :as f.tools]
[eca.features.tools.filesystem :as f.tools.filesystem]
+ [eca.features.tools.mcp :as f.mcp]
[eca.test-helper :as h]
[matcher-combinators.matchers :as m]
[matcher-combinators.test :refer [match?]]))
@@ -190,3 +191,89 @@
;; Test safe commands that should NOT be denied
(is (= :ask (f.tools/approval all-tools "eca_shell_command" {"command" "ls -la"} {} config "plan")))
(is (= :ask (f.tools/approval all-tools "eca_shell_command" {"command" "git status"} {} config "plan")))))))
+
+(deftest call-tool!-test
+ (testing "INVALID_ARGS for missing required param on native tool"
+ (is (match?
+ {:error true
+ :contents [{:type :text
+ :text "INVALID_ARGS: missing required params: `path`"}]}
+ (with-redefs [f.tools.filesystem/definitions
+ {"eca_test_native_tool"
+ {:description "Test tool"
+ :parameters {"type" "object"
+ :properties {"path" {:type "string"}}
+ :required ["path"]}
+ :handler (fn [& _]
+ {:error false
+ :contents [{:type :text :text "OK"}]})}}]
+ (f.tools/call-tool!
+ "eca_test_native_tool"
+ {}
+ "chat-1"
+ "call-1"
+ "agent"
+ (h/db*)
+ (h/config)
+ (h/messenger)
+ (h/metrics)
+ identity
+ identity))))))
+
+(deftest call-tool!-mcp-missing-required-test
+ (testing "INVALID_ARGS for missing required param on MCP tool"
+ (is (match?
+ {:error true
+ :contents [{:type :text
+ :text "INVALID_ARGS: missing required params: `code`"}]}
+ (with-redefs [f.mcp/all-tools (fn [_]
+ [{:name "mcp_eval"
+ :server {:name "clojureMCP"}
+ :description "eval code"
+ :parameters {"type" "object"
+ :properties {"code" {:type "string"}}
+ :required ["code"]}}])
+ f.mcp/call-tool! (fn [& _]
+ {:error false
+ :contents [{:type :text :text "should-not-be-called"}]})]
+ (f.tools/call-tool!
+ "mcp_eval"
+ {}
+ "chat-1"
+ "call-2"
+ "agent"
+ (h/db*)
+ (h/config)
+ (h/messenger)
+ (h/metrics)
+ identity
+ identity))))))
+
+(deftest call-tool!-missing-multiple-required-test
+ (testing "INVALID_ARGS for multiple missing required params on native tool"
+ (is (match?
+ {:error true
+ :contents [{:type :text
+ :text "INVALID_ARGS: missing required params: `path`, `content`"}]}
+ (with-redefs [f.tools.filesystem/definitions
+ {"eca_test_native_multi"
+ {:description "Test tool multi"
+ :parameters {"type" "object"
+ :properties {"path" {:type "string"}
+ "content" {:type "string"}}
+ :required ["path" "content"]}
+ :handler (fn [& _]
+ {:error false
+ :contents [{:type :text :text "OK"}]})}}]
+ (f.tools/call-tool!
+ "eca_test_native_multi"
+ {}
+ "chat-2"
+ "call-3"
+ "agent"
+ (h/db*)
+ (h/config)
+ (h/messenger)
+ (h/metrics)
+ identity
+ identity))))))