Enhance plan mode with safe-only tools and improved file operations

- Replace eca_plan_edit_file with eca_preview_file_change tool that simulates file changes without modification
- Add shell command restrictions in plan mode to prevent destructive operations
- Extract text matching logic into dedicated text-match namespace with robust content normalization for CRLF/whitespace differences
- Improve file change error handling with specific messages for not-found, ambiguous matches, and new file creation scenarios
- Add behavior parameter threading through tool system to enable mode-specific tool restrictions
- Enhance file diff generation to support new file creation and improve existing file modification previews
- Add test coverage for plan mode restrictions, text normalization, and preview functionality

Author: Jakub Zika

src/eca/features/chat.clj

@@ -216,7 +216,7 @@
                                             (if @approved?*
                                               (do
                                                 (assert-chat-not-stopped! chat-ctx)
-                                                (let [result (f.tools/call-tool! name arguments @db* config messenger)
+                                                (let [result (f.tools/call-tool! name arguments @db* config messenger behavior)
                                                       details (f.tools/tool-call-details-after-invocation name arguments details result)]
                                                   (add-to-history! {:role "tool_call" :content (assoc tool-call
                                                                                                       :details details
@@ -355,7 +355,11 @@
         rules (f.rules/all config (:workspace-folders db))
         refined-contexts (f.context/raw-contexts->refined contexts db config)
         repo-map* (delay (f.index/repo-map db config {:as-string? true}))
-        instructions (f.prompt/build-instructions refined-contexts rules repo-map* (or behavior (-> config :chat :defaultBehavior)) config)
+        instructions (f.prompt/build-instructions refined-contexts
+                                                  rules
+                                                  repo-map*
+                                                  (or behavior (-> config :chat :defaultBehavior))
+                                                  config)
         chat-ctx {:chat-id chat-id
                   :request-id request-id
                   :contexts contexts

src/eca/features/tools.clj

@@ -55,14 +55,15 @@
       (mapv #(assoc % :origin :native) (native-tools db config))
       (mapv #(assoc % :origin :mcp) (f.mcp/all-tools db))))))
 
-(defn call-tool! [^String name ^Map arguments db config messenger]
+(defn call-tool! [^String name ^Map arguments db config messenger behavior]
   (logger/info logger-tag (format "Calling tool '%s' with args '%s'" name arguments))
   (let [arguments (update-keys arguments clojure.core/name)]
     (try
       (let [result (if-let [native-tool-handler (get-in (native-definitions db config) [name :handler])]
                      (native-tool-handler arguments {:db db
                                                      :config config
-                                                     :messenger messenger})
+                                                     :messenger messenger
+                                                     :behavior behavior})
                      (f.mcp/call-tool! name arguments db))]
         (logger/debug logger-tag "Tool call result: " result)
         result)

src/eca/features/tools/filesystem.clj

@@ -3,8 +3,10 @@
    [babashka.fs :as fs]
    [clojure.java.io :as io]
    [clojure.java.shell :as shell]
+   [eca.logger :as logger]
    [clojure.string :as string]
    [eca.diff :as diff]
+   [eca.features.tools.text-match :as text-match]
    [eca.features.tools.util :as tools.util]
    [eca.shared :as shared]))
 
@@ -203,40 +205,62 @@
     (format "Searching for '%s'" pattern)
     "Searching for files"))
 
-(defn file-change-full-content [path original-content new-content all?]
-  (let [original-full-content (slurp path)
-        new-full-content (if all?
-                           (string/replace original-full-content original-content new-content)
-                           (string/replace-first original-full-content original-content new-content))]
-    (when (string/includes? original-full-content original-content)
-      {:original-full-content original-full-content
-       :new-full-content new-full-content})))
+(defn ^:private handle-file-change-result
+  "Convert file-change-full-content result to appropriate tool response"
+  [result path success-message]
+  (cond
+    (:new-full-content result)
+    (tools.util/single-text-content success-message)
 
-(defn ^:private change-file [arguments {:keys [db]} diff?]
+    (= (:error result) :not-found)
+    (tools.util/single-text-content (format "Original content not found in %s" path) :error)
+
+    (= (:error result) :ambiguous)
+    (tools.util/single-text-content
+     (format "Ambiguous match - content appears %d times in %s. Provide more specific context to identify the exact location."
+             (:match-count result) path) :error)
+
+    :else
+    (tools.util/single-text-content (format "Failed to process %s" path) :error)))
+
+(defn ^:private change-file [arguments {:keys [db]}]
   (or (tools.util/invalid-arguments arguments (concat (path-validations db)
                                                       [["path" fs/readable? "File $path is not readable"]]))
       (let [path (get arguments "path")
             original-content (get arguments "original_content")
             new-content (get arguments "new_content")
-            new-content (if diff?
-                          (str "<<<<<<< HEAD\n"
-                               original-content
-                               "\n=======\n"
-                               new-content
-                               "\n>>>>>>> eca\n")
-                          new-content)
-            all? (boolean (get arguments "all_occurrences"))]
-        (if-let [{:keys [new-full-content]} (file-change-full-content path original-content new-content all?)]
+            all? (boolean (get arguments "all_occurrences"))
+            result (text-match/apply-content-change-to-file path original-content new-content all?)]
+        (if (:new-full-content result)
           (do
-            (spit path new-full-content)
-            (tools.util/single-text-content (format "Successfully replaced content in %s." path)))
-          (tools.util/single-text-content (format "Original content not found in %s" path) :error)))))
+            (spit path (:new-full-content result))
+            (handle-file-change-result result path (format "Successfully replaced content in %s." path)))
+          (handle-file-change-result result path nil)))))
 
 (defn ^:private edit-file [arguments components]
-  (change-file arguments components false))
+  (change-file arguments components))
 
-(defn ^:private plan-edit-file [arguments components]
-  (change-file arguments components true))
+(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 (text-match/apply-content-change-to-file path original-content new-content all?)]
+            (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)]
@@ -284,7 +308,7 @@
                  :required ["path" "content"]}
     :handler #'write-file
     ;; TODO - add behaviors to config and define disabled tools there!
-    :enabled-fn (fn [{:keys [behavior]}] (not= "plan" behavior))
+    :enabled-fn #(not= "plan" (:behavior %))
     :summary-fn #'write-file-summary}
    "eca_edit_file"
    {:description (tools.util/read-tool-description "eca_edit_file")
@@ -299,30 +323,23 @@
                                                   :description "Whether to replace all occurences of the file or just the first one (default)"}}
                   :required ["path" "original_content" "new_content"]}
     :handler #'edit-file
