diff --git a/internal/config/reload_hooks_test.go b/internal/config/reload_hooks_test.go new file mode 100644 index 0000000000000000000000000000000000000000..a21dfa89a0b7c61260a96f800acc34a18a2b086b --- /dev/null +++ b/internal/config/reload_hooks_test.go @@ -0,0 +1,102 @@ +package config_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/charmbracelet/crush/internal/config" + "github.com/charmbracelet/crush/internal/hooks" + "github.com/stretchr/testify/require" +) + +// TestReloadFromDisk_CompilesHookMatchers is a regression test for a bug +// where ReloadFromDisk dropped the compiled matcher regex on every hook, +// causing a matcher like "^bash$" to match every tool call after any +// SetConfigField-triggered reload. +// +// The assertion is phrased in terms of observable Runner behavior (not +// internal field presence) so it stays valid if the Runner later owns +// matcher compilation itself. +func TestReloadFromDisk_CompilesHookMatchers(t *testing.T) { + // No t.Parallel(): we Setenv HOME/XDG_CONFIG_HOME to isolate from the + // developer's real global config, which may define its own hooks. + isolated := t.TempDir() + t.Setenv("HOME", isolated) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(isolated, ".config")) + t.Setenv("XDG_DATA_HOME", filepath.Join(isolated, ".local", "share")) + + workDir := t.TempDir() + dataDir := t.TempDir() + configPath := filepath.Join(workDir, "crush.json") + cfgJSON := `{ + "hooks": { + "PreToolUse": [ + {"matcher": "^bash$", "command": "exit 0"} + ] + } + }` + require.NoError(t, os.WriteFile(configPath, []byte(cfgJSON), 0o600)) + + store, err := config.Load(workDir, dataDir, false) + require.NoError(t, err) + + // Sanity: hook filtering works immediately after Load. + assertHookFilters(t, store) + + require.NoError(t, store.ReloadFromDisk(context.Background())) + + // The actual regression check: filtering must still work after a + // reload, not silently collapse to match-everything. + assertHookFilters(t, store) +} + +// assertHookFilters builds a Runner from the store's current hooks and +// verifies the "^bash$" matcher rejects a non-bash tool while accepting +// bash. +func assertHookFilters(t *testing.T, store *config.ConfigStore) { + t.Helper() + preHooks := store.Config().Hooks[hooks.EventPreToolUse] + require.Len(t, preHooks, 1) + + runner := hooks.NewRunner(preHooks, t.TempDir(), t.TempDir()) + + nonMatch, err := runner.Run(context.Background(), hooks.EventPreToolUse, "sess", "view", `{}`) + require.NoError(t, err) + require.Equal(t, 0, nonMatch.HookCount, "view must not match ^bash$ matcher") + + match, err := runner.Run(context.Background(), hooks.EventPreToolUse, "sess", "bash", `{}`) + require.NoError(t, err) + require.Equal(t, 1, match.HookCount, "bash must match ^bash$ matcher") +} + +// TestSetConfigField_AutoReload_PreservesHookMatcherFiltering verifies the +// dominant real-world trigger path: config writes call autoReload, +// autoReload calls ReloadFromDisk, and hook matching must remain correct. +func TestSetConfigField_AutoReload_PreservesHookMatcherFiltering(t *testing.T) { + isolated := t.TempDir() + t.Setenv("HOME", isolated) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(isolated, ".config")) + t.Setenv("XDG_DATA_HOME", filepath.Join(isolated, ".local", "share")) + + workDir := t.TempDir() + dataDir := t.TempDir() + configPath := filepath.Join(workDir, "crush.json") + cfgJSON := `{ + "hooks": { + "PreToolUse": [ + {"matcher": "^bash$", "command": "exit 0"} + ] + } + }` + require.NoError(t, os.WriteFile(configPath, []byte(cfgJSON), 0o600)) + + store, err := config.Load(workDir, dataDir, false) + require.NoError(t, err) + assertHookFilters(t, store) + + require.NoError(t, store.SetConfigField(config.ScopeGlobal, "options.debug", true)) + + assertHookFilters(t, store) +} diff --git a/internal/config/store.go b/internal/config/store.go index 86fa937d1595b83f97fa43d3abd0a39cabd5f8cc..7c3d0f80cdebf545d2601ea78f490eddef3067e7 100644 --- a/internal/config/store.go +++ b/internal/config/store.go @@ -590,6 +590,12 @@ func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error { } } + // Validate hooks after all config merging is complete so matcher + // regexes are recompiled on the reloaded config (mirrors Load). + if err := cfg.ValidateHooks(); err != nil { + return fmt.Errorf("invalid hook configuration on reload: %w", err) + } + // Preserve runtime overrides overrides := s.overrides