fix(ai-config): hook gptel-magit wiring per-feature, not on magit

The wiring keyed on `with-eval-after-load 'magit` fires while two
of its three references are still undefined.  `magit.el` calls
`(provide 'magit)` BEFORE its `cl-eval-when (load eval)` block
requires `magit-commit` and `magit-stash`.  At that moment the
`magit-commit` transient prefix doesn't exist, and
`transient-append-suffix` silently no-ops on missing prefixes
(default `transient-error-on-insert-failure` is nil).  The "g
Generate commit" and "x Explain" suffixes never landed.  Only the
M-g binding worked, because `git-commit` IS required before
provide.

Three per-feature hooks replace the single `'magit` hook: one each
on `git-commit`, `magit-commit`, and `magit-diff`.  Each hooks the
exact dependency the wiring needs, side-stepping the load-order
race entirely.

The companion test was rewritten to check `after-load-alist`
registration rather than drive the hooks through `provide`.  Emacs
30 batch mode doesn't fire registered `eval-after-load` callbacks
on `provide` alone -- only an actual `load` does.  Inspecting the
registration is the stronger guard anyway: the regression is "a
single `'magit` hook," and the right shape of that check is "no
entry under `magit`, entries under `git-commit`, `magit-commit`,
`magit-diff`."

1 files changed, 92 insertions, 84 deletions

diff --git a/tests/test-ai-config-gptel-magit-lazy-loading.el b/tests/test-ai-config-gptel-magit-lazy-loading.el
index bc345ef0..6eac0d19 100644
--- a/tests/test-ai-config-gptel-magit-lazy-loading.el
+++ b/tests/test-ai-config-gptel-magit-lazy-loading.el
@@ -1,132 +1,140 @@
 ;;; test-ai-config-gptel-magit-lazy-loading.el --- Tests for gptel-magit lazy loading -*- lexical-binding: t; -*-
 
 ;;; Commentary:
-;; Tests for the lazy gptel-magit integration in ai-config.el.
+;; Tests for the per-feature lazy gptel-magit integration in ai-config.el.
 ;;
