refactor(webclipper): scope clip URL/title to dynamic bindings

org-webclipper passed the org-protocol URL and title through globals cj/webclip-current-url / cj/webclip-current-title: the protocol handler setq them, and the "W" capture template plus its handler read them, with the handler clearing them afterward. An aborted or erroring capture left the stale values for the next clip.

Renamed them to cj/--webclip-url / cj/--webclip-title and let-bind them around the org-capture call in the protocol entry point instead of mutating globals. The template %(identity ...) forms and the handler run within that dynamic extent, so they see the values while the capture runs, and an abort/error unwinds the binding automatically — no stale state, no manual clear. This mirrors the quick-video-capture fix. Tests updated to the new contract: URL/title visible during the capture, nothing left bound after, and an aborted capture leaves no stale state.

2 files changed, 60 insertions, 45 deletions

diff --git a/modules/org-webclipper.el b/modules/org-webclipper.el
index cf4cdbf0..2a6d7164 100644
--- a/modules/org-webclipper.el
+++ b/modules/org-webclipper.el
@@ -45,11 +45,16 @@
 
 
 ;; Variables for storing org-protocol data
-(defvar cj/webclip-current-url nil
-  "Temporary storage for URL passed via org-protocol.")
+(defvar cj/--webclip-url nil
+  "URL for the active web clip, dynamically bound around `org-capture'.
+The org-protocol entry point `let'-binds this for the dynamic extent of
+its capture call, so the capture template and handler see it while the
+capture runs, and an aborted or erroring capture unwinds the binding
+instead of leaving stale state for the next capture.")
 
-(defvar cj/webclip-current-title nil
-  "Temporary storage for page title passed via org-protocol.")
+(defvar cj/--webclip-title nil
+  "Page title for the active web clip, dynamically bound around `org-capture'.
+See `cj/--webclip-url' for the binding contract.")
 
 ;; Flag to track if we've done initialization
 (defvar cj/webclipper-initialized nil
@@ -80,7 +85,7 @@
       (add-to-list 'org-capture-templates
                    '("W" "Web Clipper (Protocol)" entry
                      (file+headline webclipped-file "Webclipped Inbox")
-                     "* [[%(identity cj/webclip-current-url)][%(identity cj/webclip-current-title)]] :website:\nURL: %(identity cj/webclip-current-url)\nCaptured On:%U\n%(cj/org-protocol-webclip-handler)\n"
+                     "* [[%(identity cj/--webclip-url)][%(identity cj/--webclip-title)]] :website:\nURL: %(identity cj/--webclip-url)\nCaptured On:%U\n%(cj/org-protocol-webclip-handler)\n"
 					 :prepend t
                      :immediate-finish t)
                    t))
@@ -133,11 +138,13 @@ downstream inside the capture handler with confusing messages."
       (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")
+    ;; Bind url+title for the dynamic extent of the capture call only, so
+    ;; the template and handler see them while the capture runs and an
+    ;; aborted/erroring capture unwinds the binding rather than leaving
+    ;; stale state for the next clip.
+    (let ((cj/--webclip-url url)
+          (cj/--webclip-title (or title "Untitled")))
+      (org-capture nil "W"))
     nil))  ; Return nil to indicate we handled it
 
 (defun cj/org-protocol-webclip-handler ()
@@ -154,12 +161,8 @@ It fetches the page content and converts it to Org format."
   (require 'org-web-tools)
   (setq org-web-tools-pandoc-sleep-time 0.5)
 
-  (let ((url cj/webclip-current-url)
-        (title cj/webclip-current-title))
-    ;; Clear the stored values after using them
-    (setq cj/webclip-current-url nil
-          cj/webclip-current-title nil)
-
+  (let ((url cj/--webclip-url)
+        (title cj/--webclip-title))
     (if (not url)
         (error "No URL provided for clipping")
       (condition-case err
diff --git a/tests/test-org-webclipper-commands.el b/tests/test-org-webclipper-commands.el
index 57c7b5fc..5ec5fbfe 100644
--- a/tests/test-org-webclipper-commands.el
+++ b/tests/test-org-webclipper-commands.el
@@ -65,62 +65,74 @@ that registers the webclip entry.  Providing `'org-protocol' fires the block."
 
 ;;; cj/org-protocol-webclip
 
-(ert-deftest test-webclipper-protocol-stores-url-title-and-captures ()
-  "Normal: the protocol handler stores url+title and triggers `org-capture'."
+(ert-deftest test-webclipper-protocol-binds-url-title-during-capture ()
+  "Normal: the protocol handler makes url+title visible to the capture call."
   (let ((cj/webclipper-initialized t)
-        (cj/webclip-current-url nil)
-        (cj/webclip-current-title nil)
-        (capture-key nil))
+        (seen-url nil) (seen-title nil) (capture-key nil))
     (cl-letf (((symbol-function 'org-capture)
-               (lambda (_arg k) (setq capture-key k))))
+               (lambda (_arg k)
+                 (setq capture-key k
+                       seen-url cj/--webclip-url
+                       seen-title cj/--webclip-title))))
       (cj/org-protocol-webclip
        '(:url "https://example.com" :title "Hello")))
-    (should (equal cj/webclip-current-url "https://example.com"))
-    (should (equal cj/webclip-current-title "Hello"))
+    (should (equal seen-url "https://example.com"))
+    (should (equal seen-title "Hello"))
     (should (equal capture-key "W"))))
 
+(ert-deftest test-webclipper-protocol-leaves-no-stale-state ()
+  "Boundary: after the protocol capture returns, no url/title remains bound."
+  (let ((cj/webclipper-initialized t))
+    (cl-letf (((symbol-function 'org-capture) #'ignore))
+      (cj/org-protocol-webclip '(:url "https://example.com" :title "Hello")))
+    (should (null cj/--webclip-url))
+    (should (null cj/--webclip-title))))
+
+(ert-deftest test-webclipper-protocol-aborted-capture-clears-state ()
+  "Error: a capture that errors mid-flow still leaves no stale url/title."
+  (let ((cj/webclipper-initialized t))
+    (cl-letf (((symbol-function 'org-capture)
+               (lambda (&rest _) (error "simulated capture abort"))))
+      (ignore-errors (cj/org-protocol-webclip '(:url "https://example.com"))))
+    (should (null cj/--webclip-url))
+    (should (null cj/--webclip-title))))
+
 (ert-deftest test-webclipper-protocol-defaults-title-when-missing ()
-  "Boundary: a missing title in INFO becomes \"Untitled\"."
+  "Boundary: a missing title in INFO becomes \"Untitled\" during the capture."
   (let ((cj/webclipper-initialized t)
-        (cj/webclip-current-url nil)
-        (cj/webclip-current-title nil))
-    (cl-letf (((symbol-function 'org-capture) #'ignore))
+        (seen-title nil))
+    (cl-letf (((symbol-function 'org-capture)
+               (lambda (&rest _) (setq seen-title cj/--webclip-title))))
       (cj/org-protocol-webclip '(:url "https://x.test")))
-    (should (equal cj/webclip-current-title "Untitled"))))
+    (should (equal seen-title "Untitled"))))
 
 ;;; cj/org-protocol-webclip-handler
 
 (ert-deftest test-webclipper-protocol-handler-errors-when-no-url ()
-  "Error: handler with no stashed url signals an error."
-  (let ((cj/webclip-current-url nil)
-        (cj/webclip-current-title "Whatever"))
-    (cl-letf (((symbol-function 'require) (lambda (&rest _) t))
-              ((symbol-function 'setopt) (lambda (&rest _) nil)))
+  "Error: handler with no bound url signals an error."
+  (let ((cj/--webclip-url nil)
+        (cj/--webclip-title "Whatever"))
+    (cl-letf (((symbol-function 'require) (lambda (&rest _) t)))
       (should-error (cj/org-protocol-webclip-handler) :type 'error))))
 
 (ert-deftest test-webclipper-protocol-handler-returns-processed-content ()
-  "Normal: handler converts the stashed URL into processed org content."
-  (let ((cj/webclip-current-url "https://example.com")
-        (cj/webclip-current-title "Title"))
+  "Normal: handler converts the bound URL into processed org content."
+  (let ((cj/--webclip-url "https://example.com")
+        (cj/--webclip-title "Title"))
     (cl-letf (((symbol-function 'require) (lambda (&rest _) t))
-              ((symbol-function 'setopt) (lambda (&rest _) nil))
               ((symbol-function 'org-web-tools--url-as-readable-org)
                (lambda (_) "* Page Title\n** Sub heading\nBody.\n"))
               ((symbol-function 'message) #'ignore))
       (let ((out (cj/org-protocol-webclip-handler)))
         ;; The first H1 is stripped, sub-heading is demoted.
         (should (string-match-p "^\\*\\*\\* Sub heading" out))
-        (should (string-match-p "Body" out)))
-      ;; Stash is cleared.
-      (should (null cj/webclip-current-url))
-      (should (null cj/webclip-current-title)))))
+        (should (string-match-p "Body" out))))))
 
 (ert-deftest test-webclipper-protocol-handler-wraps-fetch-error ()
   "Error: a fetch failure is wrapped in a clear error message."
-  (let ((cj/webclip-current-url "https://example.com")
-        (cj/webclip-current-title "Title"))
+  (let ((cj/--webclip-url "https://example.com")
+        (cj/--webclip-title "Title"))
     (cl-letf (((symbol-function 'require) (lambda (&rest _) t))
-              ((symbol-function 'setopt) (lambda (&rest _) nil))
               ((symbol-function 'org-web-tools--url-as-readable-org)
                (lambda (_) (error "network down"))))
       (let ((err (should-error (cj/org-protocol-webclip-handler) :type 'error)))