-    :enabled-fn (fn [{:keys [behavior]}] (not= "plan" behavior))
-    :summary-fn (constantly "Editing file")}
-   "eca_plan_edit_file"
-   {:description  (str "Plan a file change where user needs to apply or reject the change. "
-                       "Replace a specific string or content block in a file with new content. "
-                       "Finds the exact original content and replaces it with new content. "
-                       "Be extra careful to format the original-content exactly correctly, "
-                       "taking extra care with whitespace and newlines. "
-                       "Avoid replacing whole functions, methods, or classes, change only the needed code. "
-                       "In addition to replacing strings, this can also be used to prepend, append, or delete contents from a file.")
-    :parameters  {:type "object"
-                  :properties {"path" {:type "string"
-                                       :description "The absolute file path to do the replace."}
-                               "original_content" {:type "string"
-                                                   :description "The exact content to find and replace"}
-                               "new_content" {:type "string"
-                                              :description "The new content to replace the original content with"}
-                               "all_occurrences" {:type "boolean"
-                                                  :description "Whether to replace all occurences of the file or just the first one (default)"}}
-                  :required ["path" "original_content" "new_content"]}
-    :handler #'plan-edit-file
-    ;; TODO improve plan behavior providing better tool for exit plan and present to user.
-    :enabled-fn (constantly false) #_(fn [{:keys [behavior]}] (= "plan" behavior))
-    :summary-fn (constantly "Planning edit")}
+    :enabled-fn #(not= "plan" (:behavior %))
+    :summary-fn (constantly "Editting file")}
+   "eca_preview_file_change"
+   {:description (tools.util/read-tool-description "eca_preview_file_change")
+    :parameters {:type "object"
+                 :properties {"path" {:type "string"
+                                      :description "The absolute file path to preview changes for."}
+                              "original_content" {:type "string"
+                                                  :description "The exact content to find in the file"}
+                              "new_content" {:type "string"
+                                             :description "The content to show as replacement in the preview"}
+                              "all_occurrences" {:type "boolean"
+                                                 :description "Whether to preview replacing all occurrences or just the first one (default)"}}
+                 :required ["path" "original_content" "new_content"]}
+    :handler #'preview-file-change
+    :enabled-fn #(= "plan" (:behavior %))
+    :summary-fn (constantly "Previewing change")}
    "eca_move_file"
    {:description (tools.util/read-tool-description "eca_move_file")
     :parameters  {:type "object"
@@ -332,7 +349,7 @@
                                               :description "The new absolute file path to move to."}}
                   :required ["source" "destination"]}
     :handler #'move-file
