diff --git a/README.md b/README.md index f055fc5..8cc9e97 100644 --- a/README.md +++ b/README.md @@ -277,12 +277,13 @@ You can see customizable variables/sub-groups with `M-x customize-group RET mach ### Tools -| Variable | Description | -| -------------------------- | -------------------------------------------- | -| `macher-tools` | Tool definitions for reading/editing files | -| `macher-presets-alist` | Preset definitions (macher, macher-ro, etc.) | -| `macher-tool-category` | Category for macher tools in gptel registry | -| `macher-match-max-columns` | Max line length for search results | +| Variable | Description | +| ------------------------------- | ------------------------------------------------------------------ | +| `macher-tools` | Tool definitions for reading/editing files | +| `macher-presets-alist` | Preset definitions (macher, macher-ro, etc.) | +| `macher-tool-category` | Category for macher tools in gptel registry | +| `macher-match-max-columns` | Max line length for search results | +| `macher-tool-output-max-length` | Max characters any macher tool may return (errors out if exceeded) | ## FAQ diff --git a/macher.el b/macher.el index 6d09d0b..aa58a56 100644 --- a/macher.el +++ b/macher.el @@ -784,7 +784,7 @@ adding tools to this category directly; instead, customize "(offset/limit) and line numbering.\n" "\n" "Returns file contents on success. Fails if the file is not found in the workspace " - "or if the content exceeds the maximum read length." + "or if the content exceeds the maximum tool output length." macher--workspace-postfix) :confirm nil :include nil @@ -1091,6 +1091,16 @@ Set to nil to disable the limit entirely." :type '(choice (natnum :tag "Maximum number of characters") (const :tag "No limit" nil)) :group 'macher-tools) +(defcustom macher-tool-output-max-length (* 1024 1024) + "Maximum number of characters any macher tool may return to the LLM. + +Tools that produce text output (e.g. read, search, directory listing) +signal an error if their result exceeds this length, rather than +dumping a large payload into the LLM context. Lower this if your +model has a small context window." + :type 'natnum + :group 'macher-tools) + (defcustom macher-presets-alist `( ;; Enable all macher tools. @@ -1206,11 +1216,6 @@ To add a new workspace type, add an entry to this alist and update :type '(alist :key-type symbol :value-type (plist :key-type keyword :value-type function)) :group 'macher-workspace) -;;; Constants - -(defconst macher--max-read-length (* 1024 1024) - "Max number of bytes to return from the read tool.") - ;;; Variables (defvar-local macher--workspace nil @@ -2205,6 +2210,27 @@ a redundant computation over a remote connection." ;; The workspace tools are somewhat inspired by the reference filesystem MCP. See ;; https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/README.md. +(defun macher--check-output-length (output description) + "Signal an error if OUTPUT exceeds `macher-tool-output-max-length'. +DESCRIPTION names what is being returned (e.g. \"File content\") and is +included verbatim in the error message. When the limit is exceeded, +the error includes the leading and trailing portions of OUTPUT so the +caller can see the structure of the truncated payload." + (let ((len (length output))) + (when (> len macher-tool-output-max-length) + (let* ((preview-chars (min 256 (/ len 2))) + (head (substring output 0 preview-chars)) + (tail (substring output (- len preview-chars)))) + (error + "%s too large: %d characters exceeds maximum tool output length of %d characters\nFirst %d characters:\n%s\n...\nLast %d characters:\n%s" + description + len + macher-tool-output-max-length + preview-chars + head + preview-chars + tail))))) + (defun macher--tool-read-file (context path &optional offset limit show-line-numbers) "Read the contents of a file specified by PATH within the workspace. @@ -2251,12 +2277,7 @@ found in the workspace." (round limit))) (processed-content (macher--read-string new-content parsed-offset parsed-limit show-line-numbers))) - ;; Check if the processed content exceeds the maximum read length. - (when (> (length processed-content) macher--max-read-length) - (error - "File content too large: %d bytes exceeds maximum read length of %d bytes" - (length processed-content) - macher--max-read-length)) + (macher--check-output-length processed-content "File content") processed-content))))))) (defun macher--tool-list-directory (context path &optional recursive sizes) @@ -2502,9 +2523,12 @@ Signals an error if the directory is not found in the workspace." (collect-entries full-path "" 0)) ;; Return formatted results. - (if results - (string-join (reverse results) "\n") - "Directory is empty"))) + (let ((output + (if results + (string-join (reverse results) "\n") + "Directory is empty"))) + (macher--check-output-length output "Directory listing") + output))) (defun macher--tool-edit-file (context path old-text new-text &optional replace-all) "Edit file specified by PATH within the workspace. @@ -3172,6 +3196,7 @@ piping the results through `head -N`." (let ((lines (split-string output "\n"))) (when (> (length lines) parsed-head-limit) (setq output (string-join (seq-take lines parsed-head-limit) "\n"))))) + (macher--check-output-length output "Search output") output)))) (defun macher--tool-search diff --git a/tests/test-integration.el b/tests/test-integration.el index f303b26..6d9b5fe 100644 --- a/tests/test-integration.el +++ b/tests/test-integration.el @@ -720,9 +720,9 @@ SILENT and INHIBIT-COOKIES are ignored in this mock implementation." ;; Second request should contain numbered lines (cat -n style). (expect (cadr tool-messages) :to-equal '("1\tline1\n2\tline2\n3\tline3\n4\tline4")))))) - (it "returns error when file content exceeds max read length" - ;; Create a file with content that exceeds macher--max-read-length - (let* ((large-content (make-string (1+ macher--max-read-length) ?x)) + (it "returns error when file content exceeds max tool output length" + ;; Create a file with content that exceeds macher-tool-output-max-length + (let* ((large-content (make-string (1+ macher-tool-output-max-length) ?x)) (large-file-path (expand-file-name "large-file.txt" project-dir))) (unwind-protect (progn @@ -762,7 +762,7 @@ SILENT and INHIBIT-COOKIES are ignored in this mock implementation." (let ((error-message (cadr tool-messages))) (expect (length error-message) :to-be 1) (expect "File content too large" :to-appear-once-in (car error-message)) - (expect "exceeds maximum read length" + (expect "exceeds maximum tool output length" :to-appear-once-in (car error-message))))))) ;; Clean up the large file (when (file-exists-p large-file-path) diff --git a/tests/test-unit.el b/tests/test-unit.el index dfc0d64..11f117c 100644 --- a/tests/test-unit.el +++ b/tests/test-unit.el @@ -903,6 +903,54 @@ ;; Should match the literal string "array[0]", not as regex. (expect result :to-equal "list[0] = value"))))) + (describe "macher--check-output-length" + (it "returns nil when output is within the limit" + (let ((macher-tool-output-max-length 100)) + (expect (macher--check-output-length "short" "Thing") :to-be nil) + ;; Boundary: exactly at the limit is allowed. + (expect (macher--check-output-length (make-string 100 ?x) "Thing") :to-be nil))) + + (it "signals an error when output exceeds the limit" + (let ((macher-tool-output-max-length 10) + (output (make-string 11 ?x))) + (expect (macher--check-output-length output "Thing") :to-throw 'error))) + + (it "includes description, sizes, and head/tail previews in the error" + ;; Use a large-enough payload that the preview is capped at 256 bytes. + (let* ((macher-tool-output-max-length 10) + (head (make-string 256 ?A)) + (mid (make-string 600 ?M)) + (tail (make-string 256 ?Z)) + (output (concat head mid tail)) + (err + (condition-case e + (macher--check-output-length output "Search output") + (error + e))) + (msg (error-message-string err))) + (expect msg :to-match "\\`Search output too large: 1112 characters") + (expect msg :to-match "maximum tool output length of 10 characters") + (expect msg :to-match "First 256 characters:\n") + (expect msg :to-match "Last 256 characters:\n") + ;; Previews reflect the actual head/tail of the payload. + (expect msg :to-match (regexp-quote head)) + (expect msg :to-match (regexp-quote tail)) + ;; The middle of the payload is omitted. + (expect msg :not :to-match (regexp-quote mid)))) + + (it "shrinks the preview to half the payload for small over-length outputs" + ;; A 5-byte payload over a 1-byte cap previews 2 bytes (= 5/2) on each end. + (let* ((macher-tool-output-max-length 1) + (output "abcde") + (err + (condition-case e + (macher--check-output-length output "Thing") + (error + e))) + (msg (error-message-string err))) + (expect msg :to-match "First 2 characters:\nab") + (expect msg :to-match "Last 2 characters:\nde")))) + (describe "macher--format-size" (it "formats bytes correctly" (expect (macher--format-size 0) :to-equal "0 B") @@ -1361,7 +1409,12 @@ ;; Clean up the external directory (when (file-exists-p external-dir) - (delete-directory external-dir t)))))) + (delete-directory external-dir t))))) + + (it "errors when output exceeds the configured max" + ;; Lower the cap so we don't need a huge directory to exercise the path. + (let ((macher-tool-output-max-length 5)) + (expect (macher--tool-list-directory context ".") :to-throw 'error)))) (describe "macher--search-get-xref-matches" :var (context temp-dir) @@ -2824,6 +2877,11 @@ ;; 0.6 should round to 1 (1 extra line). (expect result-0.6 :to-match "hello")))) + (it "errors when output exceeds the configured max" + ;; Lower the cap to force the error without needing a huge fixture. + (let ((macher-tool-output-max-length 5)) + (expect (macher--tool-search-helper context "hello") :to-throw 'error))) + (it "search handles nonexistent files in workspace gracefully" ;; When workspace-files includes a file that doesn't exist on disk, ;; search should still work, treating the missing file as empty.