Merge pull request #571 from Scott-Guest/kill-buffer

Fix unkillable buffer after major-mode change

Author: xenodium

agent-shell.el

@@ -2366,22 +2366,21 @@ DIFF should be in the form returned by `agent-shell--make-diff-info':
   "Clean up resources.
 
 For example, shut down ACP client."
-  (unless (derived-mode-p 'agent-shell-mode)
-    (error "Not in a shell"))
-  (agent-shell--cancel-idle-timer)
-  (agent-shell--emit-event :event 'clean-up)
-  (agent-shell--shutdown)
-  ;; Kill any open diff buffers associated with tool calls.
-  (map-do (lambda (_tool-call-id tool-call-data)
-            (when-let ((diff-buf (map-elt tool-call-data :diff-buffer)))
-              (agent-shell-diff-kill-buffer diff-buf)))
-          (map-elt (agent-shell--state) :tool-calls))
-  (when-let (((map-elt (agent-shell--state) :buffer))
-             (viewport-buffer (agent-shell-viewport--buffer
-                               :shell-buffer (map-elt (agent-shell--state) :buffer)
-                               :existing-only t))
-             (buffer-live-p viewport-buffer))
-    (kill-buffer viewport-buffer)))
+  (when (derived-mode-p 'agent-shell-mode)
+    (agent-shell--cancel-idle-timer)
+    (agent-shell--emit-event :event 'clean-up)
+    (agent-shell--shutdown)
+    ;; Kill any open diff buffers associated with tool calls.
+    (map-do (lambda (_tool-call-id tool-call-data)
+              (when-let ((diff-buf (map-elt tool-call-data :diff-buffer)))
+                (agent-shell-diff-kill-buffer diff-buf)))
+            (map-elt (agent-shell--state) :tool-calls))
+    (when-let (((map-elt (agent-shell--state) :buffer))
+               (viewport-buffer (agent-shell-viewport--buffer
+                                 :shell-buffer (map-elt (agent-shell--state) :buffer)
+                                 :existing-only t))
+               (buffer-live-p viewport-buffer))
+      (kill-buffer viewport-buffer))))
 
 (defun agent-shell--shutdown ()
   "Shut down shell activity."
@@ -2774,6 +2773,7 @@ variable (see makunbound)"))
       (setq-local filter-buffer-substring-function #'agent-shell--filter-buffer-substring)
       (agent-shell--update-header-and-mode-line)
       (add-hook 'kill-buffer-hook #'agent-shell--clean-up nil t)
+      (add-hook 'change-major-mode-hook #'agent-shell--clean-up nil t)
       (agent-shell-ui-mode +1)
       (when agent-shell-file-completion-enabled
         (agent-shell-completion-mode +1))

tests/agent-shell-tests.el

@@ -2315,5 +2315,27 @@ that fallback buffer, potentially starting the new shell in the wrong project."
   ;; Empty input returns empty output.
   (should (equal (agent-shell--sort-sessions-by-recency '()) '())))
 
+(ert-deftest agent-shell--clean-up-tolerates-mode-change-test ()
+  "Test `kill-buffer' succeeds after the major mode is manually changed.
+
+`kill-buffer-hook' is permanent-local, so the buffer-local
+`agent-shell--clean-up' entry survives a mode change,
+and it must handle that cleanly."
+  (let ((shell-buf (generate-new-buffer " *test-shell*")))
+    (unwind-protect
+        (progn
+          (with-current-buffer shell-buf
+            (setq major-mode 'agent-shell-mode)
+            (setq-local agent-shell--state
+                        (agent-shell--make-state :buffer shell-buf))
+            (add-hook 'kill-buffer-hook #'agent-shell--clean-up nil t)
+            (text-mode))
+          (kill-buffer shell-buf)
+          (should-not (buffer-live-p shell-buf)))
+      (when (buffer-live-p shell-buf)
+        (with-current-buffer shell-buf
+          (remove-hook 'kill-buffer-hook #'agent-shell--clean-up t))
+        (kill-buffer shell-buf)))))
+
 (provide 'agent-shell-tests)
 ;;; agent-shell-tests.el ends here