fix:org-roam: copy completed tasks to dailies on state transitions

  Fixed and enhanced the org-roam hook that copies completed tasks to
  daily notes:

  - Fixed hook not triggering immediately after Emacs launch by moving it
    outside the lazy-loaded use-package org-roam :config block and into
    with-eval-after-load 'org

  - Changed hook to trigger for ANY org-done-keyword (DONE, CANCELLED,
    etc.) instead of just "DONE"

  - Updated hook to only trigger on non-done → done transitions using
    org-last-state, preventing duplicate copies when changing between
    done states (e.g., DONE → CANCELLED)

  - Added docstrings to org-roam helper functions to fix checkdoc linter
    warnings

  - Created comprehensive ERT test suite with 10 tests covering:
    * Hook registration before org-roam loads (lazy-loading fix)
    * Transitions to done states (nil→DONE, TODO→DONE, IN-PROGRESS→DONE,
      WAITING→CANCELLED)
    * Non-triggering cases (done→done, transitions to non-done states)

2 files changed, 202 insertions, 14 deletions

diff --git a/modules/org-roam-config.el b/modules/org-roam-config.el
index c497fd4b..18552b1d 100644
--- a/modules/org-roam-config.el
+++ b/modules/org-roam-config.el
@@ -69,15 +69,17 @@
   ;; remove/disable if performance slows
   ;; (setq org-element-use-cache nil) ;; disables caching org files
 
-  ;; move closed tasks to today's journal when marked done
-  (add-to-list 'org-after-todo-state-change-hook
-               (lambda ()
-                 (when (equal org-state "DONE")
-                   (cj/org-roam-copy-todo-to-today))))
-
   (require 'org-roam-dailies)      ;; Ensures the keymap is available
   (org-roam-db-autosync-mode))
 
