Skip to content

Commit 91e938a

Browse files
wesmclaude
andauthored
fix: make --all-branches imply --unaddressed (#345)
## Summary - `roborev fix --all-branches` now implies `--unaddressed` instead of erroring, removing an unnecessary sharp edge - Validation order fixed so `--all-branches --branch main` correctly returns the mutual-exclusion error - Added positive test for `--all-branches` alone and updated existing flag-validation tests Closes #344 ## Test plan - [x] `TestFixCmdFlagValidation` passes (all error cases) - [x] `TestFixAllBranchesImpliesUnaddressed` passes (positive case with mock daemon) - [x] Full `TestFix*` suite passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent b469126 commit 91e938a

File tree

2 files changed

+31
-10
lines changed

2 files changed

+31
-10
lines changed

cmd/roborev/fix.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,20 @@ Examples:
5555
roborev fix --agent claude-code 123 # Use a specific agent
5656
roborev fix --unaddressed # Fix all unaddressed on current branch
5757
roborev fix --unaddressed --branch main
58-
roborev fix --unaddressed --all-branches
58+
roborev fix --all-branches # Fix all unaddressed across all branches
5959
roborev fix --batch 123 124 125 # Batch multiple jobs into one prompt
6060
roborev fix --batch # Batch all unaddressed on current branch
6161
roborev fix --list # List unaddressed jobs without fixing
6262
roborev fix --unaddressed --list # Same as above
6363
`,
6464
Args: cobra.ArbitraryArgs,
6565
RunE: func(cmd *cobra.Command, args []string) error {
66+
if allBranches && !unaddressed && !batch && !list {
67+
unaddressed = true
68+
}
6669
if branch != "" && !unaddressed && !batch && !list {
6770
return fmt.Errorf("--branch requires --unaddressed, --batch, or --list")
6871
}
69-
if allBranches && !unaddressed && !batch && !list {
70-
return fmt.Errorf("--all-branches requires --unaddressed, --batch, or --list")
71-
}
7272
if allBranches && branch != "" {
7373
return fmt.Errorf("--all-branches and --branch are mutually exclusive")
7474
}
@@ -179,7 +179,7 @@ Examples:
179179
cmd.Flags().BoolVarP(&quiet, "quiet", "q", false, "suppress progress output")
180180
cmd.Flags().BoolVar(&unaddressed, "unaddressed", false, "fix all unaddressed completed jobs for the current repo")
181181
cmd.Flags().StringVar(&branch, "branch", "", "filter by branch (default: current branch; requires --unaddressed)")
182-
cmd.Flags().BoolVar(&allBranches, "all-branches", false, "include unaddressed jobs from all branches (requires --unaddressed)")
182+
cmd.Flags().BoolVar(&allBranches, "all-branches", false, "include unaddressed jobs from all branches (implies --unaddressed)")
183183
cmd.Flags().BoolVar(&newestFirst, "newest-first", false, "process jobs newest first instead of oldest first (requires --unaddressed)")
184184
cmd.Flags().BoolVar(&batch, "batch", false, "concatenate reviews into a single prompt for the agent")
185185
cmd.Flags().BoolVar(&list, "list", false, "list unaddressed jobs without fixing (implies --unaddressed)")

cmd/roborev/fix_test.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,9 @@ func TestFixCmdFlagValidation(t *testing.T) {
308308
wantErr: "--branch requires --unaddressed",
309309
},
310310
{
311-
name: "--all-branches without --unaddressed",
312-
args: []string{"--all-branches"},
313-
wantErr: "--all-branches requires --unaddressed",
311+
name: "--all-branches with positional args",
312+
args: []string{"--all-branches", "123"},
313+
wantErr: "--unaddressed cannot be used with positional job IDs",
314314
},
315315
{
316316
name: "--unaddressed with positional args",
@@ -323,8 +323,8 @@ func TestFixCmdFlagValidation(t *testing.T) {
323323
wantErr: "--newest-first requires --unaddressed",
324324
},
325325
{
326-
name: "--all-branches with --branch",
327-
args: []string{"--unaddressed", "--all-branches", "--branch", "main"},
326+
name: "--all-branches with --branch (no explicit --unaddressed)",
327+
args: []string{"--all-branches", "--branch", "main"},
328328
wantErr: "--all-branches and --branch are mutually exclusive",
329329
},
330330
}
@@ -374,6 +374,27 @@ func TestFixNoArgsDefaultsToUnaddressed(t *testing.T) {
374374
}
375375
}
376376

377+
func TestFixAllBranchesImpliesUnaddressed(t *testing.T) {
378+
// --all-branches alone should imply --unaddressed and pass
379+
// validation, routing through unaddressed discovery.
380+
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
381+
json.NewEncoder(w).Encode(map[string]any{
382+
"jobs": []any{},
383+
"has_more": false,
384+
})
385+
}))
386+
defer cleanup()
387+
388+
cmd := fixCmd()
389+
cmd.SilenceUsage = true
390+
cmd.SilenceErrors = true
391+
cmd.SetArgs([]string{"--all-branches"})
392+
err := cmd.Execute()
393+
if err != nil {
394+
t.Errorf("--all-branches should not fail validation: %v", err)
395+
}
396+
}
397+
377398
func TestRunFixUnaddressed(t *testing.T) {
378399
repo := createTestRepo(t, map[string]string{"f.txt": "x"})
379400

0 commit comments

Comments
 (0)