Skip to content

Commit 997c637

Browse files
fix: address review findings for multi-job wait
- Add explicit cobra.ArbitraryArgs to document intent - Hoist repoRoot and ensureDaemon() before resolution loop - Poll goroutines in quiet mode to avoid interleaved output Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ea283cf commit 997c637

File tree

1 file changed

+15
-16
lines changed

1 file changed

+15
-16
lines changed

cmd/roborev/wait.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ Examples:
4444
roborev wait --job 42 # Force as job ID
4545
roborev wait --job 10 20 30 # Wait for multiple job IDs
4646
roborev wait --sha HEAD~1 # Wait for job matching HEAD~1`,
47+
Args: cobra.ArbitraryArgs,
4748
RunE: func(cmd *cobra.Command, args []string) error {
4849
// In quiet mode, suppress cobra's error output
4950
if quiet {
@@ -169,7 +170,13 @@ Examples:
169170
// waitMultiple resolves multiple args to job IDs and waits for all concurrently.
170171
// Returns exit code 1 if any job fails or has a FAIL verdict.
171172
func waitMultiple(cmd *cobra.Command, args []string, forceJobID, quiet bool) error {
172-
// Resolve all args to job IDs first (before contacting daemon)
173+
// Ensure daemon is running before any resolution
174+
if err := ensureDaemon(); err != nil {
175+
return fmt.Errorf("daemon not running: %w", err)
176+
}
177+
178+
// Resolve all args to job IDs
179+
repoRoot, _ := git.GetRepoRoot(".")
173180
jobIDs := make([]int64, 0, len(args))
174181
for _, arg := range args {
175182
if forceJobID {
@@ -181,24 +188,19 @@ func waitMultiple(cmd *cobra.Command, args []string, forceJobID, quiet bool) err
181188
} else {
182189
// Try git ref first
183190
var ref string
184-
if repoRoot, err := git.GetRepoRoot("."); err == nil {
191+
if repoRoot != "" {
185192
if _, err := git.ResolveSHA(repoRoot, arg); err == nil {
186193
ref = arg
187194
}
188195
}
189196
if ref != "" {
190-
// Resolve to SHA, then find job
191-
repoRoot, _ := git.GetRepoRoot(".")
192197
sha, err := git.ResolveSHA(repoRoot, ref)
193198
if err != nil {
194199
return fmt.Errorf("invalid git ref: %s", ref)
195200
}
196-
if err := ensureDaemon(); err != nil {
197-
return fmt.Errorf("daemon not running: %w", err)
198-
}
199201
mainRoot, _ := git.GetMainRepoRoot(".")
200202
if mainRoot == "" {
201-
mainRoot, _ = git.GetRepoRoot(".")
203+
mainRoot = repoRoot
202204
}
203205
job, err := findJobForCommit(mainRoot, sha)
204206
if err != nil {
@@ -224,14 +226,11 @@ func waitMultiple(cmd *cobra.Command, args []string, forceJobID, quiet bool) err
224226
}
225227
}
226228

227-
// Ensure daemon is running
228-
if err := ensureDaemon(); err != nil {
229-
return fmt.Errorf("daemon not running: %w", err)
230-
}
231-
232229
addr := getDaemonAddr()
233230

234-
// Wait for all jobs concurrently
231+
// Wait for all jobs concurrently.
232+
// Always poll in quiet mode to avoid interleaved output from goroutines;
233+
// we display results serially after all complete.
235234
type result struct {
236235
jobID int64
237236
err error
@@ -242,13 +241,13 @@ func waitMultiple(cmd *cobra.Command, args []string, forceJobID, quiet bool) err
242241
wg.Add(1)
243242
go func(idx int, jobID int64) {
244243
defer wg.Done()
245-
err := waitForJob(cmd, addr, jobID, quiet)
244+
err := waitForJob(cmd, addr, jobID, true)
246245
results[idx] = result{jobID: jobID, err: err}
247246
}(i, id)
248247
}
249248
wg.Wait()
250249

251-
// Collect errors: any failure means exit 1
250+
// Report results serially
252251
var firstErr error
253252
for _, r := range results {
254253
if r.err != nil {

0 commit comments

Comments
 (0)