+;; Move closed tasks to today's journal when marked done
+(with-eval-after-load 'org
+  (add-to-list 'org-after-todo-state-change-hook
+               (lambda ()
+                 (when (and (member org-state org-done-keywords)
+                            (not (member org-last-state org-done-keywords)))
+                   (cj/org-roam-copy-todo-to-today)))))
+
 ;; ------------------------- Org Roam Insert Immediate -------------------------
 
 (defun cj/org-roam-node-insert-immediate (arg &rest args)
@@ -95,10 +97,12 @@ the arguments that org-roam-node-insert expects."
 ;; ------------------------- Tag Listing And Filtering -------------------------
 
 (defun cj/org-roam-filter-by-tag (tag-name)
+  "Return a predicate function that filters org-roam nodes by TAG-NAME."
   (lambda (node)
     (member tag-name (org-roam-node-tags node))))
 
 (defun cj/org-roam-list-notes-by-tag (tag-name)
+  "Return a list of file paths for all org-roam nodes tagged with TAG-NAME."
   (mapcar #'org-roam-node-file
           (seq-filter
            (cj/org-roam-filter-by-tag tag-name)
@@ -106,12 +110,12 @@ the arguments that org-roam-node-insert expects."
 
 ;; -------------------------- Org Roam Find Functions --------------------------
 
-;;;###autoload
 (defun cj/org-roam-find-node (tag template-key template-file &optional subdir)
   "List all nodes of type TAG in completing read for selection or creation.
 Interactively find or create an Org-roam node with a given TAG. Newly
 created nodes are added to the agenda and follow a template defined by
-TEMPLATE-KEY and TEMPLATE-FILE."
+TEMPLATE-KEY and TEMPLATE-FILE. If SUBDIR is provided, new nodes are
+created in that subdirectory of `org-roam-directory'."
   (interactive)
   (add-hook 'org-capture-after-finalize-hook
 			#'cj/org-roam-add-node-to-agenda-files-finalize-hook)
@@ -122,14 +126,15 @@ TEMPLATE-KEY and TEMPLATE-FILE."
 	  :if-new (file+head ,(concat (or subdir "") "%<%Y%m%d%H%M%S>-${slug}.org") "")
 	  :unnarrowed t))))
 
-;;;###autoload
+
 (defun cj/org-roam-find-node-topic ()
-  "List nodes of type \=`topic\=` in completing read for selection or creation."
+  "List nodes of type \=`Topic\=` in completing read for selection or creation."
   (interactive)
   (cj/org-roam-find-node "Topic" "t" (concat roam-dir "templates/topic.org")))
 
-;;;###autoload
+
 (defun cj/org-roam-find-node-recipe ()
+  "List nodes of type \"Recipe\" in completing read for selection or creation."
   (interactive)
   (cj/org-roam-find-node "Recipe" "r" (concat roam-dir "templates/recipe.org") "recipes/"))
 
@@ -185,12 +190,13 @@ Otherwise return TEXT unchanged."
 		(or description url))
 	text))
 
-;;;###autoload
+
 (defun cj/move-org-branch-to-roam ()
   "Move the org subtree at point to a new org-roam node.
 The node filename will be timestamp-based with the heading name.
-The heading becomes the node title, and the entire subtree is demoted to level 1.
-If the heading contains a link, extract the description for the title."
+The heading becomes the node title, and the entire subtree is demoted to
+level 1.  If the heading contains a link, extract the description for the
+title."
   (interactive)
   ;; Lazy load org and org-roam when needed
   (require 'org)
diff --git a/tests/test-org-roam-config-copy-todo-to-today.el b/tests/test-org-roam-config-copy-todo-to-today.el
new file mode 100644
index 00000000..bcac5a26
--- /dev/null
+++ b/tests/test-org-roam-config-copy-todo-to-today.el
@@ -0,0 +1,182 @@
+;;; test-org-roam-config-copy-todo-to-today.el --- Tests for org-roam TODO completion hook -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; Tests for the org-after-todo-state-change-hook configuration that copies
+;; completed tasks to daily org-roam nodes.
+;;
+;; The hook should trigger for ANY org-mode done state (DONE, CANCELLED, etc.),
+;; not just "DONE". This is verified by checking membership in org-done-keywords.
+;;
+;; The critical behavior being tested is that the hook is registered
+;; immediately when org-mode loads, NOT when org-roam loads (which happens
+;; lazily). This ensures tasks can be copied to dailies even before the user
+;; has invoked any org-roam commands.
+
+;;; Code:
+
+(require 'ert)
+(require 'testutil-general)
+
+;;; Setup and Teardown
+
+(defun test-org-roam-todo-hook-setup ()
+  "Setup for org-roam todo hook tests."
+  (cj/create-test-base-dir))
+
+(defun test-org-roam-todo-hook-teardown ()
+  "Teardown for org-roam todo hook tests."
+  (cj/delete-test-base-dir))
+
+;;; Normal Cases
+
+(ert-deftest test-org-roam-hook-registered-after-org-loads ()
+  "The hook should be registered after org loads."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (progn
+        (require 'org)
+        (require 'org-roam-config)
+        (should (consp org-after-todo-state-change-hook)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-calls-copy-function-on-done ()
+  "The hook lambda should call copy function when state is DONE."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))  ; Set done keywords for test
+        (setq org-last-state nil)   ; No previous state (new task)
+        (setq org-state "DONE")     ; Dynamic variable used by org-mode hooks
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-calls-copy-function-on-cancelled ()
+  "The hook lambda should call copy function when state is CANCELLED."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))  ; Set done keywords for test
+        (setq org-last-state nil)       ; No previous state (new task)
+        (setq org-state "CANCELLED")    ; Dynamic variable used by org-mode hooks
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+;;; Boundary Cases
+
+(ert-deftest test-org-roam-hook-registered-before-org-roam-loads ()
+  "The hook should be registered even if org-roam has not loaded yet."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (progn
+        (require 'org)
+        (require 'org-roam-config)
+        (should (consp org-after-todo-state-change-hook))
+        (should-not (featurep 'org-roam)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-calls-copy-on-todo-to-done ()
+  "The hook should copy when transitioning FROM TODO TO DONE."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))
+        (setq org-last-state "TODO")  ; Previous state was TODO (non-done)
+        (setq org-state "DONE")       ; New state is DONE
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-calls-copy-on-in-progress-to-done ()
+  "The hook should copy when transitioning FROM IN-PROGRESS TO DONE."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))
+        (setq org-last-state "IN-PROGRESS")  ; Previous state was IN-PROGRESS (non-done)
+        (setq org-state "DONE")              ; New state is DONE
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-calls-copy-on-waiting-to-cancelled ()
+  "The hook should copy when transitioning FROM WAITING TO CANCELLED."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))
+        (setq org-last-state "WAITING")   ; Previous state was WAITING (non-done)
+        (setq org-state "CANCELLED")      ; New state is CANCELLED
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-ignores-done-to-cancelled ()
+  "The hook should NOT copy when transitioning FROM DONE TO CANCELLED (both done)."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))
+        (setq org-last-state "DONE")      ; Previous state was DONE (already done)
+        (setq org-state "CANCELLED")      ; New state is CANCELLED (also done)
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should-not copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-ignores-todo-state ()
+  "The hook should not copy when transitioning TO TODO state (non-done)."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))  ; TODO is not in done keywords
+        (setq org-state "TODO")  ; Transitioning TO TODO
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should-not copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(ert-deftest test-org-roam-hook-ignores-in-progress-state ()
+  "The hook should not copy when transitioning TO IN-PROGRESS state (non-done)."
+  (test-org-roam-todo-hook-setup)
+  (unwind-protect
+      (let ((copy-function-called nil))
+        (require 'org)
+        (require 'org-roam-config)
+        (setq org-done-keywords '("DONE" "CANCELLED"))  ; IN-PROGRESS is not in done keywords
+        (setq org-state "IN-PROGRESS")  ; Transitioning TO IN-PROGRESS
+        (cl-letf (((symbol-function 'cj/org-roam-copy-todo-to-today)
+                   (lambda () (setq copy-function-called t))))
+          (run-hooks 'org-after-todo-state-change-hook)
+          (should-not copy-function-called)))
+    (test-org-roam-todo-hook-teardown)))
+
+(provide 'test-org-roam-config-copy-todo-to-today)
+;;; test-org-roam-config-copy-todo-to-today.el ends here