From 1fe6b0006d5c88b055f588200bac7734d3d8bd73 Mon Sep 17 00:00:00 2001 From: Jordan Jackson Date: Tue, 6 Oct 2026 23:48:03 -0400 Subject: [PATCH 1/2] fix(scanner,config): skip nested worktrees, union --exclude with config, keep .codemap/ out of git status Nested git repositories (linked worktrees whose .git is a gitdir file, plain clones, submodules) are repository boundaries at any depth, under dotted directories too; the file walker, the ast-grep scan, the Go fallback and the manifest reader share one detector (issue #131). --exclude now adds to the project config's exclude list instead of replacing it; --only and --depth keep overriding (issue #150). Every codemap write that creates .codemap/ in a git checkout appends .codemap/ to /info/exclude when no rule ignores it yet, never touching a tracked .gitignore; doctor reports tracked .codemap/ files (exit test of issue #184). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0159tcanCgA3jxi2E7DdRVWJ --- README.md | 2 +- cmd/agent_edits.go | 4 +- cmd/config.go | 12 +- cmd/config_more_test.go | 37 ++++ cmd/doctor.go | 20 +++ cmd/gitignore.go | 108 ++--------- cmd/hooks.go | 5 +- cmd/setup_review_test.go | 37 ++++ cmd/skill.go | 3 +- config/config.go | 25 +++ config/config_test.go | 24 +++ handoff/storage.go | 3 +- internal/gitexclude/gitexclude.go | 239 +++++++++++++++++++++++++ internal/gitexclude/gitexclude_test.go | 170 ++++++++++++++++++ main.go | 9 +- main_helpers_test.go | 84 +++++++++ main_more_test.go | 13 +- scanner/astgrep.go | 29 +-- scanner/astgrep_test.go | 72 ++++++++ scanner/deps.go | 2 +- scanner/walker.go | 58 ++++++ scanner/walker_test.go | 117 ++++++++++++ topology/cache.go | 3 +- watch/daemon.go | 7 +- watch/state.go | 3 +- watch/transition.go | 3 +- 26 files changed, 952 insertions(+), 137 deletions(-) create mode 100644 internal/gitexclude/gitexclude.go create mode 100644 internal/gitexclude/gitexclude_test.go diff --git a/README.md b/README.md index f6ebab4..98d999e 100644 --- a/README.md +++ b/README.md @@ -431,7 +431,7 @@ codemap config show # display current config } ``` -All fields are optional; CLI flags always override config. When an MCP file search finds real matches hidden by `only`, codemap reports the paths and suggests which extensions to add — set `guidance.missing_extension_hints: false` to disable. +All fields are optional. `--only` and `--depth` override the config's values for one run; `--exclude` adds to the config's `exclude` list, so a config exclude keeps applying when you pass more patterns on the command line. When an MCP file search finds real matches hidden by `only`, codemap reports the paths and suggests which extensions to add — set `guidance.missing_extension_hints: false` to disable. ## Skills diff --git a/cmd/agent_edits.go b/cmd/agent_edits.go index ee313b9..962e4b3 100644 --- a/cmd/agent_edits.go +++ b/cmd/agent_edits.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "time" + + "codemap/internal/gitexclude" ) // The watch daemon sees every write on disk — including git merges, checkouts, @@ -54,7 +56,7 @@ func recordAgentEdit(root, sessionID, path string, now time.Time) error { if strings.TrimSpace(path) == "" { return nil } - if err := os.MkdirAll(filepath.Join(root, ".codemap"), 0o755); err != nil { + if err := gitexclude.MkdirAll(filepath.Join(root, ".codemap"), 0o755); err != nil { return err } record := agentEditRecord{ diff --git a/cmd/config.go b/cmd/config.go index 541d4a4..d6c9323 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -11,6 +11,7 @@ import ( "strings" "codemap/config" + "codemap/internal/gitexclude" "codemap/scanner" ) @@ -76,13 +77,22 @@ func EnsureProjectConfig(root string) bool { if _, err := os.Stat(filepath.Join(root, ".git")); err != nil { return false } - if _, err := initProjectConfig(root); err != nil { + result, err := initProjectConfig(root) + if err != nil { return false } + // The auto-write just created .codemap/ in a checkout that never asked + // for it, so keep it out of `git status` (issue #184): the rule goes to + // the clone-local info/exclude, never a tracked .gitignore, and only + // when no existing rule already ignores the directory. + excludeFile, _ := gitexclude.EnsureIgnoredIfNeeded(filepath.Dir(filepath.Dir(result.Path))) // Only a person at a terminal needs the notice; piped consumers that // merge stderr into a JSON stream must not see it. if info, err := os.Stderr.Stat(); err == nil && info.Mode()&os.ModeCharDevice != 0 { fmt.Fprintln(os.Stderr, "codemap: wrote .codemap/config.json (auto-detected); tune with: codemap skill show config-setup") + if excludeFile != "" { + fmt.Fprintf(os.Stderr, "codemap: added %s to %s so git status stays clean; verify with: git check-ignore -v .codemap\n", gitexclude.Entry, excludeFile) + } } return true } diff --git a/cmd/config_more_test.go b/cmd/config_more_test.go index 0872c1f..3b96b97 100644 --- a/cmd/config_more_test.go +++ b/cmd/config_more_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -224,3 +225,39 @@ func TestEnsureProjectConfigWritesOnlyInsideRepos(t *testing.T) { t.Fatalf("config unexpectedly present in non-repo: %v", err) } } + +// TestEnsureProjectConfigKeepsFreshCloneClean is the exit test from issue +// #184: a fresh clone of a repo with no codemap config, one codemap command, +// and `git status --porcelain` is empty. The auto-written config lands in +// .codemap/, which the auto-init must ignore via the clone-local +// info/exclude, never by creating or editing a tracked .gitignore. +func TestEnsureProjectConfigKeepsFreshCloneClean(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + src := makeRepoOnBranch(t, "main") + clone := filepath.Join(t.TempDir(), "clone") + runGitTestCmd(t, src, "clone", "-q", src, clone) + + if !EnsureProjectConfig(clone) { + t.Fatal("expected config to be written in a fresh clone") + } + if _, err := os.Stat(config.ConfigPath(clone)); err != nil { + t.Fatalf("config not written: %v", err) + } + status := exec.Command("git", "status", "--porcelain") + status.Dir = clone + out, err := status.Output() + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(out)) != "" { + t.Fatalf("git status --porcelain not empty after auto-init:\n%s", out) + } + if _, err := os.Stat(filepath.Join(clone, ".gitignore")); !os.IsNotExist(err) { + t.Fatal("auto-init must not create a tracked .gitignore") + } + if !gitIgnoresCodemap(t, clone) { + t.Fatal("git does not ignore .codemap after auto-init") + } +} diff --git a/cmd/doctor.go b/cmd/doctor.go index 16eccb4..de3af09 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -16,6 +16,7 @@ import ( "codemap/config" "codemap/internal/buildinfo" + "codemap/internal/gitexclude" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/pelletier/go-toml/v2" @@ -114,6 +115,7 @@ func RunDoctor(args []string, defaultRoot string) int { checkFile("project config", config.ConfigPath(root), validateJSONFile) if isGitWorkTree(root) { doctorCheckLocalCodemapIgnore(root, &failures) + doctorCheckTrackedCodemapState(root, &failures) } claudeSettings, claudeSettingsErr := claudeSettingsPath(root, *global) claudeMCP, claudeMCPErr := claudeMCPPath(root, *global) @@ -202,6 +204,24 @@ func doctorCheckLocalCodemapIgnore(root string, failures *int) { *failures++ } +// doctorCheckTrackedCodemapState reports .codemap/ files git already tracks +// (`git ls-files .codemap`): an ignore rule cannot hide those, so they show +// up in every diff until they are removed from the index. +func doctorCheckTrackedCodemapState(root string, failures *int) { + tracked, err := gitexclude.TrackedStateFiles(root) + if err != nil { + fmt.Printf("MISS tracked codemap state: %v\n", err) + *failures++ + return + } + if len(tracked) > 0 { + fmt.Printf("MISS tracked codemap state: %d file(s) under .codemap/ are committed; run: git rm -r --cached .codemap\n", len(tracked)) + *failures++ + return + } + fmt.Println("OK tracked codemap state: 0 files under .codemap/ are committed (git ls-files .codemap)") +} + func reportCodexRuntimeVersions(cliPath string) bool { var desktopPaths []string switch doctorRuntimeGOOS { diff --git a/cmd/gitignore.go b/cmd/gitignore.go index 6c9b95c..0859ebf 100644 --- a/cmd/gitignore.go +++ b/cmd/gitignore.go @@ -1,18 +1,17 @@ package cmd import ( - "bytes" "fmt" "io" - "os" - "os/exec" "path/filepath" "strings" + + "codemap/internal/gitexclude" ) // ignoreEntry keeps the runtime-state directory out of git. The trailing // slash scopes the rule to the directory. -const ignoreEntry = ".codemap/" +const ignoreEntry = gitexclude.Entry // reportEnsureIgnored ensures .codemap/ is ignored for root and reports the // outcome to w (notice, warning, or nothing). @@ -36,119 +35,38 @@ func reportEnsureIgnored(w io.Writer, root string) { // ignoreEntry to the clone-local info/exclude. It returns the file written, // or "" when the entry already exists or root is not a git work tree. func ensureCodemapIgnored(root string) (string, error) { - if !isGitWorkTree(root) { - return "", nil - } - target, present, err := localCodemapIgnore(root) - if err != nil { - return "", err - } - if present { - return "", nil - } - if err := appendIgnoreEntry(target, ignoreEntry); err != nil { - return "", err - } - return target, nil + return gitexclude.EnsureIgnored(root) } // localCodemapIgnore reports whether root's info/exclude already lists // ignoreEntry. func localCodemapIgnore(root string) (string, bool, error) { - target, err := infoExcludePath(root) - if err != nil { - return "", false, err - } - data, err := os.ReadFile(target) - if os.IsNotExist(err) { - return target, false, nil - } - if err != nil { - return "", false, err - } - return target, hasIgnoreLine(data, ignoreEntry), nil + return gitexclude.LocalEntry(root) } // isGitWorkTree reports whether root sits inside a git working tree. func isGitWorkTree(root string) bool { - cmd := exec.Command("git", "rev-parse", "--is-inside-work-tree") - cmd.Dir = root - out, err := cmd.Output() - return err == nil && strings.TrimSpace(string(out)) == "true" + return gitexclude.IsWorkTree(root) } // gitCheckIgnoreVerbose reports whether git would ignore path at root and, if -// so, the :: rule that matched, across every ignore -// source (tracked .gitignore, local exclude, global excludes, parent repos). +// so, the :: rule that matched. func gitCheckIgnoreVerbose(root, path string) (bool, string, error) { - cmd := exec.Command("git", "check-ignore", "-v", path) - cmd.Dir = root - out, err := cmd.Output() - if err == nil { - line := strings.TrimSpace(string(out)) - if i := strings.IndexByte(line, '\t'); i >= 0 { - line = line[:i] - } - return true, line, nil - } - // Exit code 1 means "not ignored"; anything else is a real error. - if exit, ok := err.(*exec.ExitError); ok && exit.ExitCode() == 1 { - return false, "", nil - } - return false, "", fmt.Errorf("git check-ignore: %w", err) + return gitexclude.CheckIgnored(root, path) } -// infoExcludePath resolves /info/exclude for root, so every -// worktree of a repo shares one local exclude file. +// infoExcludePath resolves /info/exclude for root. func infoExcludePath(root string) (string, error) { - cmd := exec.Command("git", "rev-parse", "--git-common-dir") - cmd.Dir = root - out, err := cmd.Output() - if err != nil { - return "", fmt.Errorf("git rev-parse --git-common-dir: %w", err) - } - commonDir := strings.TrimSpace(string(out)) - if !filepath.IsAbs(commonDir) { - commonDir = filepath.Join(root, commonDir) - } - return filepath.Join(commonDir, "info", "exclude"), nil + return gitexclude.InfoExcludePath(root) } -// appendIgnoreEntry adds entry on its own line to the ignore file at path, -// creating the file if needed. No-op when entry is already listed; never -// glues onto a partial last line. Atomic write, so a symlinked exclude is -// replaced rather than written through. +// appendIgnoreEntry adds entry on its own line to the ignore file at path. func appendIgnoreEntry(path, entry string) error { - existing, err := os.ReadFile(path) - if err != nil && !os.IsNotExist(err) { - return err - } - if hasIgnoreLine(existing, entry) { - return nil - } - - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - return err - } - - var buf bytes.Buffer - buf.Write(existing) - if len(existing) > 0 && !bytes.HasSuffix(existing, []byte("\n")) { - buf.WriteByte('\n') - } - buf.WriteString(entry) - buf.WriteByte('\n') - - return writeFileAtomic(path, buf.Bytes(), 0644) + return gitexclude.AppendEntry(path, entry) } // hasIgnoreLine reports whether data lists entry as a standalone, uncommented // line. func hasIgnoreLine(data []byte, entry string) bool { - for _, line := range bytes.Split(data, []byte("\n")) { - if strings.TrimSpace(string(line)) == entry { - return true - } - } - return false + return gitexclude.HasLine(data, entry) } diff --git a/cmd/hooks.go b/cmd/hooks.go index 13d8c86..fdc2b47 100644 --- a/cmd/hooks.go +++ b/cmd/hooks.go @@ -19,6 +19,7 @@ import ( "codemap/config" "codemap/handoff" + "codemap/internal/gitexclude" "codemap/internal/projectpath" "codemap/limits" "codemap/scanner" @@ -1475,7 +1476,7 @@ func hookPreCompact(root string) error { if err != nil { return err } - if err := os.MkdirAll(codemapDir, 0755); err != nil { + if err := gitexclude.MkdirAll(codemapDir, 0755); err != nil { return err } @@ -1869,7 +1870,7 @@ func updateSessionLeaseContext(ctx context.Context, root, sessionID string, acti if err != nil { return err } - if err := os.MkdirAll(codemapDir, 0o755); err != nil { + if err := gitexclude.MkdirAll(codemapDir, 0o755); err != nil { return err } lockPath := filepath.Join(codemapDir, "sessions.lock") diff --git a/cmd/setup_review_test.go b/cmd/setup_review_test.go index d00c49e..f64501f 100644 --- a/cmd/setup_review_test.go +++ b/cmd/setup_review_test.go @@ -597,3 +597,40 @@ func TestDoctorCheckLocalCodemapIgnoreFailsWhenGitUnavailable(t *testing.T) { t.Fatalf("expected MISS local-ignore diagnosis, got:\n%s", out) } } + +// TestRunDoctorReportsTrackedCodemapState locks in the doctor line for +// .codemap/ files that are already committed: an ignore rule cannot hide +// them, so doctor names the count and the git command that removes them +// from the index (issue #184). +func TestRunDoctorReportsTrackedCodemapState(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + root := makeRepoOnBranch(t, "main") + if _, err := ensureCodemapIgnored(root); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, ".codemap"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, ".codemap", "config.json"), []byte("{}\n"), 0o644); err != nil { + t.Fatal(err) + } + + out := captureOutput(func() { RunDoctor([]string{root}, ".") }) + if !strings.Contains(out, "OK tracked codemap state: 0 files under .codemap/ are committed") { + t.Fatalf("expected clean tracked-state line, got:\n%s", out) + } + + runGitTestCmd(t, root, "add", "-f", ".codemap") + runGitTestCmd(t, root, "-c", "user.name=Test", "-c", "user.email=test@example.com", "commit", "-q", "-m", "track codemap state") + + var code int + out = captureOutput(func() { code = RunDoctor([]string{root}, ".") }) + if code == 0 { + t.Fatalf("RunDoctor exit code = 0, want tracked state to fail\n%s", out) + } + if !strings.Contains(out, "MISS tracked codemap state: 1 file(s) under .codemap/ are committed; run: git rm -r --cached .codemap") { + t.Fatalf("expected tracked-state diagnosis, got:\n%s", out) + } +} diff --git a/cmd/skill.go b/cmd/skill.go index 097e48d..18fc11c 100644 --- a/cmd/skill.go +++ b/cmd/skill.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" + "codemap/internal/gitexclude" "codemap/internal/projectpath" "codemap/skills" ) @@ -99,7 +100,7 @@ func runSkillShow(root, name string) { func runSkillInit(root string) { skillsDir := filepath.Join(projectpath.CodemapDir(root), "skills") - if err := os.MkdirAll(skillsDir, 0755); err != nil { + if err := gitexclude.MkdirAll(skillsDir, 0755); err != nil { fmt.Fprintf(os.Stderr, "Error creating skills directory: %v\n", err) os.Exit(1) } diff --git a/config/config.go b/config/config.go index 656c16b..655fefb 100644 --- a/config/config.go +++ b/config/config.go @@ -225,6 +225,31 @@ func (c ProjectConfig) RoutingTopKOrDefault() int { return clampRange(c.Routing.Retrieval.TopK, defaultRoutingTopK, 1, 20) } +// MergeExcludes unions the project config's exclude patterns with the +// patterns passed on the command line: `--exclude` adds to the config's list +// rather than replacing it (issue #150), so a config exclude keeps applying +// when a caller narrows a single run further. Config patterns come first, +// then CLI patterns; blank entries and duplicates are dropped and nil is +// returned when nothing remains. +func MergeExcludes(configured, cli []string) []string { + seen := make(map[string]struct{}, len(configured)+len(cli)) + var merged []string + for _, list := range [][]string{configured, cli} { + for _, pattern := range list { + pattern = strings.TrimSpace(pattern) + if pattern == "" { + continue + } + if _, dup := seen[pattern]; dup { + continue + } + seen[pattern] = struct{}{} + merged = append(merged, pattern) + } + } + return merged +} + // ConfigPath returns the path to .codemap/config.json for the given root. func ConfigPath(root string) string { return filepath.Join(projectpath.CodemapDir(root), "config.json") diff --git a/config/config_test.go b/config/config_test.go index 7721c17..b1af1aa 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "reflect" "strings" "testing" @@ -466,3 +467,26 @@ func TestAssessSetup(t *testing.T) { } }) } + +func TestMergeExcludesUnionsConfigAndCLI(t *testing.T) { + tests := []struct { + name string + configured []string + cli []string + want []string + }{ + {name: "both empty", want: nil}, + {name: "config only", configured: []string{"vendorx"}, want: []string{"vendorx"}}, + {name: "cli only", cli: []string{"docs"}, want: []string{"docs"}}, + {name: "cli adds to config", configured: []string{"vendorx"}, cli: []string{"ZZZNOMATCH"}, want: []string{"vendorx", "ZZZNOMATCH"}}, + {name: "duplicates and blanks dropped", configured: []string{"vendorx", " "}, cli: []string{"vendorx", "", "docs"}, want: []string{"vendorx", "docs"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MergeExcludes(tt.configured, tt.cli) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("MergeExcludes(%v, %v) = %v, want %v", tt.configured, tt.cli, got, tt.want) + } + }) + } +} diff --git a/handoff/storage.go b/handoff/storage.go index 55e03c5..50184b4 100644 --- a/handoff/storage.go +++ b/handoff/storage.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + "codemap/internal/gitexclude" "codemap/internal/projectpath" "codemap/internal/runtimefile" ) @@ -91,7 +92,7 @@ func WriteLatest(root string, artifact *Artifact) error { return err } path := filepath.Join(runtimeDir, latestFilename) - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + if err := gitexclude.MkdirAll(filepath.Dir(path), 0755); err != nil { return err } diff --git a/internal/gitexclude/gitexclude.go b/internal/gitexclude/gitexclude.go new file mode 100644 index 0000000..ac88c3c --- /dev/null +++ b/internal/gitexclude/gitexclude.go @@ -0,0 +1,239 @@ +// Package gitexclude keeps codemap's own state directory out of `git status` +// without touching a tracked .gitignore: the rule goes to the clone-local +// /info/exclude, which linked worktrees share. +package gitexclude + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// Entry is the ignore rule written for codemap's state directory. The +// trailing slash scopes the rule to the directory. +const Entry = ".codemap/" + +// stateDirName is the directory Entry refers to. +const stateDirName = ".codemap" + +// IsWorkTree reports whether root sits inside a git working tree. +func IsWorkTree(root string) bool { + cmd := exec.Command("git", "rev-parse", "--is-inside-work-tree") + cmd.Dir = root + out, err := cmd.Output() + return err == nil && strings.TrimSpace(string(out)) == "true" +} + +// InfoExcludePath resolves /info/exclude for root, so every +// worktree of a repo shares one local exclude file. +func InfoExcludePath(root string) (string, error) { + cmd := exec.Command("git", "rev-parse", "--git-common-dir") + cmd.Dir = root + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("git rev-parse --git-common-dir: %w", err) + } + commonDir := strings.TrimSpace(string(out)) + if !filepath.IsAbs(commonDir) { + commonDir = filepath.Join(root, commonDir) + } + return filepath.Join(commonDir, "info", "exclude"), nil +} + +// CheckIgnored reports whether git would ignore path at root and, if so, the +// :: rule that matched, across every ignore source +// (tracked .gitignore, local exclude, global excludes, parent repos). +func CheckIgnored(root, path string) (bool, string, error) { + cmd := exec.Command("git", "check-ignore", "-v", path) + cmd.Dir = root + out, err := cmd.Output() + if err == nil { + line := strings.TrimSpace(string(out)) + if i := strings.IndexByte(line, '\t'); i >= 0 { + line = line[:i] + } + return true, line, nil + } + // Exit code 1 means "not ignored"; anything else is a real error. + if exit, ok := err.(*exec.ExitError); ok && exit.ExitCode() == 1 { + return false, "", nil + } + return false, "", fmt.Errorf("git check-ignore: %w", err) +} + +// LocalEntry reports whether root's info/exclude already lists Entry, and +// the path of that file. +func LocalEntry(root string) (string, bool, error) { + target, err := InfoExcludePath(root) + if err != nil { + return "", false, err + } + data, err := os.ReadFile(target) + if os.IsNotExist(err) { + return target, false, nil + } + if err != nil { + return "", false, err + } + return target, HasLine(data, Entry), nil +} + +// EnsureIgnored ensures Entry is listed in root's clone-local info/exclude. +// It returns the file written, or "" when the entry already exists there or +// root is not a git work tree. It records the local rule even when a tracked +// .gitignore already covers it, so `codemap setup` leaves a rule it owns. +func EnsureIgnored(root string) (string, error) { + if !IsWorkTree(root) { + return "", nil + } + target, present, err := LocalEntry(root) + if err != nil { + return "", err + } + if present { + return "", nil + } + if err := AppendEntry(target, Entry); err != nil { + return "", err + } + return target, nil +} + +// EnsureIgnoredIfNeeded is the quiet variant used when codemap creates the +// state directory on its own (issue #184): it writes Entry to info/exclude +// only when git does not already ignore .codemap/ by any rule, so a repo +// whose tracked .gitignore covers it gains no redundant local rule. It +// returns the file written, or "" when nothing was needed or root is not a +// git work tree. +func EnsureIgnoredIfNeeded(root string) (string, error) { + if !IsWorkTree(root) { + return "", nil + } + ignored, _, err := CheckIgnored(root, Entry) + if err != nil { + return "", err + } + if ignored { + return "", nil + } + return EnsureIgnored(root) +} + +// MkdirAll creates dir like os.MkdirAll and, when doing so creates a +// `.codemap` directory (dir itself or an ancestor of it) that did not exist +// before, ensures git ignores it via EnsureIgnoredIfNeeded on the directory +// that contains it. Every codemap state write that may create `.codemap/` +// goes through here so a fresh checkout's `git status` stays clean after +// any command. The ignore write is best-effort: a failure there never fails +// the directory creation, and nothing runs when `.codemap` already exists. +func MkdirAll(dir string, perm os.FileMode) error { + stateDir, hasStateDir := stateDirOf(dir) + existed := true + if hasStateDir { + _, err := os.Lstat(stateDir) + existed = err == nil + } + if err := os.MkdirAll(dir, perm); err != nil { + return err + } + if hasStateDir && !existed { + _, _ = EnsureIgnoredIfNeeded(filepath.Dir(stateDir)) + } + return nil +} + +// stateDirOf returns the nearest path on or above dir whose base is +// `.codemap`. +func stateDirOf(dir string) (string, bool) { + dir = filepath.Clean(dir) + for current := dir; ; current = filepath.Dir(current) { + if filepath.Base(current) == stateDirName { + return current, true + } + if parent := filepath.Dir(current); parent == current { + return "", false + } + } +} + +// TrackedStateFiles returns the paths under .codemap/ that git tracks at +// root (`git ls-files -- .codemap`), which is the state the ignore rule can +// no longer hide. +func TrackedStateFiles(root string) ([]string, error) { + cmd := exec.Command("git", "ls-files", "-z", "--", stateDirName) + cmd.Dir = root + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("git ls-files: %w", err) + } + var files []string + for _, entry := range bytes.Split(out, []byte{0}) { + if len(entry) > 0 { + files = append(files, string(entry)) + } + } + return files, nil +} + +// AppendEntry adds entry on its own line to the ignore file at path, +// creating the file if needed. No-op when entry is already listed; never +// glues onto a partial last line. Atomic write, so a symlinked exclude is +// replaced rather than written through. +func AppendEntry(path, entry string) error { + existing, err := os.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + return err + } + if HasLine(existing, entry) { + return nil + } + + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + + var buf bytes.Buffer + buf.Write(existing) + if len(existing) > 0 && !bytes.HasSuffix(existing, []byte("\n")) { + buf.WriteByte('\n') + } + buf.WriteString(entry) + buf.WriteByte('\n') + + return writeFileAtomic(path, buf.Bytes(), 0644) +} + +// HasLine reports whether data lists entry as a standalone, uncommented +// line. +func HasLine(data []byte, entry string) bool { + for _, line := range bytes.Split(data, []byte("\n")) { + if strings.TrimSpace(string(line)) == entry { + return true + } + } + return false +} + +func writeFileAtomic(path string, data []byte, perm os.FileMode) error { + tmp, err := os.CreateTemp(filepath.Dir(path), "."+filepath.Base(path)+".tmp-") + if err != nil { + return err + } + tmpPath := tmp.Name() + defer os.Remove(tmpPath) + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return err + } + if err := tmp.Chmod(perm); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return os.Rename(tmpPath, path) +} diff --git a/internal/gitexclude/gitexclude_test.go b/internal/gitexclude/gitexclude_test.go new file mode 100644 index 0000000..4152340 --- /dev/null +++ b/internal/gitexclude/gitexclude_test.go @@ -0,0 +1,170 @@ +package gitexclude + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func gitRun(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.com", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.com") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return string(out) +} + +func freshClone(t *testing.T) string { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + src := t.TempDir() + gitRun(t, src, "init", "-q", "-b", "main") + if err := os.WriteFile(filepath.Join(src, "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + gitRun(t, src, "add", ".") + gitRun(t, src, "commit", "-q", "-m", "init") + clone := filepath.Join(t.TempDir(), "clone") + gitRun(t, src, "clone", "-q", src, clone) + return clone +} + +func TestMkdirAllIgnoresNewStateDirInFreshClone(t *testing.T) { + root := freshClone(t) + stateDir := filepath.Join(root, ".codemap", "projects", "abc") + + if err := MkdirAll(stateDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(stateDir, "state.json"), []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + if status := gitRun(t, root, "status", "--porcelain"); strings.TrimSpace(status) != "" { + t.Fatalf("git status not clean after state write:\n%s", status) + } + if _, err := os.Stat(filepath.Join(root, ".gitignore")); !os.IsNotExist(err) { + t.Fatal("must never create a tracked .gitignore") + } + exclude, err := InfoExcludePath(root) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(exclude) + if err != nil { + t.Fatal(err) + } + if !HasLine(data, Entry) { + t.Fatalf("info/exclude missing %q:\n%s", Entry, data) + } +} + +func TestMkdirAllAddsNoLocalRuleWhenTrackedIgnoreCovers(t *testing.T) { + root := freshClone(t) + if err := os.WriteFile(filepath.Join(root, ".gitignore"), []byte(".codemap/\n"), 0o644); err != nil { + t.Fatal(err) + } + gitRun(t, root, "add", ".gitignore") + gitRun(t, root, "commit", "-q", "-m", "ignore") + + if err := MkdirAll(filepath.Join(root, ".codemap"), 0o755); err != nil { + t.Fatal(err) + } + exclude, err := InfoExcludePath(root) + if err != nil { + t.Fatal(err) + } + if data, err := os.ReadFile(exclude); err == nil && HasLine(data, Entry) { + t.Fatalf("local rule written although .gitignore already ignores .codemap/:\n%s", data) + } + if status := gitRun(t, root, "status", "--porcelain"); strings.TrimSpace(status) != "" { + t.Fatalf("git status not clean:\n%s", status) + } +} + +func TestMkdirAllOutsideGitAndWithoutStateDir(t *testing.T) { + plain := filepath.Join(t.TempDir(), "a", ".codemap", "b") + if err := MkdirAll(plain, 0o755); err != nil { + t.Fatal(err) + } + if info, err := os.Stat(plain); err != nil || !info.IsDir() { + t.Fatalf("dir not created: %v", err) + } + other := filepath.Join(t.TempDir(), "cache", "x") + if err := MkdirAll(other, 0o755); err != nil { + t.Fatal(err) + } + if _, ok := stateDirOf(other); ok { + t.Fatalf("stateDirOf(%q) found a .codemap ancestor", other) + } + if got, ok := stateDirOf(plain); !ok || filepath.Base(got) != ".codemap" { + t.Fatalf("stateDirOf(%q) = %q, %v", plain, got, ok) + } +} + +func TestEnsureIgnoredIfNeededUsesSharedExcludeInLinkedWorktree(t *testing.T) { + root := freshClone(t) + wt := filepath.Join(t.TempDir(), "wt") + gitRun(t, root, "worktree", "add", "-q", wt, "HEAD") + + wrote, err := EnsureIgnoredIfNeeded(wt) + if err != nil { + t.Fatal(err) + } + want := filepath.Join(root, ".git", "info", "exclude") + if wrote != want { + t.Fatalf("wrote %q, want the primary's shared exclude %q", wrote, want) + } + if err := os.MkdirAll(filepath.Join(wt, ".codemap"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(wt, ".codemap", "config.json"), []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + if status := gitRun(t, wt, "status", "--porcelain"); strings.TrimSpace(status) != "" { + t.Fatalf("worktree git status not clean:\n%s", status) + } + again, err := EnsureIgnoredIfNeeded(wt) + if err != nil { + t.Fatal(err) + } + if again != "" { + t.Fatalf("second call wrote %q, want nothing", again) + } +} + +func TestTrackedStateFiles(t *testing.T) { + root := freshClone(t) + files, err := TrackedStateFiles(root) + if err != nil { + t.Fatal(err) + } + if len(files) != 0 { + t.Fatalf("tracked = %v, want none", files) + } + if err := os.MkdirAll(filepath.Join(root, ".codemap", "skills"), 0o755); err != nil { + t.Fatal(err) + } + for _, name := range []string{"config.json", "skills/x.md"} { + if err := os.WriteFile(filepath.Join(root, ".codemap", filepath.FromSlash(name)), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } + gitRun(t, root, "add", "-f", ".codemap") + files, err = TrackedStateFiles(root) + if err != nil { + t.Fatal(err) + } + if len(files) != 2 { + t.Fatalf("tracked = %v, want 2 entries", files) + } +} diff --git a/main.go b/main.go index cdacf02..8bf8478 100644 --- a/main.go +++ b/main.go @@ -404,7 +404,10 @@ func main() { } } - // Load project config (CLI flags take precedence) + // Load project config. --only and --depth override the config; --exclude + // adds to the config's exclude list (issue #150), since a config exclude + // is a standing decision about the repo and one run narrowing further + // must not silently re-admit it. if tempDir == "" { cmd.EnsureProjectConfig(absRoot) } @@ -412,9 +415,7 @@ func main() { if len(only) == 0 && len(projCfg.Only) > 0 { only = projCfg.Only } - if len(exclude) == 0 && len(projCfg.Exclude) > 0 { - exclude = projCfg.Exclude - } + exclude = config.MergeExcludes(projCfg.Exclude, exclude) if *depthLimit == 0 && projCfg.Depth > 0 { *depthLimit = projCfg.Depth } diff --git a/main_helpers_test.go b/main_helpers_test.go index 1d55194..d2446c8 100644 --- a/main_helpers_test.go +++ b/main_helpers_test.go @@ -5,6 +5,7 @@ import ( "flag" "io" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -233,3 +234,86 @@ func runMainWithArgs(t *testing.T, args []string) string { return captureMainOutput(func() { main() }) } + +// TestMainExcludeFlagAddsToConfigExcludes is the regression for issue #150: +// --exclude used to replace the config's exclude list, so a non-matching CLI +// pattern silently re-admitted everything the config excluded. +func TestMainExcludeFlagAddsToConfigExcludes(t *testing.T) { + root := t.TempDir() + files := map[string]string{ + ".codemap/config.json": "{\"exclude\": [\"vendorx\"]}\n", + "main.go": "package main\n", + "src/a.go": "package src\n", + "src/b.go": "package src\n", + "vendorx/x.go": "package vendorx\n", + "vendorx/y.go": "package vendorx\n", + } + for path, content := range files { + full := filepath.Join(root, filepath.FromSlash(path)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + count := func(args ...string) int { + t.Helper() + out := runMainWithArgs(t, append([]string{"codemap", "--json"}, append(args, root)...)) + var project scanner.Project + if err := json.Unmarshal([]byte(out), &project); err != nil { + t.Fatalf("expected JSON output, got error %v with body:\n%s", err, out) + } + n := 0 + for _, file := range project.Files { + // The tree inventory lists codemap's own config; only project + // files matter here. + if !strings.HasPrefix(filepath.ToSlash(file.Path), ".codemap/") { + n++ + } + } + return n + } + + if got := count(); got != 3 { + t.Fatalf("config exclude alone: %d files, want 3 (main.go, src/a.go, src/b.go)", got) + } + if got := count("--exclude", "ZZZNOMATCH"); got != 3 { + t.Fatalf("config exclude + non-matching --exclude: %d files, want 3 (config exclude must keep applying)", got) + } + if got := count("--exclude", "src"); got != 1 { + t.Fatalf("config exclude + matching --exclude: %d files, want 1 (both sets removed)", got) + } +} + +// TestMainFreshCloneLeavesGitStatusClean is the exit test from issue #184: +// a fresh clone of a repo with no codemap config, `codemap .`, and +// `git status --porcelain` is empty. +func TestMainFreshCloneLeavesGitStatusClean(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + src := makeMainGitRepo(t, "main") + clone := filepath.Join(t.TempDir(), "clone") + runGitMainTestCmd(t, src, "clone", "-q", src, clone) + + out := runMainWithArgs(t, []string{"codemap", clone}) + if !strings.Contains(out, "Files:") { + t.Fatalf("expected a structure summary, got:\n%s", out) + } + if _, err := os.Stat(filepath.Join(clone, ".codemap", "config.json")); err != nil { + t.Fatalf("auto-init config not written: %v", err) + } + status := exec.Command("git", "status", "--porcelain") + status.Dir = clone + statusOut, err := status.Output() + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(string(statusOut)) != "" { + t.Fatalf("git status --porcelain not empty after `codemap .`:\n%s", statusOut) + } + if _, err := os.Stat(filepath.Join(clone, ".gitignore")); !os.IsNotExist(err) { + t.Fatal("codemap must not create a tracked .gitignore") + } +} diff --git a/main_more_test.go b/main_more_test.go index d63ac35..ccd49cf 100644 --- a/main_more_test.go +++ b/main_more_test.go @@ -853,7 +853,12 @@ func TestRunDepsModeJSONAndMainDispatchesDepsAndImporters(t *testing.T) { } } -func TestBinaryDependencyModesHonorCLIFiltersOverConfig(t *testing.T) { +// TestBinaryDependencyModesMergeCLIFiltersWithConfig locks in the two +// precedence rules: --only replaces the config's only list for one run, while +// --exclude adds to the config's exclude list (issue #150). The fixture's +// config is only=[ts] exclude=[a]; the CLI passes --only go --exclude c, so +// Go files are scanned, and both a/ (config) and c/ (CLI) stay excluded. +func TestBinaryDependencyModesMergeCLIFiltersWithConfig(t *testing.T) { if !scanner.NewAstGrepAnalyzer().Available() { t.Skip("ast-grep not available") } @@ -877,8 +882,8 @@ func TestBinaryDependencyModesHonorCLIFiltersOverConfig(t *testing.T) { t.Fatalf("decode deps JSON: %v\n%s", err, output) } requirePaths(t, analysisPaths(project.Files), - []string{"a/a.go", "pkg/shared/shared.go"}, - []string{"c/c.go", "ts/ignored.ts"}) + []string{"pkg/shared/shared.go"}, + []string{"a/a.go", "c/c.go", "ts/ignored.ts"}) }, }, { @@ -890,7 +895,7 @@ func TestBinaryDependencyModesHonorCLIFiltersOverConfig(t *testing.T) { if err := json.Unmarshal([]byte(output), &report); err != nil { t.Fatalf("decode importers JSON: %v\n%s", err, output) } - requirePaths(t, report.Importers, []string{"a/a.go"}, []string{"c/c.go", "ts/ignored.ts"}) + requirePaths(t, report.Importers, nil, []string{"a/a.go", "c/c.go", "ts/ignored.ts"}) }, }, { diff --git a/scanner/astgrep.go b/scanner/astgrep.go index 75ba69e..ab17ec4 100644 --- a/scanner/astgrep.go +++ b/scanner/astgrep.go @@ -290,26 +290,12 @@ func hiddenScanExcludes() []string { return names } -// findNestedGitRepos returns subdirectory names that contain their own .git -// These are separate repositories (not submodules) that should be excluded -// from scanning to avoid hanging on large nested repos. +// findNestedGitRepos returns the root-relative paths of every repository +// nested under root (linked worktrees, plain clones, submodules), at any +// depth, so the ast-grep scan and the Go fallback exclude exactly what the +// file walker skips. See NestedGitRepos. func findNestedGitRepos(root string) []string { - entries, err := os.ReadDir(root) - if err != nil { - return nil - } - - var repos []string - for _, e := range entries { - if !e.IsDir() || strings.HasPrefix(e.Name(), ".") { - continue - } - gitPath := filepath.Join(root, e.Name(), ".git") - if info, err := os.Stat(gitPath); err == nil && info.IsDir() { - repos = append(repos, e.Name()) - } - } - return repos + return NestedGitRepos(root) } // ScanDirectory analyzes all files in a directory while honoring caller @@ -401,8 +387,11 @@ func (s *AstGrepScanner) scanDirectory(parent context.Context, root string) ([]F for _, name := range hiddenScanExcludes() { args = append(args, "--globs", "!**/"+name+"/**") } + // Nested repositories are excluded by their root-relative path (the + // scan runs with cwd=root, so the glob is anchored there and cannot + // match a same-named directory elsewhere in the tree). for _, repo := range findNestedGitRepos(root) { - args = append(args, "--globs", "!"+repo+"/**") + args = append(args, "--globs", "!"+filepath.ToSlash(repo)+"/**") } args = append(args, ".") diff --git a/scanner/astgrep_test.go b/scanner/astgrep_test.go index ed40f3c..0b80df6 100644 --- a/scanner/astgrep_test.go +++ b/scanner/astgrep_test.go @@ -5,7 +5,9 @@ import ( "errors" "os" "path/filepath" + "reflect" "runtime" + "sort" "strings" "testing" "time" @@ -773,3 +775,73 @@ func TestScanDirectoryStillExcludesKnownNeverSourceDirs(t *testing.T) { } } } + +// TestScanDirectoryExcludesNestedWorktreesByPath locks in that the ast-grep +// invocation excludes every nested repository the file walker skips (issue +// #131): a linked worktree under a dotted directory and a plain clone two +// levels down, each by its root-relative path, so --deps agrees with +// `codemap .` on what is project source. +func TestScanDirectoryExcludesNestedWorktreesByPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("requires shell script execution") + } + root := nestedRepoFixture(t) + + tmpDir := t.TempDir() + capturedArgs := filepath.Join(tmpDir, "args.txt") + fakeBinary := filepath.Join(tmpDir, "fake-sg.sh") + script := "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$CAPTURE_ARGS\"\nprintf '[]\\n'\n" + if err := os.WriteFile(fakeBinary, []byte(script), 0755); err != nil { + t.Fatal(err) + } + t.Setenv("CAPTURE_ARGS", capturedArgs) + + scanner := &AstGrepScanner{rulesDir: tmpDir, binary: fakeBinary} + if _, err := scanner.ScanDirectory(context.Background(), root); err != nil { + t.Fatalf("ScanDirectory() error = %v", err) + } + args, err := os.ReadFile(capturedArgs) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"--globs\n!.claude/worktrees/agent-x/**\n", "--globs\n!tools/sub-clone/**\n"} { + if !strings.Contains(string(args), want) { + t.Errorf("expected %q in ast-grep args, got:\n%s", want, args) + } + } + if strings.Contains(string(args), "!lib/**") { + t.Errorf("plain source directory must not be excluded, got:\n%s", args) + } +} + +// TestScanDirectoryRealAstGrepSkipsNestedWorktree runs the real ast-grep on +// the same fixture and checks the nested repositories' files are not indexed. +func TestScanDirectoryRealAstGrepSkipsNestedWorktree(t *testing.T) { + scanner, err := NewAstGrepScanner() + if err != nil { + t.Fatalf("NewAstGrepScanner() error = %v", err) + } + t.Cleanup(scanner.Close) + if !scanner.Available() { + t.Skip("ast-grep not available") + } + root := nestedRepoFixture(t) + for _, name := range []string{"a.js", ".claude/worktrees/agent-x/a.js", "tools/sub-clone/a.js"} { + if err := os.WriteFile(filepath.Join(root, filepath.FromSlash(name)), []byte("const b = require('./b');\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + outcome, err := scanner.ScanDirectory(context.Background(), root) + if err != nil { + t.Fatalf("ScanDirectory() error = %v", err) + } + var paths []string + for _, a := range outcome.Analyses { + paths = append(paths, filepath.ToSlash(a.Path)) + } + sort.Strings(paths) + if !reflect.DeepEqual(paths, []string{"a.js"}) { + t.Fatalf("analyses = %v, want only a.js (nested worktree and clone excluded)", paths) + } +} diff --git a/scanner/deps.go b/scanner/deps.go index a63a6e4..30f07e3 100644 --- a/scanner/deps.go +++ b/scanner/deps.go @@ -38,7 +38,7 @@ func ReadExternalDeps(ctx context.Context, root string, manifestByteBudget int64 if info.IsDir() { // The walk root is what the user asked for, so it is never one of // the hardcoded ignored directories. - if IgnoredDirs[info.Name()] && path != root { + if path != root && (IgnoredDirs[info.Name()] || IsNestedGitRepo(path)) { return filepath.SkipDir } return nil diff --git a/scanner/walker.go b/scanner/walker.go index b10c7c0..05adc7b 100644 --- a/scanner/walker.go +++ b/scanner/walker.go @@ -3,8 +3,10 @@ package scanner import ( "bufio" "context" + "io/fs" "os" "path/filepath" + "sort" "strings" "codemap/config" @@ -147,6 +149,56 @@ var IgnoredDirs = map[string]bool{ "grammars": true, } +// IsNestedGitRepo reports whether dir is the root of a git repository other +// than the one being scanned: it holds a `.git` entry that is either a +// directory (a plain clone or an old-style submodule checkout) or a regular +// file (a linked worktree's or modern submodule's `gitdir:` pointer). Such a +// directory is a repository boundary, not project source, so every walker +// skips it. Callers never apply this to the scan root itself, whose `.git` +// is expected. +func IsNestedGitRepo(dir string) bool { + info, err := os.Lstat(filepath.Join(dir, ".git")) + if err != nil { + return false + } + return info.IsDir() || info.Mode().IsRegular() +} + +// NestedGitRepos returns the root-relative paths (OS separators, sorted) of +// every git repository nested anywhere under root: linked worktrees (for +// example `.claude/worktrees/agent-x`), plain clones, and submodules, at any +// depth and under dotted directories too. IgnoredDirs and gitignored +// directories are not entered, matching ScanFiles, and a nested repository's +// own subtree is not entered either. The file walker, the ast-grep scan and +// the Go fallback all exclude exactly this set, so `codemap .`, `context`, +// `handoff` and `--deps` agree on what is project source. +func NestedGitRepos(root string) []string { + absRoot := projectpath.CanonicalPath(root) + cache := NewGitIgnoreCache(absRoot) + var repos []string + _ = filepath.WalkDir(absRoot, func(path string, d fs.DirEntry, err error) error { + if err != nil || d == nil || !d.IsDir() || path == absRoot { + return nil + } + if IgnoredDirs[d.Name()] { + return filepath.SkipDir + } + if IsNestedGitRepo(path) { + if rel, relErr := filepath.Rel(absRoot, path); relErr == nil { + repos = append(repos, rel) + } + return filepath.SkipDir + } + cache.tryLoadGitignore(path) + if cache.ShouldIgnore(path) { + return filepath.SkipDir + } + return nil + }) + sort.Strings(repos) + return repos +} + // matchesPattern does smart pattern matching: // - ".png" or "png" → extension match (case-insensitive) // - "Fonts" → directory/component match (contains /Fonts/ or ends with /Fonts) @@ -264,6 +316,12 @@ func ScanFiles(ctx context.Context, root string, cache *GitIgnoreCache, only []s // For directories: load any .gitignore, then check if dir itself should be skipped if info.IsDir() { + // A nested repository (linked worktree, plain clone, submodule) + // is a boundary, not project source, whether or not the parent + // repo's .gitignore happens to cover it (issue #131). + if path != absRoot && IsNestedGitRepo(path) { + return filepath.SkipDir + } if cache != nil { cache.tryLoadGitignore(path) if cache.ShouldIgnore(path) { diff --git a/scanner/walker_test.go b/scanner/walker_test.go index b45f025..06726c6 100644 --- a/scanner/walker_test.go +++ b/scanner/walker_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "os" + "os/exec" "path/filepath" "reflect" "sort" @@ -857,3 +858,119 @@ func TestFilterAnalysesContextBranches(t *testing.T) { t.Fatalf("cancelled filter error = %v, want context.Canceled", err) } } + +// nestedRepoFixture builds a git repo at root with three project files, a +// linked worktree (created by `git worktree add`, so its .git is a gitdir +// pointer file) under a dotted directory, and a nested plain clone (whose +// .git is a directory). It returns root; callers must skip when git is +// missing. +func nestedRepoFixture(t *testing.T) string { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + root := t.TempDir() + git := func(dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.com", + "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.com") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + } + for _, name := range []string{"main.go", "lib/lib.go", "lib/util.go"} { + path := filepath.Join(root, name) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("package p\n"), 0o644); err != nil { + t.Fatal(err) + } + } + git(root, "init", "-q", "-b", "main") + git(root, "add", ".") + git(root, "commit", "-q", "-m", "init") + // Linked worktree under a dotted directory, like .claude/worktrees/. + git(root, "worktree", "add", "-q", filepath.Join(".claude", "worktrees", "agent-x"), "HEAD") + // Nested plain clone, two levels down. + git(root, "clone", "-q", root, filepath.Join("tools", "sub-clone")) + return root +} + +// TestScanFilesSkipsNestedWorktreesAndClones is the regression for issue +// #131: a linked worktree's .git is a file, not a directory, and IgnoredDirs +// only matched a directory literally named .git, so `git worktree add` inside +// the repo doubled the file count. +func TestScanFilesSkipsNestedWorktreesAndClones(t *testing.T) { + root := nestedRepoFixture(t) + + files, err := ScanFiles(context.Background(), root, NewGitIgnoreCache(root), nil, nil) + if err != nil { + t.Fatal(err) + } + var paths []string + for _, f := range files { + paths = append(paths, filepath.ToSlash(f.Path)) + } + sort.Strings(paths) + want := []string{"lib/lib.go", "lib/util.go", "main.go"} + if !reflect.DeepEqual(paths, want) { + t.Fatalf("ScanFiles = %v, want %v (nested worktree and clone must not be counted)", paths, want) + } + + // The scan root itself is a repository too; it must still be scanned. + wt := filepath.Join(root, ".claude", "worktrees", "agent-x") + wtFiles, err := ScanFiles(context.Background(), wt, NewGitIgnoreCache(wt), nil, nil) + if err != nil { + t.Fatal(err) + } + if len(wtFiles) != 3 { + t.Fatalf("scanning the linked worktree as root returned %d files, want 3", len(wtFiles)) + } +} + +func TestNestedGitReposFindsWorktreesAtAnyDepth(t *testing.T) { + root := nestedRepoFixture(t) + + got := NestedGitRepos(root) + want := []string{filepath.Join(".claude", "worktrees", "agent-x"), filepath.Join("tools", "sub-clone")} + if !reflect.DeepEqual(got, want) { + t.Fatalf("NestedGitRepos = %v, want %v", got, want) + } + + if !IsNestedGitRepo(filepath.Join(root, ".claude", "worktrees", "agent-x")) { + t.Fatal("a linked worktree (.git file) must be a repository boundary") + } + if !IsNestedGitRepo(filepath.Join(root, "tools", "sub-clone")) { + t.Fatal("a plain clone (.git directory) must be a repository boundary") + } + if IsNestedGitRepo(filepath.Join(root, "lib")) { + t.Fatal("a plain source directory is not a repository boundary") + } +} + +func TestReadExternalDepsSkipsNestedRepos(t *testing.T) { + root := nestedRepoFixture(t) + if err := os.WriteFile(filepath.Join(root, "go.mod"), []byte("module example.com/outer\n\ngo 1.22\n\nrequire (\n\texample.com/outerdep v1.0.0\n)\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "tools", "sub-clone", "go.mod"), []byte("module example.com/inner\n\ngo 1.22\n\nrequire (\n\texample.com/innerdep v1.0.0\n)\n"), 0o644); err != nil { + t.Fatal(err) + } + + deps, err := ReadExternalDeps(context.Background(), root, 0) + if err != nil { + t.Fatal(err) + } + for _, dep := range deps["go"] { + if strings.Contains(dep, "innerdep") { + t.Fatalf("manifest inside a nested clone leaked into external deps: %v", deps["go"]) + } + } + if len(deps["go"]) == 0 { + t.Fatalf("outer go.mod was not read: %v", deps) + } +} diff --git a/topology/cache.go b/topology/cache.go index d42394b..d3e0069 100644 --- a/topology/cache.go +++ b/topology/cache.go @@ -14,6 +14,7 @@ import ( "time" "codemap/config" + "codemap/internal/gitexclude" "codemap/scanner" ) @@ -186,7 +187,7 @@ func WriteCacheAt(cacheDir string, envelope CacheEnvelope) error { } data = append(data, '\n') - if err := os.MkdirAll(cacheDir, 0o755); err != nil { + if err := gitexclude.MkdirAll(cacheDir, 0o755); err != nil { return err } temp, err := os.CreateTemp(cacheDir, ".topology-state-*.tmp") diff --git a/watch/daemon.go b/watch/daemon.go index 18fcd98..88c1c4a 100644 --- a/watch/daemon.go +++ b/watch/daemon.go @@ -12,6 +12,7 @@ import ( "codemap/analysis" "codemap/config" + "codemap/internal/gitexclude" "codemap/internal/projectpath" "codemap/limits" "codemap/scanner" @@ -92,7 +93,7 @@ func (d *Daemon) ensurePublisher() error { if err != nil { return err } - if err := os.MkdirAll(runtimeDir, 0o755); err != nil { + if err := gitexclude.MkdirAll(runtimeDir, 0o755); err != nil { return err } d.publisher = newStatePublisher(d, filepath.Join(runtimeDir, "state.json"), "legacy-test-instance") @@ -163,13 +164,13 @@ func (d *Daemon) Start() error { return fmt.Errorf("resolve runtime state: %w", err) } codemapDir := runtimeDir - if err := os.MkdirAll(codemapDir, 0755); err != nil { + if err := gitexclude.MkdirAll(codemapDir, 0755); err != nil { return fmt.Errorf("failed to create .codemap dir: %w", err) } // Ensure the config directory exists; it is watched so config edits can // refresh the configured-file inventory. configDir := d.configDir - if err := os.MkdirAll(configDir, 0755); err != nil { + if err := gitexclude.MkdirAll(configDir, 0755); err != nil { return fmt.Errorf("failed to create .codemap dir: %w", err) } diff --git a/watch/state.go b/watch/state.go index 7c8d5b2..dbf1925 100644 --- a/watch/state.go +++ b/watch/state.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "codemap/internal/gitexclude" "codemap/internal/projectpath" "codemap/internal/runtimefile" ) @@ -85,7 +86,7 @@ func WriteProcessPID(root string, pid int) error { if err != nil { return err } - if err := os.MkdirAll(runtimeDir, 0o755); err != nil { + if err := gitexclude.MkdirAll(runtimeDir, 0o755); err != nil { return err } pidFile := filepath.Join(runtimeDir, "watch.pid") diff --git a/watch/transition.go b/watch/transition.go index 75c48ff..f213544 100644 --- a/watch/transition.go +++ b/watch/transition.go @@ -10,6 +10,7 @@ import ( "path/filepath" "time" + "codemap/internal/gitexclude" "codemap/internal/projectpath" ) @@ -44,7 +45,7 @@ func AcquireTransition(root string) (*Transition, error) { if err != nil { return nil, err } - if err := os.MkdirAll(selection.RuntimeDir, 0o755); err != nil { + if err := gitexclude.MkdirAll(selection.RuntimeDir, 0o755); err != nil { return nil, err } if info, err := os.Lstat(selection.RuntimeDir); err != nil || !info.IsDir() { From 6c3d024d7eea34d44bd543572eb1a85a2d51a902 Mon Sep 17 00:00:00 2001 From: Jordan Jackson Date: Tue, 6 Oct 2026 23:51:45 -0400 Subject: [PATCH 2/2] test(gitexclude): compare canonical paths so macOS temp symlinks do not fail the worktree test Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0159tcanCgA3jxi2E7DdRVWJ --- internal/gitexclude/gitexclude_test.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/gitexclude/gitexclude_test.go b/internal/gitexclude/gitexclude_test.go index 4152340..10fd033 100644 --- a/internal/gitexclude/gitexclude_test.go +++ b/internal/gitexclude/gitexclude_test.go @@ -22,6 +22,15 @@ func gitRun(t *testing.T, dir string, args ...string) string { return string(out) } +func canonical(t *testing.T, path string) string { + t.Helper() + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + t.Fatalf("EvalSymlinks(%q): %v", path, err) + } + return resolved +} + func freshClone(t *testing.T) string { t.Helper() if _, err := exec.LookPath("git"); err != nil { @@ -120,9 +129,11 @@ func TestEnsureIgnoredIfNeededUsesSharedExcludeInLinkedWorktree(t *testing.T) { if err != nil { t.Fatal(err) } - want := filepath.Join(root, ".git", "info", "exclude") - if wrote != want { - t.Fatalf("wrote %q, want the primary's shared exclude %q", wrote, want) + // git reports the common dir with symlinks resolved (macOS keeps + // t.TempDir under /var -> /private/var), so compare canonical paths. + want := canonical(t, filepath.Join(root, ".git", "info", "exclude")) + if got := canonical(t, wrote); got != want { + t.Fatalf("wrote %q, want the primary's shared exclude %q", got, want) } if err := os.MkdirAll(filepath.Join(wt, ".codemap"), 0o755); err != nil { t.Fatal(err)