branch: externals/ellama
commit baf078dcb0080a426a6ae38dabe22fc8bc0096db
Author: Sergey Kostyaev <[email protected]>
Commit: Sergey Kostyaev <[email protected]>
tools: warn LLM when reading binary content
---
ellama-tools.el | 82 ++++++++++++++++++++++++++++++++++++++++----------
tests/test-ellama.el | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 151 insertions(+), 15 deletions(-)
diff --git a/ellama-tools.el b/ellama-tools.el
index a349f27492..d88d6fa0df 100644
--- a/ellama-tools.el
+++ b/ellama-tools.el
@@ -306,13 +306,38 @@ TOOL-PLIST is a property list in the format expected by
`llm-make-tool'."
(interactive)
(setq ellama-tools-enabled nil))
+(defun ellama-tools--string-has-raw-bytes-p (string)
+ "Return non-nil when STRING contain binary-like chars.
+Treat Emacs raw-byte chars and NUL bytes as binary-like."
+ (let ((idx 0)
+ (len (length string))
+ found)
+ (while (and (not found) (< idx len))
+ (when (or (> (aref string idx) #x10FFFF)
+ (= (aref string idx) 0))
+ (setq found t))
+ (setq idx (1+ idx)))
+ found))
+
+(defun ellama-tools--sanitize-tool-text-output (text label)
+ "Return TEXT or a warning when TEXT is binary-like.
+LABEL is used to identify the source in the warning."
+ (if (ellama-tools--string-has-raw-bytes-p text)
+ (concat label
+ " appears to contain binary data. Reading binary data as "
+ "text is a bad idea for this tool.")
+ text))
+
(defun ellama-tools-read-file-tool (file-name)
"Read the file FILE-NAME."
(json-encode (if (not (file-exists-p file-name))
(format "File %s doesn't exists." file-name)
- (with-temp-buffer
- (insert-file-contents-literally file-name)
- (buffer-string)))))
+ (let ((content (with-temp-buffer
+ (insert-file-contents file-name)
+ (buffer-string))))
+ (ellama-tools--sanitize-tool-text-output
+ content
+ (format "File %s" file-name))))))
(ellama-tools-define-tool
'(:function
@@ -522,17 +547,42 @@ Replace OLDCONTENT with NEWCONTENT."
(defun ellama-tools-shell-command-tool (callback cmd)
"Execute shell command CMD.
CALLBACK – function called once with the result string."
- (let ((buf (get-buffer-create (concat (make-temp-name " *ellama shell
command") "*"))))
- (set-process-sentinel
- (start-process "*ellama-shell-command*" buf shell-file-name
shell-command-switch cmd)
- (lambda (process _)
- (when (not (process-live-p process))
- (funcall callback
- ;; we need to trim trailing newline
- (string-trim-right
- (with-current-buffer buf (buffer-string))
- "\n"))
- (kill-buffer buf)))))
+ (condition-case err
+ (let ((buf (get-buffer-create
+ (concat (make-temp-name " *ellama shell command") "*"))))
+ (set-process-sentinel
+ (start-process
+ "*ellama-shell-command*" buf shell-file-name shell-command-switch cmd)
+ (lambda (process _)
+ (when (not (process-live-p process))
+ (let* ((raw-output
+ ;; trim trailing newline to reduce noisy tool output
+ (string-trim-right
+ (with-current-buffer buf (buffer-string))
+ "\n"))
+ (output
+ (ellama-tools--sanitize-tool-text-output
+ raw-output
+ "Command output"))
+ (exit-code (process-exit-status process))
+ (result
+ (cond
+ ((and (string= output "") (zerop exit-code))
+ "Command completed successfully with no output.")
+ ((string= output "")
+ (format "Command failed with exit code %d and no output."
+ exit-code))
+ ((zerop exit-code)
+ output)
+ (t
+ (format "Command failed with exit code %d.\n%s"
+ exit-code output)))))
+ (funcall callback result)
+ (kill-buffer buf))))))
+ (error
+ (funcall callback
+ (format "Failed to start shell command: %s"
+ (error-message-string err)))))
;; async tool should always return nil
;; to work properly with the llm library
nil)
@@ -691,7 +741,9 @@ ANSWER-VARIANT-LIST is a list of possible answer
variants."))
(forward-line (1- to))
(end-of-line)
(point))))
- (buffer-substring-no-properties start end))))))
+ (ellama-tools--sanitize-tool-text-output
+ (buffer-substring-no-properties start end)
+ (format "File %s" file-name)))))))
(ellama-tools-define-tool
'(:function
diff --git a/tests/test-ellama.el b/tests/test-ellama.el
index 375e06abff..8f0e9864b8 100644
--- a/tests/test-ellama.el
+++ b/tests/test-ellama.el
@@ -31,6 +31,12 @@
(require 'ert)
(require 'llm-fake)
+(defconst ellama-test-root
+ (expand-file-name
+ ".."
+ (file-name-directory (or load-file-name buffer-file-name)))
+ "Project root directory for test assets.")
+
(ert-deftest test-ellama--code-filter ()
(should (equal "" (ellama--code-filter "")))
(should (equal "(hello)" (ellama--code-filter "(hello)")))
@@ -1023,6 +1029,84 @@ region, season, or type)! 🍎🍊"))))
(should (equal (ellama--string-without-last-two-lines "Line1\nLine2")
"")))
+(defun ellama-test--ensure-local-ellama-tools ()
+ "Ensure tests use local `ellama-tools.el' from project root."
+ (unless (fboundp 'ellama-tools--sanitize-tool-text-output)
+ (load-file (expand-file-name "ellama-tools.el" ellama-test-root))))
+
+(defun ellama-test--wait-shell-command-result (cmd)
+ "Run shell tool CMD and wait for a result string."
+ (ellama-test--ensure-local-ellama-tools)
+ (let ((result :pending)
+ (deadline (+ (float-time) 3.0)))
+ (ellama-tools-shell-command-tool
+ (lambda (res)
+ (setq result res))
+ cmd)
+ (while (and (eq result :pending)
+ (< (float-time) deadline))
+ (accept-process-output nil 0.01))
+ (when (eq result :pending)
+ (ert-fail (format "Timeout while waiting result for: %s" cmd)))
+ result))
+
+(ert-deftest test-ellama-shell-command-tool-empty-success-output ()
+ (should
+ (string=
+ (ellama-test--wait-shell-command-result "sh -c 'true'")
+ "Command completed successfully with no output.")))
+
+(ert-deftest test-ellama-shell-command-tool-empty-failure-output ()
+ (should
+ (string-match-p
+ "Command failed with exit code 7 and no output\\."
+ (ellama-test--wait-shell-command-result "sh -c 'exit 7'"))))
+
+(ert-deftest test-ellama-shell-command-tool-returns-stdout ()
+ (should
+ (string=
+ (ellama-test--wait-shell-command-result "printf 'ok\\n'")
+ "ok")))
+
+(ert-deftest test-ellama-shell-command-tool-rejects-binary-output ()
+ (should
+ (string-match-p
+ "binary data"
+ (ellama-test--wait-shell-command-result
+ "awk 'BEGIN { printf \"%c\", 0 }'"))))
+
+(ert-deftest test-ellama-read-file-tool-rejects-binary-content ()
+ (ellama-test--ensure-local-ellama-tools)
+ (let ((file (make-temp-file "ellama-read-file-bin-")))
+ (unwind-protect
+ (progn
+ (let ((coding-system-for-write 'no-conversion))
+ (with-temp-buffer
+ (set-buffer-multibyte nil)
+ (insert "%PDF-1.5\n%")
+ (insert (char-to-string 143))
+ (insert "\n")
+ (write-region (point-min) (point-max) file nil 'silent)))
+ (let ((result (ellama-tools-read-file-tool file)))
+ (should (string-match-p "binary data" result))
+ (should (string-match-p "bad idea" result))))
+ (when (file-exists-p file)
+ (delete-file file)))))
+
+(ert-deftest test-ellama-read-file-tool-accepts-utf8-markdown-text ()
+ (ellama-test--ensure-local-ellama-tools)
+ (let ((file (make-temp-file "ellama-read-file-utf8-" nil ".md")))
+ (unwind-protect
+ (progn
+ (with-temp-file file
+ (insert "# Research Plan\n\n")
+ (insert "Sub‑topics: temporal reasoning overview.\n"))
+ (let ((result (ellama-tools-read-file-tool file)))
+ (should-not (string-match-p "binary data" result))
+ (should (string-match-p "Research Plan" result))))
+ (when (file-exists-p file)
+ (delete-file file)))))
+
(provide 'test-ellama)
;;; test-ellama.el ends here
