Skip to content

Commit ed76ce9

Browse files
refactor(fix): share repo root resolution helpers (#575)
## Summary - extract shared helpers for worktree root, main repo root, and branch fallback resolution - reuse the helpers across fix list/open/batch/single-job flows and the matching compact path - add direct helper coverage for worktree and branch-filter behavior ## Verification - go fmt ./... - go vet ./... - go test ./... - roborev fix --list --all-branches ## Tracking - closes bd-pui.7 Co-authored-by: OpenAI Codex <noreply@openai.com>
1 parent 5bd7a8e commit ed76ce9

4 files changed

Lines changed: 127 additions & 107 deletions

File tree

cmd/roborev/compact.go

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -311,29 +311,18 @@ func runCompact(cmd *cobra.Command, opts compactOptions) error {
311311
ctx = context.Background()
312312
}
313313

314-
workDir, err := os.Getwd()
314+
roots, err := resolveCurrentRepoRoots()
315315
if err != nil {
316-
return fmt.Errorf("get working directory: %w", err)
317-
}
318-
// Use worktree root for branch detection, main repo root for
319-
// API queries (daemon stores jobs under the main repo path).
320-
localRoot := workDir
321-
if root, err := git.GetRepoRoot(workDir); err == nil {
322-
localRoot = root
323-
}
324-
repoRoot := workDir
325-
if root, err := git.GetMainRepoRoot(workDir); err == nil {
326-
repoRoot = root
316+
return err
327317
}
328318

329-
branchFilter := opts.branch
330-
if !opts.allBranches && branchFilter == "" {
331-
branchFilter = git.GetCurrentBranch(localRoot)
332-
}
319+
branchFilter := resolveCurrentBranchFilter(
320+
roots.worktreeRoot, opts.branch, opts.allBranches,
321+
)
333322

334323
// Query and limit jobs, excluding non-review types (compact, task)
335324
// to prevent recursive self-compaction loops
336-
allJobs, err := queryOpenJobs(ctx, repoRoot, branchFilter)
325+
allJobs, err := queryOpenJobs(ctx, roots.mainRepoRoot, branchFilter)
337326
if err != nil {
338327
return err
339328
}
@@ -392,7 +381,7 @@ func runCompact(cmd *cobra.Command, opts compactOptions) error {
392381
}
393382

394383
// Enqueue consolidation job
395-
consolidatedJobID, err := enqueueConsolidation(ctx, cmd, repoRoot, jobReviews, branchFilter, opts)
384+
consolidatedJobID, err := enqueueConsolidation(ctx, cmd, roots.mainRepoRoot, jobReviews, branchFilter, opts)
396385
if err != nil {
397386
return err
398387
}

cmd/roborev/fix.go

Lines changed: 39 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"net"
1111
"net/http"
1212
"net/url"
13-
"os"
1413
"strings"
1514
"time"
1615

@@ -109,20 +108,13 @@ Examples:
109108
return fmt.Errorf("--list and --batch are mutually exclusive")
110109
}
111110
if list {
112-
// When --all-branches, effectiveBranch stays "" so
113-
// queryOpenJobs omits the branch filter.
114-
effectiveBranch := branch
115-
if !allBranches && effectiveBranch == "" {
116-
workDir, err := os.Getwd()
117-
if err != nil {
118-
return fmt.Errorf("get working directory: %w", err)
119-
}
120-
repoRoot := workDir
121-
if root, err := git.GetRepoRoot(workDir); err == nil {
122-
repoRoot = root
123-
}
124-
effectiveBranch = git.GetCurrentBranch(repoRoot)
111+
roots, err := resolveCurrentRepoRoots()
112+
if err != nil {
113+
return err
125114
}
115+
effectiveBranch := resolveCurrentBranchFilter(
116+
roots.worktreeRoot, branch, allBranches,
117+
)
126118
return runFixList(cmd, effectiveBranch, newestFirst)
127119
}
128120
opts := fixOptions{
@@ -147,18 +139,13 @@ Examples:
147139
}
148140
// If no args, discover unaddressed jobs
149141
if len(jobIDs) == 0 {
150-
effectiveBranch := branch
151-
if !allBranches && effectiveBranch == "" {
152-
workDir, err := os.Getwd()
153-
if err != nil {
154-
return fmt.Errorf("get working directory: %w", err)
155-
}
156-
repoRoot := workDir
157-
if root, err := git.GetRepoRoot(workDir); err == nil {
158-
repoRoot = root
159-
}
160-
effectiveBranch = git.GetCurrentBranch(repoRoot)
142+
roots, err := resolveCurrentRepoRoots()
143+
if err != nil {
144+
return err
161145
}
146+
effectiveBranch := resolveCurrentBranchFilter(
147+
roots.worktreeRoot, branch, allBranches,
148+
)
162149
return runFixBatch(cmd, nil, effectiveBranch, allBranches, branch != "", newestFirst, opts)
163150
}
164151
return runFixBatch(cmd, jobIDs, "", false, false, false, opts)
@@ -169,18 +156,13 @@ Examples:
169156
// --branch X: use explicit branch
170157
// --all-branches: empty string (no filter)
171158
// default: current branch
172-
effectiveBranch := branch
173-
if !allBranches && effectiveBranch == "" {
174-
workDir, err := os.Getwd()
175-
if err != nil {
176-
return fmt.Errorf("get working directory: %w", err)
177-
}
178-
repoRoot := workDir
179-
if root, err := git.GetRepoRoot(workDir); err == nil {
180-
repoRoot = root
181-
}
182-
effectiveBranch = git.GetCurrentBranch(repoRoot)
159+
roots, err := resolveCurrentRepoRoots()
160+
if err != nil {
161+
return err
183162
}
163+
effectiveBranch := resolveCurrentBranchFilter(
164+
roots.worktreeRoot, branch, allBranches,
165+
)
184166
return runFixOpen(cmd, effectiveBranch, allBranches, branch != "", newestFirst, opts)
185167
}
186168

@@ -378,15 +360,9 @@ func runFixWithSeen(cmd *cobra.Command, jobIDs []int64, opts fixOptions, seen ma
378360
return err
379361
}
380362

381-
// Get working directory and repo root
382-
workDir, err := os.Getwd()
363+
roots, err := resolveCurrentRepoRoots()
383364
if err != nil {
384-
return fmt.Errorf("get working directory: %w", err)
385-
}
386-
387-
repoRoot := workDir
388-
if root, err := git.GetRepoRoot(workDir); err == nil {
389-
repoRoot = root
365+
return err
390366
}
391367

392368
// Process each job
@@ -395,7 +371,7 @@ func runFixWithSeen(cmd *cobra.Command, jobIDs []int64, opts fixOptions, seen ma
395371
cmd.Printf("\n=== Fixing job %d (%d/%d) ===\n", jobID, i+1, len(jobIDs))
396372
}
397373

398-
err := fixSingleJob(cmd, repoRoot, jobID, opts)
374+
err := fixSingleJob(cmd, roots.worktreeRoot, jobID, opts)
399375
if err != nil {
400376
if isConnectionError(err) {
401377
return fmt.Errorf("daemon connection lost: %w", err)
@@ -442,24 +418,15 @@ func runFixOpen(cmd *cobra.Command, branch string, allBranches, explicitBranch,
442418
ctx = context.Background()
443419
}
444420

445-
workDir, err := os.Getwd()
421+
roots, err := resolveCurrentRepoRoots()
446422
if err != nil {
447-
return fmt.Errorf("get working directory: %w", err)
448-
}
449-
450-
worktreeRoot := workDir
451-
if root, err := git.GetRepoRoot(workDir); err == nil {
452-
worktreeRoot = root
453-
}
454-
apiRepoRoot := worktreeRoot
455-
if root, err := git.GetMainRepoRoot(workDir); err == nil {
456-
apiRepoRoot = root
423+
return err
457424
}
458425

459426
seen := make(map[int64]bool)
460427

461428
for {
462-
jobs, err := queryOpenJobs(ctx, apiRepoRoot, branch)
429+
jobs, err := queryOpenJobs(ctx, roots.mainRepoRoot, branch)
463430
if err != nil {
464431
return err
465432
}
@@ -473,7 +440,7 @@ func runFixOpen(cmd *cobra.Command, branch string, allBranches, explicitBranch,
473440
if explicitBranch {
474441
filterBranch = branch
475442
}
476-
jobs = filterReachableJobs(worktreeRoot, filterBranch, jobs)
443+
jobs = filterReachableJobs(roots.worktreeRoot, filterBranch, jobs)
477444
}
478445

479446
// Filter out jobs we've already processed
@@ -673,28 +640,20 @@ func runFixList(cmd *cobra.Command, branch string, newestFirst bool) error {
673640
ctx = context.Background()
674641
}
675642

676-
workDir, err := os.Getwd()
643+
roots, err := resolveCurrentRepoRoots()
677644
if err != nil {
678-
return fmt.Errorf("get working directory: %w", err)
679-
}
680-
worktreeRoot := workDir
681-
if root, err := git.GetRepoRoot(workDir); err == nil {
682-
worktreeRoot = root
683-
}
684-
apiRepoRoot := worktreeRoot
685-
if root, err := git.GetMainRepoRoot(workDir); err == nil {
686-
apiRepoRoot = root
645+
return err
687646
}
688647

689-
jobs, err := queryOpenJobs(ctx, apiRepoRoot, branch)
648+
jobs, err := queryOpenJobs(ctx, roots.mainRepoRoot, branch)
690649
if err != nil {
691650
return err
692651
}
693652
// When listing a specific branch, filter by reachability/branch.
694653
// When listing all branches (branch==""), skip filtering — the
695654
// user explicitly asked for everything in this repo.
696655
if branch != "" {
697-
jobs = filterReachableJobs(worktreeRoot, branch, jobs)
656+
jobs = filterReachableJobs(roots.worktreeRoot, branch, jobs)
698657
}
699658

700659
jobIDs := make([]int64, len(jobs))
@@ -966,23 +925,14 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
966925
ctx = context.Background()
967926
}
968927

969-
workDir, err := os.Getwd()
928+
roots, err := resolveCurrentRepoRoots()
970929
if err != nil {
971-
return fmt.Errorf("get working directory: %w", err)
972-
}
973-
repoRoot := workDir
974-
if root, err := git.GetRepoRoot(workDir); err == nil {
975-
repoRoot = root
976-
}
977-
// Use main repo root for API queries (daemon stores jobs under main repo path)
978-
apiRepoRoot := repoRoot
979-
if root, err := git.GetMainRepoRoot(workDir); err == nil {
980-
apiRepoRoot = root
930+
return err
981931
}
982932

983933
// Discover jobs if none provided
984934
if len(jobIDs) == 0 {
985-
jobs, queryErr := queryOpenJobs(ctx, apiRepoRoot, branch)
935+
jobs, queryErr := queryOpenJobs(ctx, roots.mainRepoRoot, branch)
986936
if queryErr != nil {
987937
return queryErr
988938
}
@@ -991,7 +941,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
991941
if explicitBranch {
992942
filterBranch = branch
993943
}
994-
jobs = filterReachableJobs(repoRoot, filterBranch, jobs)
944+
jobs = filterReachableJobs(roots.worktreeRoot, filterBranch, jobs)
995945
}
996946
jobIDs = make([]int64, len(jobs))
997947
for i, j := range jobs {
@@ -1055,7 +1005,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
10551005
}
10561006

10571007
// Resolve agent once
1058-
fixAgent, err := resolveFixAgent(repoRoot, opts)
1008+
fixAgent, err := resolveFixAgent(roots.worktreeRoot, opts)
10591009
if err != nil {
10601010
return err
10611011
}
@@ -1064,7 +1014,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
10641014
// task job — task/analyze output has no severity labels, so the
10651015
// instruction would confuse the agent for those entries.
10661016
minSev, err := config.ResolveFixMinSeverity(
1067-
opts.minSeverity, repoRoot,
1017+
opts.minSeverity, roots.worktreeRoot,
10681018
)
10691019
if err != nil {
10701020
return fmt.Errorf("resolve min-severity: %w", err)
@@ -1081,7 +1031,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
10811031
// Split into batches by prompt size (after severity resolution
10821032
// so the severity instruction overhead is accounted for)
10831033
cfg, _ := config.LoadGlobal()
1084-
maxSize := config.ResolveMaxPromptSize(repoRoot, cfg)
1034+
maxSize := config.ResolveMaxPromptSize(roots.worktreeRoot, cfg)
10851035
batches := splitIntoBatches(entries, maxSize, minSev)
10861036

10871037
for i, batch := range batches {
@@ -1115,7 +1065,7 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
11151065
}
11161066

11171067
result, err := fixJobDirect(ctx, fixJobParams{
1118-
RepoRoot: repoRoot,
1068+
RepoRoot: roots.worktreeRoot,
11191069
Agent: fixAgent,
11201070
Output: out,
11211071
}, prompt)
@@ -1134,15 +1084,15 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches,
11341084
} else if result.NoChanges {
11351085
cmd.Println("No changes were made by the fix agent.")
11361086
} else {
1137-
if hasChanges, hcErr := git.HasUncommittedChanges(repoRoot); hcErr == nil && hasChanges {
1087+
if hasChanges, hcErr := git.HasUncommittedChanges(roots.worktreeRoot); hcErr == nil && hasChanges {
11381088
cmd.Println("Warning: Changes were made but not committed. Please review and commit manually.")
11391089
}
11401090
}
11411091
}
11421092

11431093
// Enqueue review for fix commit
11441094
if result.CommitCreated {
1145-
if enqErr := enqueueIfNeeded(ctx, batchAddr, repoRoot, result.NewCommitSHA); enqErr != nil && !opts.quiet {
1095+
if enqErr := enqueueIfNeeded(ctx, batchAddr, roots.worktreeRoot, result.NewCommitSHA); enqErr != nil && !opts.quiet {
11461096
cmd.Printf("Warning: could not enqueue review for fix commit: %v\n", enqErr)
11471097
}
11481098
}

cmd/roborev/fix_repo_roots.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package main
2+
3+
import (
4+
"fmt"
5+
"os"
6+
7+
"github.com/roborev-dev/roborev/internal/git"
8+
)
9+
10+
// currentRepoRoots captures the worktree root used for local git/file
11+
// operations and the main repo root used for daemon/API queries.
12+
type currentRepoRoots struct {
13+
worktreeRoot string
14+
mainRepoRoot string
15+
}
16+
17+
// resolveCurrentRepoRoots resolves repo roots from the current working
18+
// directory. Outside a git repo, both roots fall back to the working
19+
// directory so existing fix/compact behavior stays unchanged.
20+
func resolveCurrentRepoRoots() (currentRepoRoots, error) {
21+
workDir, err := os.Getwd()
22+
if err != nil {
23+
return currentRepoRoots{}, fmt.Errorf("get working directory: %w", err)
24+
}
25+
26+
roots := currentRepoRoots{
27+
worktreeRoot: workDir,
28+
mainRepoRoot: workDir,
29+
}
30+
if root, err := git.GetRepoRoot(workDir); err == nil {
31+
roots.worktreeRoot = root
32+
roots.mainRepoRoot = root
33+
}
34+
if root, err := git.GetMainRepoRoot(workDir); err == nil {
35+
roots.mainRepoRoot = root
36+
}
37+
38+
return roots, nil
39+
}
40+
41+
func resolveCurrentBranchFilter(worktreeRoot, branch string, allBranches bool) string {
42+
if allBranches {
43+
return ""
44+
}
45+
if branch != "" {
46+
return branch
47+
}
48+
return git.GetCurrentBranch(worktreeRoot)
49+
}

0 commit comments

Comments
 (0)