From a00628ac96d5f09f96be868755304f7bdfdc0d99 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 28 May 2026 02:45:31 -0500 Subject: fix(signal): require signel before reading its private variables cj/signel--ensure-started in modules/signal-config.el was reading signel--process-name in the first branch of its cond before the use-package autoload of signel had fired. The forward-declared (defvar signel--process-name) at L137 silences the byte-compile warning but doesn't actually bind the variable. Its value comes from signel.el's defconst, which doesn't run until signel is loaded. The first call to cj/signel-connect (C-; M SPC) after Emacs launch produced "Symbol's value as variable is void: signel--process-name" instead of starting the daemon. Surfaced tonight during the manual verify walk of the initiate-message workflow. I added (require 'signel) at the top of cj/signel--ensure-started so signel loads before any of its variables get read. The require is idempotent, so callers that hit the function after signel is already loaded pay nothing. The new ERT test test-signal-config-ensure-started-requires-signel-first asserts ordering: require must be the first call inside the function, not just called somewhere. A future refactor that moves the require below the cond would fail this test instead of passing silently. --- tests/test-signal-config.el | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'tests') diff --git a/tests/test-signal-config.el b/tests/test-signal-config.el index 6ddc7917..b362e03b 100644 --- a/tests/test-signal-config.el +++ b/tests/test-signal-config.el @@ -199,6 +199,29 @@ starting an account-less daemon." ((symbol-function 'get-process) (lambda (_) nil))) (should-error (cj/signel--ensure-started) :type 'user-error)))) +(ert-deftest test-signal-config-ensure-started-requires-signel-first () + "Error: ensure-started must `require' signel BEFORE reading any of +its private variables, so a `void-variable' error cannot fire when +called before signel has been autoloaded. Captures the bug where +`signel--process-name' was forward-declared in signal-config but not +yet bound at runtime because the `use-package' autoload had not fired. + +Asserts ordering, not just presence: a future refactor that moves the +`require' below the `cond' would still execute the require eventually +but the variable read in the cond would fire void-variable first. +The test fails if `require' isn't the first call inside the function." + (let ((call-order nil)) + (cl-letf (((symbol-function 'require) + (lambda (feature &optional _filename _noerror) + (push (list 'require feature) call-order) + t)) + ((symbol-function 'process-live-p) + (lambda (_) (push 'process-live-p call-order) t)) + ((symbol-function 'get-process) + (lambda (_) (push 'get-process call-order) 'fake-proc))) + (cj/signel--ensure-started) + (should (equal (car (reverse call-order)) '(require signel)))))) + ;;; cj/signel--fetch-contacts + cj/signel--contact-cache (ert-deftest test-signal-config-fetch-contacts-issues-list-contacts-rpc () -- cgit v1.2.3