Skip to content

Commit 3b68b32

Browse files
ericdalloericdallo
committed
Improve eca filesystem calls for better tool usage from LLM
1 parent 5f591dc commit 3b68b32

File tree

4 files changed

+104
-81
lines changed

4 files changed

+104
-81
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

55
- Fix mcp tool calls.
6+
- Improve eca filesystem calls for better tool usage from LLM.
67

78
## 0.15.0
89

‎docs/features.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ Some native tools like `filesystem` have MCP alternatives, but ECA having them b
2020
Provides access to filesystem under workspace root, listing and reading files and directories a subset of [official MCP filesystem](https://mcpserverhub.com/servers/filesystem), important for agentic operations, without the need to support NPM or other tools.
2121

2222
- `eca_read_file`: read a file content.
23-
- `eca_write_file`: write content to file.
23+
- `eca_write_file`: write content to a new file.
24+
- `eca_edit_file`: replace lines of a file with a new content.
2425
- `eca_move_file`: move/rename a file.
2526
- `eca_list_directory`: list a directory.
2627
- `eca_search_files`: search in a path for files matching a pattern.
2728
- `eca_grep`: ripgrep/grep for paths with specified content.
28-
- `eca_replace_in_file`: replace a text with another one in file.
2929

3030
#### Shell
3131

‎src/eca/features/tools/filesystem.clj‎

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -163,21 +163,23 @@
163163
(tools.util/single-text-content (string/join "\n" paths))
164164
(tools.util/single-text-content "No files found for given pattern" :error)))))
165165

