Skip to content

fix(project): case-sensitive isGitRepo + injectable GitChecker#163

Open
neversettle17-101 wants to merge 2 commits into
mainfrom
feat/issue-97
Open

fix(project): case-sensitive isGitRepo + injectable GitChecker#163
neversettle17-101 wants to merge 2 commits into
mainfrom
feat/issue-97

Conversation

@neversettle17-101

Copy link
Copy Markdown
Collaborator

Summary

Resolves the two isGitRepo findings deferred from #95 / PR #96 (an implementation existed there and was reverted in e3d2533; this restores it).

1. Case-sensitivity bug on Linux

The old isGitRepo used strings.EqualFold(top, path), which wrongly treats /home/u/MyRepo and /home/u/myrepo as the same path on case-sensitive filesystems. Replaced with samePath, which is case-insensitive only on darwin/windows (gated by runtime.GOOS) and uses == everywhere else.

2. Untestable git check

The git check shelled out to exec.Command("git", ...) directly, so project.Service couldn't be tested without a real git binary and working tree. The check now sits behind a GitChecker interface injected into Service:

  • execGitChecker — production impl, shells out to git.
  • NewWithGitChecker(store, git) — lets tests substitute a fake.

Tests

  • TestAdd_UsesInjectedGitChecker / TestAdd_RejectsNonRepoViaGitChecker — exercise Add through a fake GitChecker, no git binary.
  • TestSamePath_CaseSensitivity — guards the platform contract for samePath.

Acceptance criteria

  • isGitRepo correct on case-sensitive filesystems
  • Project service testable without a real git binary

Closes #97

🤖 Generated with Claude Code

@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Introduces a GitChecker interface injected into Service, moving the git binary shell-out to a swappable seam, and fixes a case-sensitivity bug in the old isGitRepo by replacing unconditional strings.EqualFold with samePath, which uses case-insensitive comparison only on darwin/windows and strict == on Linux.

  • git.go defines GitChecker, execGitChecker (production), and samePath with the platform-gated logic; the free-function isGitRepo wrapper continues to serve workspace_registration.go, which is explicitly out of scope for this injection.
  • service.go gains a git GitChecker field, a NewWithGitChecker constructor, and swaps isGitRepo(path) for m.git.IsRepo(path) in Add.
  • git_test.go / export_test.go add unit tests for the injected checker and the samePath platform contract, both exercisable without a real git binary.

Confidence Score: 5/5

Safe to merge; the refactor is well-scoped, backward-compatible, and all changed paths are covered by tests.

The case-sensitivity fix replaces a clear incorrect behaviour with a correct platform-gated implementation. The GitChecker injection follows a clean constructor-injection pattern; New delegates to NewWithGitChecker so no call sites break. The free-function isGitRepo wrapper is still reachable from workspace_registration.go so there is no dead code. Tests exercise both the happy path and the rejection path through the fake seam.

No files require special attention.

Important Files Changed

Filename Overview
backend/internal/service/project/git.go New file; correctly extracts GitChecker interface, execGitChecker production impl, and platform-aware samePath. isGitRepo free function confirmed used by workspace_registration.go.
backend/internal/service/project/service.go Adds git GitChecker field, NewWithGitChecker constructor, and replaces direct isGitRepo call with m.git.IsRepo; backward-compatible via New delegating to NewWithGitChecker.
backend/internal/service/project/git_test.go Adds three focused tests: injectable checker acceptance, rejection path, and samePath platform contract — none require a real git binary.
backend/internal/service/project/export_test.go Cleanly exposes samePath via SamePathForTest without widening the package's public surface; standard Go export_test pattern.

Reviews (3): Last reviewed commit: "test(project): drop real-git assertion f..." | Re-trigger Greptile

Comment thread backend/internal/service/project/git_test.go
Comment thread backend/internal/service/project/git.go
neversettle17-101 and others added 2 commits June 12, 2026 14:32
Restores the GitChecker seam and case-sensitivity fix deferred from #95
(reverted in e3d2533).

- samePath compares cleaned, symlink-resolved paths and is case-insensitive
  only on darwin/windows; on case-sensitive filesystems (Linux) paths
  differing only in case are now distinct.
- Git repo check moves behind a GitChecker interface injected into Service,
  so the service is testable with a fake — no real git binary or working tree.

Closes #97

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The execGitChecker.IsRepo(tempdir) check in TestSamePath_CaseSensitivity
silently passed when git was absent from PATH (cmd.Output() fails -> false),
so it never exercised the git path it implied. Remove it; the test now only
asserts the samePath platform contract, which has no git dependency. Drops the
now-unused NewExecGitCheckerForTest shim.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LLD: isGitRepo case-sensitivity bug + untestable git check (deferred from #95)

1 participant