diff options
| author | Craig Jennings <c@cjennings.net> | 2026-07-14 00:07:06 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-07-14 00:07:06 -0500 |
| commit | 703b4841253ca610e9b50a8e8519863eaf22a6b2 (patch) | |
| tree | 162e9cff886d7e8e6bdca41113dc245cee438d61 | |
| parent | 03a1f26b2879f678a48c036a9e6f4b9ace864c3b (diff) | |
| download | dotemacs-703b4841253ca610e9b50a8e8519863eaf22a6b2.tar.gz dotemacs-703b4841253ca610e9b50a8e8519863eaf22a6b2.zip | |
fix(signal): honor the cache-clear promise and mark empty fetches warm
Two picker-cache bugs from the config audit, verified against current code first.
The cache docstring promised clearing on signel-stop, but nothing cleared it, so a stale contact list survived a relink or reconnect. I advised a named clear function :after signel-stop, which makes the docstring true.
An empty listContacts result cached nil, indistinguishable from a cold cache, so a zero-contact account re-ran the picker's blocking fetch (up to the 3s timeout) on every open. The fetch now caches an empty sentinel and the picker reads through cj/signel--cached-contacts. One existing test pinned the nil behavior and now asserts the sentinel instead.
| -rw-r--r-- | modules/signal-config.el | 34 | ||||
| -rw-r--r-- | tests/test-signal-config--contact-cache.el | 61 | ||||
| -rw-r--r-- | tests/test-signal-config.el | 11 |
3 files changed, 95 insertions, 11 deletions
diff --git a/modules/signal-config.el b/modules/signal-config.el index edb7d0dc..2ee4403f 100644 --- a/modules/signal-config.el +++ b/modules/signal-config.el @@ -199,10 +199,27 @@ time." (declare-function signel--send-rpc "signel" (method params &optional target-buffer success-callback)) (defvar cj/signel--contact-cache nil - "Cached `(LABEL . RECIPIENT)' alist for the contact picker. + "Contact-picker cache: nil (cold), `empty', or a `(LABEL . RECIPIENT)' alist. Populated by `cj/signel--fetch-contacts' on first invocation (or after a -`cj/signel-refresh-contacts'), and cleared on `signel-stop' / restart so -a stale list can't survive a reconnect. In-memory only.") +`cj/signel-refresh-contacts'). A fetched-and-empty account caches the +symbol `empty' rather than nil, so it reads as warm and the picker does +not re-run its blocking fetch on every open -- read through +`cj/signel--cached-contacts'. Cleared back to cold by +`cj/signel--clear-contact-cache', advised onto `signel-stop' so a stale +list can't survive a reconnect. In-memory only.") + +(defun cj/signel--clear-contact-cache (&rest _) + "Return the contact cache to cold (nil) so the next picker refetches. +Advised `:after' `signel-stop': a relink or reconnect may change the +contact list, so a cache from the previous connection must not survive." + (setq cj/signel--contact-cache nil)) + +(advice-add 'signel-stop :after #'cj/signel--clear-contact-cache) + +(defun cj/signel--cached-contacts () + "Return the cached contact alist, treating the `empty' sentinel as none." + (unless (eq cj/signel--contact-cache 'empty) + cj/signel--contact-cache)) (defcustom cj/signel-fetch-timeout 3.0 "Seconds the picker blocks on `accept-process-output' for a cold-cache fetch. @@ -249,8 +266,10 @@ fires a void-variable error before the autoload would trigger." Issues a `listContacts' RPC and registers a success callback that runs the result through `cj/signal--parse-contacts' (the verified parser) and stores the resulting `(LABEL . RECIPIENT)' alist in the cache. An empty -result populates the cache as nil; a failure goes through the dispatch -error path and never invokes the callback, so the prior cache survives. +result caches the `empty' sentinel -- nil would read as a cold cache and +re-run the picker's blocking fetch on every open. A failure goes +through the dispatch error path and never invokes the callback, so the +prior cache survives. AFTER-CALLBACK, when non-nil, is invoked with no arguments after the cache has been populated -- the picker uses this to unblock its @@ -258,7 +277,8 @@ bounded-wait on cold caches." (signel--send-rpc "listContacts" nil nil (lambda (result) - (setq cj/signel--contact-cache (cj/signal--parse-contacts result)) + (setq cj/signel--contact-cache + (or (cj/signal--parse-contacts result) 'empty)) (when after-callback (funcall after-callback))))) (defun cj/signel-refresh-contacts () @@ -306,7 +326,7 @@ opens the chosen recipient in `signel-chat'." "Signal contact fetch timed out after %.1fs; try again or run M-x cj/signel-refresh-contacts (see *signel-log* for detail)" cj/signel-fetch-timeout)))) (let* ((note-self (cons "Note to Self" signel-account)) - (candidates (cons note-self cj/signel--contact-cache)) + (candidates (cons note-self (cj/signel--cached-contacts))) (table (lambda (string pred action) (if (eq action 'metadata) `(metadata diff --git a/tests/test-signal-config--contact-cache.el b/tests/test-signal-config--contact-cache.el new file mode 100644 index 00000000..720e0c3a --- /dev/null +++ b/tests/test-signal-config--contact-cache.el @@ -0,0 +1,61 @@ +;;; test-signal-config--contact-cache.el --- Contact-cache lifecycle tests -*- lexical-binding: t; -*- + +;;; Commentary: +;; The picker's contact cache has two lifecycle bugs from the 2026-06 config +;; audit: (1) its docstring promised clearing on signel-stop but nothing +;; cleared it, so a stale list survived a relink/reconnect; (2) a +;; fetched-and-empty list was cached as nil, indistinguishable from a cold +;; cache, so a zero-contact account re-ran the blocking fetch (up to +;; `cj/signel-fetch-timeout') on every picker open. The fix names the empty +;; state with an `empty' sentinel and clears the cache via a named function +;; advised onto `signel-stop'. +;; +;; Boundary mocks only (the RPC send, the prompt); the cache logic runs +;; real. The ensure-started branches the audit called untested were +;; already covered in test-signal-config.el -- that claim was stale. + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'signal-config) + +;;; ------------------------- clear-on-stop (F1) ------------------------------ + +(ert-deftest test-signal-config-clear-contact-cache-resets () + "Normal: the clear function empties the cache back to cold (nil)." + (let ((cj/signel--contact-cache '(("Al (+1)" . "+1")))) + (cj/signel--clear-contact-cache) + (should (null cj/signel--contact-cache)))) + +(ert-deftest test-signal-config-clear-contact-cache-advises-stop () + "Normal: `signel-stop' carries the clear advice, so the docstring's +\"cleared on stop/restart\" promise is real." + (should (advice-member-p #'cj/signel--clear-contact-cache 'signel-stop))) + +;;; ------------------------- empty sentinel (F2) ------------------------------ + +(ert-deftest test-signal-config-fetch-empty-caches-sentinel () + "Boundary: a fetched-and-empty list caches the `empty' sentinel, not nil. +nil means cold cache; without the sentinel a zero-contact account re-ran +the blocking fetch on every picker open." + (let ((cj/signel--contact-cache nil) + (captured-callback nil)) + (cl-letf (((symbol-function 'signel--send-rpc) + (lambda (_method _params _buf callback) + (setq captured-callback callback)))) + (cj/signel--fetch-contacts) + (funcall captured-callback '())) + (should (eq cj/signel--contact-cache 'empty)))) + +(ert-deftest test-signal-config-cached-contacts-unwraps-sentinel () + "Normal: the cache reader returns the alist, and nil for the sentinel." + (let ((cj/signel--contact-cache '(("Al (+1)" . "+1")))) + (should (equal (cj/signel--cached-contacts) '(("Al (+1)" . "+1"))))) + (let ((cj/signel--contact-cache 'empty)) + (should (null (cj/signel--cached-contacts))))) + +(provide 'test-signal-config--contact-cache) +;;; test-signal-config--contact-cache.el ends here diff --git a/tests/test-signal-config.el b/tests/test-signal-config.el index 7556efdb..f8bf4410 100644 --- a/tests/test-signal-config.el +++ b/tests/test-signal-config.el @@ -251,9 +251,12 @@ and stores the (LABEL . RECIPIENT) alist in `cj/signel--contact-cache'." (should (equal cj/signel--contact-cache '(("Alice (+15555550100)" . "+15555550100")))))) -(ert-deftest test-signal-config-fetch-contacts-empty-result-clears-cache () - "Boundary: an empty listContacts result populates the cache as nil, -distinct from a failure path (which never invokes the success callback)." +(ert-deftest test-signal-config-fetch-contacts-empty-result-caches-sentinel () + "Boundary: an empty listContacts result caches the `empty' sentinel. +nil would read as a cold cache and re-run the picker's blocking fetch on +every open; the sentinel marks warm-but-empty. Still distinct from a +failure path, which never invokes the success callback. (This test +formerly pinned the nil behavior -- the 2026-06 audit's F2 bug.)" (let (sent-callback) (cl-letf (((symbol-function 'signel--send-rpc) (lambda (_method _params _target callback) @@ -261,7 +264,7 @@ distinct from a failure path (which never invokes the success callback)." (setq cj/signel--contact-cache '(("stale" . "+10000000000"))) (cj/signel--fetch-contacts) (funcall sent-callback [])) - (should-not cj/signel--contact-cache))) + (should (eq cj/signel--contact-cache 'empty)))) ;;; cj/signel-refresh-contacts |
