Skip to content

Commit c76ce4a

Browse files
wesmclaude
andcommitted
Pre-release fixes: agent fallback, prompts with diffs, race conditions
- Add actual diffs to review prompts (agents can now see code changes) - Add 250KB size limit for prompts, falls back to git hashes for large diffs - Fix job claiming race condition with atomic UPDATE+subquery - Fix codex temp file collision using os.CreateTemp - Fix roborevd -config flag being ignored - Fix CLI commands to use runtime file for daemon port detection - Add automatic agent fallback (codex <-> claude-code) - Display short refs (7 chars) in CLI and TUI - Simplify database schema (remove migration machinery) - Update README with agent fallback documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 121b5b1 commit c76ce4a

File tree

13 files changed

+228
-362
lines changed

13 files changed

+228
-362
lines changed

README.md

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,39 @@ The daemon starts automatically when needed and handles port conflicts by findin
8686

8787
## Agents
8888

89+
roborev supports multiple AI review agents:
90+
8991
- `codex` - OpenAI Codex CLI
9092
- `claude-code` - Anthropic Claude Code CLI
9193

92-
Agent selection priority:
93-
1. `--agent` flag on enqueue
94+
### Automatic Fallback
95+
96+
roborev automatically detects which agents are installed and falls back gracefully:
97+
98+
- If `codex` is requested but not installed, roborev uses `claude` instead
99+
- If `claude-code` is requested but not installed, roborev uses `codex` instead
100+
- If neither is installed, the job fails with a helpful error message
101+
102+
### Explicit Agent Selection
103+
104+
To use a specific agent for a repository, create `.roborev.toml` in the repo root:
105+
106+
```toml
107+
agent = "claude-code"
108+
```
109+
110+
Or set a global default in `~/.roborev/config.toml`:
111+
112+
```toml
113+
default_agent = "claude-code"
114+
```
115+
116+
### Selection Priority
117+
118+
1. `--agent` flag on enqueue command
94119
2. Per-repo `.roborev.toml`
95-
3. Global config
96-
4. Default: `codex`
120+
3. Global `~/.roborev/config.toml`
121+
4. Automatic detection (uses first available: codex, claude-code)
97122

98123
## Commands
99124

