refactor(org-refile): name the two refile advices so they can be removed

- Both were anonymous lambdas advising org-refile and org-refile-get-targets.
- An anonymous advice can't be advice-removed by reference.
- So deleting one from source left a live daemon still running it.
- Extracted both to named defuns and installed them remove-then-add.
- Now each reloads to a single copy and is removable by reference.
- Added unit tests for the target-visiting logic, which had no coverage.

2 files changed, 114 insertions, 14 deletions

diff --git a/modules/org-refile-config.el b/modules/org-refile-config.el
index 5f826cac..d94e4965 100644
--- a/modules/org-refile-config.el
+++ b/modules/org-refile-config.el
@@ -185,6 +185,25 @@ ARG DEFAULT-BUFFER RFLOC and MSG parameters passed to org-refile."
 
 ;; --------------------------------- Org Refile --------------------------------
 
+(declare-function org-save-all-org-buffers "org")
+
+(defun cj/org-refile--save-all-buffers (&rest _)
+  "Save every open Org buffer.  Installed as `:after' advice on `org-refile'.
+Named (not an anonymous lambda) so the :config reload can `advice-remove'
+it by reference and a test can assert its installation."
+  (org-save-all-org-buffers))
+
+(defun cj/org-refile--ensure-targets-in-org-mode (&rest _)
+  "Put every string-named refile target buffer into `org-mode' first.
+Installed as `:before' advice on `org-refile-get-targets'.  Fixes targets
+opened before Org loaded getting stuck in `fundamental-mode'.  A non-string
+target car (a function or symbol spec) is skipped.  Named for the same
+remove-by-reference and testability reasons as the save helper above."
+  (dolist (target org-refile-targets)
+    (let ((file (car target)))
+      (when (stringp file)
+        (cj/org-refile-ensure-org-mode file)))))
+
 (use-package org-refile
   :ensure nil ;; built-in
   :defer .5
@@ -193,20 +212,14 @@ ARG DEFAULT-BUFFER RFLOC and MSG parameters passed to org-refile."
 		("C-c C-w"   . cj/org-refile)
 		("C-c w"     . cj/org-refile-in-file))
   :config
-  ;; save all open org buffers after a refile is complete
-  (advice-add 'org-refile :after
-			  (lambda (&rest _)
-				(org-save-all-org-buffers)))
-
-  ;; Ensure refile target buffers are in org-mode before processing
-  ;; Fixes issue where buffers opened before org loaded get stuck in fundamental-mode
-  (advice-add 'org-refile-get-targets :before
-              (lambda (&rest _)
-                "Ensure all refile target buffers are in org-mode."
-                (dolist (target org-refile-targets)
-                  (let ((file (car target)))
-                    (when (stringp file)
-                      (cj/org-refile-ensure-org-mode file)))))))
+  ;; Install both advices by named-function reference with a remove-then-add
+  ;; guard.  Anonymous lambdas here couldn't be `advice-remove'd (deleting the
+  ;; advice from source left a live daemon still running it) and couldn't be
+  ;; tested; the named helpers above are both.
+  (advice-remove 'org-refile #'cj/org-refile--save-all-buffers)
+  (advice-add 'org-refile :after #'cj/org-refile--save-all-buffers)
+  (advice-remove 'org-refile-get-targets #'cj/org-refile--ensure-targets-in-org-mode)
+  (advice-add 'org-refile-get-targets :before #'cj/org-refile--ensure-targets-in-org-mode))
 
 (provide 'org-refile-config)
 ;;; org-refile-config.el ends here.
diff --git a/tests/test-org-refile-config--advice-helpers.el b/tests/test-org-refile-config--advice-helpers.el
new file mode 100644
index 00000000..0d9979d8
--- /dev/null
+++ b/tests/test-org-refile-config--advice-helpers.el
@@ -0,0 +1,87 @@
+;;; test-org-refile-config--advice-helpers.el --- Tests for the refile advice helpers -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Unit tests for the two named advice helpers extracted from anonymous lambdas
+;; in the org-refile use-package :config block:
+;;
+;;   cj/org-refile--save-all-buffers          (:after org-refile)
+;;   cj/org-refile--ensure-targets-in-org-mode (:before org-refile-get-targets)
+;;
+;; They were anonymous `(lambda (&rest _) ...)' advices, which cannot be
+;; `advice-remove'd by reference and cannot be tested.  Naming them makes both
+;; possible.  The install-by-reference and removability are verified live in the
+;; daemon (the :config block doesn't run under batch make test); these tests
+;; pin the extracted logic.
+;;
+;; Test organization:
+;; - Normal Cases: ensure-targets visits each string-named target
+;; - Boundary Cases: empty targets, non-string cars, mixed list
+;; - Error Cases: a nil target list is a no-op
+;;
+;;; Code:
+
+(require 'ert)
+(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
+(require 'org-refile-config)
+
+;; The module's bare `(defvar org-refile-targets)' marks the symbol special only
+;; within its own file, so it isn't special here.  Declare it (with a value) so
+;; the `let' bindings below bind it dynamically, the way the sibling
+;; test-org-refile-config-commands.el does.
+(defvar org-refile-targets nil)
+
+;;; cj/org-refile--ensure-targets-in-org-mode
+
+(ert-deftest test-org-refile-ensure-targets-visits-each-string-target ()
+  "Normal: every string-named target file is passed to the ensure helper."
+  (let ((org-refile-targets '(("/a.org" :maxlevel . 3)
+                              ("/b.org" :maxlevel . 3)))
+        (seen '()))
+    (cl-letf (((symbol-function 'cj/org-refile-ensure-org-mode)
+               (lambda (f) (push f seen))))
+      (cj/org-refile--ensure-targets-in-org-mode))
+    (should (equal '("/a.org" "/b.org") (nreverse seen)))))
+
+;;; Boundary
+
+(ert-deftest test-org-refile-ensure-targets-skips-non-string-cars ()
+  "Boundary: a target whose car is not a string (a function/symbol spec) is
+skipped rather than passed to the ensure helper."
+  (let ((org-refile-targets `((,(lambda () '("/x.org")) :maxlevel . 2)
+                              ("/real.org" :maxlevel . 2)
+                              (org-agenda-files :maxlevel . 2)))
+        (seen '()))
+    (cl-letf (((symbol-function 'cj/org-refile-ensure-org-mode)
+               (lambda (f) (push f seen))))
+      (cj/org-refile--ensure-targets-in-org-mode))
+    (should (equal '("/real.org") seen))))
+
+(ert-deftest test-org-refile-ensure-targets-empty-list-is-noop ()
+  "Boundary: no targets means the ensure helper is never called."
+  (let ((org-refile-targets '())
+        (called nil))
+    (cl-letf (((symbol-function 'cj/org-refile-ensure-org-mode)
+               (lambda (_f) (setq called t))))
+      (cj/org-refile--ensure-targets-in-org-mode))
+    (should-not called)))
+
+;;; Error
+
+(ert-deftest test-org-refile-ensure-targets-nil-targets-does-not-signal ()
+  "Error: a nil `org-refile-targets' completes without signaling."
+  (let ((org-refile-targets nil))
+    (cl-letf (((symbol-function 'cj/org-refile-ensure-org-mode) #'ignore))
+      (should (progn (cj/org-refile--ensure-targets-in-org-mode) t)))))
+
+;;; cj/org-refile--save-all-buffers
+
+(ert-deftest test-org-refile-save-all-buffers-delegates ()
+  "Normal: the save helper calls `org-save-all-org-buffers'."
+  (let ((called nil))
+    (cl-letf (((symbol-function 'org-save-all-org-buffers)
+               (lambda (&rest _) (setq called t))))
+      (cj/org-refile--save-all-buffers))
+    (should called)))
+
+(provide 'test-org-refile-config--advice-helpers)
+;;; test-org-refile-config--advice-helpers.el ends here