Add git status detection when navigating into git repos#13
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Prism’s statusline git integration so git branch/status and diff stats can appear after navigating from a non-git start directory into a git repository, by resolving an “effective” git directory based on CurrentDir when ProjectDir isn’t a repo.
Changes:
- Add “effective git dir” resolution for
linesChangedin the statusline to fall back fromProjectDirtoCurrentDir’s git root. - Update the
gitplugin to use an effective git directory (with caching) rather than relying solely onProjectDir. - Add tests covering effective git dir resolution and
linesChangedbehavior whenProjectDiris not a git repo.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| internal/statusline/statusline.go | Uses an effective git directory for diff stats and adds helper(s) to resolve git root from CurrentDir. |
| internal/plugins/git.go | Resolves git root from CurrentDir when needed and caches effective-dir resolution under git:effective: keys. |
| internal/statusline/statusline_test.go | Adds tests for effective git dir selection and linesChanged diff stats when starting outside a git repo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/statusline/statusline.go
Outdated
| cmd := exec.Command("git", "--no-optional-locks", "rev-parse", "--show-toplevel") | ||
| cmd.Dir = currentDir | ||
| output, err := cmd.Output() | ||
| if err != nil { |
There was a problem hiding this comment.
getEffectiveGitDir runs git rev-parse with exec.Command and no timeout/context. If git hangs (e.g., slow/remote FS or a stuck subprocess), status line rendering can block. Consider using exec.CommandContext with a short timeout and caching the failure result.
internal/statusline/statusline.go
Outdated
| // isGitDir checks if a directory is inside a git repository | ||
| func isGitDir(dir string) bool { | ||
| cmd := exec.Command("git", "--no-optional-locks", "rev-parse", "--git-dir") | ||
| cmd.Dir = dir | ||
| return cmd.Run() == nil |
There was a problem hiding this comment.
The helper name isGitDir is misleading: it checks whether dir is inside a git repo (via rev-parse --git-dir), not whether the path is a .git directory. Renaming to isGitRepo/isInGitRepo would make call sites clearer.
internal/plugins/git.go
Outdated
| // Check if this is a git repo | ||
| if !isGitRepo(ctx, projectDir) { | ||
| if !isGitRepo(ctx, gitDir) { | ||
| return "", nil | ||
| } |
There was a problem hiding this comment.
Execute calls getEffectiveGitDir, which already runs isGitRepo(projectDir) on the fast path, but then Execute immediately calls isGitRepo(gitDir) again. This adds an extra git rev-parse subprocess every cache refresh. Consider having getEffectiveGitDir also return whether the path is a repo (or have the fast path skip the later repo check) to avoid redundant git calls.
| os.WriteFile(readmeFile, []byte("# Test\n"), 0644) | ||
| cmd := exec.Command("git", "add", "README.md") | ||
| cmd.Dir = gitRepoDir | ||
| cmd.Run() | ||
| cmd = exec.Command("git", "commit", "-m", "Initial commit") | ||
| cmd.Dir = gitRepoDir | ||
| cmd.Run() |
There was a problem hiding this comment.
Test setup ignores errors from os.WriteFile and git add (cmd.Run() result). If these steps fail, the test can still pass without exercising the intended scenario. Please check and fail the test on errors (consistent with setupTestGitRepo).
| os.WriteFile(readmeFile, []byte("# Test\n"), 0644) | |
| cmd := exec.Command("git", "add", "README.md") | |
| cmd.Dir = gitRepoDir | |
| cmd.Run() | |
| cmd = exec.Command("git", "commit", "-m", "Initial commit") | |
| cmd.Dir = gitRepoDir | |
| cmd.Run() | |
| if err := os.WriteFile(readmeFile, []byte("# Test\n"), 0644); err != nil { | |
| t.Fatalf("failed to write README.md: %v", err) | |
| } | |
| cmd := exec.Command("git", "add", "README.md") | |
| cmd.Dir = gitRepoDir | |
| if err := cmd.Run(); err != nil { | |
| t.Fatalf("failed to run git add: %v", err) | |
| } | |
| cmd = exec.Command("git", "commit", "-m", "Initial commit") | |
| cmd.Dir = gitRepoDir | |
| if err := cmd.Run(); err != nil { | |
| t.Fatalf("failed to run git commit: %v", err) | |
| } |
|
|
||
| result := sl.getEffectiveGitDir() | ||
| // Resolve symlinks for comparison (macOS /var -> /private/var) | ||
| expectedDir, _ := filepath.EvalSymlinks(gitRepoDir) |
There was a problem hiding this comment.
expectedDir, _ := filepath.EvalSymlinks(gitRepoDir) ignores the error. If symlink evaluation fails, expectedDir will be empty and the assertion will be misleading. Handle the error (e.g., fall back to gitRepoDir or fail the test) so failures are actionable.
| expectedDir, _ := filepath.EvalSymlinks(gitRepoDir) | |
| expectedDir, err := filepath.EvalSymlinks(gitRepoDir) | |
| if err != nil { | |
| t.Fatalf("failed to evaluate symlinks for %q: %v", gitRepoDir, err) | |
| } |
internal/statusline/statusline.go
Outdated
| cacheKey := "git:effective:" + currentDir | ||
| if cached, ok := statusCache.Get(cacheKey); ok { | ||
| return cached // empty string cached means "not a git repo" | ||
| } | ||
|
|
There was a problem hiding this comment.
git:effective: results are cached per currentDir. Since cache.Cache doesn’t evict expired entries, visiting many different directories can permanently grow statusCache even though entries expire. Consider caching by resolved git root (or adding eviction/cleanup) to keep cache size bounded.
| cmds := [][]string{ | ||
| {"git", "init"}, | ||
| {"git", "config", "user.email", "test@test.com"}, | ||
| {"git", "config", "user.name", "Test"}, | ||
| } |
There was a problem hiding this comment.
The repo initialization/commit boilerplate (git init + config + initial commit) is duplicated across multiple new tests, making them harder to maintain and easier to diverge. Consider extracting a helper that creates a git repo at a specified path (similar to setupTestGitRepo).
| cmd.Run() | ||
| cmd = exec.Command("git", "commit", "-m", "Initial commit") | ||
| cmd.Dir = gitRepoDir | ||
| cmd.Run() |
There was a problem hiding this comment.
git commit is executed with cmd.Run() but the error is ignored. If the commit fails, the test may proceed in an unexpected repo state. Please assert cmd.Run() succeeds (and consider capturing stderr for easier debugging).
| cmd.Run() | |
| cmd = exec.Command("git", "commit", "-m", "Initial commit") | |
| cmd.Dir = gitRepoDir | |
| cmd.Run() | |
| if out, err := cmd.CombinedOutput(); err != nil { | |
| t.Fatalf("failed to run git add: %v, output: %s", err, string(out)) | |
| } | |
| cmd = exec.Command("git", "commit", "-m", "Initial commit") | |
| cmd.Dir = gitRepoDir | |
| if out, err := cmd.CombinedOutput(); err != nil { | |
| t.Fatalf("failed to run git commit: %v, output: %s", err, string(out)) | |
| } |
internal/plugins/git.go
Outdated
| cacheKey := fmt.Sprintf("git:effective:%s", currentDir) | ||
| if cached, ok := p.cache.Get(cacheKey); ok { | ||
| if cached == "" { | ||
| return "" | ||
| } |
There was a problem hiding this comment.
git:effective: cache keys are based on full currentDir. Because the cache doesn’t evict expired items automatically, traversing many directories can leave lots of expired git:effective: keys behind. Consider caching by resolved git root (or adding eviction/cleanup) to keep cache size bounded.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| defer os.RemoveAll(parentDir) | ||
|
|
||
| // Create a git repo as a subdirectory | ||
| gitRepoDir := filepath.Join(parentDir, "my-repo") | ||
| if err := os.MkdirAll(gitRepoDir, 0755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| cmds := [][]string{ | ||
| {"git", "init"}, | ||
| {"git", "config", "user.email", "test@test.com"}, | ||
| {"git", "config", "user.name", "Test"}, | ||
| } | ||
| for _, args := range cmds { | ||
| cmd := exec.Command(args[0], args[1:]...) | ||
| cmd.Dir = gitRepoDir | ||
| if err := cmd.Run(); err != nil { | ||
| t.Fatalf("failed to run %v: %v", args, err) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
These tests re-implement git repo initialization/initial commit logic that already exists in setupTestGitRepo. Consider extracting a helper that can create a repo at a caller-provided path (or returns both parent dir + repo dir) to reduce duplication and keep future git test setup changes in one place.
| defer os.RemoveAll(parentDir) | |
| // Create a git repo as a subdirectory | |
| gitRepoDir := filepath.Join(parentDir, "my-repo") | |
| if err := os.MkdirAll(gitRepoDir, 0755); err != nil { | |
| t.Fatal(err) | |
| } | |
| cmds := [][]string{ | |
| {"git", "init"}, | |
| {"git", "config", "user.email", "test@test.com"}, | |
| {"git", "config", "user.name", "Test"}, | |
| } | |
| for _, args := range cmds { | |
| cmd := exec.Command(args[0], args[1:]...) | |
| cmd.Dir = gitRepoDir | |
| if err := cmd.Run(); err != nil { | |
| t.Fatalf("failed to run %v: %v", args, err) | |
| } | |
| } | |
| // Use shared test helper to create a git repo with an initial commit. | |
| // This avoids duplicating git initialization logic here. | |
| gitRepoDir := setupTestGitRepo(t) | |
| defer os.RemoveAll(gitRepoDir) | |
| // parentDir is the non-git directory that contains the repo. | |
| // This mirrors the original intent of having a parent directory | |
| // with a git repo as a subdirectory, without reimplementing git setup. | |
| parentDir := filepath.Dir(gitRepoDir) | |
| // (rest of the test continues to use parentDir and gitRepoDir) |
| // Create a git repo as a subdirectory | ||
| gitRepoDir := filepath.Join(parentDir, "my-repo") | ||
| if err := os.MkdirAll(gitRepoDir, 0755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| cmds := [][]string{ | ||
| {"git", "init"}, | ||
| {"git", "config", "user.email", "test@test.com"}, | ||
| {"git", "config", "user.name", "Test"}, | ||
| } | ||
| for _, args := range cmds { | ||
| cmd := exec.Command(args[0], args[1:]...) | ||
| cmd.Dir = gitRepoDir | ||
| if err := cmd.Run(); err != nil { | ||
| t.Fatalf("failed to run %v: %v", args, err) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This test duplicates the same git repo setup sequence used in other new tests and in setupTestGitRepo earlier in this file. Suggest factoring out a helper for “create git repo under parentDir” to avoid repeating the init/config/commit boilerplate three times.
| // Create a git repo as a subdirectory | ||
| gitRepoDir := filepath.Join(parentDir, "my-repo") | ||
| if err := os.MkdirAll(gitRepoDir, 0755); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| cmds := [][]string{ | ||
| {"git", "init"}, | ||
| {"git", "config", "user.email", "test@test.com"}, | ||
| {"git", "config", "user.name", "Test"}, | ||
| } | ||
| for _, args := range cmds { | ||
| cmd := exec.Command(args[0], args[1:]...) | ||
| cmd.Dir = gitRepoDir | ||
| if err := cmd.Run(); err != nil { | ||
| t.Fatalf("failed to run %v: %v", args, err) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This test repeats the git init/config/commit boilerplate that appears in the other new fallback tests. Consider reusing setupTestGitRepo by extending it (e.g., accept a target dir) or adding a dedicated helper, to keep the test file maintainable.
internal/statusline/statusline.go
Outdated
| // getEffectiveGitDir returns the best directory for git operations. | ||
| // Tries ProjectDir first, falls back to finding git root from CurrentDir. | ||
| func (sl *StatusLine) getEffectiveGitDir() string { | ||
| projectDir := sl.input.Workspace.ProjectDir | ||
| currentDir := sl.input.Workspace.CurrentDir | ||
|
|
||
| // Fast path: ProjectDir is set and is a git repo | ||
| if projectDir != "" { | ||
| if isGitRepo(projectDir) { | ||
| return projectDir | ||
| } |
There was a problem hiding this comment.
There are now two separate implementations of “effective git dir” resolution (here and in internal/plugins/git.go). To prevent drift (different timeouts/caching/edge-case handling), consider extracting shared git-root detection into a small internal helper package and reusing it from both places.
internal/plugins/git.go
Outdated
| // getEffectiveGitDir returns the best directory to use for git operations. | ||
| // Tries ProjectDir first (fast path), falls back to finding git root from CurrentDir. | ||
| func (p *GitPlugin) getEffectiveGitDir(ctx context.Context, input plugin.Input) string { | ||
| projectDir := input.Prism.ProjectDir | ||
| currentDir := input.Prism.CurrentDir | ||
|
|
||
| // Fast path: ProjectDir is a git repo | ||
| if projectDir != "" && isGitRepo(ctx, projectDir) { | ||
| return projectDir | ||
| } | ||
|
|
||
| // Fallback: find git root from CurrentDir | ||
| if currentDir == "" { | ||
| return "" | ||
| } | ||
|
|
||
| // Check cache for effective dir resolution | ||
| if p.cache != nil { | ||
| cacheKey := fmt.Sprintf("git:effective:%s", currentDir) | ||
| if cached, ok := p.cache.Get(cacheKey); ok { | ||
| if cached == "" { | ||
| return "" | ||
| } | ||
| return cached | ||
| } | ||
| } | ||
|
|
||
| gitRoot := findGitRootFromDir(ctx, currentDir) | ||
|
|
||
| // Cache the result (even empty, to avoid repeated lookups) | ||
| if p.cache != nil { | ||
| cacheKey := fmt.Sprintf("git:effective:%s", currentDir) | ||
| p.cache.Set(cacheKey, gitRoot, cache.GitTTL) | ||
| } | ||
|
|
||
| return gitRoot | ||
| } |
There was a problem hiding this comment.
GitPlugin now resolves an effective git directory via ProjectDir/CurrentDir, but this new behavior isn’t covered by tests (unlike other native plugins in this package). Adding a small git plugin test suite (e.g., non-git ProjectDir + git CurrentDir, and subdir-of-repo cases) would prevent regressions in branch/dirty/upstream rendering and caching keys.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/plugins/git_test.go
Outdated
| {"git", "init"}, | ||
| {"git", "config", "user.email", "test@test.com"}, | ||
| {"git", "config", "user.name", "Test"}, | ||
| {"git", "checkout", "-b", "main"}, |
There was a problem hiding this comment.
setupGitRepo runs git checkout -b main after git init. This can fail on systems where init.defaultBranch is already main (branch already exists), making the tests flaky. Prefer a branch operation that works whether or not the branch exists (e.g., git checkout -B main or git branch -M main).
| {"git", "checkout", "-b", "main"}, | |
| {"git", "checkout", "-B", "main"}, |
internal/plugins/git_test.go
Outdated
|
|
||
| // Create a subdirectory that IS a git repo | ||
| gitRepoDir := filepath.Join(parentDir, "repo") | ||
| if err := os.Mkdir(gitRepoDir, 0o755); err != nil { |
There was a problem hiding this comment.
File mode literals in this repo appear to consistently use the 0755 style. This new test uses 0o755, which is valid Go but inconsistent with the surrounding codebase; consider switching to 0755 for consistency.
internal/plugins/git_test.go
Outdated
| parentDir := t.TempDir() | ||
|
|
||
| gitRepoDir := filepath.Join(parentDir, "repo") | ||
| if err := os.Mkdir(gitRepoDir, 0o755); err != nil { |
There was a problem hiding this comment.
File mode literals in this repo appear to consistently use the 0755 style. This new test uses 0o755, which is valid Go but inconsistent with the surrounding codebase; consider switching to 0755 for consistency.
internal/plugins/git_test.go
Outdated
| setupGitRepo(t, gitRepoDir) | ||
|
|
||
| subDir := filepath.Join(gitRepoDir, "subdir") | ||
| if err := os.Mkdir(subDir, 0o755); err != nil { |
There was a problem hiding this comment.
File mode literals in this repo appear to consistently use the 0755 style. This new test uses 0o755, which is valid Go but inconsistent with the surrounding codebase; consider switching to 0755 for consistency.
| func (sl *StatusLine) getEffectiveGitDir() string { | ||
| return git.EffectiveDir( | ||
| context.Background(), | ||
| sl.input.Workspace.ProjectDir, | ||
| sl.input.Workspace.CurrentDir, | ||
| statusCache, | ||
| ) |
There was a problem hiding this comment.
getEffectiveGitDir uses context.Background() which means these git probes can take up to the full internal timeout(s) independent of the statusline render timeout, and getGitDiffStats still runs git without any context/timeout. Consider threading a single timeout context through getEffectiveGitDir (and switching getGitDiffStats to exec.CommandContext) so a slow/hung git call can't stall statusline rendering.
When Claude Code starts in a non-git directory (e.g., ~/Development) and navigates into a subdirectory that is a git repo, the git plugin and linesChanged section now detect and display git information by falling back to CurrentDir when ProjectDir is not a git repo. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add 500ms timeout to git commands in getEffectiveGitDir and isGitRepo - Rename isGitDir to isGitRepo for clarity - Remove redundant isGitRepo check in git plugin Execute() - Add error checks in test setup (os.WriteFile, git add/commit, filepath.EvalSymlinks) - Add cache cleanup for unbounded growth (CleanExpired on Set when >50 entries) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract internal/git package with EffectiveDir, IsRepo, and FindRoot to eliminate duplicate getEffectiveGitDir implementations - Refactor git plugin and statusline to use shared git helpers - Add setupTestGitRepoAt helper and deduplicate git repo setup in tests - Add git plugin test suite for effective-dir fallback behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use -B instead of -b for git checkout in setupGitRepo to avoid failure when the main branch already exists. Normalize file mode literals from 0o755 to 0755 for consistency with the rest of the codebase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3fa71a2 to
7d5fdc5
Compare
Summary
~/Development) and navigates into a git repo subdirectory, git branch/status and diff stats now appear in the status linegitplugin andlinesChangedsection fall back toCurrentDirwhenProjectDiris not a git repogit:effective:prefix so idle hook invalidation (DeleteByPrefix("git:")) still worksTest plan
go build ./cmd/prismcompiles cleanlygo test ./...— all existing tests pass (no regressions)🤖 Generated with Claude Code