git.iko.soy/crush
on main help sign in
fix(hooks): recompile matchers after config reload
38a846dChristian Rocha
internal/config/reload_hooks_test.gonew file+102
compare with main
unifiedsplit
@@ -0,0 +1,102 @@
1+package config_test
2+
3+import (
4+ "context"
5+ "os"
6+ "path/filepath"
7+ "testing"
8+
9+ "github.com/charmbracelet/crush/internal/config"
10+ "github.com/charmbracelet/crush/internal/hooks"
11+ "github.com/stretchr/testify/require"
12+)
13+
14+// TestReloadFromDisk_CompilesHookMatchers is a regression test for a bug
15+// where ReloadFromDisk dropped the compiled matcher regex on every hook,
16+// causing a matcher like "^bash$" to match every tool call after any
17+// SetConfigField-triggered reload.
18+//
19+// The assertion is phrased in terms of observable Runner behavior (not
20+// internal field presence) so it stays valid if the Runner later owns
21+// matcher compilation itself.
22+func TestReloadFromDisk_CompilesHookMatchers(t *testing.T) {
23+ // No t.Parallel(): we Setenv HOME/XDG_CONFIG_HOME to isolate from the
24+ // developer's real global config, which may define its own hooks.
25+ isolated := t.TempDir()
26+ t.Setenv("HOME", isolated)
27+ t.Setenv("XDG_CONFIG_HOME", filepath.Join(isolated, ".config"))
28+ t.Setenv("XDG_DATA_HOME", filepath.Join(isolated, ".local", "share"))
29+
30+ workDir := t.TempDir()
31+ dataDir := t.TempDir()
32+ configPath := filepath.Join(workDir, "crush.json")
33+ cfgJSON := `{
34+ "hooks": {
35+ "PreToolUse": [
36+ {"matcher": "^bash$", "command": "exit 0"}
37+ ]
38+ }
39+ }`
40+ require.NoError(t, os.WriteFile(configPath, []byte(cfgJSON), 0o600))
41+
42+ store, err := config.Load(workDir, dataDir, false)
43+ require.NoError(t, err)
44+
45+ // Sanity: hook filtering works immediately after Load.
46+ assertHookFilters(t, store)
47+
48+ require.NoError(t, store.ReloadFromDisk(context.Background()))
49+
50+ // The actual regression check: filtering must still work after a
51+ // reload, not silently collapse to match-everything.
52+ assertHookFilters(t, store)
53+}
54+
55+// assertHookFilters builds a Runner from the store's current hooks and
56+// verifies the "^bash$" matcher rejects a non-bash tool while accepting
57+// bash.
58+func assertHookFilters(t *testing.T, store *config.ConfigStore) {
59+ t.Helper()
60+ preHooks := store.Config().Hooks[hooks.EventPreToolUse]
61+ require.Len(t, preHooks, 1)
62+
63+ runner := hooks.NewRunner(preHooks, t.TempDir(), t.TempDir())
64+
65+ nonMatch, err := runner.Run(context.Background(), hooks.EventPreToolUse, "sess", "view", `{}`)
66+ require.NoError(t, err)
67+ require.Equal(t, 0, nonMatch.HookCount, "view must not match ^bash$ matcher")
68+
69+ match, err := runner.Run(context.Background(), hooks.EventPreToolUse, "sess", "bash", `{}`)
70+ require.NoError(t, err)
71+ require.Equal(t, 1, match.HookCount, "bash must match ^bash$ matcher")
72+}
73+
74+// TestSetConfigField_AutoReload_PreservesHookMatcherFiltering verifies the
75+// dominant real-world trigger path: config writes call autoReload,
76+// autoReload calls ReloadFromDisk, and hook matching must remain correct.
77+func TestSetConfigField_AutoReload_PreservesHookMatcherFiltering(t *testing.T) {
78+ isolated := t.TempDir()
79+ t.Setenv("HOME", isolated)
80+ t.Setenv("XDG_CONFIG_HOME", filepath.Join(isolated, ".config"))
81+ t.Setenv("XDG_DATA_HOME", filepath.Join(isolated, ".local", "share"))
82+
83+ workDir := t.TempDir()
84+ dataDir := t.TempDir()
85+ configPath := filepath.Join(workDir, "crush.json")
86+ cfgJSON := `{
87+ "hooks": {
88+ "PreToolUse": [
89+ {"matcher": "^bash$", "command": "exit 0"}
90+ ]
91+ }
92+ }`
93+ require.NoError(t, os.WriteFile(configPath, []byte(cfgJSON), 0o600))
94+
95+ store, err := config.Load(workDir, dataDir, false)
96+ require.NoError(t, err)
97+ assertHookFilters(t, store)
98+
99+ require.NoError(t, store.SetConfigField(config.ScopeGlobal, "options.debug", true))
100+
101+ assertHookFilters(t, store)
102+}
internal/config/store.go+6
@@ -590,6 +590,12 @@ func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {
590 }
590 }
591 }
591 }
592
592
593+ // Validate hooks after all config merging is complete so matcher
594+ // regexes are recompiled on the reloaded config (mirrors Load).
595+ if err := cfg.ValidateHooks(); err != nil {
596+ return fmt.Errorf("invalid hook configuration on reload: %w", err)
597+ }
598+
593 // Preserve runtime overrides
599 // Preserve runtime overrides
594 overrides := s.overrides
600 overrides := s.overrides
595
601