diff --git a/backend/internal/service/project/export_test.go b/backend/internal/service/project/export_test.go new file mode 100644 index 00000000..55e792f2 --- /dev/null +++ b/backend/internal/service/project/export_test.go @@ -0,0 +1,7 @@ +package project + +// Test-only handles to unexported internals, so external tests can assert the +// isGitRepo behavior without widening the package's public surface. + +// SamePathForTest exposes samePath to tests. +var SamePathForTest = samePath diff --git a/backend/internal/service/project/git.go b/backend/internal/service/project/git.go new file mode 100644 index 00000000..4f75cbbc --- /dev/null +++ b/backend/internal/service/project/git.go @@ -0,0 +1,53 @@ +package project + +import ( + "os/exec" + "path/filepath" + "runtime" + "strings" +) + +// GitChecker reports whether a filesystem path is the root of a git repository. +// It is the seam that lets the project Service be exercised without a real git +// binary or working tree. +type GitChecker interface { + IsRepo(path string) bool +} + +// isGitRepo reports whether path is a git repository root, using the production +// git checker. Free-function workspace registration calls this directly; the +// Service's own repo check goes through the injectable GitChecker seam instead. +func isGitRepo(path string) bool { return execGitChecker{}.IsRepo(path) } + +// execGitChecker is the production GitChecker: it shells out to git. +type execGitChecker struct{} + +func (execGitChecker) IsRepo(path string) bool { + cmd := exec.Command("git", "-C", path, "rev-parse", "--show-toplevel") + out, err := cmd.Output() + if err != nil { + return false + } + top := filepath.Clean(strings.TrimSpace(string(out))) + path = filepath.Clean(path) + top, err = filepath.EvalSymlinks(top) + if err != nil { + return false + } + path, err = filepath.EvalSymlinks(path) + if err != nil { + return false + } + return samePath(top, path) +} + +// samePath compares two cleaned, symlink-resolved paths. It is case-insensitive +// only on filesystems that are conventionally case-insensitive (macOS, Windows); +// on case-sensitive filesystems (Linux), "/home/u/Repo" and "/home/u/repo" are +// distinct directories and must not be treated as equal. +func samePath(a, b string) bool { + if runtime.GOOS == "darwin" || runtime.GOOS == "windows" { + return strings.EqualFold(a, b) + } + return a == b +} diff --git a/backend/internal/service/project/git_test.go b/backend/internal/service/project/git_test.go new file mode 100644 index 00000000..24cc46e0 --- /dev/null +++ b/backend/internal/service/project/git_test.go @@ -0,0 +1,60 @@ +package project_test + +import ( + "context" + "runtime" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/service/project" + "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" +) + +// fakeGitChecker is a GitChecker that answers from an in-memory set of repo +// paths — no git binary, no working tree. It is the seam that lets the project +// service be exercised in a unit test. +type fakeGitChecker struct{ repos map[string]bool } + +func (f fakeGitChecker) IsRepo(path string) bool { return f.repos[path] } + +func newManagerWithGit(t *testing.T, git project.GitChecker) project.Manager { + t.Helper() + store, err := sqlite.Open(t.TempDir()) + if err != nil { + t.Fatalf("open store: %v", err) + } + t.Cleanup(func() { _ = store.Close() }) + return project.NewWithGitChecker(store, git) +} + +func TestAdd_UsesInjectedGitChecker(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + m := newManagerWithGit(t, fakeGitChecker{repos: map[string]bool{dir: true}}) + + // A path the fake recognizes as a repo is accepted — without shelling out to git. + if _, err := m.Add(ctx, project.AddInput{Path: dir, ProjectID: ptr("ao")}); err != nil { + t.Fatalf("Add on a fake-recognized repo: %v", err) + } +} + +func TestAdd_RejectsNonRepoViaGitChecker(t *testing.T) { + ctx := context.Background() + m := newManagerWithGit(t, fakeGitChecker{repos: map[string]bool{}}) + + _, err := m.Add(ctx, project.AddInput{Path: t.TempDir(), ProjectID: ptr("ao")}) + wantCode(t, err, "NOT_A_GIT_REPO") +} + +// TestSamePath_CaseSensitivity guards the isGitRepo fix: paths differing only in +// case must be treated as distinct on case-sensitive filesystems (Linux) and as +// equal on the conventionally case-insensitive ones (macOS, Windows). +func TestSamePath_CaseSensitivity(t *testing.T) { + // Document the platform contract so a regression in samePath is caught here. + caseInsensitive := runtime.GOOS == "darwin" || runtime.GOOS == "windows" + if got := project.SamePathForTest("/a/Repo", "/a/repo"); got != caseInsensitive { + t.Fatalf("samePath(/a/Repo, /a/repo) = %v on %s, want %v", got, runtime.GOOS, caseInsensitive) + } + if !project.SamePathForTest("/a/repo", "/a/repo") { + t.Fatalf("samePath of identical paths must be true") + } +} diff --git a/backend/internal/service/project/service.go b/backend/internal/service/project/service.go index adc92438..d29180a0 100644 --- a/backend/internal/service/project/service.go +++ b/backend/internal/service/project/service.go @@ -39,6 +39,7 @@ type Manager interface { // Service implements project registration and lookup use-cases for controllers. type Service struct { store Store + git GitChecker // addMu serialises the whole body of Add. Workspace registration performs // filesystem mutations (git init, .gitignore writes, commits) that are not // covered by the store's own writeMu, so path/id conflict checks plus the @@ -48,9 +49,16 @@ type Service struct { var _ Manager = (*Service)(nil) -// New returns a project service backed by the given durable store. +// New returns a project service backed by the given durable store, using the +// production GitChecker that shells out to git. func New(store Store) *Service { - return &Service{store: store} + return NewWithGitChecker(store, execGitChecker{}) +} + +// NewWithGitChecker returns a project service with an injectable GitChecker, +// letting tests substitute a fake for the repo check. +func NewWithGitChecker(store Store, git GitChecker) *Service { + return &Service{store: store, git: git} } // List returns every active registered project. @@ -173,7 +181,7 @@ func (m *Service) Add(ctx context.Context, in AddInput) (Project, error) { p.WorkspaceRepos = workspaceReposFromRecords(repos) return p, nil } - if !isGitRepo(path) { + if !m.git.IsRepo(path) { return Project{}, apierr.Invalid("NOT_A_GIT_REPO", "Repository path must point to a git repository", nil) } row.RepoOriginURL = resolveGitOriginURL(path) @@ -288,29 +296,6 @@ func normalizePath(raw string) (string, error) { return filepath.Clean(abs), nil } -func isGitRepo(path string) bool { - cmd := exec.Command("git", "-C", path, "rev-parse", "--show-toplevel") - out, err := cmd.Output() - if err != nil { - return false - } - top := filepath.Clean(strings.TrimSpace(string(out))) - path = filepath.Clean(path) - top, err = filepath.EvalSymlinks(top) - if err != nil { - return false - } - path, err = filepath.EvalSymlinks(path) - if err != nil { - return false - } - - if strings.EqualFold(top, path) { - return true - } - return top == path -} - func defaultProjectID(path string) domain.ProjectID { id := strings.ToLower(filepath.Base(path)) id = strings.TrimSpace(id)