Skip to content

Commit 9a7c3ff

Browse files
mariusvniekerkclaudewesm
authored
fix: handle empty git ref when fixing compact review jobs (#327)
I think this likely gets us to a fix for #323 ## Summary - Compact jobs are stored with an empty `git_ref` (only a display label is stored). Attempting to fix a compact review would pass `""` to `worktree.Create`, causing `create worktree: ref must not be empty`. - The fix job handler now resolves the ref via: explicit request override → parent job's `git_ref` → parent job's branch → `HEAD` (with a log warning). - When the TUI detects no git ref and no branch (the HEAD fallback case), it shows a new confirmation modal (`tuiViewFixGitRef`) pre-filled with `"HEAD"` so the user can verify or correct the target before proceeding. ## Test plan - [ ] Fix a compact review job from the TUI — should no longer fail with `create worktree: ref must not be empty` - [ ] When compact job has a branch set, fix proceeds normally using that branch as the ref - [ ] When compact job has neither git ref nor branch, the new git ref modal appears pre-filled with `HEAD`, allowing the user to edit before confirming - [ ] `go test ./...` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
1 parent 2c9e0cc commit 9a7c3ff

5 files changed

Lines changed: 350 additions & 6 deletions

File tree

cmd/roborev/tui.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ const (
112112
tuiViewLog
113113
tuiViewTasks // Background fix tasks view
114114
tuiViewFixPrompt // Fix prompt confirmation modal
115+
tuiViewFixGitRef // Git ref confirmation modal (HEAD fallback for compact jobs)
115116
tuiViewPatch // Patch viewer for fix jobs
116117
)
117118

@@ -271,6 +272,7 @@ type tuiModel struct {
271272
fixPromptText string // Editable fix prompt text
272273
fixPromptJobID int64 // Parent job ID for fix prompt modal
273274
fixPromptFromView tuiView // View to return to after fix prompt closes
275+
fixPromptGitRef string // Editable git ref for HEAD-fallback confirmation
274276
fixShowHelp bool // Show help overlay in tasks view
275277
patchText string // Current patch text for patch viewer
276278
patchScroll int // Scroll offset in patch viewer
@@ -2417,6 +2419,9 @@ func (m tuiModel) View() string {
24172419
if m.currentView == tuiViewFixPrompt {
24182420
return m.renderFixPromptView()
24192421
}
2422+
if m.currentView == tuiViewFixGitRef {
2423+
return m.renderFixGitRefView()
2424+
}
24202425
if m.currentView == tuiViewPatch {
24212426
return m.renderPatchView()
24222427
}
@@ -3925,6 +3930,30 @@ func (m tuiModel) renderFixPromptView() string {
39253930
return b.String()
39263931
}
39273932

3933+
// renderFixGitRefView renders the git ref confirmation modal shown when a fix job
3934+
// cannot determine the target ref automatically (e.g. compact jobs with no branch).
3935+
func (m tuiModel) renderFixGitRefView() string {
3936+
var b strings.Builder
3937+
3938+
b.WriteString(tuiTitleStyle.Render(fmt.Sprintf("Fix Review #%d", m.fixPromptJobID)))
3939+
b.WriteString("\x1b[K\n\n")
3940+
3941+
b.WriteString(" Could not determine the target branch for this review.\n")
3942+
b.WriteString(" Enter a branch name or commit SHA to apply the fix against:\n\n")
3943+
3944+
refDisplay := m.fixPromptGitRef
3945+
if refDisplay == "" {
3946+
refDisplay = "HEAD"
3947+
}
3948+
fmt.Fprintf(&b, " > %s_\n", refDisplay)
3949+
b.WriteString("\n")
3950+
3951+
b.WriteString(tuiHelpStyle.Render("enter: confirm | esc: cancel"))
3952+
b.WriteString("\x1b[K\x1b[J")
3953+
3954+
return b.String()
3955+
}
3956+
39283957
// fetchJobByID fetches a single job by ID from the daemon API.
39293958
func (m tuiModel) fetchJobByID(jobID int64) (*storage.ReviewJob, error) {
39303959
var result struct {
@@ -3966,14 +3995,17 @@ func (m tuiModel) fetchFixJobs() tea.Cmd {
39663995
}
39673996

39683997
// triggerFix triggers a background fix job for a parent review.
3969-
func (m tuiModel) triggerFix(parentJobID int64, prompt string) tea.Cmd {
3998+
func (m tuiModel) triggerFix(parentJobID int64, prompt, gitRef string) tea.Cmd {
39703999
return func() tea.Msg {
39714000
req := map[string]any{
39724001
"parent_job_id": parentJobID,
39734002
}
39744003
if prompt != "" {
39754004
req["prompt"] = prompt
39764005
}
4006+
if gitRef != "" {
4007+
req["git_ref"] = gitRef
4008+
}
39774009
var job storage.ReviewJob
39784010
err := m.postJSON("/api/job/fix", req, &job)
39794011
if err != nil {

cmd/roborev/tui_handlers.go

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ func (m tuiModel) handleKeyMsg(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
2323
return m.handleFilterKey(msg)
2424
case tuiViewLog:
2525
return m.handleLogKey(msg)
26+
case tuiViewFixGitRef:
27+
return m.handleFixGitRefKey(msg)
2628
case tuiViewFixPrompt:
2729
return m.handleFixPromptKey(msg)
2830
case tuiViewTasks:
@@ -1478,11 +1480,20 @@ func (m tuiModel) handleFixKey() (tea.Model, tea.Cmd) {
14781480
return m, nil
14791481
}
14801482

1481-
// Open fix prompt modal
1483+
// Open fix prompt modal. For compact/task jobs with no git ref and no branch,
1484+
// show the git ref confirmation step first so the user can specify a target.
14821485
m.fixPromptJobID = job.ID
1483-
m.fixPromptText = "" // Empty means use default prompt from server
1486+
m.fixPromptText = ""
14841487
m.fixPromptFromView = m.currentView
1485-
m.currentView = tuiViewFixPrompt
1488+
m.fixPromptGitRef = ""
1489+
isRange := strings.Contains(job.GitRef, "..")
1490+
if (job.GitRef == "" || isRange) && job.Branch == "" {
1491+
// No usable single ref — ask the user to confirm or correct the target.
1492+
m.fixPromptGitRef = "HEAD"
1493+
m.currentView = tuiViewFixGitRef
1494+
} else {
1495+
m.currentView = tuiViewFixPrompt
1496+
}
14861497
return m, nil
14871498
}
14881499

@@ -1499,6 +1510,46 @@ func (m tuiModel) handleToggleTasksKey() (tea.Model, tea.Cmd) {
14991510
return m, nil
15001511
}
15011512

1513+
// handleFixGitRefKey handles key input in the git ref confirmation modal.
1514+
// This modal is shown when fixing a compact job that has no branch information,
1515+
// so the user can specify (or confirm) the target ref before proceeding.
1516+
func (m tuiModel) handleFixGitRefKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
1517+
switch msg.String() {
1518+
case "ctrl+c":
1519+
return m, tea.Quit
1520+
case "esc":
1521+
m.currentView = m.fixPromptFromView
1522+
m.fixPromptGitRef = ""
1523+
m.fixPromptJobID = 0
1524+
return m, nil
1525+
case "enter":
1526+
if strings.TrimSpace(m.fixPromptGitRef) == "" {
1527+
m.flashMessage = "Git ref is required"
1528+
m.flashExpiresAt = time.Now().Add(2 * time.Second)
1529+
m.flashView = tuiViewFixGitRef
1530+
return m, nil
1531+
}
1532+
// Move on to the fix instructions prompt with the ref confirmed.
1533+
m.currentView = tuiViewFixPrompt
1534+
return m, nil
1535+
case "backspace":
1536+
if len(m.fixPromptGitRef) > 0 {
1537+
runes := []rune(m.fixPromptGitRef)
1538+
m.fixPromptGitRef = string(runes[:len(runes)-1])
1539+
}
1540+
return m, nil
1541+
default:
1542+
if len(msg.Runes) > 0 {
1543+
for _, r := range msg.Runes {
1544+
if unicode.IsPrint(r) {
1545+
m.fixPromptGitRef += string(r)
1546+
}
1547+
}
1548+
}
1549+
return m, nil
1550+
}
1551+
}
1552+
15021553
// handleFixPromptKey handles key input in the fix prompt confirmation modal.
15031554
func (m tuiModel) handleFixPromptKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
15041555
switch msg.String() {
@@ -1507,15 +1558,18 @@ func (m tuiModel) handleFixPromptKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
15071558
case "esc":
15081559
m.currentView = m.fixPromptFromView
15091560
m.fixPromptText = ""
1561+
m.fixPromptGitRef = ""
15101562
m.fixPromptJobID = 0
15111563
return m, nil
15121564
case "enter":
15131565
jobID := m.fixPromptJobID
15141566
prompt := m.fixPromptText
1567+
gitRef := m.fixPromptGitRef
15151568
m.currentView = tuiViewTasks
15161569
m.fixPromptText = ""
1570+
m.fixPromptGitRef = ""
15171571
m.fixPromptJobID = 0
1518-
return m, m.triggerFix(jobID, prompt)
1572+
return m, m.triggerFix(jobID, prompt, gitRef)
15191573
case "backspace":
15201574
if len(m.fixPromptText) > 0 {
15211575
runes := []rune(m.fixPromptText)

cmd/roborev/tui_review_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3151,4 +3151,63 @@ func TestTUILogNavFromTasks(t *testing.T) {
31513151
}
31523152
}
31533153

3154+
func TestTUIFixGitRefRejectsEmpty(t *testing.T) {
3155+
// Pressing Enter with an empty git ref should show a flash
3156+
// message and stay on the git ref modal.
3157+
m := newTuiModel("http://localhost")
3158+
m.currentView = tuiViewFixGitRef
3159+
m.fixPromptJobID = 1
3160+
m.fixPromptGitRef = ""
3161+
m.fixPromptFromView = tuiViewQueue
3162+
3163+
m2, _ := pressSpecial(m, tea.KeyEnter)
3164+
3165+
if m2.currentView != tuiViewFixGitRef {
3166+
t.Errorf(
3167+
"should stay on git ref view, got %d",
3168+
m2.currentView,
3169+
)
3170+
}
3171+
if m2.flashMessage == "" {
3172+
t.Error("expected flash message for empty ref")
3173+
}
3174+
}
3175+
3176+
func TestTUIFixGitRefRejectsWhitespaceOnly(t *testing.T) {
3177+
m := newTuiModel("http://localhost")
3178+
m.currentView = tuiViewFixGitRef
3179+
m.fixPromptJobID = 1
3180+
m.fixPromptGitRef = " "
3181+
m.fixPromptFromView = tuiViewQueue
3182+
3183+
m2, _ := pressSpecial(m, tea.KeyEnter)
3184+
3185+
if m2.currentView != tuiViewFixGitRef {
3186+
t.Errorf(
3187+
"should stay on git ref view for whitespace-only, got %d",
3188+
m2.currentView,
3189+
)
3190+
}
3191+
if m2.flashMessage == "" {
3192+
t.Error("expected flash message for whitespace-only ref")
3193+
}
3194+
}
3195+
3196+
func TestTUIFixGitRefAcceptsNonEmpty(t *testing.T) {
3197+
m := newTuiModel("http://localhost")
3198+
m.currentView = tuiViewFixGitRef
3199+
m.fixPromptJobID = 1
3200+
m.fixPromptGitRef = "main"
3201+
m.fixPromptFromView = tuiViewQueue
3202+
3203+
m2, _ := pressSpecial(m, tea.KeyEnter)
3204+
3205+
if m2.currentView != tuiViewFixPrompt {
3206+
t.Errorf(
3207+
"should transition to fix prompt, got %d",
3208+
m2.currentView,
3209+
)
3210+
}
3211+
}
3212+
31543213
// Branch filter tests

internal/daemon/server.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1871,6 +1871,7 @@ func (s *Server) handleFixJob(w http.ResponseWriter, r *http.Request) {
18711871
var req struct {
18721872
ParentJobID int64 `json:"parent_job_id"`
18731873
Prompt string `json:"prompt,omitempty"` // Optional custom prompt override
1874+
GitRef string `json:"git_ref,omitempty"` // Optional: explicit ref for worktree (user-confirmed for compact jobs)
18741875
StaleJobID int64 `json:"stale_job_id,omitempty"` // Optional: server looks up patch from this job for rebase
18751876
}
18761877
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
@@ -1951,10 +1952,34 @@ func (s *Server) handleFixJob(w http.ResponseWriter, r *http.Request) {
19511952
}
19521953
model := config.ResolveModelForWorkflow("", parentJob.RepoPath, cfg, "fix", reasoning)
19531954

1955+
// Normalize and validate user-provided git ref to prevent option
1956+
// injection (e.g. " --something") when passed to git worktree add.
1957+
req.GitRef = strings.TrimSpace(req.GitRef)
1958+
if req.GitRef != "" && !isValidGitRef(req.GitRef) {
1959+
writeError(w, http.StatusBadRequest, "invalid git_ref")
1960+
return
1961+
}
1962+
1963+
// Resolve the git ref for the fix worktree.
1964+
// Range refs (sha..sha) and empty refs (compact jobs) are not valid for
1965+
// git worktree add, so fall back to branch then "HEAD".
1966+
// An explicit git_ref from the request (user-confirmed via TUI) takes precedence.
1967+
fixGitRef := req.GitRef
1968+
if fixGitRef == "" && !strings.Contains(parentJob.GitRef, "..") {
1969+
fixGitRef = parentJob.GitRef
1970+
}
1971+
if fixGitRef == "" {
1972+
fixGitRef = parentJob.Branch
1973+
}
1974+
if fixGitRef == "" {
1975+
fixGitRef = "HEAD"
1976+
log.Printf("fix job for parent %d: no git ref or branch available, falling back to HEAD", req.ParentJobID)
1977+
}
1978+
19541979
// Enqueue the fix job
19551980
job, err := s.db.EnqueueJob(storage.EnqueueOpts{
19561981
RepoID: parentJob.RepoID,
1957-
GitRef: parentJob.GitRef,
1982+
GitRef: fixGitRef,
19581983
Branch: parentJob.Branch,
19591984
Agent: agentName,
19601985
Model: model,
@@ -1973,6 +1998,21 @@ func (s *Server) handleFixJob(w http.ResponseWriter, r *http.Request) {
19731998
writeJSONWithStatus(w, http.StatusCreated, job)
19741999
}
19752000

2001+
// isValidGitRef checks that a user-provided git ref is safe to pass
2002+
// to git commands. Rejects empty strings, leading dashes (option
2003+
// injection), and control characters.
2004+
func isValidGitRef(ref string) bool {
2005+
if ref == "" || ref[0] == '-' {
2006+
return false
2007+
}
2008+
for _, r := range ref {
2009+
if r < 0x20 || r == 0x7f {
2010+
return false
2011+
}
2012+
}
2013+
return true
2014+
}
2015+
19762016
// handleGetPatch returns the stored patch for a completed fix job.
19772017
func (s *Server) handleGetPatch(w http.ResponseWriter, r *http.Request) {
19782018
if r.Method != http.MethodGet {

0 commit comments

Comments
 (0)