From a005c7636f30b710e27a6812ca989506d5df7531 Mon Sep 17 00:00:00 2001
From: Craig Jennings <c@cjennings.net>
Date: Sat, 16 May 2026 03:45:29 -0500
Subject: refactor(org-workflow): four hygiene fixes from the module-by-module
 re-review

- org-roam-config.el: extract `cj/--org-roam-should-copy-completed-task-p'
  and gate the `org-after-todo-state-change-hook' on it.  Skips
  fileless buffers (org-capture, indirect, temp Org) where
  `buffer-file-name' is nil and the downstream copy used to crash.
  Same gcal.org skip preserved.  Five existing tests updated to
  bind `buffer-file-name' inside `run-hooks' so the positive-case
  hook still fires.

- org-webclipper.el: drop the redundant
  `org-protocol-protocol-alist' registration inside
  `cj/webclipper-ensure-initialized'.  The
  `with-eval-after-load 'org-protocol' block at the bottom of the
  module is the single registration site now; comment in the
  initializer explains why.  Split the matching test into two:
  one for template registration (the initializer's actual job) and
  one for protocol registration (which now fires from the
  after-load block when `org-protocol' provides).

- org-webclipper.el: validate `:url' and `:title' in
  `cj/org-protocol-webclip'.  `:url' must be a non-empty string;
  `:title' must be a string when provided.  Signals `user-error'
  with the unexpected value instead of silently setting the
  globals to nil and failing downstream in the capture handler.

- mu4e-org-contacts-integration.el: declare `contacts-file' (via
  `eval-when-compile (defvar ...)') and `cj/get-all-contact-emails'
  (via `declare-function') near the top of the file.  Byte-compile
  in isolation no longer warns about free variables / unknown
  functions; the cross-module dependency is explicit at the top.
---
 tests/test-org-roam-config-copy-todo-to-today.el | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

(limited to 'tests/test-org-roam-config-copy-todo-to-today.el')

diff --git a/tests/test-org-roam-config-copy-todo-to-today.el b/tests/test-org-roam-config-copy-todo-to-today.el
index bcac5a26..c642020d 100644
--- a/tests/test-org-roam-config-copy-todo-to-today.el
+++ b/tests/test-org-roam-config-copy-todo-to-today.el
@@ -51,7 +51,8 @@
         (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)
+          (let ((buffer-file-name "/tmp/test-org-roam-fake.org"))
+            (run-hooks 'org-after-todo-state-change-hook))
           (should copy-function-called)))
     (test-org-roam-todo-hook-teardown)))
 
@@ -67,7 +68,8 @@
         (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)
+          (let ((buffer-file-name "/tmp/test-org-roam-fake.org"))
+            (run-hooks 'org-after-todo-state-change-hook))
           (should copy-function-called)))
     (test-org-roam-todo-hook-teardown)))
 
@@ -96,7 +98,8 @@
         (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)
+          (let ((buffer-file-name "/tmp/test-org-roam-fake.org"))
+            (run-hooks 'org-after-todo-state-change-hook))
           (should copy-function-called)))
     (test-org-roam-todo-hook-teardown)))
 
@@ -112,7 +115,8 @@
         (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)
+          (let ((buffer-file-name "/tmp/test-org-roam-fake.org"))
+            (run-hooks 'org-after-todo-state-change-hook))
           (should copy-function-called)))
     (test-org-roam-todo-hook-teardown)))
 
@@ -128,7 +132,8 @@
         (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)
+          (let ((buffer-file-name "/tmp/test-org-roam-fake.org"))
+            (run-hooks 'org-after-todo-state-change-hook))
           (should copy-function-called)))
     (test-org-roam-todo-hook-teardown)))
 
-- 
cgit v1.2.3