Skip to content

Commit 3430760

Browse files
wesmclaude
andauthored
Fix --all-branches and --branch filtering from worktrees (#534)
## Summary - Fix `roborev fix --all-branches` and `--branch <name>` not discovering jobs from other branches when run from a git worktree — `filterReachableJobs` was called with an empty branch override, causing it to filter by the worktree's branch instead of the requested branch - Deprecate `--open` and `--unaddressed` flags — open job discovery is now the default behavior when no positional job IDs are provided; both flags are hidden and silently ignored for backwards compatibility - Thread `allBranches` and `explicitBranch` through `runFixOpen`/`runFixBatch` to distinguish three filtering modes: default (commit-graph reachability), `--branch X` (branch-field matching for cross-branch fixing), and `--all-branches` (skip filtering) - Add tests for all-branches discovery, explicit branch filtering, and worktree reachability --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e511664 commit 3430760

File tree

2 files changed

+241
-80
lines changed

2 files changed

+241
-80
lines changed

cmd/roborev/fix.go

Lines changed: 67 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525

2626
var (
2727
// Retry up to 3 times after the initial daemon request. Each retry waits
28-
// for the daemon to come back for up to a minute so `roborev fix --open`
28+
// for the daemon to come back for up to a minute so `roborev fix`
2929
// can survive daemon restarts without immediately aborting.
3030
fixDaemonMaxRetries = 3
3131
fixDaemonRecoveryWait = 1 * time.Minute
@@ -43,8 +43,8 @@ func fixCmd() *cobra.Command {
4343
reasoning string
4444
minSeverity string
4545
quiet bool
46-
open bool
47-
unaddressed bool // deprecated alias for open
46+
open bool // deprecated, silently ignored
47+
unaddressed bool // deprecated, silently ignored
4848
allBranches bool
4949
newestFirst bool
5050
branch string
@@ -65,20 +65,19 @@ The agent runs synchronously in your terminal, streaming output as it
6565
works. The review output is printed first so you can see what needs
6666
fixing. When complete, the job is closed.
6767
68-
Use --open to automatically discover and fix all open completed jobs
69-
for the current repo.
68+
With no arguments, discovers and fixes all open completed jobs on the
69+
current branch.
7070
7171
Examples:
72+
roborev fix # Fix all open jobs on current branch
7273
roborev fix 123 # Fix a single job
7374
roborev fix 123 124 125 # Fix multiple jobs sequentially
7475
roborev fix --agent claude-code 123 # Use a specific agent
75-
roborev fix --open # Fix all open jobs on current branch
76-
roborev fix --open --branch main
76+
roborev fix --branch main # Fix all open jobs on main
7777
roborev fix --all-branches # Fix all open jobs across all branches
7878
roborev fix --batch 123 124 125 # Batch multiple jobs into one prompt
7979
roborev fix --batch # Batch all open jobs on current branch
8080
roborev fix --list # List open jobs without fixing
81-
roborev fix --open --list # Same as above
8281
`,
8382
Args: cobra.ArbitraryArgs,
8483
RunE: func(cmd *cobra.Command, args []string) error {
@@ -91,27 +90,17 @@ Examples:
9190
_ = git.EnsureAbsoluteHooksPath(root)
9291
}
9392

94-
// Support deprecated --unaddressed as alias for --open
95-
if unaddressed {
96-
open = true
97-
}
98-
if allBranches && !open && !batch && !list {
99-
open = true
100-
}
101-
if branch != "" && !open && !batch && !list {
102-
return fmt.Errorf("--branch requires --open, --batch, or --list")
103-
}
10493
if allBranches && branch != "" {
10594
return fmt.Errorf("--all-branches and --branch are mutually exclusive")
10695
}
107-
if newestFirst && !open && !batch && !list {
108-
return fmt.Errorf("--newest-first requires --open, --batch, or --list")
96+
if allBranches && len(args) > 0 {
97+
return fmt.Errorf("--all-branches cannot be used with positional job IDs")
10998
}
110-
if open && len(args) > 0 {
111-
return fmt.Errorf("--open cannot be used with positional job IDs")
99+
if branch != "" && len(args) > 0 {
100+
return fmt.Errorf("--branch cannot be used with positional job IDs")
112101
}
113-
if batch && open {
114-
return fmt.Errorf("--batch and --open are mutually exclusive (--batch without args already discovers open jobs)")
102+
if newestFirst && len(args) > 0 {
103+
return fmt.Errorf("--newest-first cannot be used with positional job IDs")
115104
}
116105
if list && len(args) > 0 {
117106
return fmt.Errorf("--list cannot be used with positional job IDs")
@@ -170,13 +159,16 @@ Examples:
170159
}
171160
effectiveBranch = git.GetCurrentBranch(repoRoot)
172161
}
173-
return runFixBatch(cmd, nil, effectiveBranch, newestFirst, opts)
162+
return runFixBatch(cmd, nil, effectiveBranch, allBranches, branch != "", newestFirst, opts)
174163
}
175-
return runFixBatch(cmd, jobIDs, "", false, opts)
164+
return runFixBatch(cmd, jobIDs, "", false, false, false, opts)
176165
}
177166

178-
if open || len(args) == 0 {
179-
// Default to current branch unless --branch or --all-branches is set
167+
if len(args) == 0 {
168+
// Resolve branch for API query filtering.
169+
// --branch X: use explicit branch
170+
// --all-branches: empty string (no filter)
171+
// default: current branch
180172
effectiveBranch := branch
181173
if !allBranches && effectiveBranch == "" {
182174
workDir, err := os.Getwd()
@@ -189,7 +181,7 @@ Examples:
189181
}
190182
effectiveBranch = git.GetCurrentBranch(repoRoot)
191183
}
192-
return runFixOpen(cmd, effectiveBranch, newestFirst, opts)
184+
return runFixOpen(cmd, effectiveBranch, allBranches, branch != "", newestFirst, opts)
193185
}
194186

195187
// Parse job IDs
@@ -211,13 +203,14 @@ Examples:
211203
cmd.Flags().StringVar(&reasoning, "reasoning", "", "reasoning level: fast, standard, medium, thorough, or maximum")
212204
cmd.Flags().StringVar(&minSeverity, "min-severity", "", "minimum finding severity to address: critical, high, medium, or low")
213205
cmd.Flags().BoolVarP(&quiet, "quiet", "q", false, "suppress progress output")
214-
cmd.Flags().BoolVar(&open, "open", false, "fix all open completed jobs for the current repo")
215-
cmd.Flags().BoolVar(&unaddressed, "unaddressed", false, "deprecated: use --open")
216-
cmd.Flags().StringVar(&branch, "branch", "", "filter by branch (default: current branch; requires --open)")
217-
cmd.Flags().BoolVar(&allBranches, "all-branches", false, "include open jobs from all branches (implies --open)")
218-
cmd.Flags().BoolVar(&newestFirst, "newest-first", false, "process jobs newest first instead of oldest first (requires --open)")
206+
cmd.Flags().BoolVar(&open, "open", false, "deprecated: open is now the default behavior")
207+
cmd.Flags().BoolVar(&unaddressed, "unaddressed", false, "deprecated: open is now the default behavior")
208+
cmd.Flags().StringVar(&branch, "branch", "", "filter by branch (default: current branch)")
209+
cmd.Flags().BoolVar(&allBranches, "all-branches", false, "include open jobs from all branches")
210+
cmd.Flags().BoolVar(&newestFirst, "newest-first", false, "process jobs newest first instead of oldest first")
219211
cmd.Flags().BoolVar(&batch, "batch", false, "concatenate reviews into a single prompt for the agent")
220-
cmd.Flags().BoolVar(&list, "list", false, "list open jobs without fixing (implies --open)")
212+
cmd.Flags().BoolVar(&list, "list", false, "list open jobs without fixing")
213+
_ = cmd.Flags().MarkHidden("open")
221214
_ = cmd.Flags().MarkHidden("unaddressed")
222215
registerAgentCompletion(cmd)
223216
registerReasoningCompletion(cmd)
@@ -427,7 +420,19 @@ func runFixWithSeen(cmd *cobra.Command, jobIDs []int64, opts fixOptions, seen ma
427420
return nil
428421
}
429422

430-
func runFixOpen(cmd *cobra.Command, branch string, newestFirst bool, opts fixOptions) error {
423+
// runFixOpen discovers and fixes open jobs.
424+
//
425+
// branch is used for the API query filter (current branch, explicit
426+
// --branch, or "" for --all-branches). allBranches controls local
427+
// filtering: when true, filterReachableJobs is skipped entirely.
428+
// When false and the user passed --branch, the explicit branch is
429+
// forwarded as a branchOverride for branch-field matching. When false
430+
// and no --branch was passed, "" is used so filterReachableJobs falls
431+
// back to commit-graph reachability.
432+
//
433+
// explicitBranch should be true when the caller set --branch (as
434+
// opposed to auto-resolving the current branch).
435+
func runFixOpen(cmd *cobra.Command, branch string, allBranches, explicitBranch, newestFirst bool, opts fixOptions) error {
431436
// Ensure daemon is running
432437
if err := ensureDaemon(); err != nil {
433438
return err
@@ -458,7 +463,18 @@ func runFixOpen(cmd *cobra.Command, branch string, newestFirst bool, opts fixOpt
458463
if err != nil {
459464
return err
460465
}
461-
jobs = filterReachableJobs(worktreeRoot, "", jobs)
466+
// --all-branches: skip filtering, user wants everything.
467+
// --branch X: pass branch so filterReachableJobs uses
468+
// branch-field matching (cross-branch from worktree).
469+
// Default: pass "" so filterReachableJobs uses commit-graph
470+
// reachability for SHA/range refs.
471+
if !allBranches {
472+
filterBranch := ""
473+
if explicitBranch {
474+
filterBranch = branch
475+
}
476+
jobs = filterReachableJobs(worktreeRoot, filterBranch, jobs)
477+
}
462478

463479
// Filter out jobs we've already processed
464480
var newIDs []int64
@@ -501,10 +517,11 @@ func runFixOpen(cmd *cobra.Command, branch string, newestFirst bool, opts fixOpt
501517
// graph; non-SHA refs (dirty, empty, task labels) fall back to
502518
// branch matching. branchOverride is the explicit --branch value
503519
// for non-mutating flows (e.g. --list); when set, all job types
504-
// use branch matching, so cross-branch listing works for SHA/range
505-
// jobs too. Mutating flows (--open, --batch) must pass "" so that
506-
// fixes are never applied to the wrong checkout. On git errors the
507-
// job is kept (fail open) to avoid silently dropping work.
520+
// use branch matching so cross-branch listing works for SHA/range
521+
// jobs too. Mutating flows (fix, --batch) pass "" so that
522+
// commit-graph reachability is checked. Callers that want all
523+
// branches (--all-branches) skip this function entirely. On git
524+
// errors the job is kept (fail open) to avoid silently dropping work.
508525
func filterReachableJobs(
509526
worktreeRoot, branchOverride string,
510527
jobs []storage.ReviewJob,
@@ -737,7 +754,7 @@ func runFixList(cmd *cobra.Command, branch string, newestFirst bool) error {
737754
}
738755

739756
cmd.Printf("To apply a fix: roborev fix <job_id>\n")
740-
cmd.Printf("To apply all: roborev fix --open\n")
757+
cmd.Printf("To apply all: roborev fix\n")
741758

742759
return nil
743760
}
@@ -934,7 +951,7 @@ type batchEntry struct {
934951

935952
// runFixBatch discovers jobs (or uses provided IDs), splits them into batches
936953
// respecting max prompt size, and runs each batch as a single agent invocation.
937-
func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, newestFirst bool, opts fixOptions) error {
954+
func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, allBranches, explicitBranch, newestFirst bool, opts fixOptions) error {
938955
if err := ensureDaemon(); err != nil {
939956
return err
940957
}
@@ -963,7 +980,13 @@ func runFixBatch(cmd *cobra.Command, jobIDs []int64, branch string, newestFirst
963980
if queryErr != nil {
964981
return queryErr
965982
}
966-
jobs = filterReachableJobs(repoRoot, "", jobs)
983+
if !allBranches {
984+
filterBranch := ""
985+
if explicitBranch {
986+
filterBranch = branch
987+
}
988+
jobs = filterReachableJobs(repoRoot, filterBranch, jobs)
989+
}
967990
jobIDs = make([]int64, len(jobs))
968991
for i, j := range jobs {
969992
jobIDs[i] = j.ID

0 commit comments

Comments
 (0)