cmd/roborev/main.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,10 @@ func statusCmd() *cobra.Command {
355355
Use: "status",
356356
Short: "Show daemon and queue status",
357357
RunE: func(cmd *cobra.Command, args []string) error {
358-
// Check if daemon is running
358+
// Check if daemon is running (use runtime file to find actual port)
359+
addr := getDaemonAddr()
359360
client := &http.Client{Timeout: 2 * time.Second}
360-
resp, err := client.Get(serverAddr + "/api/status")
361+
resp, err := client.Get(addr + "/api/status")
361362
if err != nil {
362363
fmt.Println("Daemon: not running")
363364
fmt.Println()
@@ -378,7 +379,7 @@ func statusCmd() *cobra.Command {
378379
fmt.Println()
379380

380381
// Get recent jobs
381-
resp, err = client.Get(serverAddr + "/api/jobs?limit=10")
382+
resp, err = client.Get(addr + "/api/jobs?limit=10")
382383
if err != nil {
383384
return nil
384385
}
@@ -433,8 +434,9 @@ func showCmd() *cobra.Command {
433434
}
434435
}
435436

437+
addr := getDaemonAddr()
436438
client := &http.Client{Timeout: 5 * time.Second}
437-
resp, err := client.Get(serverAddr + "/api/review?sha=" + sha)
439+
resp, err := client.Get(addr + "/api/review?sha=" + sha)
438440
if err != nil {
439441
return fmt.Errorf("failed to connect to daemon (is it running?)")
440442
}
@@ -524,7 +526,8 @@ func respondCmd() *cobra.Command {
524526
"response": message,
525527
})
526528

527-
resp, err := http.Post(serverAddr+"/api/respond", "application/json", bytes.NewReader(reqBody))
529+
addr := getDaemonAddr()
530+
resp, err := http.Post(addr+"/api/respond", "application/json", bytes.NewReader(reqBody))
528531
if err != nil {
529532
return fmt.Errorf("failed to connect to daemon: %w", err)
530533
}

cmd/roborev/tui.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,7 @@ func (m tuiModel) renderQueueView() string {
316316
}
317317

318318
func (m tuiModel) renderJobLine(job storage.ReviewJob) string {
319-
ref := job.GitRef
320-
if len(ref) > 17 {
321-
ref = ref[:17]
322-
}
319+
ref := shortRef(job.GitRef)
323320

324321
repo := job.RepoName
325322
if len(repo) > 15 {
@@ -406,10 +403,7 @@ func (m tuiModel) renderReviewView() string {
406403

407404
review := m.currentReview
408405
if review.Job != nil {
409-
ref := review.Job.GitRef
410-
if len(ref) > 17 {
411-
ref = ref[:17]
412-
}
406+
ref := shortRef(review.Job.GitRef)
413407
title := fmt.Sprintf("Review: %s (%s)", ref, review.Agent)
414408
b.WriteString(tuiTitleStyle.Render(title))
415409
} else {

cmd/roborevd/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ func main() {
2424
log.SetFlags(log.Ldate | log.Ltime | log.Lshortfile)
2525
log.Println("Starting roborevd...")
2626

27-
// Load configuration
28-
cfg, err := config.LoadGlobal()
27+
// Load configuration from specified path
28+
cfg, err := config.LoadGlobalFrom(*configPath)
2929
if err != nil {
3030
log.Printf("Warning: failed to load config from %s: %v", *configPath, err)
3131
cfg = config.DefaultConfig()

internal/agent/agent.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package agent
33
import (
44
"context"
55
"fmt"
6+
"os/exec"
67
)
78

89
// Agent defines the interface for code review agents
@@ -14,6 +15,13 @@ type Agent interface {
1415
Review(ctx context.Context, repoPath, commitSHA, prompt string) (output string, err error)
1516
}
1617

18+
// CommandAgent is an agent that uses an external command
19+
type CommandAgent interface {
20+
Agent
21+
// CommandName returns the executable command name
22+
CommandName() string
23+
}
24+
1725
// Registry holds available agents
1826
var registry = make(map[string]Agent)
1927

@@ -39,3 +47,51 @@ func Available() []string {
3947
}
4048
return names
4149
}
50+
51+
// IsAvailable checks if an agent's command is installed on the system
52+
func IsAvailable(name string) bool {
53+
a, ok := registry[name]
54+
if !ok {
55+
return false
56+
}
57+
58+
// Check if agent implements CommandAgent interface
59+
if ca, ok := a.(CommandAgent); ok {
60+
_, err := exec.LookPath(ca.CommandName())
61+
return err == nil
62+
}
63+
64+
// Non-command agents (like test) are always available
65+
return true
66+
}
67+
68+
// GetAvailable returns an available agent, trying the requested one first,
69+
// then falling back to alternatives. Returns error only if no agents available.
70+
func GetAvailable(preferred string) (Agent, error) {
71+
// Try preferred agent first
72+
if preferred != "" && IsAvailable(preferred) {
73+
return Get(preferred)
74+
}
75+
76+
// Fallback order: codex, claude-code
77+
fallbacks := []string{"codex", "claude-code"}
78+
for _, name := range fallbacks {
79+
if name != preferred && IsAvailable(name) {
80+
return Get(name)
81+
}
82+
}
83+
84+
// List what's actually available for error message
85+
var available []string
86+
for name := range registry {
87+
if IsAvailable(name) {
88+
available = append(available, name)
89+
}
90+
}
91+
92+
if len(available) == 0 {
93+
return nil, fmt.Errorf("no agents available (install codex or claude)")
94+
}
95+
96+
return Get(available[0])
97+
}

internal/agent/claude.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ func (a *ClaudeAgent) Name() string {
2424
return "claude-code"
2525
}
2626

27+
func (a *ClaudeAgent) CommandName() string {
28+
return a.Command
29+
}
30+
2731
func (a *ClaudeAgent) Review(ctx context.Context, repoPath, commitSHA, prompt string) (string, error) {
2832
// Use claude CLI in print mode (non-interactive)
2933
// --print outputs the response without the interactive TUI

internal/agent/codex.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"os"
88
"os/exec"
9-
"path/filepath"
109
)
1110

1211
// CodexAgent runs code reviews using the Codex CLI
@@ -26,10 +25,18 @@ func (a *CodexAgent) Name() string {
2625
return "codex"
2726
}
2827

28+
func (a *CodexAgent) CommandName() string {
29+
return a.Command
30+
}
31+
2932
func (a *CodexAgent) Review(ctx context.Context, repoPath, commitSHA, prompt string) (string, error) {
30-
// Create temp file for output
31-
tmpDir := os.TempDir()
32-
outputFile := filepath.Join(tmpDir, fmt.Sprintf("roborev-%s.txt", commitSHA[:8]))
33+
// Create unique temp file for output
34+
tmpFile, err := os.CreateTemp("", "roborev-*.txt")
35+
if err != nil {
36+
return "", fmt.Errorf("create temp file: %w", err)
37+
}
38+
outputFile := tmpFile.Name()
39+
tmpFile.Close()
3340
defer os.Remove(outputFile)
3441

3542
// Use codex exec with output capture

internal/config/config.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,15 @@ func GlobalConfigPath() string {
4343
return filepath.Join(home, ".roborev", "config.toml")
4444
}
4545

46-
// LoadGlobal loads the global configuration, using defaults for missing values
46+
// LoadGlobal loads the global configuration from the default path
4747
func LoadGlobal() (*Config, error) {
48+
return LoadGlobalFrom(GlobalConfigPath())
49+
}
50+
51+
// LoadGlobalFrom loads the global configuration from a specific path
52+
func LoadGlobalFrom(path string) (*Config, error) {
4853
cfg := DefaultConfig()
4954

50-
path := GlobalConfigPath()
5155
if _, err := os.Stat(path); os.IsNotExist(err) {
5256
return cfg, nil
5357
}

internal/daemon/worker.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,25 +109,31 @@ func (wp *WorkerPool) processJob(workerID string, job *storage.ReviewJob) {
109109
return
110110
}
111111

112-
// Get the agent
113-
a, err := agent.Get(job.Agent)
112+
// Get the agent (falls back to available agent if preferred not installed)
113+
a, err := agent.GetAvailable(job.Agent)
114114
if err != nil {
115-
log.Printf("[%s] Error getting agent %s: %v", workerID, job.Agent, err)
115+
log.Printf("[%s] Error getting agent: %v", workerID, err)
116116
wp.db.FailJob(job.ID, fmt.Sprintf("get agent: %v", err))
117117
return
118118
}
119119

120+
// Use the actual agent name (may differ from requested if fallback occurred)
121+
agentName := a.Name()
122+
if agentName != job.Agent {
123+
log.Printf("[%s] Agent %s not available, using %s", workerID, job.Agent, agentName)
124+
}
125+
120126
// Run the review
121-
log.Printf("[%s] Running %s review...", workerID, job.Agent)
127+
log.Printf("[%s] Running %s review...", workerID, agentName)
122128
output, err := a.Review(ctx, job.RepoPath, job.GitRef, reviewPrompt)
123129
if err != nil {
124130
log.Printf("[%s] Agent error: %v", workerID, err)
125131
wp.db.FailJob(job.ID, fmt.Sprintf("agent: %v", err))
126132
return
127133
}
128134

129-
// Store the result
130-
if err := wp.db.CompleteJob(job.ID, job.Agent, reviewPrompt, output); err != nil {
135+
// Store the result (use actual agent name, not requested)
136+
if err := wp.db.CompleteJob(job.ID, agentName, reviewPrompt, output); err != nil {
131137
log.Printf("[%s] Error storing review: %v", workerID, err)
132138
return
133139
}

internal/prompt/prompt.go

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import (
88
"github.com/wesm/roborev/internal/storage"
99
)
1010

11+
// MaxPromptSize is the maximum size of a prompt in bytes (250KB)
12+
// If the prompt with diffs exceeds this, we fall back to just commit info
13+
const MaxPromptSize = 250 * 1024
14+
1115
// SystemPromptSingle is the base instruction for single commit reviews
1216
const SystemPromptSingle = `You are a code reviewer. Review the git commit shown below for:
1317
@@ -102,8 +106,43 @@ func (b *Builder) buildSinglePrompt(repoPath, sha string, repoID int64, contextC
102106
if len(shortSHA) > 7 {
103107
shortSHA = shortSHA[:7]
104108
}
109+
110+
// Get commit info
111+
info, err := git.GetCommitInfo(repoPath, sha)
112+
if err != nil {
113+
return "", fmt.Errorf("get commit info: %w", err)
114+
}
115+
105116
sb.WriteString("## Current Commit\n\n")
106-
sb.WriteString(fmt.Sprintf("Review the following commit: %s\n", shortSHA))
117+
sb.WriteString(fmt.Sprintf("**Commit:** %s\n", shortSHA))
118+
sb.WriteString(fmt.Sprintf("**Author:** %s\n", info.Author))
119+
sb.WriteString(fmt.Sprintf("**Subject:** %s\n\n", info.Subject))
120+
121+
// Get and include the diff
122+
diff, err := git.GetDiff(repoPath, sha)
123+
if err != nil {
124+
return "", fmt.Errorf("get diff: %w", err)
125+
}
126+
127+
// Build diff section
128+
var diffSection strings.Builder
129+
diffSection.WriteString("### Diff\n\n")
130+
diffSection.WriteString("```diff\n")
131+
diffSection.WriteString(diff)
132+
if !strings.HasSuffix(diff, "\n") {
133+
diffSection.WriteString("\n")
134+
}
135+
diffSection.WriteString("```\n")
136+
137+
// Check if adding the diff would exceed max prompt size
138+
if sb.Len()+diffSection.Len() > MaxPromptSize {
139+
// Fall back to just commit info without diff
140+
sb.WriteString("### Diff\n\n")
141+
sb.WriteString("(Diff too large to include - please review the commit directly)\n")
142+
sb.WriteString(fmt.Sprintf("View with: git show %s\n", sha))
143+
} else {
144+
sb.WriteString(diffSection.String())
145+
}
107146

108147
return sb.String(), nil
109148
}
@@ -151,6 +190,32 @@ func (b *Builder) buildRangePrompt(repoPath, rangeRef string, repoID int64, cont
151190
}
152191
sb.WriteString("\n")
153192

193+
// Get and include the combined diff for the range
194+
diff, err := git.GetRangeDiff(repoPath, rangeRef)
195+
if err != nil {
196+
return "", fmt.Errorf("get range diff: %w", err)
197+
}
198+
199+
// Build diff section
200+
var diffSection strings.Builder
201+
diffSection.WriteString("### Combined Diff\n\n")
202+
diffSection.WriteString("```diff\n")
203+
diffSection.WriteString(diff)
204+
if !strings.HasSuffix(diff, "\n") {
205+
diffSection.WriteString("\n")
206+
}
207+
diffSection.WriteString("```\n")
208+
209+
// Check if adding the diff would exceed max prompt size
210+
if sb.Len()+diffSection.Len() > MaxPromptSize {
211+
// Fall back to just commit info without diff
212+
sb.WriteString("### Combined Diff\n\n")
213+
sb.WriteString("(Diff too large to include - please review the commits directly)\n")
214+
sb.WriteString(fmt.Sprintf("View with: git diff %s\n", rangeRef))
215+
} else {
216+
sb.WriteString(diffSection.String())
217+
}
218+
154219
return sb.String(), nil
155220
}
156221

0 commit comments

Comments
 (0)