Skip to content

feat: support multiple job IDs in roborev wait#377

Merged
wesm merged 4 commits intoroborev-dev:mainfrom
mariusvniekerk:wait-flags
Feb 26, 2026
Merged

feat: support multiple job IDs in roborev wait#377
wesm merged 4 commits intoroborev-dev:mainfrom
mariusvniekerk:wait-flags

Conversation

@mariusvniekerk
Copy link
Collaborator

Summary

  • Extend roborev wait to accept multiple job IDs/refs, waiting for all concurrently
  • roborev wait --job 251 252 253 or roborev wait 251 252 253 now works
  • Exit code 1 if any job fails; 0 only if all pass
  • Goroutines poll in quiet mode to avoid interleaved output

Test plan

  • TestWaitMultipleJobIDs — all jobs pass, expect exit 0
  • TestWaitMultipleJobIDsOneFails — one fails, expect exit 1
  • TestWaitMultipleJobIDsValidation--sha incompatible with multiple args
  • All 52 existing wait tests still pass
  • Full suite (3503 tests) passes

🤖 Generated with Claude Code

@roborev-ci
Copy link

roborev-ci bot commented Feb 26, 2026

roborev: Combined Review (997c637)

Verdict: The PR introduces multi-job
wait functionality with a few bugs regarding error handling and missing output, but no security vulnerabilities were found.

🔴 High

  • Silent failure path / Missing review output in multi-job mode
    • File: cmd/roborev/wait.go (lines 151-1
      63, 220, 244)
    • Description: The implementation calls waitForJob with quiet=true, which suppresses review output. While the code checks for errors, it never displays the actual review text or actionable output for job failures (such as *exitError). This
      silently drops results and can return exit code 1 in non-quiet mode without providing the user with any context.
    • Suggested Fix: After wg.Wait(), iterate over the completed jobs to fetch and print the actual review results serially, or capture the per-job outcome/output from
      waitForJob to display a summary later.

🟡 Medium

  • First-error selection can mask a more important later error
    • File: cmd/roborev/wait.go (lines 235, 249)
    • Description:
      Error handling retains only the first error by argument order. If the first job returns ErrJobNotFound but a subsequent job experiences a real operational failure (e.g., transport/server error), the function may return exitError{1} and hide the actual root cause.
    • Suggested Fix: Prior
      itize returning non-*exitError/non-ErrJobNotFound errors first, or optionally aggregate errors and return the most actionable one.

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

mariusvniekerk and others added 4 commits February 26, 2026 13:10
Allow `roborev wait 251 252 253` to wait for multiple jobs concurrently.
All jobs are polled in parallel and exit code is 1 if any job fails.
Works with both `--job` flag and auto-detection of numeric job IDs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 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>
waitMultiple had three bugs:

1. Only ErrJobNotFound was reported to the user; FAIL verdicts,
   failed/canceled jobs, and fetch errors were silently swallowed
   (exit 1 with no message). Now all failures print a per-job line.

2. git.ResolveSHA was called twice for the same arg — once to check
   validity, then again to get the value. Collapsed to a single call.

3. ensureDaemon() was called before local validation (parsing job IDs,
   resolving git refs), unlike the single-arg path. Restructured into
   three phases: local validation, ensureDaemon, daemon-backed lookups.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add table-driven tests for the default branch in waitMultiple's
per-job error reporting: failed jobs, canceled jobs, and review
fetch errors (done status with missing review). These paths were
untested despite being central to the fix in the previous commit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 26, 2026

roborev: Combined Review (e313f91)

Verdict: All agents agree the code is clean; no medium, high, or critical issues were found.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

@wesm wesm merged commit 44cf6ad into roborev-dev:main Feb 26, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants