fix(slack): harden notifications like signel's

cj/slack-notify passed raw message bodies to a bare start-process. A multiline message made a giant toast, and a machine without the notify script errored inside the condition-case, so the notification silently vanished. It now routes through the same shape signel uses: whitespace collapsed and the body truncated at 120 chars, sound gated by a new cj/slack-notify-sound defcustom (silent by default), and a notifications-notify fallback when the script is absent. The fallback requires notifications.el first, which isn't autoloaded. cj/slack-test-notify exercises the same delivery path.

The shared cj/messenger-notify extraction that collapses the signel and slack copies stays with the messenger-unification task. Ten tests cover the formatter, the delivery routing, and the DM/mention predicate wiring the audit called untested.

2 files changed, 159 insertions, 6 deletions

diff --git a/modules/slack-config.el b/modules/slack-config.el
index adf38804..9eb9f838 100644
--- a/modules/slack-config.el
+++ b/modules/slack-config.el
@@ -52,6 +52,7 @@
 (defvar slack-message-custom-notifier)
 (defvar slack-teams)
 
+(declare-function notifications-notify "notifications")
 (declare-function slack-buffer-add-reaction-to-message "slack-buffer")
 (declare-function slack-buffer-latest-ts "slack-buffer")
 (declare-function slack-buffer-team "slack-buffer")
@@ -207,6 +208,46 @@ Errors if called outside a Slack message buffer."
               :around #'cj/slack--safe-reaction-echo-description))
 
 ;; ----------------------------- Notifications ---------------------------------
