Merge pull request #8 from rhblind/7-tool-calling

7 tool calling

Author: rhblind

CLAUDE.md

@@ -102,6 +102,23 @@ This is an Emacs MCP (Model Context Protocol) Server implementation written in p
                 (required . ("expression")))
 ```
 
+### JSON Boolean Values
+
+**NEVER use `:json-false` in this codebase. It is banned.**
+
+Always use `t` for JSON `true` and `:false` for JSON `false`:
+```elisp
+;; Correct
+:annotations '((readOnlyHint . t)
+               (destructiveHint . :false))
+
+;; BANNED - will break the MCP protocol!
+:annotations '((readOnlyHint . t)
+               (destructiveHint . :json-false))  ; DO NOT USE
+```
+
+Why: `:json-false` is what Emacs returns when *parsing* JSON input, but `json-serialize` only accepts `:false` for output. This codebase produces JSON for the MCP protocol. Using `:json-false` will cause serialization errors and break client communication.
+
 ### Autoload Cookies
 
 **Use `;;;###autoload` only for user-facing interactive commands:**
@@ -164,6 +181,12 @@ Respects `mcp-server-tools-filter' - disabled tools cannot be called."
 
 This project requires **Emacs 27.1+** for native JSON support (`json-serialize`, `json-parse-string`).
 
+### Version Bumping
+
+When releasing a new version, update **both** locations in `mcp-server.el`:
+1. Header comment: `;; Version: X.Y.Z`
+2. Runtime constant: `(defconst mcp-server-version "X.Y.Z" ...)`
+
 ## Key Architecture Components
 
 ### Modular Transport System
@@ -252,11 +275,49 @@ Tools can be selectively enabled via `mcp-server-emacs-tools-enabled`:
 
 ## Security Model
 
-### Permission System
-- Dangerous operations require user confirmation
-- Permission decisions are cached per session  
+### Tool Annotations (MCP Specification)
+
+Tools expose behavior hints via annotations that MCP clients use to determine
+whether to prompt users for permission:
+
+| Annotation | Description |
+|------------|-------------|
+| `readOnlyHint` | `true` if tool doesn't modify anything |
+| `destructiveHint` | `true` if tool may cause destructive changes |
+| `idempotentHint` | `true` if repeated calls have no additional effect |
+| `openWorldHint` | `true` if tool interacts with external entities |
+
+Current tool annotations:
+- `eval-elisp`: destructive, non-idempotent, open-world (can do anything)
+- `get-diagnostics`: read-only, idempotent, closed-world (safe)
+
+### Permission Handling
+
+The security model has two layers:
+
+1. **MCP client prompting** - Clients like Claude Code use tool annotations
+   (`destructiveHint`, etc.) to decide whether to prompt users for tool access.
+
+2. **Emacs blocklist** - Dangerous functions (e.g., `delete-file`, `shell-command`)
+   are always blocked, regardless of whether the tool was allowed by the client.
+
+By default (`mcp-server-security-prompt-for-permissions` = `nil`):
+- Dangerous operations are **blocked silently** (no minibuffer prompt)
+- Safe operations are allowed
+- The blocklist is always enforced
+
+To enable Emacs-side prompting (approve dangerous operations case-by-case):
+```elisp
+(setq mcp-server-security-prompt-for-permissions t)
+```
+
+This prompts in the minibuffer instead of blocking, letting users approve
+dangerous operations individually.
+
+### Permission Caching
+- Permission decisions are cached per session
 - Comprehensive audit trail of all actions
-- Configurable prompting behavior
+- View audit log: `M-x mcp-server-security-show-audit-log`
 
 ### Input Validation
 - JSON Schema validation for all tool inputs
@@ -266,7 +327,7 @@ Tools can be selectively enabled via `mcp-server-emacs-tools-enabled`:
 ### Execution Sandboxing
 - 30-second default timeout for operations
 - Memory usage monitoring
-- Restricted access to dangerous functions
+- Restricted access to dangerous functions (when Emacs prompting enabled)
 
 ## Client Integration Examples
 
@@ -416,4 +477,4 @@ The modular transport design allows adding new transport mechanisms:
 - Implement the `mcp-server-transport` interface
 - Register with `mcp-server-transport-register`
 - Support for stdio, TCP, WebSocket, etc.
-- Always update relevant files with significant changes. For example, when changing tests make sure to update the tests/README.md file.
+- Always update relevant files with significant changes. For example, when changing tests make sure to update the tests/README.md file.

mcp-server-security.el

@@ -88,8 +88,13 @@ Use this to whitelist specific functions you trust the LLM to use freely."
   :type 'integer
   :group 'mcp-server)
 
-(defcustom mcp-server-security-prompt-for-permissions t
-  "Whether to prompt user for dangerous operations."
+(defcustom mcp-server-security-prompt-for-permissions nil
+  "Whether to prompt user in Emacs for dangerous operations.
+When nil (the default), dangerous operations are blocked without prompting.
+The MCP client uses tool annotations to determine whether to prompt for
+tool-level permission, but the blocklist is always enforced.
+Set to t to prompt in the Emacs minibuffer instead of blocking, allowing
+users to approve dangerous operations on a case-by-case basis."
   :type 'boolean
   :group 'mcp-server)
 
@@ -148,7 +153,7 @@ Returns t if permitted, nil otherwise."
           ('no
            (mcp-server-security--log-audit operation data nil)
            nil)))
-    ;; If not prompting, deny dangerous operations by default
+    ;; When not prompting, still block dangerous operations
     (let ((granted (not (mcp-server-security--is-dangerous-operation operation))))
       (puthash cache-key granted mcp-server-security--permission-cache)
       (mcp-server-security--log-audit operation data granted)
@@ -213,20 +218,20 @@ Returns the input if safe, signals an error otherwise."
     ;; Check for shell command injection
     (when (string-match-p "[;&|`$]" input)
       (error "Input contains potentially dangerous shell characters"))
