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.

1 files changed, 20 insertions, 11 deletions

diff --git a/modules/org-webclipper.el b/modules/org-webclipper.el
index be4c1cb6..cf4cdbf0 100644
--- a/modules/org-webclipper.el
+++ b/modules/org-webclipper.el
@@ -68,12 +68,12 @@
     (require 'org-web-tools)
     (require 'user-constants) ;; for webclipped-file
 
-    ;; Register the org-protocol handler
-    (add-to-list 'org-protocol-protocol-alist
-                 '("webclip"
-                   :protocol "webclip"
-                   :function cj/org-protocol-webclip
-                   :kill-client t))
+    ;; The org-protocol handler registration lives in the
+    ;; `with-eval-after-load 'org-protocol' block at the bottom of
+    ;; this module -- that's the more robust home (it survives
+    ;; org-protocol being loaded before or after this module).  Two
+    ;; registration sites would silently drift if the alist entry
+    ;; shape ever changes.
 
     ;; Add capture templates if not already present
     (unless (assoc "W" org-capture-templates)
@@ -119,14 +119,23 @@ Returns the processed content as a string with:
 
 (defun cj/org-protocol-webclip (info)
   "Process org-protocol webclip requests.
-INFO is a plist containing :url and :title from the org-protocol call."
+INFO is a plist containing :url and :title from the org-protocol call.
+Signals `user-error' when :url is missing, nil, empty, or non-string -- an
+unexpected plist shape used to silently set the globals to nil and fail
+downstream inside the capture handler with confusing messages."
   (cj/webclipper-ensure-initialized)
   (let ((url (plist-get info :url))
         (title (plist-get info :title)))
-    (when url
-      ;; Store the URL and title for the capture template to use
-      (setq cj/webclip-current-url url
-            cj/webclip-current-title (or title "Untitled")))
+    (unless (and (stringp url) (not (string-empty-p url)))
+      (user-error
+       "org-protocol webclip: expected non-empty :url string, got %S" url))
+    (when (and title (not (stringp title)))
+      (user-error
+       "org-protocol webclip: :title must be a string when provided, got %S"
+       title))
+    ;; Store the URL and title for the capture template to use
+    (setq cj/webclip-current-url url
+          cj/webclip-current-title (or title "Untitled"))
     ;; Trigger the capture
     (org-capture nil "W")
     nil))  ; Return nil to indicate we handled it