Skip to content

Commit ddffe2b

Browse files
committed
Address runtime debug review feedback
1 parent f2f00e9 commit ddffe2b

4 files changed

Lines changed: 67 additions & 12 deletions

File tree

ai-code-mcp-debug-tools.el

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,14 @@
127127
(defun ai-code-mcp-debug-tools--require-enabled ()
128128
"Signal an error unless debug inspection tools are enabled."
129129
(unless (ai-code-mcp-debug-tools--enabled-p)
130-
(error "Emacs debug MCP tools are disabled for this session")))
130+
(error
131+
"Enable ai-code-mcp-debug-tools-enabled to use the global Emacs debug MCP tools")))
131132

132133
(defun ai-code-mcp-debug-tools--require-eval-enabled ()
133134
"Signal an error unless `eval_elisp' is enabled."
134135
(unless (ai-code-mcp-debug-tools--eval-enabled-p)
135-
(error "The eval_elisp tool is disabled for this session")))
136+
(error
137+
"Enable ai-code-mcp-debug-tools-enable-eval-elisp to use the global eval_elisp MCP tool")))
136138

137139
(defun ai-code-mcp-debug-tools-setup ()
138140
"Register optional MCP debugging tools when enabled."

ai-code.el

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -480,9 +480,11 @@ ARG is the prefix argument."
480480
clipboard-context)))))
481481
(ai-code--insert-prompt final-prompt)))))
482482

483-
(defun ai-code--emacs-runtime-debug-prompt (description enable-eval-elisp)
483+
(defun ai-code--emacs-runtime-debug-prompt (description eval-available-p
484+
request-eval-elisp)
484485
"Return an Emacs runtime debugging prompt from DESCRIPTION.
485-
ENABLE-EVAL-ELISP describes whether `eval_elisp' is available."
486+
EVAL-AVAILABLE-P reports whether `eval_elisp' is globally enabled.
487+
REQUEST-EVAL-ELISP reports whether this debug run may use it."
486488
(format
487489
(concat
488490
"Use the Emacs MCP tools available in this session to debug my Emacs runtime.\n"
@@ -493,9 +495,13 @@ ENABLE-EVAL-ELISP describes whether `eval_elisp' is available."
493495
"Explain what you find, then recommend the smallest fix or next step.\n\n"
494496
"Runtime issue description:\n"
495497
"%s")
496-
(if enable-eval-elisp
497-
"eval_elisp is enabled in your Emacs MCP config."
498-
"eval_elisp is disabled in your Emacs MCP config, so rely on non-eval inspection tools unless you first enable ai-code-mcp-debug-tools-enable-eval-elisp.")
498+
(cond
499+
((and eval-available-p request-eval-elisp)
500+
"eval_elisp is enabled in your Emacs MCP config and is allowed for this debugging run.")
501+
(eval-available-p
502+
"eval_elisp is enabled in your Emacs MCP config, but it was not requested for this debugging run.")
503+
(t
504+
"eval_elisp is disabled in your Emacs MCP config, so rely on non-eval inspection tools unless you first enable ai-code-mcp-debug-tools-enable-eval-elisp."))
499505
description))
500506

501507
;;;###autoload
@@ -508,20 +514,23 @@ ENABLE-EVAL-ELISP describes whether `eval_elisp' is available."
508514
(let* ((description
509515
(ai-code-read-string
510516
"Describe the Emacs runtime issue (it can be an interactive function or a key binding): "))
511-
(enable-eval-elisp
517+
(eval-available-p
518+
(bound-and-true-p ai-code-mcp-debug-tools-enable-eval-elisp))
519+
(request-eval-elisp
512520
(y-or-n-p
513521
"Allow AI to eval Emacs Lisp while debugging this Emacs runtime issue? ")))
514522
(when description
515-
(when (and enable-eval-elisp
516-
(not (bound-and-true-p ai-code-mcp-debug-tools-enable-eval-elisp)))
523+
(when (and request-eval-elisp
524+
(not eval-available-p))
517525
(user-error
518526
"Enable ai-code-mcp-debug-tools-enable-eval-elisp before requesting eval_elisp debugging"))
519527
(when-let* ((prompt
520528
(ai-code-read-string
521529
"Confirm and edit Emacs runtime debug prompt: "
522530
(ai-code--emacs-runtime-debug-prompt
523531
description
524-
enable-eval-elisp))))
532+
eval-available-p
533+
request-eval-elisp))))
525534
(ai-code--insert-prompt prompt)))))
526535

527536
;;;###autoload
@@ -683,7 +692,6 @@ Shows the current backend label to the right."
683692
("p" "Open prompt history file" ai-code-open-prompt-file)
684693
("m" "Debug python MCP server" ai-code-debug-mcp)
685694
("N" "Toggle notifications" ai-code-notifications-toggle)
686-
;; DONE: add a menu item: Debug your emacs runtime. It will temporarily enable ai-code-mcp-debug-tools-enabled, and ask user if they want to enable ai-code-mcp-debug-tools-enable-eval-elisp (eval elisp with AI?) to further help debugging. User can describe what happens (We prompt them that it can debug an interactive function or a key-binding). The final prompt will assemble with user description and then tell AI to user emacs mcp tools to debug. After user confirm the prompt, send to AI.
687695
("d" "Debug Emacs runtime" ai-code-debug-emacs-runtime)
688696
("h" "Help / Quick Start" ai-code-onboarding-open-quickstart))
689697

