refactor(integrations): five hygiene fixes from the module-by-module re-review

- markdown-config.el: two related fixes on `markdown-preview'.
  First, the URL was `https://localhost:8080/imp' but simple-httpd
  serves plaintext on port 8080 -- the browser hit a TLS handshake
  against a non-TLS listener and the preview never rendered.  Changed
  to `http://' and switched from `browse-url-generic' to plain
  `browse-url' so the user's default protocol handler picks the
  browser.  Second, the function used to start the network listener
  as a side effect of opening a preview; that's split into a
  separate `cj/markdown-preview-server-start' command and
  `markdown-preview' now signals a `user-error' (with the recovery
  command in the message) when the server isn't running.

- slack-config.el: wrap the
  `which-key-add-keymap-based-replacements' call in
  `with-eval-after-load 'which-key'.  Matches the pattern other
  config modules use and means a slow / missing which-key load
  won't block requiring slack-config.

- ai-vterm.el: pass the inner shell-command-string through
  `shell-quote-argument' before wrapping in the tmux invocation.
  The default value with embedded double quotes was safe under the
  prior literal-single-quote wrap, but a user-customized
  `cj/ai-vterm-agent-command' containing a single quote silently
  broke the shell parse.  Two existing tests updated to tolerate
  the post-quote escape shape; new regression test asserts a
  single-quote-bearing custom command survives.

- eshell-config.el: scope the `TERM=xterm-256color' override to
  eshell-spawned processes only via an `eshell-mode' hook that
  prepends to a buffer-local `process-environment'.  The previous
  global `setenv' at config-time changed `TERM' for every
  subsequent `start-process' across the Emacs session, so any
  subprocess (not just eshell pipelines) inherited
  `xterm-256color' regardless of whether the receiver could
  interpret the escapes.

4 files changed, 61 insertions, 22 deletions