166-
(defn ^:private replace-in-file [arguments {:keys [db]}]
166+
(defn ^:private edit-file [arguments {:keys [db]}]
167167
(or (tools.util/invalid-arguments arguments (concat (path-validations db)
168-
[["path" fs/readable? "File $path is not readable"]]))
168+
[["path" fs/readable? "File $path is not readable"]
169+
["start_line" #(and (integer? %) (pos? %)) "$start_line must be a positive integer"]
170+
["end_line" #(and (integer? %) (pos? %)) "$end_line must be a positive integer"]
171+
["content" #(string? %) "$content must be a string"]]))
169172
(let [path (get arguments "path")
170-
original-content (get arguments "original_content")
171-
new-content (get arguments "new_content")
172-
all? (boolean (get arguments "all_occurrences"))
173-
content (slurp path)]
174-
(if (string/includes? content original-content)
175-
(let [content (if all?
176-
(string/replace content original-content new-content)
177-
(string/replace-first content original-content new-content))]
178-
(spit path content)
179-
(tools.util/single-text-content (format "Successfully replaced content in %s." path)))
180-
(tools.util/single-text-content (format "Original content not found in %s" path) :error)))))
173+
start-line (dec (int (get arguments "start_line"))) ; convert 1-based to 0-based
174+
end-line (dec (int (get arguments "end_line"))) ; inclusive, 0-based
175+
new-lines (string/split-lines (get arguments "content"))
176+
old-lines (vec (string/split-lines (slurp path)))
177+
before (subvec old-lines 0 start-line)
178+
after (subvec old-lines (inc end-line))
179+
new-content-lines (vec (concat before new-lines after))
180+
new-file-content (string/join "\n" new-content-lines)]
181+
(spit path new-file-content)
182+
(tools.util/single-text-content (format "Successfully replaced lines %d-%d in %s." (inc start-line) (inc end-line) path)))))
181183

182184
(defn ^:private move-file [arguments {:keys [db]}]
183185
(let [workspace-dirs (tools.util/workspace-roots-strs db)]
@@ -203,11 +205,10 @@
203205
:required ["path"]}
204206
:handler #'list-directory}
205207
"eca_read_file"
206-
{:description (str "Read the complete contents of a file from the file system. "
207-
"Handles various text encodings and provides detailed error messages "
208-
"if the file cannot be read. Use this tool when you need to examine "
208+
{:description (str "Read the contents of a file from the file system. "
209+
"Use this tool when you need to examine "
209210
"the contents of a single file. Optionally use the 'line_offset' and/or 'limit' "
210-
"parameters to read specific contents of the file when you know the lines."
211+
"parameters to read specific contents of the file when you know the range."
211212
"**Only works within the directories: $workspaceRoots.**")
212213
:parameters {:type "object"
213214
:properties {"path" {:type "string"
@@ -219,9 +220,7 @@
219220
:required ["path"]}
220221
:handler #'read-file}
221222
"eca_write_file"
222-
{:description (str "Create a new file or completely overwrite an existing file with new content. "
223-
"Use with caution as it will overwrite existing files without warning. "
224-
"Handles text content with proper encoding. "
223+
{:description (str "Create a new file with new content. To edit existing files use eca_edit_file. "
225224
"**Only works within the directories: $workspaceRoots.**")
226225
:parameters {:type "object"
227226
:properties {"path" {:type "string"
@@ -230,6 +229,21 @@
230229
:description "The content of the new file"}}
231230
:required ["path" "content"]}
232231
:handler #'write-file}
232+
"eca_edit_file"
233+
{:description (str "Change the specified file replacing the lines range with the given content. "
234+
"Make sure to have the existing file content updated via your context or eca_read_file before calling this. "
235+
"This can also be used to prepend, append, or delete contents from a file.")
236+
:parameters {:type "object"
237+
:properties {"path" {:type "string"
238+
:description "The absolute file path to do the replace."}
239+
"start_line" {:type "number"
240+
:description "The 1-based start line number"}
241+
"end_line" {:type "number"
242+
:description "The 1-based end line number"}
243+
"content" {:type "string"
244+
:description "The new content to be inserted"}}
245+
:required ["path" "start_line" "end_line" "content"]}
246+
:handler #'edit-file}
233247
"eca_move_file"
234248
{:description (str "Move or rename files and directories. Can move files between directories "
235249
"and rename them in a single operation. If the destination exists, the "
@@ -274,21 +288,4 @@
274288
"max_results" {:type "integer"
275289
:description "Maximum number of results to return (default: 1000)"}}
276290
:required ["path" "pattern"]}
277-
:handler #'grep}
278-
"eca_replace_in_file"
279-
{:description (str "Replace a specific string or content block in a file with new content. "
280-
"Finds the exact original content and replaces it with new content. "
281-
"Be extra careful to format the original-content exactly correctly, "
282-
"taking extra care with whitespace and newlines. In addition to replacing strings, "
283-
"this can also be used to prepend, append, or delete contents from a file.")
284-
:parameters {:type "object"
285-
:properties {"path" {:type "string"
286-
:description "The absolute file path to do the replace."}
287-
"original_content" {:type "string"
288-
:description "The exact content to find and replace"}
289-
"new_content" {:type "string"
290-
:description "The new content to replace the original content with"}
291-
"all_occurrences" {:type "boolean"
292-
:description "Whether to replace all occurences of the file or just the first one (default)"}}
293-
:required ["path" "original_content" "new_content"]}
294-
:handler #'replace-in-file}})
291+
:handler #'grep}})

‎test/eca/features/tools/filesystem_test.clj‎

Lines changed: 66 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727
{:error true
2828
:contents [{:type :text
2929
:text (format "Access denied - path %s outside allowed directories: %s"
30-
(h/file-path "/foo/qux")
31-
(h/file-path "/foo/bar/baz"))}]}
30+
(h/file-path "/foo/qux")
31+
(h/file-path "/foo/bar/baz"))}]}
3232
(with-redefs [fs/canonicalize (constantly (h/file-path "/foo/qux"))
3333
fs/exists? (constantly true)]
3434
((get-in f.tools.filesystem/definitions ["eca_list_directory" :handler])
@@ -39,9 +39,9 @@
3939
{:error false
4040
:contents [{:type :text
4141
:text (format (str "[FILE] %s\n"
42-
"[DIR] %s\n")
43-
(h/file-path "/foo/bar/baz/some.clj")
44-
(h/file-path "/foo/bar/baz/qux"))}]}
42+
"[DIR] %s\n")
43+
(h/file-path "/foo/bar/baz/some.clj")
44+
(h/file-path "/foo/bar/baz/qux"))}]}
4545
(with-redefs [fs/exists? (constantly true)
4646
fs/starts-with? (constantly true)
4747
fs/list-dir (constantly [(fs/path (h/file-path "/foo/bar/baz/some.clj"))
@@ -119,8 +119,8 @@
119119
{:error true
120120
:contents [{:type :text
121121
:text (format "Access denied - path %s outside allowed directories: %s"
122-
(h/file-path "/foo/qux/new_file.clj")
123-
(h/file-path "/foo/bar"))}]}
122+
(h/file-path "/foo/qux/new_file.clj")
123+
(h/file-path "/foo/bar"))}]}
124124
(with-redefs [f.tools.filesystem/allowed-path? (constantly false)]
125125
((get-in f.tools.filesystem/definitions ["eca_write_file" :handler])
126126
{"path" (h/file-path "/foo/qux/new_file.clj")}
@@ -153,8 +153,8 @@
153153
{:error false
154154
:contents [{:type :text
155155
:text (str (h/file-path "/project/foo/bar/baz.txt") "\n"
156-
(h/file-path "/project/foo/qux.txt") "\n"
157-
(h/file-path "/project/foo/qux.clj"))}]}
156+
(h/file-path "/project/foo/qux.txt") "\n"
157+
(h/file-path "/project/foo/qux.clj"))}]}
158158
(with-redefs [fs/exists? (constantly true)
159159
fs/glob (constantly [(fs/path (h/file-path "/project/foo/bar/baz.txt"))
160160
(fs/path (h/file-path "/project/foo/qux.txt"))
@@ -168,7 +168,7 @@
168168
{:error false
169169
:contents [{:type :text
170170
:text (str (h/file-path "/project/foo/bar/baz.txt") "\n"
171-
(h/file-path "/project/foo/qux.txt"))}]}
171+
(h/file-path "/project/foo/qux.txt"))}]}
172172
(with-redefs [fs/exists? (constantly true)
173173
fs/glob (constantly [(fs/path (h/file-path "/project/foo/bar/baz.txt"))
174174
(fs/path (h/file-path "/project/foo/qux.txt"))])]
@@ -268,63 +268,88 @@
268268
(with-redefs [fs/exists? (constantly true)
269269
fs/readable? (constantly false)
270270
f.tools.filesystem/allowed-path? (constantly true)]
271-
((get-in f.tools.filesystem/definitions ["eca_replace_in_file" :handler])
271+
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
272272
{"path" (h/file-path "/foo/qux")
273-
"original_content" "some-cool-text"
274-
"new_content" "another-boring-text"}
273+
"start_line" 2
274+
"end_line" 3
275+
"content" "updated"}
275276
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar/baz") :name "foo"}]}})))))
276-
(testing "original content not found"
277-
(is (match?
278-
{:error true
279-
:contents [{:type :text
280-
:text (format "Original content not found in %s" (h/file-path "/project/foo/my-file.txt"))}]}
277+
278+
(testing "invalid line range (start_line > end_line)"
279+
(is (thrown?
280+
Exception
281281
(with-redefs [fs/exists? (constantly true)
282282
fs/readable? (constantly true)
283283
f.tools.filesystem/allowed-path? (constantly true)
284-
slurp (constantly "Hey, here is some-cool-text in this file!")]
285-
((get-in f.tools.filesystem/definitions ["eca_replace_in_file" :handler])
286-
{"path" (h/file-path "/project/foo/my-file.txt")
287-
"original_content" "other-cool-text"
288-
"new_content" "another-boring-text"}
289-
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}})))))
290-
(testing "original content found and replaced first"
284+
slurp (constantly "a\nb\nc")] ; just for completeness
285+
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
286+
{"path" (h/file-path "/foo/qux")
287+
"start_line" 3
288+
"end_line" 2
289+
"content" "no-op"}
290+
{:db {:workspace-folders [{:uri (h/file-uri "file:///foo/bar") :name "bar"}]}})))))
291+
292+
(testing "replace 2nd and 3rd lines in file"
291293
(let [file-content* (atom {})]
292294
(is (match?
293295
{:error false
294296
:contents [{:type :text
295-
:text (format "Successfully replaced content in %s." (h/file-path "/project/foo/my-file.txt"))}]}
297+
:text (format "Successfully replaced lines 2-3 in %s." (h/file-path "/project/foo/my-file.txt"))}]}
296298
(with-redefs [fs/exists? (constantly true)
297299
fs/readable? (constantly true)
298300
f.tools.filesystem/allowed-path? (constantly true)
299-
slurp (constantly "Hey, here is some-cool-text in this file! here as well: some-cool-text")
301+
slurp (constantly "line1\nold2\nold3\nline4")
300302
spit (fn [f content] (swap! file-content* assoc f content))]
301-
((get-in f.tools.filesystem/definitions ["eca_replace_in_file" :handler])
303+
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
302304
{"path" (h/file-path "/project/foo/my-file.txt")
303-
"original_content" "some-cool-text"
304-
"new_content" "another-boring-text"}
305+
"start_line" 2
306+
"end_line" 3
307+
"content" "new2\nnew3"}
305308
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}}))))
306309
(is (match?
307-
{(h/file-path "/project/foo/my-file.txt") "Hey, here is another-boring-text in this file! here as well: some-cool-text"}
310+
{(h/file-path "/project/foo/my-file.txt") "line1\nnew2\nnew3\nline4"}
308311
@file-content*))))
309-
(testing "original content found and replaced all"
312+
313+
(testing "replace first line only in file"
314+
(let [file-content* (atom {})]
315+
(is (match?
316+
{:error false
317+
:contents [{:type :text
318+
:text (format "Successfully replaced lines 1-1 in %s." (h/file-path "/project/foo/my-file.txt"))}]}
319+
(with-redefs [fs/exists? (constantly true)
320+
fs/readable? (constantly true)
321+
f.tools.filesystem/allowed-path? (constantly true)
322+
slurp (constantly "orig1\nkeep2\nkeep3")
323+
spit (fn [f content] (swap! file-content* assoc f content))]
324+
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
325+
{"path" (h/file-path "/project/foo/my-file.txt")
326+
"start_line" 1
327+
"end_line" 1
328+
"content" "updated1"}
329+
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}}))))
330+
(is (match?
331+
{(h/file-path "/project/foo/my-file.txt") "updated1\nkeep2\nkeep3"}
332+
@file-content*))))
333+
334+
(testing "replace last line in file"
310335
(let [file-content* (atom {})]
311336
(is (match?
312337
{:error false
313338
:contents [{:type :text
314-
:text (format "Successfully replaced content in %s." (h/file-path "/project/foo/my-file.txt"))}]}
339+
:text (format "Successfully replaced lines 4-4 in %s." (h/file-path "/project/foo/my-file.txt"))}]}
315340
(with-redefs [fs/exists? (constantly true)
316341
fs/readable? (constantly true)
317342
f.tools.filesystem/allowed-path? (constantly true)
318-
slurp (constantly "Hey, here is some-cool-text in this file! here as well: some-cool-text")
343+
slurp (constantly "keep1\nkeep2\nkeep3\nlast")
319344
spit (fn [f content] (swap! file-content* assoc f content))]
320-
((get-in f.tools.filesystem/definitions ["eca_replace_in_file" :handler])
345+
((get-in f.tools.filesystem/definitions ["eca_edit_file" :handler])
321346
{"path" (h/file-path "/project/foo/my-file.txt")
322-
"original_content" "some-cool-text"
323-
"new_content" "another-boring-text"
324-
"all_occurrences" true}
347+
"start_line" 4
348+
"end_line" 4
349+
"content" "newlast"}
325350
{:db {:workspace-folders [{:uri (h/file-uri "file:///project/foo") :name "foo"}]}}))))
326351
(is (match?
327-
{(h/file-path "/project/foo/my-file.txt") "Hey, here is another-boring-text in this file! here as well: another-boring-text"}
352+
{(h/file-path "/project/foo/my-file.txt") "keep1\nkeep2\nkeep3\nnewlast"}
328353
@file-content*)))))
329354

330355
(deftest move-file-test
@@ -354,8 +379,8 @@
354379
{:error false
355380
:contents [{:type :text
356381
:text (format "Successfully moved %s to %s"
357-
(h/file-path "/foo/bar/some_file.clj")
358-
(h/file-path "/foo/bar/other_file.clj"))}]}
382+
(h/file-path "/foo/bar/some_file.clj")
383+
(h/file-path "/foo/bar/other_file.clj"))}]}
359384
(with-redefs [fs/exists? (fn [path] (not (string/includes? path "other_file.clj")))
360385
f.tools.filesystem/allowed-path? (constantly true)
361386
fs/move (constantly true)]

0 commit comments

Comments
 (0)