From 269bc30e2038750b0dbff76f329d5b07775a13e5 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Sun, 5 Apr 2026 16:42:50 -0400 Subject: [PATCH] feat(tools/crush_info): handle config staleness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track config file staleness and expose dirty/changed/missing state so crush_info can warn when in-memory config is outdated after on-disk edits. This also makes reload use freshly swapped config state before model/agent setup to avoid stale runtime wiring. 💘 Generated with Crush Assisted-by: Kimi-K2.5 via Crush --- internal/agent/tools/crush_info.go | 31 ++++ internal/agent/tools/crush_info_test.go | 72 ++++++++ internal/config/load.go | 4 + internal/config/store.go | 231 +++++++++++++++++++++++- internal/config/store_test.go | 225 +++++++++++++++++++++++ 5 files changed, 555 insertions(+), 8 deletions(-) diff --git a/internal/agent/tools/crush_info.go b/internal/agent/tools/crush_info.go index 1e90c6ba1fa41888493b651be2ade92a3cf04f9f..bbc96b60954273fe4342d5634529f608c8b7dead 100644 --- a/internal/agent/tools/crush_info.go +++ b/internal/agent/tools/crush_info.go @@ -36,6 +36,7 @@ func buildCrushInfo(cfg *config.ConfigStore, lspManager *lsp.Manager) string { var b strings.Builder writeConfigFiles(&b, cfg) + writeConfigStaleness(&b, cfg) writeModels(&b, cfg) writeProviders(&b, cfg) writeLSP(&b, lspManager, cfg) @@ -56,6 +57,36 @@ func writeConfigFiles(b *strings.Builder, cfg *config.ConfigStore) { b.WriteString("\n") } +func writeConfigStaleness(b *strings.Builder, cfg *config.ConfigStore) { + staleness := cfg.ConfigStaleness() + + b.WriteString("[config]\n") + fmt.Fprintf(b, "dirty = %v\n", staleness.Dirty) + + if len(staleness.Changed) > 0 { + sorted := slices.Clone(staleness.Changed) + slices.Sort(sorted) + fmt.Fprintf(b, "changed_paths = %s\n", strings.Join(sorted, ", ")) + } + + if len(staleness.Missing) > 0 { + sorted := slices.Clone(staleness.Missing) + slices.Sort(sorted) + fmt.Fprintf(b, "missing_paths = %s\n", strings.Join(sorted, ", ")) + } + + if len(staleness.Errors) > 0 { + var paths []string + for path := range staleness.Errors { + paths = append(paths, path) + } + slices.Sort(paths) + fmt.Fprintf(b, "errors = %s\n", strings.Join(paths, ", ")) + } + + b.WriteString("\n") +} + func writeModels(b *strings.Builder, cfg *config.ConfigStore) { c := cfg.Config() if len(c.Models) == 0 { diff --git a/internal/agent/tools/crush_info_test.go b/internal/agent/tools/crush_info_test.go index bf9a2f01420b025652435e2099238fe15b069021..83b14884b1bab4f7d41f30e7b6c3c85794a456cc 100644 --- a/internal/agent/tools/crush_info_test.go +++ b/internal/agent/tools/crush_info_test.go @@ -2,6 +2,8 @@ package tools import ( "errors" + "os" + "path/filepath" "strings" "testing" "time" @@ -298,3 +300,73 @@ func TestCrushInfo_EmptySectionsOmitted(t *testing.T) { require.NotContains(t, output, "[lsp]") require.NotContains(t, output, "[mcp]") } + +func TestCrushInfo_ConfigStaleness_Clean(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{}`), 0o600)) + + store := config.NewTestStore(&config.Config{ + Providers: csync.NewMap[string, config.ProviderConfig](), + }, configPath) + + // Capture snapshot (normally done in Load) + store.CaptureStalenessSnapshot([]string{configPath}) + + output := buildCrushInfo(store, nil) + require.Contains(t, output, "[config]") + require.Contains(t, output, "dirty = false") + require.NotContains(t, output, "changed_paths") + require.NotContains(t, output, "missing_paths") +} + +func TestCrushInfo_ConfigStaleness_Dirty(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": false}`), 0o600)) + + store := config.NewTestStore(&config.Config{ + Providers: csync.NewMap[string, config.ProviderConfig](), + }, configPath) + + // Capture initial snapshot + store.CaptureStalenessSnapshot([]string{configPath}) + + // Modify file to trigger dirty state + time.Sleep(10 * time.Millisecond) + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": true}`), 0o600)) + + output := buildCrushInfo(store, nil) + require.Contains(t, output, "[config]") + require.Contains(t, output, "dirty = true") + require.Contains(t, output, "changed_paths") + require.Contains(t, output, configPath) +} + +func TestCrushInfo_ConfigStaleness_MissingPath(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + require.NoError(t, os.WriteFile(configPath, []byte(`{}`), 0o600)) + + store := config.NewTestStore(&config.Config{ + Providers: csync.NewMap[string, config.ProviderConfig](), + }, configPath) + + // Capture initial snapshot + store.CaptureStalenessSnapshot([]string{configPath}) + + // Delete file to trigger missing state + require.NoError(t, os.Remove(configPath)) + + output := buildCrushInfo(store, nil) + require.Contains(t, output, "[config]") + require.Contains(t, output, "dirty = true") + require.Contains(t, output, "missing_paths") + require.Contains(t, output, configPath) +} diff --git a/internal/config/load.go b/internal/config/load.go index 2de15c1068c576cd24867995790a02a8c9cba533..68dc04f58fe00fec0580e9cf1d39465b36c6b782 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -104,6 +104,10 @@ func Load(workingDir, dataDir string, debug bool) (*ConfigStore, error) { return nil, fmt.Errorf("failed to configure selected models: %w", err) } store.SetupAgents() + + // Capture initial staleness snapshot + store.captureStalenessSnapshot(loadedPaths) + return store, nil } diff --git a/internal/config/store.go b/internal/config/store.go index ab38e013c45c52ccc4335014162f2d0c4e183b61..66e434d852063446996df3ad87af7da8b5214cb8 100644 --- a/internal/config/store.go +++ b/internal/config/store.go @@ -11,6 +11,7 @@ import ( "charm.land/catwalk/pkg/catwalk" hyperp "github.com/charmbracelet/crush/internal/agent/hyper" + "github.com/charmbracelet/crush/internal/env" "github.com/charmbracelet/crush/internal/oauth" "github.com/charmbracelet/crush/internal/oauth/copilot" "github.com/charmbracelet/crush/internal/oauth/hyper" @@ -18,6 +19,14 @@ import ( "github.com/tidwall/sjson" ) +// fileSnapshot captures metadata about a config file at a point in time. +type fileSnapshot struct { + Path string + Exists bool + Size int64 + ModTime int64 // UnixNano +} + // RuntimeOverrides holds per-session settings that are never persisted to // disk. They are applied on top of the loaded Config and survive only for // the lifetime of the process (or workspace). @@ -29,14 +38,16 @@ type RuntimeOverrides struct { // pure-data Config, runtime state (working directory, resolver, known // providers), and persistence to both global and workspace config files. type ConfigStore struct { - config *Config - workingDir string - resolver VariableResolver - globalDataPath string // ~/.local/share/crush/crush.json - workspacePath string // .crush/crush.json - loadedPaths []string // config files that were successfully loaded - knownProviders []catwalk.Provider - overrides RuntimeOverrides + config *Config + workingDir string + resolver VariableResolver + globalDataPath string // ~/.local/share/crush/crush.json + workspacePath string // .crush/crush.json + loadedPaths []string // config files that were successfully loaded + knownProviders []catwalk.Provider + overrides RuntimeOverrides + trackedConfigPaths []string // unique, normalized config file paths + snapshots map[string]fileSnapshot // path -> snapshot at last capture } // Config returns the pure-data config struct (read-only after load). @@ -383,3 +394,207 @@ func (s *ConfigStore) ImportCopilot() (*oauth.Token, bool) { slog.Info("GitHub Copilot successfully imported") return token, true } + +// StalenessResult contains the result of a staleness check. +type StalenessResult struct { + Dirty bool + Changed []string + Missing []string + Errors map[string]error // stat errors by path +} + +// ConfigStaleness checks whether any tracked config files have changed on disk +// since the last snapshot. Returns dirty=true if any files changed or went +// missing, along with sorted lists of affected paths. Stat errors are +// captured in Errors map but still treated as non-existence for dirty detection. +func (s *ConfigStore) ConfigStaleness() StalenessResult { + var result StalenessResult + result.Errors = make(map[string]error) + + for _, path := range s.trackedConfigPaths { + snapshot, hadSnapshot := s.snapshots[path] + + info, err := os.Stat(path) + exists := err == nil && !info.IsDir() + + if err != nil && !os.IsNotExist(err) { + // Capture permission/IO errors separately from non-existence + result.Errors[path] = err + result.Dirty = true + } + + if !exists { + if hadSnapshot && snapshot.Exists { + // File existed before but now missing + result.Missing = append(result.Missing, path) + result.Dirty = true + } + continue + } + + // File exists now + if !hadSnapshot || !snapshot.Exists { + // File didn't exist before but does now + result.Changed = append(result.Changed, path) + result.Dirty = true + continue + } + + // Check for content or metadata changes + if snapshot.Size != info.Size() || snapshot.ModTime != info.ModTime().UnixNano() { + result.Changed = append(result.Changed, path) + result.Dirty = true + } + } + + // Sort for deterministic output + slices.Sort(result.Changed) + slices.Sort(result.Missing) + + return result +} + +// RefreshStalenessSnapshot captures fresh snapshots of all tracked config files. +// Call this after reloading config to clear dirty state. +func (s *ConfigStore) RefreshStalenessSnapshot() error { + if s.snapshots == nil { + s.snapshots = make(map[string]fileSnapshot) + } + + for _, path := range s.trackedConfigPaths { + info, err := os.Stat(path) + exists := err == nil && !info.IsDir() + + snapshot := fileSnapshot{ + Path: path, + Exists: exists, + } + + if exists { + snapshot.Size = info.Size() + snapshot.ModTime = info.ModTime().UnixNano() + } + + s.snapshots[path] = snapshot + } + + return nil +} + +// CaptureStalenessSnapshot captures snapshots for the given paths, building the +// tracked config paths list. Paths are deduplicated and normalized. +func (s *ConfigStore) CaptureStalenessSnapshot(paths []string) { + // Build unique set of normalized paths + seen := make(map[string]struct{}) + for _, p := range paths { + if p == "" { + continue + } + // Normalize path + abs, err := filepath.Abs(p) + if err != nil { + abs = p + } + seen[abs] = struct{}{} + } + + // Also track workspace and global config paths if set + if s.workspacePath != "" { + abs, err := filepath.Abs(s.workspacePath) + if err == nil { + seen[abs] = struct{}{} + } + } + if s.globalDataPath != "" { + abs, err := filepath.Abs(s.globalDataPath) + if err == nil { + seen[abs] = struct{}{} + } + } + + // Build sorted list for deterministic ordering + s.trackedConfigPaths = make([]string, 0, len(seen)) + for p := range seen { + s.trackedConfigPaths = append(s.trackedConfigPaths, p) + } + slices.Sort(s.trackedConfigPaths) + + // Capture initial snapshots + s.RefreshStalenessSnapshot() +} + +// captureStalenessSnapshot is an alias for CaptureStalenessSnapshot for internal use. +func (s *ConfigStore) captureStalenessSnapshot(paths []string) { + s.CaptureStalenessSnapshot(paths) +} + +// ReloadFromDisk re-runs the config load/merge flow and updates the in-memory +// config safely. It rebuilds the staleness snapshot after successful reload. +func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error { + if s.workingDir == "" { + return fmt.Errorf("cannot reload: working directory not set") + } + + configPaths := lookupConfigs(s.workingDir) + cfg, loadedPaths, err := loadFromConfigPaths(configPaths) + if err != nil { + return fmt.Errorf("failed to reload config: %w", err) + } + + // Apply defaults (using existing data directory if set) + var dataDir string + if s.config != nil && s.config.Options != nil { + dataDir = s.config.Options.DataDirectory + } + cfg.setDefaults(s.workingDir, dataDir) + + // Merge workspace config if present + workspacePath := filepath.Join(cfg.Options.DataDirectory, fmt.Sprintf("%s.json", appName)) + if wsData, err := os.ReadFile(workspacePath); err == nil && len(wsData) > 0 { + merged, mergeErr := loadFromBytes(append([][]byte{mustMarshalConfig(cfg)}, wsData)) + if mergeErr == nil { + dataDir := cfg.Options.DataDirectory + *cfg = *merged + cfg.setDefaults(s.workingDir, dataDir) + loadedPaths = append(loadedPaths, workspacePath) + } + } + + // Preserve runtime overrides + overrides := s.overrides + + // Reconfigure providers + env := env.New() + resolver := NewShellVariableResolver(env) + providers, err := Providers(cfg) + if err != nil { + return fmt.Errorf("failed to load providers during reload: %w", err) + } + + if err := cfg.configureProviders(s, env, resolver, providers); err != nil { + return fmt.Errorf("failed to configure providers during reload: %w", err) + } + + // Update store state BEFORE running model/agent setup (so they see new config) + s.config = cfg + s.loadedPaths = loadedPaths + s.resolver = resolver + s.knownProviders = providers + s.overrides = overrides + s.workspacePath = workspacePath + + // Mirror startup flow: setup models and agents against NEW config + if !cfg.IsConfigured() { + slog.Warn("No providers configured after reload") + } else { + if err := configureSelectedModels(s, providers); err != nil { + return fmt.Errorf("failed to configure selected models during reload: %w", err) + } + s.SetupAgents() + } + + // Rebuild staleness tracking + s.captureStalenessSnapshot(loadedPaths) + + return nil +} diff --git a/internal/config/store_test.go b/internal/config/store_test.go index 91107943b4be67023ebcd9b2473fae53d5148b73..b3a4eab5b3d90cbb9dcfcc50a447c7e58ab8904b 100644 --- a/internal/config/store_test.go +++ b/internal/config/store_test.go @@ -1,10 +1,12 @@ package config import ( + "context" "errors" "os" "path/filepath" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -150,3 +152,226 @@ func TestScope_String(t *testing.T) { require.Equal(t, "workspace", ScopeWorkspace.String()) require.Contains(t, Scope(99).String(), "Scope(99)") } + +func TestConfigStaleness_CleanImmediatelyAfterSnapshot(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + + // Create a config file + content := []byte(`{"options": {"debug": true}}`) + require.NoError(t, os.WriteFile(configPath, content, 0o600)) + + store := &ConfigStore{ + config: &Config{}, + globalDataPath: configPath, + } + store.captureStalenessSnapshot([]string{configPath}) + + result := store.ConfigStaleness() + require.False(t, result.Dirty) + require.Empty(t, result.Changed) + require.Empty(t, result.Missing) +} + +func TestConfigStaleness_DetectsFileContentChange(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + + // Create initial config file + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": false}`), 0o600)) + + store := &ConfigStore{ + config: &Config{}, + globalDataPath: configPath, + } + store.captureStalenessSnapshot([]string{configPath}) + + // Modify the file + time.Sleep(10 * time.Millisecond) // Ensure different mtime + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": true}`), 0o600)) + + result := store.ConfigStaleness() + require.True(t, result.Dirty) + require.Contains(t, result.Changed, configPath) + require.Empty(t, result.Missing) +} + +func TestConfigStaleness_DetectsFileDeletion(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + + // Create initial config file + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": true}`), 0o600)) + + store := &ConfigStore{ + config: &Config{}, + globalDataPath: configPath, + } + store.captureStalenessSnapshot([]string{configPath}) + + // Delete the file + require.NoError(t, os.Remove(configPath)) + + result := store.ConfigStaleness() + require.True(t, result.Dirty) + require.Empty(t, result.Changed) + require.Contains(t, result.Missing, configPath) +} + +func TestConfigStaleness_DetectsNewFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + + // Don't create file initially + store := &ConfigStore{ + config: &Config{}, + globalDataPath: configPath, + } + store.captureStalenessSnapshot([]string{configPath}) + + // Now create the file + time.Sleep(10 * time.Millisecond) + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": true}`), 0o600)) + + result := store.ConfigStaleness() + require.True(t, result.Dirty) + require.Contains(t, result.Changed, configPath) + require.Empty(t, result.Missing) +} + +func TestConfigStaleness_SortedOutput(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + pathA := filepath.Join(dir, "a.json") + pathB := filepath.Join(dir, "b.json") + pathC := filepath.Join(dir, "c.json") + + // Create all files + for _, p := range []string{pathA, pathB, pathC} { + require.NoError(t, os.WriteFile(p, []byte(`{}`), 0o600)) + } + + store := &ConfigStore{ + config: &Config{}, + globalDataPath: pathA, + } + // Add in reverse order to test sorting + store.captureStalenessSnapshot([]string{pathC, pathA, pathB}) + + // Modify all files + time.Sleep(10 * time.Millisecond) + for _, p := range []string{pathA, pathB, pathC} { + require.NoError(t, os.WriteFile(p, []byte(`{"changed": true}`), 0o600)) + } + + result := store.ConfigStaleness() + require.True(t, result.Dirty) + // Should be sorted alphabetically + require.Equal(t, []string{pathA, pathB, pathC}, result.Changed) +} + +func TestConfigStaleness_RefreshClearsDirtyState(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + + // Create initial config file + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": false}`), 0o600)) + + store := &ConfigStore{ + config: &Config{}, + globalDataPath: configPath, + } + store.captureStalenessSnapshot([]string{configPath}) + + // Modify the file + time.Sleep(10 * time.Millisecond) + require.NoError(t, os.WriteFile(configPath, []byte(`{"debug": true}`), 0o600)) + + // Verify dirty + result := store.ConfigStaleness() + require.True(t, result.Dirty) + + // Refresh snapshot + require.NoError(t, store.RefreshStalenessSnapshot()) + + // Verify clean now + result = store.ConfigStaleness() + require.False(t, result.Dirty) + require.Empty(t, result.Changed) + require.Empty(t, result.Missing) +} + +// TestReloadFromDisk_UsesNewConfigValues is a regression test ensuring that +// ReloadFromDisk updates store state BEFORE running model/agent setup, +// so the new config values are used rather than stale pre-reload values. +func TestReloadFromDisk_UsesNewConfigValues(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "crush.json") + + // Create initial config with one model preference + initialConfig := `{ + "models": { + "large": {"provider": "openai", "model": "gpt-4"} + }, + "providers": { + "openai": { + "api_key": "test-key", + "models": [{"id": "gpt-4", "name": "GPT-4"}] + } + } + }` + require.NoError(t, os.WriteFile(configPath, []byte(initialConfig), 0o600)) + + // Load initial config properly + store, err := Load(dir, dir, false) + require.NoError(t, err) + + // Set globalDataPath for the test (Load doesn't set this directly) + store.globalDataPath = configPath + store.CaptureStalenessSnapshot([]string{configPath}) + + // Verify initial model + require.Equal(t, "openai", store.config.Models[SelectedModelTypeLarge].Provider) + require.Equal(t, "gpt-4", store.config.Models[SelectedModelTypeLarge].Model) + + // Modify config on disk to change model + updatedConfig := `{ + "models": { + "large": {"provider": "anthropic", "model": "claude-3"} + }, + "providers": { + "openai": { + "api_key": "test-key", + "models": [{"id": "gpt-4", "name": "GPT-4"}] + }, + "anthropic": { + "api_key": "test-key-2", + "models": [{"id": "claude-3", "name": "Claude 3"}] + } + } + }` + time.Sleep(10 * time.Millisecond) + require.NoError(t, os.WriteFile(configPath, []byte(updatedConfig), 0o600)) + + // Reload from disk + ctx := context.Background() + err = store.ReloadFromDisk(ctx) + require.NoError(t, err) + + // Verify the NEW config values are now in effect (regression check) + require.Equal(t, "anthropic", store.config.Models[SelectedModelTypeLarge].Provider) + require.Equal(t, "claude-3", store.config.Models[SelectedModelTypeLarge].Model) +}