diff --git a/modules/ai-vterm.el b/modules/ai-vterm.el
index 18320eee..4a8f7e0e 100644
--- a/modules/ai-vterm.el
+++ b/modules/ai-vterm.el
@@ -233,11 +233,19 @@ so the tmux window survives the AI command exiting -- the session stays
 alive with a bare bash prompt for recovery, and reattach works the same way."
   (let ((session (cj/--ai-vterm-tmux-session-name dir))
         (start-dir (expand-file-name dir)))
-    (format "tmux new-session -A -s %s -n %s -c %s '%s'"
+    ;; Pass the inner shell-command-string through `shell-quote-argument'
+    ;; so any single quotes embedded in a user-customized
+    ;; `cj/ai-vterm-agent-command' don't break the literal single-quote
+    ;; wrap below.  The default value carries embedded double quotes
+    ;; (\"Read .ai/protocols.org and follow all instructions.\") which
+    ;; was safe in the prior shape but a single-quoted custom value
+    ;; silently broke the shell parse.
+    (format "tmux new-session -A -s %s -n %s -c %s %s"
             (shell-quote-argument session)
             (shell-quote-argument cj/ai-vterm-tmux-window-name)
             (shell-quote-argument start-dir)
-            (concat cj/ai-vterm-agent-command "; exec bash"))))
+            (shell-quote-argument
+             (concat cj/ai-vterm-agent-command "; exec bash")))))
 
 (defun cj/--ai-vterm-has-marker-p (dir)
   "Return non-nil when DIR contains .ai/protocols.org."
diff --git a/modules/eshell-config.el b/modules/eshell-config.el
index 62b3a7ab..4d180d1c 100644
--- a/modules/eshell-config.el
+++ b/modules/eshell-config.el
@@ -128,8 +128,17 @@
   :hook
   (eshell-before-prompt-hook . (lambda ()
                                  (setq xterm-color-preserve-properties t)))
-  :config
-  (setenv "TERM" "xterm-256color"))
+  ;; Scope `TERM=xterm-256color' to eshell-spawned processes only by
+  ;; binding the env var on the eshell mode hook.  The previous global
+  ;; `setenv' at config-time changed `process-environment' for the
+  ;; whole Emacs process, so every subsequent `start-process' inherited
+  ;; `xterm-256color' regardless of whether the receiver was a terminal
+  ;; that could actually interpret the escapes.
+  :hook
+  (eshell-mode . (lambda ()
+                   (setq-local process-environment
+                               (cons "TERM=xterm-256color"
+                                     process-environment)))))
 
 (use-package eshell-syntax-highlighting
   :after esh-mode
diff --git a/modules/markdown-config.el b/modules/markdown-config.el
index 2536904a..1fc4cb0e 100644
--- a/modules/markdown-config.el
+++ b/modules/markdown-config.el
@@ -33,16 +33,34 @@
 
 ;;;; --------------------- WIP: Markdown-Preview ---------------------
 
+(defun cj/markdown-preview-server-start ()
+  "Start the simple-httpd listener that serves the live markdown preview.
+Idempotent: re-running while the server is already up is a no-op."
+  (interactive)
+  (require 'simple-httpd)
+  (httpd-start)
+  (message "markdown preview server running on http://localhost:8080/imp"))
+
 ;; the filter to apply to markdown before impatient-mode pushes it to the server
 (defun markdown-preview ()
+  "Open the current buffer as a live HTML preview at http://localhost:8080/imp.
+The simple-httpd listener must already be running -- see
+`cj/markdown-preview-server-start'.  Starting a network listener as a
+side effect of opening a preview is surprising, so the server start
+lives in a separate command."
   (interactive)
-  (httpd-start)
+  (unless (and (boundp 'httpd-process) httpd-process)
+    (user-error "markdown preview server not running; run `M-x cj/markdown-preview-server-start' first"))
   (impatient-mode 1)
   (setq imp-user-filter #'cj/markdown-html)
   (cl-incf imp-last-state)
   (imp--notify-clients)
-  ;;  (browse-url-generic-function 'browse-url-xdg-open)
-  (browse-url-generic "https://localhost:8080/imp" 1))
+  ;; Use plain `browse-url' (not `browse-url-generic') so the user's
+  ;; default protocol handler picks the browser, and use `http://' --
+  ;; simple-httpd serves plaintext; the previous `https://' URL caused
+  ;; a TLS handshake against a non-TLS listener and the preview never
+  ;; rendered.
+  (browse-url "http://localhost:8080/imp"))
 
 (defun cj/markdown-html (buffer)
   (princ (with-current-buffer buffer
diff --git a/modules/slack-config.el b/modules/slack-config.el
index 3e9ae283..b51db444 100644
--- a/modules/slack-config.el
+++ b/modules/slack-config.el
@@ -253,21 +253,25 @@ swallows exceptions via `websocket-try-callback'."
 (define-key cj/slack-keymap (kbd "Q") #'cj/slack-close-all-buffers)
 (define-key cj/slack-keymap (kbd "S") #'cj/slack-stop)
 
-(which-key-add-keymap-based-replacements cj/slack-keymap
-  "" "slack menu"
-  "s" "start slack"
-  "c" "unread rooms"
-  "C" "select channel"
-  "d" "direct message"
-  "w" "compose message"
-  "r" "reply / thread"
-  "e" "insert emoji"
-  "!" "add reaction"
-  "@" "embed @mention"
-  "#" "embed #channel"
-  "q" "mark read & bury"
-  "Q" "close all slack"
-  "S" "disconnect")
+;; Register which-key labels lazily so this module's require doesn't
+;; depend on which-key being loaded.  Other config modules use the same
+;; pattern.
+(with-eval-after-load 'which-key
+  (which-key-add-keymap-based-replacements cj/slack-keymap
+    "" "slack menu"
+    "s" "start slack"
+    "c" "unread rooms"
+    "C" "select channel"
+    "d" "direct message"
+    "w" "compose message"
+    "r" "reply / thread"
+    "e" "insert emoji"
+    "!" "add reaction"
+    "@" "embed @mention"
+    "#" "embed #channel"
+    "q" "mark read & bury"
+    "Q" "close all slack"
+    "S" "disconnect"))
 
 ;; Send from compose buffer with C-<return>
 (with-eval-after-load 'slack-message-compose-buffer