diff --git a/internal/cli/run.go b/internal/cli/run.go index 5b40162..196ecc8 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -197,7 +197,8 @@ func (r *Runner) setupRunContext(ctx context.Context, opts *RunOptions) (*runCon noopCleanup := func() {} // All workflows run in isolated git worktrees for security - repoRoot, err := util.FindGitRoot(ctx, opts.WorkingDir) + // Use GetGitRepoInfo to get all git info in optimized fewer subprocess calls + gitInfo, err := util.GetGitRepoInfo(ctx, opts.WorkingDir) if err != nil { return nil, noopCleanup, fmt.Errorf( "not a git repository: %w\n"+ @@ -207,18 +208,14 @@ func (r *Runner) setupRunContext(ctx context.Context, opts *RunOptions) (*runCon ) } - // Pass verified=true since FindGitRoot already confirmed this is a git repository - ws, err := worktree.CreateWorkspace(ctx, repoRoot, true) + // Pass verified=true since GetGitRepoInfo already confirmed this is a git repository + ws, err := worktree.CreateWorkspace(ctx, gitInfo.Root, true) if err != nil { return nil, noopCleanup, fmt.Errorf("failed to create workspace: %w", err) } slog.Info("created isolated workspace", "path", ws.Path) - // Get git info from original repo - sha, _ := util.GitHeadSHA(ctx, repoRoot) - ref, _ := util.GitHeadRef(ctx, repoRoot) - cleanup := func() { if err := worktree.RemoveWorkspace(ctx, ws); err != nil { slog.Warn("failed to remove workspace", "path", ws.Path, "error", err) @@ -229,9 +226,9 @@ func (r *Runner) setupRunContext(ctx context.Context, opts *RunOptions) (*runCon return &runContext{ workDir: ws.Path, - repoRoot: repoRoot, - sha: sha, - ref: ref, + repoRoot: gitInfo.Root, + sha: gitInfo.HeadSHA, + ref: gitInfo.HeadRef, workspace: ws, }, cleanup, nil } diff --git a/internal/util/git.go b/internal/util/git.go index 6222b3e..8bb8287 100644 --- a/internal/util/git.go +++ b/internal/util/git.go @@ -8,6 +8,18 @@ import ( "strings" ) +// GitRepoInfo contains information about a git repository. +// This struct is populated by a single git command to minimize subprocess overhead. +type GitRepoInfo struct { + // Root is the absolute path to the repository root directory. + Root string + // HeadSHA is the SHA of the current HEAD commit. + HeadSHA string + // HeadRef is the symbolic ref of HEAD (e.g., refs/heads/main). + // Empty string if HEAD is detached. + HeadRef string +} + // runGit executes a git command and returns the trimmed stdout. func runGit(ctx context.Context, dir string, args ...string) (string, error) { cmd := exec.CommandContext(ctx, "git", args...) @@ -55,3 +67,44 @@ func GitHeadRef(ctx context.Context, repoRoot string) (string, error) { } return out, nil } + +// GetGitRepoInfo retrieves git repository information with a single git command. +// This is more efficient than calling FindGitRoot, GitHeadSHA, and GitHeadRef separately, +// as it reduces the number of subprocess invocations from 3 to 2. +func GetGitRepoInfo(ctx context.Context, startDir string) (*GitRepoInfo, error) { + // Get root and HEAD SHA in a single command + // git rev-parse outputs each value on a separate line + out, err := runGit(ctx, startDir, "rev-parse", "--show-toplevel", "HEAD") + if err != nil { + return nil, fmt.Errorf("not a git repository: %w", err) + } + + lines := strings.Split(out, "\n") + if len(lines) < 2 { + return nil, fmt.Errorf("unexpected git rev-parse output: %s", out) + } + + info := &GitRepoInfo{ + Root: lines[0], + HeadSHA: lines[1], + } + + // Get symbolic ref separately (it's a different git command) + // Use -q to suppress errors for detached HEAD + ref, err := runGit(ctx, startDir, "symbolic-ref", "-q", "HEAD") + if err != nil { + // git symbolic-ref -q exits with code 1 for detached HEAD (not an error) + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + info.HeadRef = "" + } else { + // For other errors, we still return the info we have + // HeadRef is optional, so we don't fail the whole operation + info.HeadRef = "" + } + } else { + info.HeadRef = ref + } + + return info, nil +} diff --git a/internal/util/git_test.go b/internal/util/git_test.go index 02cc59d..a39f70b 100644 --- a/internal/util/git_test.go +++ b/internal/util/git_test.go @@ -283,3 +283,172 @@ func TestFindGitRoot_MultipleDirectories(t *testing.T) { } } } + +// TestGetGitRepoInfo tests the unified GetGitRepoInfo function +func TestGetGitRepoInfo(t *testing.T) { + ctx := context.Background() + + t.Run("returns all info for valid repository", func(t *testing.T) { + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get cwd: %v", err) + } + + repoRoot := findTestRepoRoot(t, cwd) + + info, err := GetGitRepoInfo(ctx, repoRoot) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + + // Verify Root + if info.Root != repoRoot { + t.Errorf("expected Root=%s, got %s", repoRoot, info.Root) + } + + // Verify HeadSHA is 40 hex characters + if len(info.HeadSHA) != 40 { + t.Errorf("expected 40 char SHA, got %d chars: %s", len(info.HeadSHA), info.HeadSHA) + } + + // Verify HeadRef starts with refs/ or is empty (detached HEAD) + if info.HeadRef != "" && !strings.HasPrefix(info.HeadRef, "refs/") { + t.Errorf("expected HeadRef to be empty or start with refs/, got: %s", info.HeadRef) + } + }) + + t.Run("returns same values as individual functions", func(t *testing.T) { + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get cwd: %v", err) + } + + repoRoot := findTestRepoRoot(t, cwd) + + // Get info via unified function + info, err := GetGitRepoInfo(ctx, repoRoot) + if err != nil { + t.Fatalf("GetGitRepoInfo error: %v", err) + } + + // Get info via individual functions + root, err := FindGitRoot(ctx, repoRoot) + if err != nil { + t.Fatalf("FindGitRoot error: %v", err) + } + sha, err := GitHeadSHA(ctx, repoRoot) + if err != nil { + t.Fatalf("GitHeadSHA error: %v", err) + } + ref, err := GitHeadRef(ctx, repoRoot) + if err != nil { + t.Fatalf("GitHeadRef error: %v", err) + } + + // Compare results + if info.Root != root { + t.Errorf("Root mismatch: GetGitRepoInfo=%s, FindGitRoot=%s", info.Root, root) + } + if info.HeadSHA != sha { + t.Errorf("HeadSHA mismatch: GetGitRepoInfo=%s, GitHeadSHA=%s", info.HeadSHA, sha) + } + if info.HeadRef != ref { + t.Errorf("HeadRef mismatch: GetGitRepoInfo=%s, GitHeadRef=%s", info.HeadRef, ref) + } + }) + + t.Run("works from subdirectory", func(t *testing.T) { + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get cwd: %v", err) + } + + repoRoot := findTestRepoRoot(t, cwd) + subDir := filepath.Join(repoRoot, "internal", "util") + + info, err := GetGitRepoInfo(ctx, subDir) + if err != nil { + t.Fatalf("expected no error, got: %v", err) + } + + if info.Root != repoRoot { + t.Errorf("expected Root=%s, got %s", repoRoot, info.Root) + } + }) + + t.Run("returns error for non-git directory", func(t *testing.T) { + _, err := GetGitRepoInfo(ctx, "/tmp") + if err == nil { + t.Error("expected error for non-git directory, got nil") + } + }) +} + +// TestGetGitRepoInfo_DetachedHead tests GetGitRepoInfo with detached HEAD +func TestGetGitRepoInfo_DetachedHead(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + ctx := context.Background() + + // Initialize a new git repository + cmd := exec.CommandContext(ctx, "git", "init") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to init git repo: %v", err) + } + + // Configure git user for the commit + cmd = exec.CommandContext(ctx, "git", "config", "user.email", "test@test.com") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to config git: %v", err) + } + cmd = exec.CommandContext(ctx, "git", "config", "user.name", "Test") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to config git: %v", err) + } + + // Create a file and commit it + testFile := filepath.Join(tmpDir, "test.txt") + if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil { + t.Fatalf("failed to create test file: %v", err) + } + cmd = exec.CommandContext(ctx, "git", "add", ".") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to git add: %v", err) + } + cmd = exec.CommandContext(ctx, "git", "commit", "--no-gpg-sign", "-m", "initial") + cmd.Dir = tmpDir + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("failed to git commit: %v, output: %s", err, output) + } + + // Get the commit SHA + info, err := GetGitRepoInfo(ctx, tmpDir) + if err != nil { + t.Fatalf("failed to get git info: %v", err) + } + + // Detach HEAD by checking out the commit SHA directly + cmd = exec.CommandContext(ctx, "git", "checkout", info.HeadSHA) + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to checkout detached HEAD: %v", err) + } + + // Now GetGitRepoInfo should return empty HeadRef (detached HEAD) + info, err = GetGitRepoInfo(ctx, tmpDir) + if err != nil { + t.Errorf("GetGitRepoInfo() error = %v; expected no error for detached HEAD", err) + } + if info.HeadRef != "" { + t.Errorf("GetGitRepoInfo().HeadRef = %q; expected empty string for detached HEAD", info.HeadRef) + } + if info.HeadSHA == "" { + t.Error("GetGitRepoInfo().HeadSHA should not be empty for detached HEAD") + } +} diff --git a/internal/worktree/worktree.go b/internal/worktree/worktree.go index a284a8d..0f8ceff 100644 --- a/internal/worktree/worktree.go +++ b/internal/worktree/worktree.go @@ -9,6 +9,8 @@ import ( "os/exec" "path/filepath" "strings" + + "github.com/watany-dev/raptor/internal/util" ) const ( @@ -23,9 +25,10 @@ const ( // If verified is true, skips git repository verification (use when caller has already verified). func CreateWorkspace(ctx context.Context, repoRoot string, verified bool) (*Workspace, error) { // Verify this is a git repository (skip if already verified by caller) + // Uses util.FindGitRoot to avoid duplicating git command logic if !verified { - if err := verifyGitRepo(ctx, repoRoot); err != nil { - return nil, err + if _, err := util.FindGitRoot(ctx, repoRoot); err != nil { + return nil, fmt.Errorf("not a git repository: %w", err) } } @@ -90,20 +93,6 @@ func RemoveWorkspace(ctx context.Context, ws *Workspace) error { return nil } -// verifyGitRepo checks if the given path is a valid git repository. -func verifyGitRepo(ctx context.Context, path string) error { - cmd := exec.CommandContext(ctx, "git", "rev-parse", "--git-dir") - cmd.Dir = path - - var stderr strings.Builder - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - return fmt.Errorf("not a git repository (%s): %w", strings.TrimSpace(stderr.String()), err) - } - - return nil -} // randReader is the random reader used for generating IDs. // It can be replaced in tests to simulate errors.