Remove eval_elisp mode and symbol restrictions

Kang Tu · Kang Tu · commit ad4078513fa7 · 2026-04-26T18:24:58.000-07:00
diff --git a/README.org b/README.org
@@ -225,7 +225,7 @@ These tools add:
 - =get_feature_load_state=: inspect whether an Emacs feature is loaded and where it comes from
 - =get_recent_messages=: return the latest lines from =*Messages*=
 - =get_last_error_backtrace=: return the most recently recorded Emacs command error snapshot
-- =eval_elisp=: evaluate a single Emacs Lisp form in a chosen buffer context when explicitly enabled
+- =eval_elisp=: evaluate a single Emacs Lisp form in a chosen buffer context when explicitly enabled (no restrictions once enabled)
 
 The old =ai-code-mcp-editor-tools.el= module has been removed. Its live-editor read tools are now built in, while =eval_elisp= remains an explicit opt-in.
 
@@ -235,11 +235,7 @@ To register =eval_elisp=:
 (setq ai-code-mcp-debug-tools-enable-eval-elisp t)
 #+end_src
 
-=eval_elisp= only allows =query= mode by default. To allow editor-local side effects too:
-
-#+begin_src emacs-lisp
-(setq ai-code-mcp-debug-tools-allow-effect-eval t)
-#+end_src
+Once enabled, =eval_elisp= can evaluate any Emacs Lisp form without restrictions. It takes =code= (a single top-level form), optional =buffer_name= or =file_path= for evaluation context, and optional =timeout_ms=. This is useful for letting the AI apply fixes by evaluating modified function definitions directly.
 
 screenshot inside Codex cli:
 
diff --git a/ai-code-mcp-debug-tools.el b/ai-code-mcp-debug-tools.el
@@ -8,7 +8,6 @@
 
 ;;; Code:
 