test/test_ai-code-mcp-debug-tools.el

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,20 @@
131131
(should-error
132132
(ai-code-mcp-eval-elisp "(+ 1 2)"))))
133133

134+
(ert-deftest ai-code-test-mcp-debug-tools-errors-name-global-flags ()
135+
"Global gating errors should point to the relevant defcustom names."
136+
(let ((ai-code-mcp-debug-tools-enabled nil)
137+
(ai-code-mcp-debug-tools-enable-eval-elisp nil))
138+
(should (string-match-p
139+
"ai-code-mcp-debug-tools-enabled"
140+
(error-message-string
141+
(should-error (ai-code-mcp-get-recent-messages)))))
142+
(let ((ai-code-mcp-debug-tools-enabled t))
143+
(should (string-match-p
144+
"ai-code-mcp-debug-tools-enable-eval-elisp"
145+
(error-message-string
146+
(should-error (ai-code-mcp-eval-elisp "(+ 1 2)"))))))))
147+
134148
(ert-deftest ai-code-test-mcp-tools-list-warns-eval-elisp-is-unrestricted ()
135149
"Eval tool metadata should warn about unrestricted side effects."
136150
(let ((ai-code-mcp-server-tools nil)

test/test_ai-code.el

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,37 @@
622622
(should (string-match-p "Describe the Emacs runtime issue"
623623
description-prompt))))
624624

625+
(ert-deftest ai-code-test-debug-emacs-runtime-distinguishes-config-from-run-consent ()
626+
"Debug Emacs runtime should separate global eval availability from per-run consent."
627+
(let (confirm-read-args)
628+
(let ((ai-code-mcp-debug-tools-enabled t)
629+
(ai-code-mcp-debug-tools-enable-eval-elisp t))
630+
(cl-letf (((symbol-function 'y-or-n-p)
631+
(lambda (_prompt) nil))
632+
((symbol-function 'ai-code-read-string)
633+
(lambda (prompt &optional initial-input _candidate-list)
634+
(if (string-match-p "Confirm and edit Emacs runtime debug prompt" prompt)
635+
(progn
636+
(setq confirm-read-args (list prompt initial-input))
637+
initial-input)
638+
"C-c x runs the wrong interactive command")))
639+
((symbol-function 'ai-code--insert-prompt)
640+
(lambda (&rest _args) nil)))
641+
(ai-code-debug-emacs-runtime)))
642+
(should (string-match-p
643+
"eval_elisp is enabled in your Emacs MCP config"
644+
(cadr confirm-read-args)))
645+
(should (string-match-p
646+
"not requested for this debugging run"
647+
(cadr confirm-read-args)))))
648+
649+
(ert-deftest ai-code-test-debug-emacs-runtime-removes-stale-done-comment ()
650+
"The source should not keep the stale DONE note for the runtime debug menu item."
651+
(with-temp-buffer
652+
(insert-file-contents (expand-file-name "ai-code.el" default-directory))
653+
(should-not
654+
(search-forward ";; DONE: add a menu item: Debug your emacs runtime." nil t))))
655+
625656
(ert-deftest ai-code-test-menu-ai-cli-session-includes-select-terminal-entry ()
626657
"Test that the AI CLI session menu exposes terminal backend selection."
627658
(let ((suffix (transient-get-suffix 'ai-code--menu-ai-cli-session "l")))

0 commit comments

Comments
 (0)