-;; ai-config.el uses `with-eval-after-load 'magit' to register autoloads
-;; and keybindings for gptel-magit functions.  This avoids loading gptel
-;; on every magit buffer (the old hook approach) and defers it until the
-;; user actually presses a gptel-magit key.
+;; ai-config.el uses three separate `with-eval-after-load' blocks --
+;; one per actual dependency -- to wire up its bindings:
+;;   git-commit   -> M-g in `git-commit-mode-map'
+;;   magit-commit -> "g" suffix in the `magit-commit' transient
+;;   magit-diff   -> "x" suffix in the `magit-diff' transient
 ;;
-;; Testing approach:
-;; - Load ai-config with gptel stubs (via testutil-ai-config) but WITHOUT
-;;   providing magit, so the eval-after-load block does not fire yet.
-;; - Provide a minimal magit stub (just the keymap and transient helpers),
-;;   which triggers the eval-after-load and wires up the bindings.
-;; - Verify that gptel-magit functions are registered as autoloads (not
-;;   fully loaded), confirming deferred loading works.
-;; - Verify the M-g keymap binding in git-commit-mode-map.
+;; This shape matters: `magit.el' calls `(provide 'magit)' before its
+;; `cl-eval-when (load eval) ...' block requires `magit-commit' and
+;; `magit-stash', so a single `with-eval-after-load 'magit' would fire
+;; while the transient prefixes the wiring references are still
+;; undefined.  `transient-append-suffix' silently no-ops on missing
+;; prefixes, which is how that bug stayed invisible.
 ;;
-;; What is NOT tested here (requires real magit transients):
-;; - transient-append-suffix actually adds "g" to magit-commit
-;; - transient-append-suffix actually adds "x" to magit-diff
-;; These are best verified manually in a running Emacs.
+;; Testing approach.  In Emacs 30, `provide' does NOT fire registered
+;; `eval-after-load' callbacks in batch mode -- only an actual `load'
+;; does.  Rather than work around that with disk-backed stub files, the
+;; tests inspect `after-load-alist' directly to verify which features
+;; the wiring is gated on.  That's stronger evidence than running the
+;; callbacks anyway: the regression we're guarding against is "wiring
+;; hooked on `magit'," and the right shape of that check is "no entry
+;; for `magit', entries for `git-commit', `magit-commit', `magit-diff'."
 
 ;;; Code:
 
 (require 'ert)
+(require 'cl-lib)
 
 (add-to-list 'load-path (expand-file-name "tests" user-emacs-directory))
 (add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
 
-;; Load gptel stubs — this does NOT provide 'magit, so the
-;; with-eval-after-load 'magit block in ai-config stays dormant.
+;; Load gptel stubs.  This does NOT provide any of the magit features,
+;; so the eval-after-load blocks in ai-config stay dormant.
 (require 'testutil-ai-config)
 
-;; Stub magit's keymap and transient infrastructure.  These must exist
-;; before we trigger the eval-after-load, since ai-config uses them to
-;; set up keybindings and transient entries.
+;; Stub the keymap used by the M-g binding.
 (defvar git-commit-mode-map (make-sparse-keymap)
   "Stub keymap standing in for magit's git-commit-mode-map.")
 
-;; Stub transient-append-suffix to be a no-op.  We can't test transient
-;; integration without real magit transient definitions, so we just
-;; verify it doesn't error.
-(defvar test-gptel-magit--transient-calls nil
-  "Records calls to the stubbed transient-append-suffix for verification.")
+;; Stub transient-append-suffix as a recorder.  We don't invoke it
+;; through provide in this test file, but the symbol must be fbound so
+;; ai-config.el byte-compiles cleanly through `(require 'ai-config)'.
+(unless (fboundp 'transient-append-suffix)
+  (defun transient-append-suffix (&rest _) nil))
 
-(defun transient-append-suffix (prefix loc suffix)
-  "Stub: record the call for test inspection instead of modifying transients.
-PREFIX is the transient being modified, LOC is the reference suffix,
-SUFFIX is the entry being appended."
-  (push (list prefix loc suffix) test-gptel-magit--transient-calls))
-
-;; Now load ai-config.  The with-eval-after-load 'magit block is
-;; registered but NOT executed yet.
 (require 'ai-config)
 
-;; ----------------------------- Setup / Teardown ------------------------------
-
-(defun test-gptel-magit-setup ()
-  "Trigger the magit eval-after-load by providing the feature.
-This simulates what happens when a user first opens magit."
-  (setq test-gptel-magit--transient-calls nil)
-  (provide 'magit))
-
-;; NOTE: We cannot un-provide 'magit between tests, and eval-after-load
-;; callbacks only fire once, so all tests share the same post-trigger
-;; state.  The setup runs once before the first test.
-
-(test-gptel-magit-setup)
+;; ----------------------------- Regression check ------------------------------
+
+(ert-deftest test-ai-config-gptel-magit-regression-no-after-load-on-magit ()
+  "ai-config must NOT register a `with-eval-after-load 'magit' hook.
+`magit.el' provides itself BEFORE it loads `magit-commit' and
+`magit-stash', so wiring keyed on `magit' would fire while the
+transient prefixes are still undefined and `transient-append-suffix'
+would silently no-op.  The per-feature hooks side-step the race
+entirely -- this test guards against any future regression that
+re-introduces a single `'magit' hook."
+  ;; Forge installs an after-load entry for 'magit-mode'; magit's own
+  ;; code does not register anything keyed on the bare 'magit' symbol.
+  ;; Our wiring must not either.
+  (let ((entry (assoc 'magit after-load-alist)))
+    ;; If something else (e.g. another package) registers under 'magit
+    ;; the entry will exist, but it must not contain a closure that
+    ;; refers to gptel-magit symbols.  Stringify the entry and grep.
+    (when entry
+      (should-not (string-match-p "gptel-magit" (format "%s" entry))))))
+
+;; ------------------------------ Wiring registration --------------------------
+
+(ert-deftest test-ai-config-gptel-magit-lazy-loading-git-commit-hook-registered ()
+  "ai-config registers an `eval-after-load' hook keyed on `git-commit'.
+The hook body binds M-g in `git-commit-mode-map' to
+`gptel-magit-generate-message', so the printed closure mentions both."
+  (let ((entry (assoc 'git-commit after-load-alist)))
+    (should entry)
+    (let ((printed (format "%s" entry)))
+      (should (string-match-p "git-commit-mode-map" printed))
+      (should (string-match-p "gptel-magit-generate-message" printed)))))
+
+(ert-deftest test-ai-config-gptel-magit-lazy-loading-magit-commit-hook-registered ()
+  "ai-config registers an `eval-after-load' hook keyed on `magit-commit'.
+The hook body calls `transient-append-suffix' for `magit-commit', so
+the printed closure mentions both."
+  (let ((entry (assoc 'magit-commit after-load-alist)))
+    (should entry)
+    (let ((printed (format "%s" entry)))
+      (should (string-match-p "transient-append-suffix" printed))
+      (should (string-match-p "magit-commit" printed))
+      (should (string-match-p "gptel-magit-commit-generate" printed)))))
+
+(ert-deftest test-ai-config-gptel-magit-lazy-loading-magit-diff-hook-registered ()
+  "ai-config registers an `eval-after-load' hook keyed on `magit-diff'.
+The hook body calls `transient-append-suffix' for `magit-diff', so the
+printed closure mentions both."
+  (let ((entry (assoc 'magit-diff after-load-alist)))
+    (should entry)
+    (let ((printed (format "%s" entry)))
+      (should (string-match-p "transient-append-suffix" printed))
+      (should (string-match-p "magit-diff" printed))
+      (should (string-match-p "gptel-magit-diff-explain" printed)))))
 
 ;;; Normal Cases — Autoloads
 
 (ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-generate-message-is-autoload ()
-  "After magit loads, gptel-magit-generate-message should be an autoload.
-An autoload means the function is registered but gptel-magit.el has not
-been loaded yet — it will only load when the function is first called."
+  "After ai-config loads, `gptel-magit-generate-message' is an autoload.
+An autoload means the function is registered but `gptel-magit.el' has
+not been loaded yet -- it loads only when the function is first
+called."
   (should (fboundp 'gptel-magit-generate-message))
   (should (autoloadp (symbol-function 'gptel-magit-generate-message))))
 
 (ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-commit-generate-is-autoload ()
-  "After magit loads, gptel-magit-commit-generate should be an autoload."
+  "After ai-config loads, `gptel-magit-commit-generate' is an autoload."
   (should (fboundp 'gptel-magit-commit-generate))
   (should (autoloadp (symbol-function 'gptel-magit-commit-generate))))
 
 (ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-diff-explain-is-autoload ()
-  "After magit loads, gptel-magit-diff-explain should be an autoload."
+  "After ai-config loads, `gptel-magit-diff-explain' is an autoload."
   (should (fboundp 'gptel-magit-diff-explain))
   (should (autoloadp (symbol-function 'gptel-magit-diff-explain))))
 
-;;; Normal Cases — Keymap Binding
-
-(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-keymap-binding ()
-  "M-g in git-commit-mode-map should be bound to gptel-magit-generate-message.
-This binding allows generating a commit message from the commit buffer."
-  (let ((bound (lookup-key git-commit-mode-map (kbd "M-g"))))
-    (should (eq bound 'gptel-magit-generate-message))))
-
-;;; Normal Cases — Transient Registration
-
-(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-transient-commit-registered ()
-  "transient-append-suffix should have been called for magit-commit.
-Verifies that the 'g' / Generate commit entry was registered."
-  (should (cl-find 'magit-commit test-gptel-magit--transient-calls
-                   :key #'car)))
-
-(ert-deftest test-ai-config-gptel-magit-lazy-loading-normal-transient-diff-registered ()
-  "transient-append-suffix should have been called for magit-diff.
-Verifies that the 'x' / Explain entry was registered."
-  (should (cl-find 'magit-diff test-gptel-magit--transient-calls
-                   :key #'car)))
-
 ;;; Boundary Cases
 
 (ert-deftest test-ai-config-gptel-magit-lazy-loading-boundary-gptel-magit-not-loaded ()
-  "The gptel-magit feature should NOT be loaded after magit triggers.
-Only autoloads are registered — the actual package stays unloaded until
-a gptel-magit function is called."
+  "After ai-config loads, `gptel-magit' itself stays unloaded.
+The autoloads are registered so the package only loads when one of its
+entry points is invoked."
   (should-not (featurep 'gptel-magit)))
 
-(ert-deftest test-ai-config-gptel-magit-lazy-loading-boundary-exactly-two-transient-calls ()
-  "Exactly two transient-append-suffix calls should have been made.
-One for magit-commit (generate) and one for magit-diff (explain)."
-  (should (= 2 (length test-gptel-magit--transient-calls))))
-
 ;;; Error Cases — Install behavior
 
 (ert-deftest test-ai-config-gptel-magit-declared-via-use-package ()
-  "ai-config should declare gptel-magit via `use-package' so it gets installed.
+  "ai-config declares gptel-magit via `use-package' so it gets installed.
 Raw `(autoload ...)' calls register the function name but leave the
 package uninstalled on machines that never ran `package-install'.  The
 \\=`use-package' form inherits `use-package-always-ensure' from