fix(modeline): drop per-render truename, guard vc fetch against signals

The VC modeline cache rebuilt its key every render, and the key included file-truename, so a stat ran on every redisplay rather than once per refresh as the comment claimed. Now it keys on (file show-remote). A moved symlink target is caught at the next TTL refresh, when vc-backend resolves the link fresh. And cj/modeline-vc-fetch is wrapped in condition-case returning nil, so a git signal on a slow or unmounted filesystem degrades to no-VC-info instead of breaking all redisplay.

4 files changed, 70 insertions, 59 deletions

diff --git a/modules/modeline-config.el b/modules/modeline-config.el
index 0e6e5d0fb..21ecd7e47 100644
--- a/modules/modeline-config.el
+++ b/modules/modeline-config.el
@@ -137,12 +137,12 @@ Uses built-in cached values for performance.")
         cj/modeline-vc-cache-set-p nil))
 
 (defun cj/modeline-vc-cache-key (file)
-  "Return the cache key for FILE.
-Includes the resolved `file-truename' so that if FILE is a symlink whose
-target moves to a different VC tree, the key changes and the cache is not
-served a stale backend.  The extra `file-truename' is one stat per refresh,
-cheap next to the VC calls the cache avoids."
-  (list file (file-truename file) cj/modeline-vc-show-remote))
+  "Return the cache key for FILE: the file path and `cj/modeline-vc-show-remote'.
+`file-truename' is deliberately omitted -- the mode-line rebuilds this key on
+every render to check cache validity, so a stat here would run per redisplay.
+A symlink whose target moves to a different VC tree is picked up at the next
+TTL refresh, when `vc-backend' resolves the link fresh."
+  (list file cj/modeline-vc-show-remote))
 
 (defun cj/modeline-vc-cache-valid-p (key now)
   "Return non-nil when cached VC data is valid for KEY at NOW."
@@ -157,18 +157,25 @@ Return a plist with `:branch' and `:state', or nil when FILE has no VC data.
 Uses `vc-git--symbolic-ref' for branch names when available (it returns the
 symbolic ref like \"main\" instead of a SHA when HEAD is on a branch), but
 falls back to `vc-working-revision' if the internal accessor is missing --
-the symbol is internal and can be renamed or removed between Emacs versions."
-  (unless (and (file-remote-p file) (not cj/modeline-vc-show-remote))
-    (when-let* ((backend (vc-backend file))
-                (branch (vc-working-revision file backend)))
-      (when (eq backend 'Git)
-        (unless (fboundp 'vc-git--symbolic-ref)
-          (require 'vc-git nil 'noerror))
-        (when (fboundp 'vc-git--symbolic-ref)
-          (when-let* ((symbolic (ignore-errors (vc-git--symbolic-ref file))))
-            (setq branch symbolic))))
-      (list :branch branch
-            :state (vc-state file backend)))))
+the symbol is internal and can be renamed or removed between Emacs versions.
+
+The whole VC probe is wrapped in `condition-case' returning nil.  These are
+synchronous git calls that, on TTL expiry, run while the mode-line is built;
+on a slow or unmounted filesystem a signal here would land in redisplay and
+break it.  Caching nil degrades to \"no VC info\" instead."
+  (condition-case nil
+      (unless (and (file-remote-p file) (not cj/modeline-vc-show-remote))
+        (when-let* ((backend (vc-backend file))
+                    (branch (vc-working-revision file backend)))
+          (when (eq backend 'Git)
+            (unless (fboundp 'vc-git--symbolic-ref)
+              (require 'vc-git nil 'noerror))
+            (when (fboundp 'vc-git--symbolic-ref)
+              (when-let* ((symbolic (ignore-errors (vc-git--symbolic-ref file))))
+                (setq branch symbolic))))
+          (list :branch branch
+                :state (vc-state file backend))))
+    (error nil)))
 
 (defun cj/modeline-vc-info ()
   "Return cached modeline VC data for the current buffer."
diff --git a/tests/test-modeline-config-vc-cache-key.el b/tests/test-modeline-config-vc-cache-key.el
index ae869f4b8..6ba7985c2 100644
--- a/tests/test-modeline-config-vc-cache-key.el
+++ b/tests/test-modeline-config-vc-cache-key.el
@@ -1,56 +1,36 @@
 ;;; test-modeline-config-vc-cache-key.el --- Tests for VC modeline cache key -*- lexical-binding: t; -*-
 
 ;;; Commentary:
-;; The VC modeline cache keys on the file.  A symlink whose target moves to a
-;; different VC tree must invalidate the cache, so the key includes the
-;; resolved `file-truename', not just the symlink path.
+;; The VC modeline cache keys on the file path and the `cj/modeline-vc-show-remote'
+;; flag only.  `file-truename' is deliberately NOT in the key: it would run on
+;; every redisplay (the mode-line rebuilds the key each render to check validity),
+;; and a moved symlink target is picked up at the next TTL refresh anyway, since
+;; `vc-backend' resolves the link fresh.  The per-render stat isn't worth it.
 
 ;;; Code:
 
 (require 'ert)
-(require 'cl-lib)
 
 (add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
 (require 'modeline-config)
 
-;;; Normal Cases
-
-(ert-deftest test-modeline-vc-cache-key-includes-truename ()
-  "Normal: the cache key includes the resolved truename of the file."
-  (let ((f (make-temp-file "cj-mlkey-")))
-    (unwind-protect
-        (should (member (file-truename f) (cj/modeline-vc-cache-key f)))
-      (delete-file f))))
-
-;;; Boundary Cases
-
-(ert-deftest test-modeline-vc-cache-key-changes-when-symlink-target-moves ()
-  "Boundary: re-pointing a symlink to a new target changes the cache key.
-The symlink path is identical both times; only its truename differs, so a
-key that ignored the truename would serve a stale VC backend."
-  (let* ((dir (make-temp-file "cj-mlkey-dir-" t))
-         (target-a (expand-file-name "a" dir))
-         (target-b (expand-file-name "b" dir))
-         (link (expand-file-name "link" dir)))
-    (unwind-protect
-        (progn
-          (write-region "" nil target-a)
-          (write-region "" nil target-b)
-          (make-symbolic-link target-a link)
-          (let ((key-a (cj/modeline-vc-cache-key link)))
-            (delete-file link)
-            (make-symbolic-link target-b link)
-            (let ((key-b (cj/modeline-vc-cache-key link)))
-              (should-not (equal key-a key-b)))))
-      (delete-directory dir t))))
+(ert-deftest test-modeline-vc-cache-key-is-file-and-show-remote ()
+  "Normal: the key is (FILE SHOW-REMOTE), with no per-render file-truename stat."
+  (let ((cj/modeline-vc-show-remote nil))
+    (should (equal (cj/modeline-vc-cache-key "/x/y.el") '("/x/y.el" nil)))))
+
+(ert-deftest test-modeline-vc-cache-key-tracks-show-remote ()
+  "Boundary: toggling show-remote yields a different key (separate cache entry)."
+  (should-not (equal (let ((cj/modeline-vc-show-remote nil))
+                       (cj/modeline-vc-cache-key "/x/y.el"))
+                     (let ((cj/modeline-vc-show-remote t))
+                       (cj/modeline-vc-cache-key "/x/y.el")))))
 
 (ert-deftest test-modeline-vc-cache-key-stable-for-same-file ()
-  "Boundary: the key is stable across calls for an unchanged file."
-  (let ((f (make-temp-file "cj-mlkey-stable-")))
-    (unwind-protect
-        (should (equal (cj/modeline-vc-cache-key f)
-                       (cj/modeline-vc-cache-key f)))
-      (delete-file f))))
+  "Boundary: the key is stable across calls for an unchanged file + show-remote."
+  (let ((cj/modeline-vc-show-remote nil))
+    (should (equal (cj/modeline-vc-cache-key "/x/y.el")
+                   (cj/modeline-vc-cache-key "/x/y.el")))))
 
 (provide 'test-modeline-config-vc-cache-key)
 ;;; test-modeline-config-vc-cache-key.el ends here
diff --git a/tests/test-modeline-config-vc-cache.el b/tests/test-modeline-config-vc-cache.el
index b6aafbfbe..dab755442 100644
--- a/tests/test-modeline-config-vc-cache.el
+++ b/tests/test-modeline-config-vc-cache.el
@@ -98,5 +98,12 @@
       (should (text-property-any 0 (length rendered)
                                  'mouse-face 'mode-line-highlight rendered)))))
 
+(ert-deftest test-modeline-config-vc-fetch-swallows-vc-errors ()
+  "Error: a signal from the VC backend is swallowed (returns nil) rather than
+propagating into the mode-line redisplay path, where it would break all redisplay."
+  (cl-letf (((symbol-function 'file-remote-p) (lambda (&rest _) nil))
+            ((symbol-function 'vc-backend) (lambda (&rest _) (error "git boom"))))
+    (should (null (cj/modeline-vc-fetch "/tmp/project/file.el")))))
+
 (provide 'test-modeline-config-vc-cache)
 ;;; test-modeline-config-vc-cache.el ends here
diff --git a/todo.org b/todo.org
index d2478ad07..dd53ce811 100644
--- a/todo.org
+++ b/todo.org
@@ -195,6 +195,16 @@ Spec draft: [[file:docs/theme-studio-palette-generator-spec.org][theme-studio-pa
 
 Build a constraint-first palette generator for Theme Studio: start from bg/fg, generate editable palette columns in OKLCH, preview candidate columns before applying them, and apply proposals by append/replace/regenerate modes while preserving stable column ids and existing assignments where possible. V1 is palette-only, not automatic face assignment; generator output becomes normal editable columns after apply.
 
+*** TODO [#C] theme-studio palette generator source modes for base-only vs ground-aware palettes :feature:
+:PROPERTIES:
+:LAST_REVIEWED: 2026-06-14
+:END:
+Tentative follow-up from walking through the generator algorithms. Consider splitting the current =source: palette= behavior into two explicit source modes, names TBD:
+- =base color palette= — current behavior; use non-ground base color columns only and ignore bg, fg, ground spans, and color spans.
+- =ground and base palette= — use bg/fg plus non-ground base color columns as generator anchors, useful when colored ground endpoints should shape fill-gap or harmony choices.
+
+This may be cancelled if the extra distinction makes the generator harder to understand. Before implementing, decide final names and whether ground-aware source should include only bg/fg or also ground span steps.
+
 ** TODO [#B] theme-studio save button does not overwrite the current theme file :bug:
 :PROPERTIES:
 :LAST_REVIEWED: 2026-06-13
@@ -1031,8 +1041,10 @@ From the 2026-06 config audit, =modules/help-config.el=:
 - =:111= — auto-mode-alist maps .info to an interactive command that KILLS the buffer mid find-file — programmatic =find-file-noselect= of any .info destroys buffers and pops Info windows. Drop the entry; keep the explicit command. Zero test coverage on this module (the two broken paths are exactly the untested ones).
 Fixed 2026-06-13: (1) extracted the save/cancel/open decision into a pure =cj/--info-open-plan= and routed =cj/open-with-info-mode= through it — no more =cl-return-from=, declining cancels cleanly; (2) deleted the dead =:hook= and the empty =:preface=; (3) dropped the destructive =auto-mode-alist= .info entry (kept =cj/open-with-info-mode= as an M-x command and =cj/browse-info-files= on C-h i). New test-help-config.el covers the planner (open / save-then-open / cancel) — 3 green; module loads clean. Stale daemon state (the .info auto-mode entry + the bogus info-mode-hook entry) cleared by hand so the running session is correct without a restart. Interactive Info open + find-file-no-longer-destructive are a VERIFY.
 
-** TODO [#B] modeline runs synchronous git on the redisplay path, unguarded :bug:solo:
+** DONE [#B] modeline runs synchronous git on the redisplay path, unguarded :bug:solo:
+CLOSED: [2026-06-14 Sun]
 =modules/modeline-config.el:173,154,145= — the mode-line :eval calls vc-backend/vc-state/vc-working-revision (synchronous git) on TTL expiry; a slow or unmounted filesystem stalls ALL redisplay. The cache key computes =file-truename= on every render (the "one stat per refresh" comment is wrong), and nothing is condition-case-wrapped, so a signal lands inside the mode-line eval. Defer the truename behind the TTL check; wrap the fetch in condition-case caching nil. From the 2026-06 config audit.
+Fixed 2026-06-14 (Approach A): dropped =file-truename= from =cj/modeline-vc-cache-key= (key is now =(file show-remote)=, no per-render stat; a moved symlink is caught at the next TTL refresh when vc-backend resolves the link fresh). Wrapped =cj/modeline-vc-fetch= in =condition-case ... (error nil)= so a git signal on a slow/unmounted FS degrades to no-VC-info instead of breaking redisplay. Rewrote the two truename cache-key tests to assert the cheap key; added a fetch-swallows-vc-errors test. 9 modeline vc tests green; live daemon confirms the 2-element key and nil-on-error fetch.
 
 ** TODO [#B] Stale elpa gptel shadows the local fork — likely the gptel-magit root :bug:quick:solo:
 =elpa/gptel-0.9.8.5= is still installed alongside the =~/code/gptel= fork (=ai-config.el:383=); package activation puts the elpa dir + autoloads on load-path, so which copy wins depends on ordering, and a mixed load (fork .el + elpa .elc) produces "impossible" bugs. =gptel-magit= (elpa) declares gptel as a dependency, so IT may be pulling the stale copy — check this first when working the open "[#B] Investigate gptel-magit not working properly" task. Fix: =package-delete= the elpa gptel + remove from .localrepo so the fork is the only copy on disk. From the 2026-06 config audit.
@@ -4429,6 +4441,11 @@ From the 2026-06-11 messenger-unification brainstorm. Google Voice has no offici
 
 ** TODO Manual testing and validation
 Exercised once the phases above land.
+*** VERIFY modeline still shows the git branch and state
+What we're verifying: the VC-cache simplification didn't change what the modeline shows on a normal repo. Fixed in modules/modeline-config.el (live in the daemon after reload).
+- Open a file inside a git repo
+- Glance at the mode-line VC segment
+Expected: the branch name and state still render as before (e.g. "main" with the usual state face). The change only drops a per-render stat and guards against git errors; normal display is unchanged.
 *** VERIFY info-mode open is non-destructive and cancels cleanly
 What we're verifying: opening a .info file no longer auto-kills the buffer, and the explicit cj/open-with-info-mode prompt cancels cleanly on decline. Fixed in modules/help-config.el; stale daemon state already cleared, so this also survives a fresh restart.
 - find-file a .info file (e.g. one under elpa) — it should open as an ordinary buffer, not vanish into Info