From e2e0bc093145650a45a1e663bc9674ea3542bea1 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Tue, 12 May 2026 09:22:57 -0400 Subject: [PATCH] fix(config): scope .crush discovery to the current repo When Crush starts up it looks for an existing .crush directory by walking from the working directory toward the filesystem root. In layouts with several worktrees of the same project under a common parent, that walk could end up putting crush.db files in the project root in worktrees. This stops the walk at the git working tree root, when one can be detected, and otherwise at the working directory itself. Each project should now get its own .crush as expected. Co-Authored-By: Charm Crush --- internal/config/load.go | 44 ++++++++++++++++++++++++- internal/config/load_test.go | 63 ++++++++++++++++++++++++++++++++++++ internal/fsext/lookup.go | 18 ++++++++++- 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/internal/config/load.go b/internal/config/load.go index bc75422a082d0a20d081a700ae3c126bd727b132..011a09f9ee90885776cd6d198fd6082cf2cbb2a4 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -424,7 +424,7 @@ func (c *Config) setDefaults(workingDir, dataDir string) { if dataDir != "" { c.Options.DataDirectory = dataDir } else if c.Options.DataDirectory == "" { - if path, ok := fsext.LookupClosest(workingDir, defaultDataDirectory); ok { + if path, ok := fsext.LookupClosestBounded(workingDir, projectBoundary(workingDir), defaultDataDirectory); ok { c.Options.DataDirectory = path } else { c.Options.DataDirectory = filepath.Join(workingDir, defaultDataDirectory) @@ -873,6 +873,48 @@ func isInsideWorktree() bool { return err == nil && strings.TrimSpace(string(bts)) == "true" } +// worktreeRoot returns the absolute path of the git working tree root for +// dir, or the empty string if dir is not inside a working tree (bare +// repositories, missing git binary, plain directories, or any other +// failure mode). Linked worktrees and submodules each report their own +// top-level, which is what callers want when bounding lookups. +func worktreeRoot(dir string) string { + cmd := exec.CommandContext( + context.Background(), + "git", "rev-parse", "--show-toplevel", + ) + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + return "" + } + root := strings.TrimSpace(string(out)) + if root == "" { + return "" + } + abs, err := filepath.Abs(root) + if err != nil { + return "" + } + return abs +} + +// projectBoundary returns the directory at which an upward configuration +// search rooted at dir should stop. It is the git working tree root when +// one can be detected, otherwise dir itself. Returning dir as a +// fallback keeps Crush from silently adopting state files placed above +// the current project. +func projectBoundary(dir string) string { + if root := worktreeRoot(dir); root != "" { + return root + } + abs, err := filepath.Abs(dir) + if err != nil { + return dir + } + return abs +} + // GlobalSkillsDirs returns the default directories for Agent Skills. // Skills in these directories are auto-discovered and their files can be read // without permission prompts. diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 0a50cce5ea38c8f73bfa60111d410dc15a60fd4e..50a532c0ce8a9a330f82b9b7fb54c2fb3dc8aa22 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -4,6 +4,7 @@ import ( "io" "log/slog" "os" + "os/exec" "path/filepath" "testing" @@ -111,6 +112,68 @@ func TestConfig_setDefaults(t *testing.T) { require.Equal(t, filepath.Join(workingDir, "state"), cfg.Options.DataDirectory) }) + + t.Run("does not adopt .crush from a parent project", func(t *testing.T) { + parent := t.TempDir() + + // .crush in the parent: it should not be reused by the child + // because there is no git context joining them. + require.NoError(t, os.Mkdir(filepath.Join(parent, defaultDataDirectory), 0o755)) + + child := filepath.Join(parent, "child") + require.NoError(t, os.Mkdir(child, 0o755)) + + cfg := &Config{} + cfg.setDefaults(child, "") + + require.Equal(t, + filepath.Clean(filepath.Join(child, defaultDataDirectory)), + filepath.Clean(cfg.Options.DataDirectory), + ) + }) + + t.Run("does not climb out of git worktree to find .crush", func(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + + parent := t.TempDir() + + // Stray .crush above the worktree root. + require.NoError(t, os.Mkdir(filepath.Join(parent, defaultDataDirectory), 0o755)) + + worktree := filepath.Join(parent, "worktree") + require.NoError(t, os.Mkdir(worktree, 0o755)) + + sub := filepath.Join(worktree, "pkg") + require.NoError(t, os.Mkdir(sub, 0o755)) + + // Make worktree a real git repo so the boundary detection + // resolves to it, mirroring what happens with linked worktrees + // in real usage. + gitInit := exec.CommandContext(t.Context(), "git", "init", "-q") + gitInit.Dir = worktree + require.NoError(t, gitInit.Run()) + + cfg := &Config{} + cfg.setDefaults(sub, "") + + // Resolve symlinks because TempDir on macOS sits under /var + // which is a symlink to /private/var. The data directory has + // not been created yet, so resolve its parent and join. + gotDir, gotName := filepath.Split(cfg.Options.DataDirectory) + gotEvalDir, err := filepath.EvalSymlinks(filepath.Clean(gotDir)) + require.NoError(t, err) + gotEval := filepath.Join(gotEvalDir, gotName) + + strayEval, err := filepath.EvalSymlinks(filepath.Join(parent, defaultDataDirectory)) + require.NoError(t, err) + require.NotEqual(t, strayEval, gotEval, "must not adopt parent .crush") + + subEval, err := filepath.EvalSymlinks(sub) + require.NoError(t, err) + require.Equal(t, filepath.Join(subEval, defaultDataDirectory), gotEval) + }) } func TestConfig_configureProviders(t *testing.T) { diff --git a/internal/fsext/lookup.go b/internal/fsext/lookup.go index e9d43f791bb5259dc437f6749310e741d56271cc..696219ac89209b718ed0e106a4bc6a879105961c 100644 --- a/internal/fsext/lookup.go +++ b/internal/fsext/lookup.go @@ -197,6 +197,11 @@ func traverseUp(dir string, walkFn func(dir string, owner int) error) error { // traverseUp instead. If stopDir is set but is not an ancestor of dir // the walk still stops at the filesystem root, so callers cannot // accidentally produce an infinite walk by passing a sibling path. +// +// Boundary comparison is performed against symlink-resolved paths so +// that callers passing logically equivalent paths (a symlinked /var vs +// the underlying /private/var, for example) still terminate at the +// expected directory. func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) error) error { cwd, err := filepath.Abs(dir) if err != nil { @@ -210,6 +215,7 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e return fmt.Errorf("cannot convert stop dir to absolute path: %w", err) } } + canonStop := canonicalize(stop) owner, err := Owner(dir) if err != nil { @@ -219,7 +225,7 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e for { err := walkFn(cwd, owner) if err == nil || errors.Is(err, filepath.SkipDir) { - if cwd == stop { + if canonicalize(cwd) == canonStop { return nil } @@ -240,6 +246,16 @@ func traverseUpBounded(dir, stopDir string, walkFn func(dir string, owner int) e } } +// canonicalize resolves any symbolic links in path. If resolution fails +// (typically because path does not exist yet) the original path is +// returned cleaned, so callers can still perform stable equality checks. +func canonicalize(path string) string { + if resolved, err := filepath.EvalSymlinks(path); err == nil { + return resolved + } + return filepath.Clean(path) +} + // probeEnt checks if entity at given path exists and belongs to given owner func probeEnt(fspath string, owner int) error { _, err := os.Stat(fspath)