Chore: Remove eval_elisp mode and symbol restrictions#322
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad4078513f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (ai-code-mcp-debug-tools--encode-eval-result | ||
| mode | ||
| target-buffer | ||
| before-messages |
There was a problem hiding this comment.
With symbol restrictions removed, eval_elisp can now run forms like (kill-buffer (current-buffer)); after that, this path still unconditionally encodes context_after for the original target-buffer. If that buffer was killed during evaluation, ai-code-mcp-debug-tools--context-summary raises on a dead buffer and the tool call returns an uncaught error instead of the expected JSON payload. This regression is triggered specifically by effectful forms that kill the evaluation buffer and breaks the new unrestricted workflow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. ai-code-mcp-debug-tools--context-summary now checks whether the buffer is still live first. If the target buffer gets killed during eval, the tool still returns a valid JSON payload and context_after degrades to nil instead of raising a second error. I also added an ERT regression test for (kill-buffer (current-buffer)).
There was a problem hiding this comment.
Pull request overview
This PR simplifies the Emacs MCP debug tool eval_elisp by removing its prior safety/mode restrictions, making it an opt-in “full eval” capability intended for troubleshooting and applying fixes from an AI session.
Changes:
- Remove
eval_elispmode handling and symbol deny-lists; evaluation becomes unrestricted once enabled. - Simplify the
eval_elisptool schema and response payload (nomodefield). - Update ERT tests and README documentation to reflect the new behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ai-code-mcp-debug-tools.el |
Removes eval modes/symbol restrictions and updates tool schema + evaluation implementation. |
test/test_ai-code-mcp-debug-tools.el |
Adjusts and adds tests to validate unrestricted eval behavior and updated payload shape. |
README.org |
Documents that eval_elisp has no restrictions once enabled and removes effect-mode instructions. |
| (eval form t))))))) | ||
| (save-current-buffer | ||
| (with-current-buffer target-buffer | ||
| (eval form t)))))) |
There was a problem hiding this comment.
I did not apply this change in this pass. The intent of this PR is to keep eval_elisp as an explicit opt-in full-eval capability so callers can observe the real point / narrowing / side-effect results. Wrapping everything in save-excursion / save-restriction would change that contract, so I left the behavior as-is and only fixed the killed-buffer failure mode.
| #+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. |
There was a problem hiding this comment.
Updated. The README now documents capture_messages and include_backtrace so it matches the current schema and implementation.
| '(:function ai-code-mcp-eval-elisp | ||
| :name "eval_elisp" | ||
| :description "Evaluate a single Emacs Lisp form." | ||
| :args ((:name "code" | ||
| :type string | ||
| :description "Single Emacs Lisp form to evaluate.") |
There was a problem hiding this comment.
Updated. The eval_elisp description exposed via tools/list now makes it explicit that this runs arbitrary Emacs Lisp and can have side effects, so it is less likely to be mistaken for a read-only query tool.
My own user experience: emacs mcp tools are pretty useful during trouble shooting elisp config code or package. If some function or key-binding doesn't works well, we can just ask AI coding session to check that. It will call these tools to understand what happens.
After it identify the root cause, AI might want to make change, and apply it. The current eval_elisp function is heavily restricted for safety purpose, so that it is not easy to use.
This PR Remove all kinds of restriction on it and only left one line to turn it on / off, so that AI can easily use that when needed.
I'll play with it. You can use it with your own risk.