+;; Mirrors signel's notification hardening (body truncation, sound gating,
+;; script-with-fallback delivery).  The shared cj/messenger-notify extraction
+;; that collapses the two copies belongs to the messenger-unification task.
+
+(defcustom cj/slack-notify-sound nil
+  "When non-nil, Slack notifications play the notify script's sound.
+Nil (the default) passes --silent so the toast is visual only."
+  :type 'boolean
+  :group 'slack)
+
+(defconst cj/slack--notify-body-max 120
+  "Maximum character length of a desktop-notification body.
+Longer message text truncates to this length ending in an ellipsis;
+the full text is always in the Slack buffer.")
+
+(defun cj/slack--format-notify-body (text)
+  "Collapse whitespace in TEXT and truncate it for a notification body.
+Whitespace runs (including newlines) become single spaces, the result
+is trimmed, and anything over `cj/slack--notify-body-max' characters
+truncates to that length with a trailing ellipsis."
+  (let ((flat (string-trim (replace-regexp-in-string "[ \t\n\r]+" " " text))))
+    (if (<= (length flat) cj/slack--notify-body-max)
+        flat
+      (concat (substring flat 0 (1- cj/slack--notify-body-max)) "…"))))
+
+(defun cj/slack--send-notification (title body)
+  "Deliver a desktop notification with TITLE and BODY.
+Routes through the external notify script when it is on PATH (type
+info, sound gated by `cj/slack-notify-sound'), falling back to
+`notifications-notify' otherwise.  Previously a missing script made
+`start-process' error inside the caller's condition-case, so the
+notification silently vanished."
+  (let ((script (executable-find "notify")))
+    (if script
+        (apply #'start-process "slack-notify" nil script "info" title body
+               (unless cj/slack-notify-sound (list "--silent")))
+      ;; notifications.el is not autoloaded; load it on the first fallback.
+      (unless (fboundp 'notifications-notify)
+        (require 'notifications))
+      (notifications-notify :title title :body body))))
 
 (defun cj/slack-notify (message room team)
   "Send desktop notification for DMs and @mentions only.
@@ -218,18 +259,17 @@ swallows exceptions via `websocket-try-callback'."
       (when (and (not (slack-message-minep message team))
                  (or (slack-im-p room)
                      (slack-message-mentioned-p message team)))
-        (let ((title (format "Slack: %s" (slack-room-display-name room team)))
-              (body (or (slack-message-body message team) "")))
-          (start-process "slack-notify" nil
-                         "notify" "info" title body)))
+        (cj/slack--send-notification
+         (format "Slack: %s" (slack-room-display-name room team))
+         (cj/slack--format-notify-body
+          (or (slack-message-body message team) ""))))
     (error (message "cj/slack-notify error: %S" err))))
 
 (defun cj/slack-test-notify ()
   "Send a test desktop notification to verify the notify pipeline works."
   (interactive)
   (condition-case err
-      (start-process "slack-notify-test" nil
-                     "notify" "info" "Slack: Test" "Notification pipeline works")
+      (cj/slack--send-notification "Slack: Test" "Notification pipeline works")
     (error (message "cj/slack-test-notify error: %S" err))))
 
 (defun cj/slack-mark-read-and-bury ()
diff --git a/tests/test-slack-config--notify.el b/tests/test-slack-config--notify.el
new file mode 100644
index 00000000..d830ae29
--- /dev/null
+++ b/tests/test-slack-config--notify.el
@@ -0,0 +1,113 @@
+;;; test-slack-config--notify.el --- Slack notification hardening tests -*- lexical-binding: t; -*-
+
+;;; Commentary:
+;; The config audit found `cj/slack-notify' missing signel's hardening: no
+;; body truncation (giant toasts), no whitespace collapse, no sound gating,
+;; and no `notifications-notify' fallback when the notify script is absent
+;; (the raw `start-process' error was swallowed by the condition-case, so
+;; the notification silently vanished).  This mirrors signel's shape in
+;; place; the shared cj/messenger-notify extraction belongs to the
+;; messenger-unification task.
+;;
+;; The slack package's own predicates (`slack-im-p', `slack-message-minep',
+;; `slack-message-mentioned-p') are package boundaries and are mocked; the
+;; formatter and routing logic run real.
+
+;;; Code:
+
+(require 'ert)
+(require 'cl-lib)
+
+(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
+(require 'slack-config)
+
+;;; ------------------------------ body formatter -----------------------------
+
+(ert-deftest test-slack-config-format-notify-body-collapses-whitespace ()
+  "Normal: whitespace runs (including newlines) become single spaces."
+  (should (equal (cj/slack--format-notify-body "a  b\nc\t\td")
+                 "a b c d")))
+
+(ert-deftest test-slack-config-format-notify-body-truncates-long ()
+  "Boundary: text over the max truncates to max length ending in an ellipsis."
+  (let ((long (make-string 500 ?x)))
+    (let ((formatted (cj/slack--format-notify-body long)))
+      (should (= (length formatted) cj/slack--notify-body-max))
+      (should (string-suffix-p "…" formatted)))))
+
+(ert-deftest test-slack-config-format-notify-body-empty ()
+  "Boundary: empty and whitespace-only input format to the empty string."
+  (should (equal (cj/slack--format-notify-body "") ""))
+  (should (equal (cj/slack--format-notify-body " \n\t ") "")))
+
+;;; ----------------------------- delivery routing ----------------------------
+
+(ert-deftest test-slack-config-send-notification-script-silent-by-default ()
+  "Normal: with the notify script on PATH and sound off, --silent is passed."
+  (let ((argv nil) (cj/slack-notify-sound nil))
+    (cl-letf (((symbol-function 'executable-find)
+               (lambda (prog &rest _) (when (equal prog "notify") "/bin/notify")))
+              ((symbol-function 'start-process)
+               (lambda (_name _buf &rest args) (setq argv args))))
+      (cj/slack--send-notification "Slack: general" "hello"))
+    (should (equal argv '("/bin/notify" "info" "Slack: general" "hello" "--silent")))))
+
+(ert-deftest test-slack-config-send-notification-sound-enabled ()
+  "Boundary: with sound enabled, --silent is not passed."
+  (let ((argv nil) (cj/slack-notify-sound t))
+    (cl-letf (((symbol-function 'executable-find)
+               (lambda (prog &rest _) (when (equal prog "notify") "/bin/notify")))
+              ((symbol-function 'start-process)
+               (lambda (_name _buf &rest args) (setq argv args))))
+      (cj/slack--send-notification "Slack: general" "hello"))
+    (should (equal argv '("/bin/notify" "info" "Slack: general" "hello")))))
+
+(ert-deftest test-slack-config-send-notification-fallback-without-script ()
+  "Error: with no notify script, delivery falls back to notifications-notify."
+  (let ((fallback nil))
+    (cl-letf (((symbol-function 'executable-find) (lambda (&rest _) nil))
+              ((symbol-function 'notifications-notify)
+               (lambda (&rest args) (setq fallback args))))
+      (cj/slack--send-notification "Slack: general" "hello"))
+    (should (equal (plist-get fallback :title) "Slack: general"))
+    (should (equal (plist-get fallback :body) "hello"))))
+
+;;; --------------------------- predicate wiring ------------------------------
+
+(defmacro test-slack-notify--with-message (minep im-p mentioned-p &rest body)
+  "Run BODY with the slack package predicates mocked to the given values.
+Also mocks room/body accessors and captures delivery into `sent'."
+  (declare (indent 3))
+  `(let ((sent nil))
+     (cl-letf (((symbol-function 'slack-message-minep) (lambda (&rest _) ,minep))
+               ((symbol-function 'slack-im-p) (lambda (&rest _) ,im-p))
+               ((symbol-function 'slack-message-mentioned-p) (lambda (&rest _) ,mentioned-p))
+               ((symbol-function 'slack-room-display-name) (lambda (&rest _) "general"))
+               ((symbol-function 'slack-message-body) (lambda (&rest _) "the message"))
+               ((symbol-function 'cj/slack--send-notification)
+                (lambda (title body) (setq sent (list title body)))))
+       (cj/slack-notify 'msg 'room 'team)
+       ,@body)))
+
+(ert-deftest test-slack-config-notify-dm-notifies ()
+  "Normal: a DM from someone else raises a notification."
+  (test-slack-notify--with-message nil t nil
+    (should (equal sent '("Slack: general" "the message")))))
+
+(ert-deftest test-slack-config-notify-mention-notifies ()
+  "Normal: an @mention in a channel raises a notification."
+  (test-slack-notify--with-message nil nil t
+    (should (equal sent '("Slack: general" "the message")))))
+
+(ert-deftest test-slack-config-notify-own-message-silent ()
+  "Boundary: your own message never notifies, even in a DM."
+  (test-slack-notify--with-message t t t
+    (should-not sent)))
+
+(ert-deftest test-slack-config-notify-plain-channel-silent ()
+  "Boundary: a channel message with no mention stays silent."
+  (test-slack-notify--with-message nil nil nil
+    (should-not sent)))
+
+(provide 'test-slack-config--notify)
+;;; test-slack-config--notify.el ends here