-    :enabled-fn (fn [{:keys [behavior]}] (not= "plan" behavior))
+    :enabled-fn #(not= "plan" (:behavior %))
     :summary-fn (constantly "Moving file")}
    "eca_grep"
    {:description (tools.util/read-tool-description "eca_grep")
@@ -353,19 +370,34 @@
   (let [path (get arguments "path")
         original-content (get arguments "original_content")
         new-content (get arguments "new_content")
-        all? (get arguments "all_occurrences")]
-    (when-let [{:keys [original-full-content
-                       new-full-content]} (and path (fs/exists? path) original-content new-content
-                                               (file-change-full-content path original-content new-content all?))]
-      (let [{:keys [added removed diff]} (diff/diff original-full-content new-full-content path)]
+        all? (get arguments "all_occurrences")
+        file-exists? (and path (fs/exists? path))]
+    (cond
+      (and file-exists? original-content new-content)
+      (let [result (text-match/apply-content-change-to-file path original-content new-content all?)
+            original-full-content (:original-full-content result)]
+        (when original-full-content
+          (if-let [new-full-content (:new-full-content result)]
+            (let [{:keys [added removed diff]} (diff/diff original-full-content new-full-content path)]
+              {:type :fileChange
+               :path path
+               :linesAdded added
+               :linesRemoved removed
+               :diff diff})
+            (logger/warn "tool-call-details-before-invocation - NO DIFF GENERATED because match failed for path:" path))))
+
+      (and (not file-exists?) (= original-content "") new-content path)
+      (let [{:keys [added removed diff]} (diff/diff "" new-content path)]
         {:type :fileChange
          :path path
          :linesAdded added
          :linesRemoved removed
-         :diff diff}))))
+         :diff diff})
+
+      :else nil)))
 
-(defmethod tools.util/tool-call-details-before-invocation :eca_plan_edit_file [name arguments]
-  (tools.util/tool-call-details-before-invocation :eca_edit_file name arguments))
+(defmethod tools.util/tool-call-details-before-invocation :eca_preview_file_change [_name arguments]
+  (tools.util/tool-call-details-before-invocation :eca_edit_file arguments))
 
 (defmethod tools.util/tool-call-details-before-invocation :eca_write_file [_name arguments]
   (let [path (get arguments "path")

src/eca/features/tools/shell.clj

@@ -1,26 +1,55 @@
 (ns eca.features.tools.shell
-  (:require
-   [babashka.fs :as fs]
-   [babashka.process :as p]
-   [clojure.string :as string]
-   [eca.config :as config]
-   [eca.features.tools.util :as tools.util]
-   [eca.logger :as logger]
-   [eca.shared :as shared :refer [multi-str]]))
+  (:require [babashka.fs :as fs]
+            [babashka.process :as p]
+            [clojure.string :as string]
+            [eca.config :as config]
+            [eca.features.tools.util :as tools.util]
+            [eca.logger :as logger]
+            [eca.shared :as shared]))
 
 (set! *warn-on-reflection* true)
 
 (def ^:private logger-tag "[TOOLS-SHELL]")
 
-(defn ^:private shell-command [arguments {:keys [db config]}]
+;; Plan mode command restrictions
+;; TODO - We might see that this is not needed and prompt handles it.
+;;        If we don't get 'Command bocked in plan model' error for some time, let's remove it.
+;;        Maybe we should use allowed patterns only, especially for user's custom behaviors.
+(def ^:private plan-safe-commands-default
+  #{"git" "ls" "find" "grep" "rg" "ag" "cat" "head" "tail"
+    "pwd" "which" "file" "stat" "tree" "date" "whoami"
+    "env" "echo" "wc" "du" "df"})
+
+(def ^:private plan-forbidden-patterns-default
+  [#">"                              ; output redirection
+   #"\|\s*(tee|dd|xargs)"            ; dangerous pipes
+   #"\b(sed|awk|perl)\s+.*-i"       ; in-place editing
+   #"\b(rm|mv|cp|touch|mkdir)\b"    ; file operations
+   #"git\s+(add|commit|push)"       ; git mutations
+   #"npm\s+install"                 ; package installs
+   #"-c\s+[\"'].*open.*[\"']w[\"']" ; programmatic writes
+   #"bash.*-c.*>"])                 ; nested shell redirects
+
+(defn ^:private safe-for-plan-mode? [command-string]
+  (let [cmd (-> command-string (string/split #"\s+") first)]
+    (and (contains? plan-safe-commands-default cmd)
+         (not (some #(re-find % command-string) plan-forbidden-patterns-default)))))
+
+(defn ^:private shell-command [arguments {:keys [db config behavior]}]
   (let [command-args (get arguments "command")
         command (first (string/split command-args #"\s+"))
         user-work-dir (get arguments "working_directory")
-        exclude-cmds (-> config :nativeTools :shell :excludeCommands set)]
+        exclude-cmds (-> config :nativeTools :shell :excludeCommands set)
+        plan-mode? (= "plan" behavior)]
     (or (tools.util/invalid-arguments arguments [["working_directory" #(or (nil? %)
                                                                            (fs/exists? %)) "working directory $working_directory does not exist"]
-                                                 ["commmand" (constantly (not (contains? exclude-cmds command))) (format "Cannot run command '%s' because it is excluded by eca config."
-                                                                                                                         command)]])
+                                                 ["commmand" (constantly (not (contains? exclude-cmds command)))
+                                                  (format "Command '%s' is excluded by configuration" command-args)]])
+        ;; Check plan mode restrictions
+        (when (and plan-mode? (not (safe-for-plan-mode? command-args)))
+          {:error true
+           :contents [{:type :text
+                       :text "Command blocked in plan mode. Only read-only analysis commands are allowed."}]})
         (let [work-dir (or (some-> user-work-dir fs/canonicalize str)
                            (some-> (:workspace-folders db)
                                    first