aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-14 00:07:06 -0500
committerCraig Jennings <c@cjennings.net>2026-07-14 00:07:06 -0500
commit703b4841253ca610e9b50a8e8519863eaf22a6b2 (patch)
tree162e9cff886d7e8e6bdca41113dc245cee438d61
parent03a1f26b2879f678a48c036a9e6f4b9ace864c3b (diff)
downloaddotemacs-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.el34
-rw-r--r--tests/test-signal-config--contact-cache.el61
-rw-r--r--tests/test-signal-config.el11
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