-    
+
     ;; Check for path traversal
     (when (string-match-p "\\.\\./\\|~/" input)
       (error "Input contains potentially dangerous path patterns"))
-    
+
     ;; Check for excessive length
     (when (> (length input) 10000)
       (error "Input exceeds maximum length")))
-  
+
   ;; Check for suspicious elisp code patterns in strings
   (when (and (stringp input)
              (string-match-p "(\\s-*\\(?:eval\\|load\\|shell-command\\)" input))
     (error "Input contains potentially dangerous elisp patterns"))
-  
+
   input)
 
 (defun mcp-server-security-sanitize-string (str)
@@ -245,7 +250,7 @@ Returns the input if safe, signals an error otherwise."
   "Safely evaluate FORM with security restrictions."
   ;; Check if form contains dangerous functions
   (mcp-server-security--check-form-safety form)
-  
+
   ;; Execute with timeout and memory limits
   (mcp-server-security--execute-with-limits
    (lambda () (eval form))))
@@ -258,8 +263,9 @@ Returns the input if safe, signals an error otherwise."
     (when (and (member form mcp-server-security-dangerous-functions)
                (not (member form mcp-server-security-allowed-dangerous-functions)))
       (unless (mcp-server-security-check-permission form)
-        (error "Permission denied for function: %s" form))))
-   
+        (error "Security: `%s' is blocked. Add it to `mcp-server-security-allowed-dangerous-functions' \
+to allow, or set `mcp-server-security-prompt-for-permissions' to t to prompt" form))))
+
    ;; Check lists (function calls)
    ((listp form)
     (when form
@@ -270,26 +276,27 @@ Returns the input if safe, signals an error otherwise."
           (when (and (member func mcp-server-security-dangerous-functions)
                      (not (member func mcp-server-security-allowed-dangerous-functions)))
             (unless (mcp-server-security-check-permission func args)
-              (error "Permission denied for function: %s" func)))
-          
+              (error "Security: `%s' is blocked. Add it to `mcp-server-security-allowed-dangerous-functions' \
+to allow, or set `mcp-server-security-prompt-for-permissions' to t to prompt" func)))
+
           ;; Special checks for file access functions
           (when (memq func '(find-file find-file-noselect view-file insert-file-contents))
             (let ((file-path (car args)))
               (when (and file-path (stringp file-path))
                 (when (mcp-server-security--is-sensitive-file file-path)
-                  (unless (mcp-server-security-check-permission 
+                  (unless (mcp-server-security-check-permission
                            (format "access-sensitive-file:%s" func) file-path)
                     (error "Permission denied for sensitive file access: %s" file-path))))))
-          
+
           ;; Special checks for buffer access functions
           (when (memq func '(switch-to-buffer set-buffer with-current-buffer))
-            (let ((buffer-name (if (eq func 'with-current-buffer) 
+            (let ((buffer-name (if (eq func 'with-current-buffer)
                                    (car args)
                                  (car args))))
               (when (and buffer-name (stringp buffer-name))
                 (when (mcp-server-security--is-sensitive-buffer buffer-name)
                   (error "Access denied to sensitive buffer: %s" buffer-name))))))
-        
+
         ;; Recursively check arguments
         (dolist (arg args)
           (mcp-server-security--check-form-safety arg)))))))
@@ -298,18 +305,18 @@ Returns the input if safe, signals an error otherwise."
   "Execute FUNC with time and memory limits."
   (let ((start-time (current-time))
         (start-gc-cons-threshold gc-cons-threshold))
-    
+
     ;; Set conservative GC threshold for memory monitoring
     (setq gc-cons-threshold 1000000)
-    
+
     (unwind-protect
         (with-timeout (mcp-server-security-max-execution-time
                        (error "Execution timeout exceeded"))
           (funcall func))
-      
+
       ;; Restore GC threshold
       (setq gc-cons-threshold start-gc-cons-threshold)
-      
+
       ;; Log execution time
       (let ((elapsed (float-time (time-subtract (current-time) start-time))))
         (when (> elapsed 5.0)
@@ -324,7 +331,7 @@ Returns the input if safe, signals an error otherwise."
                  (data . ,data)
                  (granted . ,granted))))
     (push entry mcp-server-security--audit-log)
-    
+
     ;; Keep only last 1000 entries
     (when (> (length mcp-server-security--audit-log) 1000)
       (setq mcp-server-security--audit-log

mcp-server-transport.el

@@ -82,7 +82,7 @@
 
 (defun mcp-server-transport-send (transport-name client-id message)
   "Send MESSAGE to CLIENT-ID using transport NAME."
-  (mcp-server-transport--debug "transport-send START - transport=%s, client=%s" 
+  (mcp-server-transport--debug "transport-send START - transport=%s, client=%s"
                                transport-name client-id)
   (let ((transport (mcp-server-transport-get transport-name)))
     (mcp-server-transport--debug "transport object = %S" transport)
@@ -150,9 +150,9 @@ This bypasses the normal JSON serialization."
     :null)
    ;; If it's an alist (list of key-value pairs where each pair is (key . value))
    ;; Check: list of cons cells where each cons has a non-list car (the key)
-   ((and (listp obj) 
-         (not (null obj)) 
-         (seq-every-p (lambda (item) 
+   ((and (listp obj)
+         (not (null obj))
+         (seq-every-p (lambda (item)
                         (and (consp item)
                              (not (listp (car item))))) obj))
     (let ((ht (make-hash-table :test 'equal)))
@@ -162,7 +162,7 @@ This bypasses the normal JSON serialization."
                      (car pair)))
               (value (mcp-server-transport--alist-to-json (cdr pair))))
           (when (string= key "isError")
-            (mcp-server-transport--debug "Processing isError - raw value: %S, converted value: %S" 
+            (mcp-server-transport--debug "Processing isError - raw value: %S, converted value: %S"
                                          (cdr pair) value))
           (puthash key value ht)))
       ht))
@@ -172,16 +172,16 @@ This bypasses the normal JSON serialization."
    ;; If it's a symbol, convert to string (except for special values)
    ((symbolp obj)
     (cond
-     ((eq obj t) 
+     ((eq obj t)
       (mcp-server-transport--debug "Converting symbol t to JSON true")
       t)
-     ((eq obj :null) 
+     ((eq obj :null)
       (mcp-server-transport--debug "Converting symbol :null to JSON null")
       :null)
-     ((eq obj :false) 
+     ((eq obj :false)
       (mcp-server-transport--debug "Converting symbol :false to JSON false")
       :false)
-     (t 
+     (t
       (mcp-server-transport--debug "Unexpected symbol %S (name: %s, type: %s) being converted to string"
                                    obj (symbol-name obj) (type-of obj))
       (symbol-name obj))))
@@ -199,10 +199,10 @@ This bypasses the normal JSON serialization."
   "Validate that MESSAGE is a proper JSON-RPC 2.0 message."
   (unless (alist-get 'jsonrpc message)
     (error "Missing jsonrpc field"))
-  
+
   (unless (string= (alist-get 'jsonrpc message) "2.0")
     (error "Invalid jsonrpc version: %s" (alist-get 'jsonrpc message)))
-  
+
   message)
 
 ;;; Client Management Utilities
@@ -226,22 +226,22 @@ This bypasses the normal JSON serialization."
 Returns updated buffer with remaining partial data."
   (let ((combined (concat buffer new-data))
         (remaining ""))
-    
+
     ;; Process complete lines
     (while (string-match "\n" combined)
       (let* ((line-end (match-end 0))
              (line (substring combined 0 (1- line-end))))
-        
+
         ;; Remove processed line from combined buffer
         (setq combined (substring combined line-end))
-        
+
         ;; Process the line if it's not empty
         (when (> (length (string-trim line)) 0)
           (condition-case err
               (funcall line-processor line)
             (error
              (mcp-server-transport--error "Error processing line: %s" (error-message-string err)))))))
-    
+
     ;; Return remaining partial data
     combined))