-(require 'cl-lib)
 (require 'json)
 (require 'ai-code-mcp-common)
 (require 'nadvice)
@@ -31,11 +30,6 @@
   :type 'boolean
   :group 'ai-code-mcp-debug-tools)
 
-(defcustom ai-code-mcp-debug-tools-allow-effect-eval nil
-  "When non-nil, allow `eval_elisp' to run in effect mode."
-  :type 'boolean
-  :group 'ai-code-mcp-debug-tools)
-
 (defvar ai-code-mcp--last-error-record nil
   "Most recent Emacs error snapshot recorded for MCP diagnostics tools.")
 
@@ -91,10 +85,6 @@
     :args ((:name "code"
             :type string
             :description "Single Emacs Lisp form to evaluate.")
-           (:name "mode"
-            :type string
-            :description "Evaluation mode."
-            :optional t)
            (:name "buffer_name"
             :type string
             :description "Optional buffer context."
@@ -117,21 +107,6 @@
             :optional t)))
   "Optional MCP eval tool specification.")
 
-(defconst ai-code-mcp-debug-tools--always-denied-symbols
-  '(append-to-file async-shell-command call-interactively call-process
-    command-execute compile copy-file delete-file delete-frame
-    delete-window eval funcall kill-buffer load load-file
-    make-directory make-network-process make-process rename-file
-    recompile require save-buffer save-buffers-kill-emacs shell-command
-    start-process url-retrieve write-file write-region)
-  "Symbols that `eval_elisp' rejects in every mode.")
-
-(defconst ai-code-mcp-debug-tools--query-denied-symbols
-  '(add-hook delete-region erase-buffer indent-region insert kill-region
-    newline put remove-hook replace-buffer-contents set setf setq
-    setq-local switch-to-buffer yank)
-  "Additional symbols that `eval_elisp' rejects in query mode.")
-
 (defun ai-code-mcp-debug-tools-setup ()
   "Register optional MCP debugging tools when enabled."
   (when ai-code-mcp-debug-tools-enabled
@@ -224,31 +199,6 @@
       (line . ,(alist-get 'line position))
       (column . ,(alist-get 'column position)))))
 
-(defun ai-code-mcp-debug-tools--symbol-denied-p (form denied-symbols)
-  "Return the first symbol in FORM that appears in DENIED-SYMBOLS."
-  (cond
-   ((symbolp form)
-    (and (memq form denied-symbols) form))
-   ((consp form)
-    (let ((head (car form)))
-      (cond
-       ((and (symbolp head)
-              (memq head denied-symbols))
-        head)
-       (t
-        (or (ai-code-mcp-debug-tools--symbol-denied-p head denied-symbols)
-            (cl-some
-             (lambda (item)
-               (ai-code-mcp-debug-tools--symbol-denied-p
-                item denied-symbols))
-             (cdr form)))))))
-   ((vectorp form)
-    (cl-some
-     (lambda (item)
-       (ai-code-mcp-debug-tools--symbol-denied-p item denied-symbols))
-     (append form nil)))
-   (t nil)))
-
 (defun ai-code-mcp-debug-tools--parse-single-form (code)
   "Parse CODE and return exactly one top-level Emacs Lisp form."
   (let* ((read-result (read-from-string code))
@@ -278,16 +228,15 @@
     (buffer-string)))
 
 (defun ai-code-mcp-debug-tools--encode-eval-result
-    (mode target-buffer before-messages capture-messages timed-out
+    (target-buffer before-messages capture-messages timed-out
           value changed-buffers &optional error-object backtrace)
-  "Return a JSON response for MODE in TARGET-BUFFER.
+  "Return a JSON eval response for TARGET-BUFFER.
 BEFORE-MESSAGES and CAPTURE-MESSAGES control message collection.
 TIMED-OUT records timeout state, VALUE carries the result,
 CHANGED-BUFFERS lists modified buffers, and ERROR-OBJECT or BACKTRACE
 describe failures."
   (json-encode
    `((ok . ,(ai-code-mcp--json-bool (null error-object)))
-     (mode . ,mode)
      (value_repr . ,(and (null error-object) (prin1-to-string value)))
      (value_type . ,(and (null error-object)
                          (symbol-name (type-of value))))
@@ -302,10 +251,10 @@ describe failures."
                         target-buffer))
      (timed_out . ,(ai-code-mcp--json-bool timed-out)))))
 
-(defun ai-code-mcp-debug-tools--run-eval (form mode target-buffer timeout-ms
+(defun ai-code-mcp-debug-tools--run-eval (form target-buffer timeout-ms
                                                capture-messages
                                                include-backtrace)
-  "Evaluate FORM in MODE within TARGET-BUFFER using TIMEOUT-MS.
+  "Evaluate FORM within TARGET-BUFFER using TIMEOUT-MS.
 CAPTURE-MESSAGES controls message collection, and INCLUDE-BACKTRACE
 keeps the backtrace on failures."
   (let ((before-messages (ai-code-mcp--message-lines))
@@ -320,16 +269,9 @@ keeps the backtrace on failures."
                          (setq timed-out t)
                          (throw 'ai-code-mcp-debug-tools-timeout nil))
             (setq value
-                  (if (string= mode "query")
-                      (save-current-buffer
-                        (with-current-buffer target-buffer
-                          (save-excursion
-                            (save-match-data
-                              (save-restriction
-                                (eval form t))))))
-                    (save-current-buffer
-                      (with-current-buffer target-buffer
-                        (eval form t)))))))
+                  (save-current-buffer
+                    (with-current-buffer target-buffer
+                      (eval form t))))))
       (error
        (setq error-object
              (ai-code-mcp-debug-tools--error-alist
@@ -343,7 +285,6 @@ keeps the backtrace on failures."
              "timeout"
              "Evaluation exceeded the configured timeout")))
     (ai-code-mcp-debug-tools--encode-eval-result
-     mode
      target-buffer
      before-messages
      capture-messages
@@ -606,14 +547,14 @@ existing bound variable."
        (limit . ,limit)
        (messages . ,(vconcat messages))))))
 
-(defun ai-code-mcp-eval-elisp (code &optional mode buffer-name file-path
+(defun ai-code-mcp-eval-elisp (code &optional buffer-name file-path
                                     capture-messages include-backtrace
                                     timeout-ms)
-  "Evaluate CODE as a single form using MODE and BUFFER-NAME.
-Return a JSON payload for BUFFER-NAME, FILE-PATH,
-CAPTURE-MESSAGES, INCLUDE-BACKTRACE, and TIMEOUT-MS."
-  (let* ((mode (or mode "query"))
-         (capture-messages (ai-code-mcp-debug-tools--bool-arg
+  "Evaluate CODE as a single Emacs Lisp form.
+Return a JSON payload.  BUFFER-NAME or FILE-PATH select the evaluation
+context.  CAPTURE-MESSAGES, INCLUDE-BACKTRACE, and TIMEOUT-MS control
+diagnostics."
+  (let* ((capture-messages (ai-code-mcp-debug-tools--bool-arg
                             capture-messages
                             t))
          (include-backtrace (ai-code-mcp-debug-tools--bool-arg
@@ -624,93 +565,32 @@ CAPTURE-MESSAGES, INCLUDE-BACKTRACE, and TIMEOUT-MS."
                          buffer-name
                          file-path))
          (parse-error nil)
-         form
-         always-denied
-         query-denied)
-    (unless (member mode '("query" "effect"))
-      (error "Argument mode must be either query or effect"))
+         form)
     (unless (and (integerp timeout-ms) (> timeout-ms 0))
       (error "Argument timeout_ms must be a positive integer"))
     (condition-case err
         (setq form (ai-code-mcp-debug-tools--parse-single-form code))
       (error
        (setq parse-error err)))
-    (cond
-     (parse-error
-      (ai-code-mcp-debug-tools--encode-eval-result
-       mode
-       target-buffer
-       (ai-code-mcp--message-lines)
-       capture-messages
-       nil
-       nil
-       '()
-       (ai-code-mcp-debug-tools--error-alist
-        (symbol-name (car parse-error))
-        (error-message-string parse-error))
-       (and include-backtrace
-            (ai-code-mcp-debug-tools--backtrace-string))))
-     (t
-      (setq always-denied
-            (ai-code-mcp-debug-tools--symbol-denied-p
-             form
-             ai-code-mcp-debug-tools--always-denied-symbols))
-      (setq query-denied
-            (and (string= mode "query")
-                 (ai-code-mcp-debug-tools--symbol-denied-p
-                  form
-                  ai-code-mcp-debug-tools--query-denied-symbols)))
-      (cond
-       (always-denied
-        (ai-code-mcp-debug-tools--encode-eval-result
-         mode
-         target-buffer
-         (ai-code-mcp--message-lines)
-         capture-messages
-         nil
-         nil
-         '()
-         (ai-code-mcp-debug-tools--error-alist
-          "symbol_denied"
-          (format "Symbol `%s' is not allowed in eval_elisp"
-                  always-denied))
-         nil))
-       (query-denied
+    (if parse-error
         (ai-code-mcp-debug-tools--encode-eval-result
-         mode
          target-buffer
          (ai-code-mcp--message-lines)
          capture-messages
          nil
          nil
          '()
          (ai-code-mcp-debug-tools--error-alist
-          "query_symbol_denied"
-          (format "Symbol `%s' is not allowed in query mode"
-                  query-denied))
-         nil))
-       ((and (string= mode "effect")
-             (not ai-code-mcp-debug-tools-allow-effect-eval))
-        (ai-code-mcp-debug-tools--encode-eval-result
-         mode
-         target-buffer
-        (ai-code-mcp--message-lines)
-        capture-messages
-        nil
-        nil
-        '()
-        (ai-code-mcp-debug-tools--error-alist
-         "effect_mode_disabled"
-         "Effect mode is disabled by configuration")
-        nil))
-       (t
-        (ai-code-mcp-debug-tools--run-eval
-         form
-         mode
-         target-buffer
-         timeout-ms
-         capture-messages
-         include-backtrace)))))))
+          (symbol-name (car parse-error))
+          (error-message-string parse-error))
+         (and include-backtrace
+              (ai-code-mcp-debug-tools--backtrace-string)))
+      (ai-code-mcp-debug-tools--run-eval
+       form
+       target-buffer
+       timeout-ms
+       capture-messages
+       include-backtrace))))
 
 (add-to-list 'ai-code-mcp-server-tool-setup-functions
              #'ai-code-mcp-debug-tools-setup)
diff --git a/test/test_ai-code-mcp-debug-tools.el b/test/test_ai-code-mcp-debug-tools.el
@@ -121,8 +121,8 @@
                                (alist-get 'tools tools-result))))
       (should (member "eval_elisp" tool-names)))))
 
