diff --git a/cmd/roborev/compact.go b/cmd/roborev/compact.go index 7aa55d54..c43b7791 100644 --- a/cmd/roborev/compact.go +++ b/cmd/roborev/compact.go @@ -311,29 +311,18 @@ func runCompact(cmd *cobra.Command, opts compactOptions) error { ctx = context.Background() } - workDir, err := os.Getwd() + roots, err := resolveCurrentRepoRoots() if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - // Use worktree root for branch detection, main repo root for - // API queries (daemon stores jobs under the main repo path). - localRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - localRoot = root - } - repoRoot := workDir - if root, err := git.GetMainRepoRoot(workDir); err == nil { - repoRoot = root + return err } - branchFilter := opts.branch - if !opts.allBranches && branchFilter == "" { - branchFilter = git.GetCurrentBranch(localRoot) - } + branchFilter := resolveCurrentBranchFilter( + roots.worktreeRoot, opts.branch, opts.allBranches, + ) // Query and limit jobs, excluding non-review types (compact, task) // to prevent recursive self-compaction loops - allJobs, err := queryOpenJobs(ctx, repoRoot, branchFilter) + allJobs, err := queryOpenJobs(ctx, roots.mainRepoRoot, branchFilter) if err != nil { return err } @@ -392,7 +381,7 @@ func runCompact(cmd *cobra.Command, opts compactOptions) error { } // Enqueue consolidation job - consolidatedJobID, err := enqueueConsolidation(ctx, cmd, repoRoot, jobReviews, branchFilter, opts) + consolidatedJobID, err := enqueueConsolidation(ctx, cmd, roots.mainRepoRoot, jobReviews, branchFilter, opts) if err != nil { return err } diff --git a/cmd/roborev/fix.go b/cmd/roborev/fix.go index 18e46228..7104cd2d 100644 --- a/cmd/roborev/fix.go +++ b/cmd/roborev/fix.go @@ -10,7 +10,6 @@ import ( "net" "net/http" "net/url" - "os" "strings" "time" @@ -109,20 +108,13 @@ Examples: return fmt.Errorf("--list and --batch are mutually exclusive") } if list { - // When --all-branches, effectiveBranch stays "" so - // queryOpenJobs omits the branch filter. - effectiveBranch := branch - if !allBranches && effectiveBranch == "" { - workDir, err := os.Getwd() - if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - repoRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - repoRoot = root - } - effectiveBranch = git.GetCurrentBranch(repoRoot) + roots, err := resolveCurrentRepoRoots() + if err != nil { + return err } + effectiveBranch := resolveCurrentBranchFilter( + roots.worktreeRoot, branch, allBranches, + ) return runFixList(cmd, effectiveBranch, newestFirst) } opts := fixOptions{ @@ -147,18 +139,13 @@ Examples: } // If no args, discover unaddressed jobs if len(jobIDs) == 0 { - effectiveBranch := branch - if !allBranches && effectiveBranch == "" { - workDir, err := os.Getwd() - if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - repoRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - repoRoot = root - } - effectiveBranch = git.GetCurrentBranch(repoRoot) + roots, err := resolveCurrentRepoRoots() + if err != nil { + return err } + effectiveBranch := resolveCurrentBranchFilter( + roots.worktreeRoot, branch, allBranches, + ) return runFixBatch(cmd, nil, effectiveBranch, allBranches, branch != "", newestFirst, opts) } return runFixBatch(cmd, jobIDs, "", false, false, false, opts) @@ -169,18 +156,13 @@ Examples: // --branch X: use explicit branch // --all-branches: empty string (no filter) // default: current branch - effectiveBranch := branch - if !allBranches && effectiveBranch == "" { - workDir, err := os.Getwd() - if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - repoRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - repoRoot = root - } - effectiveBranch = git.GetCurrentBranch(repoRoot) + roots, err := resolveCurrentRepoRoots() + if err != nil { + return err } + effectiveBranch := resolveCurrentBranchFilter( + roots.worktreeRoot, branch, allBranches, + ) return runFixOpen(cmd, effectiveBranch, allBranches, branch != "", newestFirst, opts) } @@ -378,15 +360,9 @@ func runFixWithSeen(cmd *cobra.Command, jobIDs []int64, opts fixOptions, seen ma return err } - // Get working directory and repo root - workDir, err := os.Getwd() + roots, err := resolveCurrentRepoRoots() if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - - repoRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - repoRoot = root + return err } // Process each job @@ -395,7 +371,7 @@ func runFixWithSeen(cmd *cobra.Command, jobIDs []int64, opts fixOptions, seen ma cmd.Printf("\n=== Fixing job %d (%d/%d) ===\n", jobID, i+1, len(jobIDs)) } - err := fixSingleJob(cmd, repoRoot, jobID, opts) + err := fixSingleJob(cmd, roots.worktreeRoot, jobID, opts) if err != nil { if isConnectionError(err) { return fmt.Errorf("daemon connection lost: %w", err) @@ -442,24 +418,15 @@ func runFixOpen(cmd *cobra.Command, branch string, allBranches, explicitBranch, ctx = context.Background() } - workDir, err := os.Getwd() + roots, err := resolveCurrentRepoRoots() if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - - worktreeRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - worktreeRoot = root - } - apiRepoRoot := worktreeRoot - if root, err := git.GetMainRepoRoot(workDir); err == nil { - apiRepoRoot = root + return err } seen := make(map[int64]bool) for { - jobs, err := queryOpenJobs(ctx, apiRepoRoot, branch) + jobs, err := queryOpenJobs(ctx, roots.mainRepoRoot, branch) if err != nil { return err } @@ -473,7 +440,7 @@ func runFixOpen(cmd *cobra.Command, branch string, allBranches, explicitBranch, if explicitBranch { filterBranch = branch } - jobs = filterReachableJobs(worktreeRoot, filterBranch, jobs) + jobs = filterReachableJobs(roots.worktreeRoot, filterBranch, jobs) } // Filter out jobs we've already processed @@ -673,20 +640,12 @@ func runFixList(cmd *cobra.Command, branch string, newestFirst bool) error { ctx = context.Background() } - workDir, err := os.Getwd() + roots, err := resolveCurrentRepoRoots() if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - worktreeRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - worktreeRoot = root - } - apiRepoRoot := worktreeRoot - if root, err := git.GetMainRepoRoot(workDir); err == nil { - apiRepoRoot = root + return err } - jobs, err := queryOpenJobs(ctx, apiRepoRoot, branch) + jobs, err := queryOpenJobs(ctx, roots.mainRepoRoot, branch) if err != nil { return err } @@ -694,7 +653,7 @@ func runFixList(cmd *cobra.Command, branch string, newestFirst bool) error { // When listing all branches (branch==""), skip filtering — the // user explicitly asked for everything in this repo. if branch != "" { - jobs = filterReachableJobs(worktreeRoot, branch, jobs) + jobs = filterReachableJobs(roots.worktreeRoot, branch, jobs) } jobIDs := make([]int64, len(jobs)) @@ -966,23 +925,14 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, ctx = context.Background() } - workDir, err := os.Getwd() + roots, err := resolveCurrentRepoRoots() if err != nil { - return fmt.Errorf("get working directory: %w", err) - } - repoRoot := workDir - if root, err := git.GetRepoRoot(workDir); err == nil { - repoRoot = root - } - // Use main repo root for API queries (daemon stores jobs under main repo path) - apiRepoRoot := repoRoot - if root, err := git.GetMainRepoRoot(workDir); err == nil { - apiRepoRoot = root + return err } // Discover jobs if none provided if len(jobIDs) == 0 { - jobs, queryErr := queryOpenJobs(ctx, apiRepoRoot, branch) + jobs, queryErr := queryOpenJobs(ctx, roots.mainRepoRoot, branch) if queryErr != nil { return queryErr } @@ -991,7 +941,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, if explicitBranch { filterBranch = branch } - jobs = filterReachableJobs(repoRoot, filterBranch, jobs) + jobs = filterReachableJobs(roots.worktreeRoot, filterBranch, jobs) } jobIDs = make([]int64, len(jobs)) for i, j := range jobs { @@ -1055,7 +1005,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, } // Resolve agent once - fixAgent, err := resolveFixAgent(repoRoot, opts) + fixAgent, err := resolveFixAgent(roots.worktreeRoot, opts) if err != nil { return err } @@ -1064,7 +1014,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, // task job — task/analyze output has no severity labels, so the // instruction would confuse the agent for those entries. minSev, err := config.ResolveFixMinSeverity( - opts.minSeverity, repoRoot, + opts.minSeverity, roots.worktreeRoot, ) if err != nil { return fmt.Errorf("resolve min-severity: %w", err) @@ -1081,7 +1031,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, // Split into batches by prompt size (after severity resolution // so the severity instruction overhead is accounted for) cfg, _ := config.LoadGlobal() - maxSize := config.ResolveMaxPromptSize(repoRoot, cfg) + maxSize := config.ResolveMaxPromptSize(roots.worktreeRoot, cfg) batches := splitIntoBatches(entries, maxSize, minSev) for i, batch := range batches { @@ -1115,7 +1065,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, } result, err := fixJobDirect(ctx, fixJobParams{ - RepoRoot: repoRoot, + RepoRoot: roots.worktreeRoot, Agent: fixAgent, Output: out, }, prompt) @@ -1134,7 +1084,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, } else if result.NoChanges { cmd.Println("No changes were made by the fix agent.") } else { - if hasChanges, hcErr := git.HasUncommittedChanges(repoRoot); hcErr == nil && hasChanges { + if hasChanges, hcErr := git.HasUncommittedChanges(roots.worktreeRoot); hcErr == nil && hasChanges { cmd.Println("Warning: Changes were made but not committed. Please review and commit manually.") } } @@ -1142,7 +1092,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, // Enqueue review for fix commit if result.CommitCreated { - if enqErr := enqueueIfNeeded(ctx, batchAddr, repoRoot, result.NewCommitSHA); enqErr != nil && !opts.quiet { + if enqErr := enqueueIfNeeded(ctx, batchAddr, roots.worktreeRoot, result.NewCommitSHA); enqErr != nil && !opts.quiet { cmd.Printf("Warning: could not enqueue review for fix commit: %v\n", enqErr) } } diff --git a/cmd/roborev/fix_repo_roots.go b/cmd/roborev/fix_repo_roots.go new file mode 100644 index 00000000..f018f1f2 --- /dev/null +++ b/cmd/roborev/fix_repo_roots.go @@ -0,0 +1,49 @@ +package main + +import ( + "fmt" + "os" + + "github.com/roborev-dev/roborev/internal/git" +) + +// currentRepoRoots captures the worktree root used for local git/file +// operations and the main repo root used for daemon/API queries. +type currentRepoRoots struct { + worktreeRoot string + mainRepoRoot string +} + +// resolveCurrentRepoRoots resolves repo roots from the current working +// directory. Outside a git repo, both roots fall back to the working +// directory so existing fix/compact behavior stays unchanged. +func resolveCurrentRepoRoots() (currentRepoRoots, error) { + workDir, err := os.Getwd() + if err != nil { + return currentRepoRoots{}, fmt.Errorf("get working directory: %w", err) + } + + roots := currentRepoRoots{ + worktreeRoot: workDir, + mainRepoRoot: workDir, + } + if root, err := git.GetRepoRoot(workDir); err == nil { + roots.worktreeRoot = root + roots.mainRepoRoot = root + } + if root, err := git.GetMainRepoRoot(workDir); err == nil { + roots.mainRepoRoot = root + } + + return roots, nil +} + +func resolveCurrentBranchFilter(worktreeRoot, branch string, allBranches bool) string { + if allBranches { + return "" + } + if branch != "" { + return branch + } + return git.GetCurrentBranch(worktreeRoot) +} diff --git a/cmd/roborev/fix_test.go b/cmd/roborev/fix_test.go index ba1f0d5c..5f17f3fa 100644 --- a/cmd/roborev/fix_test.go +++ b/cmd/roborev/fix_test.go @@ -2025,6 +2025,38 @@ func TestFixWorktreeRepoResolution(t *testing.T) { }) } +func TestResolveCurrentRepoRoots(t *testing.T) { + repo, worktreeDir := setupWorktree(t) + chdir(t, worktreeDir) + + resolvedWorktreeDir, err := filepath.EvalSymlinks(worktreeDir) + require.NoError(t, err) + + roots, err := resolveCurrentRepoRoots() + require.NoError(t, err) + assert.Equal(t, resolvedWorktreeDir, roots.worktreeRoot) + assert.Equal(t, repo.Dir, roots.mainRepoRoot) +} + +func TestResolveCurrentBranchFilter(t *testing.T) { + t.Run("uses worktree branch when branch omitted", func(t *testing.T) { + _, worktreeDir := setupWorktree(t) + assert.Equal(t, "wt-branch", resolveCurrentBranchFilter(worktreeDir, "", false)) + }) + + t.Run("returns explicit branch", func(t *testing.T) { + repo := newTestGitRepo(t) + repo.CommitFile("a.txt", "a", "initial") + assert.Equal(t, "feature/custom", resolveCurrentBranchFilter(repo.Dir, "feature/custom", false)) + }) + + t.Run("returns empty string for all branches", func(t *testing.T) { + repo := newTestGitRepo(t) + repo.CommitFile("a.txt", "a", "initial") + assert.Empty(t, resolveCurrentBranchFilter(repo.Dir, "", true)) + }) +} + func TestJobVerdict(t *testing.T) { pass := "P" fail := "F"