-(ert-deftest ai-code-test-mcp-eval-elisp-query-uses-target-buffer ()
-  "Query evaluation should run against the requested buffer context."
+(ert-deftest ai-code-test-mcp-eval-elisp-uses-target-buffer ()
+  "Eval should run against the requested buffer context."
   (let ((ai-code-mcp-server-tools nil)
         (ai-code-mcp-debug-tools-enabled t)
         (ai-code-mcp-debug-tools-enable-eval-elisp t)
@@ -178,21 +178,24 @@
       (when (buffer-live-p target-buffer)
         (kill-buffer target-buffer)))))
 
-(ert-deftest ai-code-test-mcp-eval-elisp-query-rejects-denied-symbols ()
-  "Query evaluation should reject denied symbols before running them."
+(ert-deftest ai-code-test-mcp-eval-elisp-allows-mutation ()
+  "Eval should allow mutation symbols when eval is enabled."
   (let ((ai-code-mcp-server-tools nil)
         (ai-code-mcp-debug-tools-enabled t)
-        (ai-code-mcp-debug-tools-enable-eval-elisp t))
-    (let* ((payload
-            (ai-code-test-mcp-debug-tools--read-json-payload
-             (ai-code-mcp-dispatch
-              "tools/call"
-              '((name . "eval_elisp")
-                (arguments . ((code . "(insert \"boom\")")))))))
-           (error-object (alist-get 'error payload)))
-      (should (equal :json-false (alist-get 'ok payload)))
-      (should (equal "query_symbol_denied"
-                     (alist-get 'type error-object))))))
+        (ai-code-mcp-debug-tools-enable-eval-elisp t)
+        (buffer (generate-new-buffer " *ai-code-mcp-eval-insert*")))
+    (unwind-protect
+        (let ((payload
+               (ai-code-test-mcp-debug-tools--read-json-payload
+                (ai-code-mcp-dispatch
+                 "tools/call"
+                 `((name . "eval_elisp")
+                   (arguments . ((code . "(progn (insert \"boom\") (buffer-string))")
+                                 (buffer_name . ,(buffer-name buffer)))))))))
+          (should (equal t (alist-get 'ok payload)))
+          (should (equal "\"boom\"" (alist-get 'value_repr payload))))
+      (when (buffer-live-p buffer)
+        (kill-buffer buffer)))))
 
 (ert-deftest ai-code-test-mcp-get-variable-value-returns-bound-variable ()
   "Variable value tool should stringify the requested Emacs variable."
@@ -479,6 +482,40 @@
       (should (string-match-p "ai-code-test-mcp-frame-a"
                               (car frames))))))
 
+(ert-deftest ai-code-test-mcp-eval-elisp-allows-previously-denied-symbols ()
+  "Eval should allow all symbols when enabled, including funcall."
+  (let ((ai-code-mcp-server-tools nil)
+        (ai-code-mcp-debug-tools-enabled t)
+        (ai-code-mcp-debug-tools-enable-eval-elisp t))
+    (let ((payload
+           (ai-code-test-mcp-debug-tools--read-json-payload
+            (ai-code-mcp-dispatch
+             "tools/call"
+             '((name . "eval_elisp")
+               (arguments . ((code . "(funcall #'+ 1 2)"))))))))
+      (should (equal t (alist-get 'ok payload)))
+      (should (equal "3" (alist-get 'value_repr payload))))))
+
+(ert-deftest ai-code-test-mcp-eval-elisp-defines-function ()
+  "Eval should define a function and make it callable."
+  (let ((ai-code-mcp-server-tools nil)
+        (ai-code-mcp-debug-tools-enabled t)
+        (ai-code-mcp-debug-tools-enable-eval-elisp t)
+        (func-name "ai-code-test-mcp-eval-defined-func"))
+    (unwind-protect
+        (let ((payload
+               (ai-code-test-mcp-debug-tools--read-json-payload
+                (ai-code-mcp-dispatch
+                 "tools/call"
+                 `((name . "eval_elisp")
+                   (arguments . ((code . "(defun ai-code-test-mcp-eval-defined-func () 42)"))))))))
+          (should (equal t (alist-get 'ok payload)))
+          (should-not (alist-get 'mode payload))
+          (should (fboundp (intern func-name)))
+          (should (= 42 (funcall (intern func-name)))))
+      (when (fboundp (intern-soft func-name))
+        (fmakunbound (intern func-name))))))
+
 (provide 'test_ai-code-mcp-debug-tools)
 
 ;;; test_ai-code-mcp-debug-tools.el ends here
