diff --git a/cmd/roborev/main.go b/cmd/roborev/main.go index 6b70fd6..26ebd06 100644 --- a/cmd/roborev/main.go +++ b/cmd/roborev/main.go @@ -542,7 +542,8 @@ func daemonRunCmd() *cobra.Command { if err := server.Stop(); err != nil { log.Printf("Shutdown error: %v", err) } - os.Exit(0) + // Note: Don't call os.Exit here - let server.Start() return naturally + // after Stop() is called. This allows proper cleanup and testability. }() // Start server (blocks until shutdown) @@ -2352,6 +2353,15 @@ func shortJobRef(job storage.ReviewJob) string { return shortRef(job.GitRef) } +// formatAgentLabel returns the agent display string, including model if set. +// Format: "agent" or "agent: model" +func formatAgentLabel(agent string, model string) string { + if model != "" { + return fmt.Sprintf("%s: %s", agent, model) + } + return agent +} + // generateHookContent creates the post-commit hook script content. // It bakes the path to the currently running binary for consistency. // Falls back to PATH lookup if the baked path becomes unavailable. diff --git a/cmd/roborev/main_test.go b/cmd/roborev/main_test.go index 76de432..7c4f68c 100644 --- a/cmd/roborev/main_test.go +++ b/cmd/roborev/main_test.go @@ -1475,11 +1475,13 @@ func TestDaemonRunStartsAndShutdownsCleanly(t *testing.T) { } // Create the daemon run command with custom flags + // Use a high base port to avoid conflicts with production (7373). + // FindAvailablePort will auto-increment if 17373 is busy. cmd := daemonRunCmd() cmd.SetArgs([]string{ "--db", dbPath, "--config", configPath, - "--addr", "127.0.0.1:0", // Use port 0 to get a free port + "--addr", "127.0.0.1:17373", }) // Run daemon in goroutine @@ -1510,9 +1512,52 @@ func TestDaemonRunStartsAndShutdownsCleanly(t *testing.T) { // Daemon is still running - good } - // The daemon is blocking in server.Start(), so we can't easily stop it - // without sending a signal. For this unit test, we just verify it started. - // A full integration test would require a separate process. + // Wait for daemon to be fully started and responsive + // The runtime file is written before ListenAndServe, so we need to verify + // the HTTP server is actually accepting connections. + // Use longer timeout for race detector which adds significant overhead. + var info *daemon.RuntimeInfo + myPID := os.Getpid() + deadline = time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + runtimes, err := daemon.ListAllRuntimes() + if err == nil { + // Find the runtime for OUR daemon (matching our PID), not a stale one + for _, rt := range runtimes { + if rt.PID == myPID && daemon.IsDaemonAlive(rt.Addr) { + info = rt + break + } + } + if info != nil { + break + } + } + time.Sleep(100 * time.Millisecond) + } + if info == nil { + // Provide more context for debugging CI failures + runtimes, _ := daemon.ListAllRuntimes() + t.Fatalf("daemon did not create runtime file or is not responding (myPID=%d, found %d runtimes)", myPID, len(runtimes)) + } + + // The daemon runs in a goroutine within this test process. + // Use os.Interrupt to trigger the signal handler. + proc, err := os.FindProcess(myPID) + if err != nil { + t.Fatalf("failed to find own process: %v", err) + } + if err := proc.Signal(os.Interrupt); err != nil { + t.Fatalf("failed to send interrupt signal: %v", err) + } + + // Wait for daemon to exit (longer timeout for race detector) + select { + case <-errCh: + // Daemon exited - good + case <-time.After(10 * time.Second): + t.Fatal("daemon did not exit within 10 second timeout") + } } // TestDaemonStopNotRunning verifies daemon stop reports when no daemon is running diff --git a/cmd/roborev/tui.go b/cmd/roborev/tui.go index 6a7e14a..693bc3a 100644 --- a/cmd/roborev/tui.go +++ b/cmd/roborev/tui.go @@ -125,6 +125,7 @@ type tuiModel struct { err error updateAvailable string // Latest version if update available, empty if up to date updateIsDevBuild bool // True if running a dev build + versionMismatch bool // True if daemon version doesn't match TUI version // Pagination state hasMore bool // true if there are more jobs to load @@ -2223,6 +2224,8 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.consecutiveErrors = 0 // Reset on successful fetch if m.status.Version != "" { m.daemonVersion = m.status.Version + // Check for version mismatch between TUI and daemon + m.versionMismatch = m.daemonVersion != version.Version } // Show flash notification when config is reloaded // Use counter (not timestamp) to detect reloads that happen within the same second @@ -2710,6 +2713,13 @@ func (m tuiModel) renderQueueView() string { } b.WriteString("\x1b[K\n") // Clear to end of line + // Version mismatch error (persistent, red) + if m.versionMismatch { + errorStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("196")).Bold(true) // Bright red + b.WriteString(errorStyle.Render(fmt.Sprintf("VERSION MISMATCH: TUI %s != Daemon %s - restart TUI or daemon", version.Version, m.daemonVersion))) + b.WriteString("\x1b[K\n") + } + // Help (two lines) helpLine1 := "↑/↓: navigate | enter: review | y: copy | m: commit msg | q: quit | ?: help" helpLine2 := "f: filter | h: hide addressed | a: toggle addressed | x: cancel" @@ -2916,6 +2926,7 @@ func (m tuiModel) renderReviewView() string { // Build title string and compute its length for line calculation var title string var titleLen int + var locationLineLen int if review.Job != nil { ref := shortJobRef(*review.Job) idStr := fmt.Sprintf("#%d ", review.Job.ID) @@ -2926,34 +2937,31 @@ func (m tuiModel) renderReviewView() string { } repoStr := m.getDisplayName(review.Job.RepoPath, defaultName) - // Build agent string, including model if explicitly set - agentStr := review.Agent - if review.Job.Model != "" { - agentStr = fmt.Sprintf("%s: %s", review.Agent, review.Job.Model) - } + agentStr := formatAgentLabel(review.Agent, review.Job.Model) title = fmt.Sprintf("Review %s%s (%s)", idStr, repoStr, agentStr) - titleLen = len(title) + titleLen = runewidth.StringWidth(title) b.WriteString(tuiTitleStyle.Render(title)) b.WriteString("\x1b[K") // Clear to end of line // Show location line: repo path (or identity/name), git ref, and branch b.WriteString("\n") - pathLine := review.Job.RepoPath - if pathLine == "" { + locationLine := review.Job.RepoPath + if locationLine == "" { // No local path - use repo name/identity as fallback - pathLine = review.Job.RepoName + locationLine = review.Job.RepoName } - if pathLine != "" { - pathLine += " " + ref + if locationLine != "" { + locationLine += " " + ref } else { - pathLine = ref + locationLine = ref } if m.currentBranch != "" { - pathLine += " on " + m.currentBranch + locationLine += " on " + m.currentBranch } - b.WriteString(tuiStatusStyle.Render(pathLine)) + locationLineLen = runewidth.StringWidth(locationLine) + b.WriteString(tuiStatusStyle.Render(locationLine)) b.WriteString("\x1b[K") // Clear to end of line // Show verdict and addressed status on next line @@ -3017,11 +3025,17 @@ func (m tuiModel) renderReviewView() string { helpLines = (len(helpText) + m.width - 1) / m.width } - // headerHeight = title + location line (1) + status line (1) + help + verdict/addressed (0|1) - headerHeight := titleLines + 1 + helpLines + // Compute location line count (repo path + ref + branch can wrap) + locationLines := 0 if review.Job != nil { - headerHeight++ // Add 1 for location line (repo path + ref + branch) + locationLines = 1 + if m.width > 0 && locationLineLen > m.width { + locationLines = (locationLineLen + m.width - 1) / m.width + } } + + // headerHeight = title + location line + status line (1) + help + verdict/addressed (0|1) + headerHeight := titleLines + locationLines + 1 + helpLines hasVerdict := review.Job != nil && review.Job.Verdict != nil && *review.Job.Verdict != "" if hasVerdict || review.Addressed { headerHeight++ // Add 1 for verdict/addressed line @@ -3068,6 +3082,13 @@ func (m tuiModel) renderReviewView() string { } b.WriteString("\x1b[K\n") // Clear status line + // Version mismatch error (persistent, red) + if m.versionMismatch { + errorStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("196")).Bold(true) + b.WriteString(errorStyle.Render(fmt.Sprintf("VERSION MISMATCH: TUI %s != Daemon %s - restart TUI or daemon", version.Version, m.daemonVersion))) + b.WriteString("\x1b[K\n") + } + b.WriteString(tuiHelpStyle.Render(helpText)) b.WriteString("\x1b[K") // Clear help line b.WriteString("\x1b[J") // Clear to end of screen to prevent artifacts @@ -3084,11 +3105,7 @@ func (m tuiModel) renderPromptView() string { review := m.currentReview if review.Job != nil { ref := shortJobRef(*review.Job) - // Build agent string, including model if explicitly set - agentStr := review.Agent - if review.Job.Model != "" { - agentStr = fmt.Sprintf("%s: %s", review.Agent, review.Job.Model) - } + agentStr := formatAgentLabel(review.Agent, review.Job.Model) title := fmt.Sprintf("Prompt: %s (%s)", ref, agentStr) b.WriteString(tuiTitleStyle.Render(title)) } else { diff --git a/cmd/roborev/tui_test.go b/cmd/roborev/tui_test.go index 5299d4f..759f358 100644 --- a/cmd/roborev/tui_test.go +++ b/cmd/roborev/tui_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" "regexp" "strings" "testing" @@ -16,11 +17,33 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/mattn/go-runewidth" "github.com/roborev-dev/roborev/internal/storage" + "github.com/roborev-dev/roborev/internal/version" ) +// testServerAddr is a placeholder address used in tests that don't make real HTTP calls. +// Tests that need actual HTTP should use httptest.NewServer and pass ts.URL. +const testServerAddr = "http://test.invalid:9999" + +// setupTuiTestEnv isolates the test from the production roborev environment +// by setting ROBOREV_DATA_DIR to a temp directory. This prevents tests from +// reading production daemon.json or affecting production state. +func setupTuiTestEnv(t *testing.T) { + t.Helper() + tmpDir := t.TempDir() + origDataDir := os.Getenv("ROBOREV_DATA_DIR") + os.Setenv("ROBOREV_DATA_DIR", tmpDir) + t.Cleanup(func() { + if origDataDir != "" { + os.Setenv("ROBOREV_DATA_DIR", origDataDir) + } else { + os.Unsetenv("ROBOREV_DATA_DIR") + } + }) +} + // mockConnError creates a connection error (url.Error) for testing func mockConnError(msg string) error { - return &url.Error{Op: "Get", URL: "http://localhost:7373", Err: errors.New(msg)} + return &url.Error{Op: "Get", URL: testServerAddr, Err: errors.New(msg)} } // stripANSI removes ANSI escape sequences from a string @@ -3925,6 +3948,60 @@ func TestTUIRenderReviewViewWithoutModel(t *testing.T) { } } +func TestTUIRenderPromptViewWithModel(t *testing.T) { + m := newTuiModel("http://localhost") + m.width = 100 + m.height = 30 + m.currentView = tuiViewPrompt + m.currentReview = &storage.Review{ + ID: 10, + Agent: "codex", + Prompt: "Review this code", + Job: &storage.ReviewJob{ + ID: 1, + GitRef: "abc1234", + Agent: "codex", + Model: "o3", + }, + } + + output := m.View() + + // Should contain agent with model in format "(codex: o3)" + if !strings.Contains(output, "(codex: o3)") { + t.Errorf("Expected Prompt view to contain '(codex: o3)', got:\n%s", output) + } +} + +func TestTUIRenderPromptViewWithoutModel(t *testing.T) { + m := newTuiModel("http://localhost") + m.width = 100 + m.height = 30 + m.currentView = tuiViewPrompt + m.currentReview = &storage.Review{ + ID: 10, + Agent: "codex", + Prompt: "Review this code", + Job: &storage.ReviewJob{ + ID: 1, + GitRef: "abc1234", + Agent: "codex", + Model: "", // No model set + }, + } + + output := m.View() + + // Should contain just the agent "(codex)" without model + if !strings.Contains(output, "(codex)") { + t.Errorf("Expected Prompt view to contain '(codex)', got:\n%s", output) + } + // Should NOT contain the colon separator that would indicate a model + if strings.Contains(output, "(codex:") { + t.Error("Expected Prompt view NOT to contain '(codex:' when no model is set") + } +} + func TestTUIRenderReviewViewNoBranchForRange(t *testing.T) { m := newTuiModel("http://localhost") m.width = 100 @@ -4033,6 +4110,57 @@ func TestTUIRenderReviewViewVerdictOnLine2(t *testing.T) { } } +func TestTUIRenderReviewViewAddressedWithoutVerdict(t *testing.T) { + // Test that [ADDRESSED] appears on line 2 when no verdict is present + m := newTuiModel("http://localhost") + m.width = 100 + m.height = 30 + m.currentView = tuiViewReview + m.currentReview = &storage.Review{ + ID: 10, + Output: "Line 1\nLine 2\nLine 3", + Addressed: true, + Job: &storage.ReviewJob{ + ID: 1, + GitRef: "abc1234", + RepoName: "myrepo", + Agent: "codex", + Verdict: nil, // No verdict + }, + } + + output := m.View() + lines := strings.Split(output, "\n") + + // Require at least 4 lines: title, location, addressed, content + if len(lines) < 4 { + t.Fatalf("Expected at least 4 lines, got %d:\n%s", len(lines), output) + } + + // Line 0: Title + if !strings.Contains(lines[0], "Review") { + t.Errorf("Line 0 should contain 'Review', got: %s", lines[0]) + } + + // Line 1: Location (ref) + if !strings.Contains(lines[1], "abc1234") { + t.Errorf("Line 1 should contain ref 'abc1234', got: %s", lines[1]) + } + + // Line 2: [ADDRESSED] (no verdict) + if !strings.Contains(lines[2], "[ADDRESSED]") { + t.Errorf("Line 2 should contain '[ADDRESSED]', got: %s", lines[2]) + } + if strings.Contains(lines[2], "Verdict") { + t.Errorf("Line 2 should not contain 'Verdict' when no verdict is set, got: %s", lines[2]) + } + + // Line 3: Content + if !strings.Contains(lines[3], "Line 1") { + t.Errorf("Line 3 should contain content 'Line 1', got: %s", lines[3]) + } +} + func TestTUIBranchClearedOnFailedJobNavigation(t *testing.T) { // Test that navigating from a successful review with branch to a failed job clears the branch m := newTuiModel("http://localhost") @@ -4301,15 +4429,15 @@ func TestTUIVisibleLinesCalculationNarrowTerminalWithVerdict(t *testing.T) { } func TestTUIVisibleLinesCalculationLongTitleWraps(t *testing.T) { - // Test that long titles correctly wrap and reduce visible lines + // Test that long titles and location lines correctly wrap and reduce visible lines // New layout: // - Title: "Review #1 very-long-repository-name-here (claude-code)" = ~54 chars, ceil(54/50) = 2 lines - // - Location line: repo + ref + branch = 1 line + // - Location line: "very-long-repository-name-here abc1234567890..de on feature/very-long-branch-name" = 81 chars, ceil(81/50) = 2 lines // - Addressed line: 1 line (since Addressed=true) // - Status line: 1 line // - Help: ceil(91/50) = 2 lines - // Non-content: 2 + 1 + 1 + 1 + 2 = 7 - // visibleLines = 12 - 7 = 5 + // Non-content: 2 + 2 + 1 + 1 + 2 = 8 + // visibleLines = 12 - 8 = 4 m := newTuiModel("http://localhost") m.width = 50 m.height = 12 @@ -4338,14 +4466,14 @@ func TestTUIVisibleLinesCalculationLongTitleWraps(t *testing.T) { } } - expectedContent := 5 + expectedContent := 4 if contentCount != expectedContent { t.Errorf("Expected %d content lines with long wrapping title, got %d", expectedContent, contentCount) } // Should show scroll indicator with correct range - if !strings.Contains(output, "[1-5 of 20 lines]") { - t.Errorf("Expected scroll indicator '[1-5 of 20 lines]', output: %s", output) + if !strings.Contains(output, "[1-4 of 20 lines]") { + t.Errorf("Expected scroll indicator '[1-4 of 20 lines]', output: %s", output) } // Should contain the long repo name and branch @@ -6134,8 +6262,91 @@ func TestFormatClipboardContent(t *testing.T) { } } +func TestTUIVersionMismatchDetection(t *testing.T) { + setupTuiTestEnv(t) + + t.Run("detects version mismatch", func(t *testing.T) { + m := newTuiModel(testServerAddr) + + // Simulate receiving status with different version + status := tuiStatusMsg(storage.DaemonStatus{ + Version: "different-version", + }) + + updated, _ := m.Update(status) + m2 := updated.(tuiModel) + + if !m2.versionMismatch { + t.Error("Expected versionMismatch=true when daemon version differs") + } + if m2.daemonVersion != "different-version" { + t.Errorf("Expected daemonVersion='different-version', got %q", m2.daemonVersion) + } + }) + + t.Run("no mismatch when versions match", func(t *testing.T) { + m := newTuiModel(testServerAddr) + + // Simulate receiving status with same version as TUI + status := tuiStatusMsg(storage.DaemonStatus{ + Version: version.Version, + }) + + updated, _ := m.Update(status) + m2 := updated.(tuiModel) + + if m2.versionMismatch { + t.Error("Expected versionMismatch=false when versions match") + } + }) + + t.Run("displays error banner in queue view when mismatched", func(t *testing.T) { + m := newTuiModel(testServerAddr) + m.width = 100 + m.height = 30 + m.currentView = tuiViewQueue + m.versionMismatch = true + m.daemonVersion = "old-version" + + output := m.View() + + if !strings.Contains(output, "VERSION MISMATCH") { + t.Error("Expected queue view to show VERSION MISMATCH error") + } + if !strings.Contains(output, "old-version") { + t.Error("Expected error to show daemon version") + } + }) + + t.Run("displays error banner in review view when mismatched", func(t *testing.T) { + m := newTuiModel(testServerAddr) + m.width = 100 + m.height = 30 + m.currentView = tuiViewReview + m.versionMismatch = true + m.daemonVersion = "old-version" + m.currentReview = &storage.Review{ + ID: 1, + Output: "Test review", + Job: &storage.ReviewJob{ + ID: 1, + GitRef: "abc123", + RepoName: "test", + Agent: "test", + }, + } + + output := m.View() + + if !strings.Contains(output, "VERSION MISMATCH") { + t.Error("Expected review view to show VERSION MISMATCH error") + } + }) +} + func TestTUIConfigReloadFlash(t *testing.T) { - m := newTuiModel("http://localhost:7373") + setupTuiTestEnv(t) + m := newTuiModel(testServerAddr) t.Run("no flash on first status fetch", func(t *testing.T) { // First status fetch with a ConfigReloadCounter should NOT flash @@ -6160,7 +6371,7 @@ func TestTUIConfigReloadFlash(t *testing.T) { t.Run("flash on config reload after first fetch", func(t *testing.T) { // Start with a model that has already fetched status once - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.statusFetchedOnce = true m.lastConfigReloadCounter = 1 @@ -6183,7 +6394,7 @@ func TestTUIConfigReloadFlash(t *testing.T) { t.Run("flash when ConfigReloadCounter changes from zero to non-zero", func(t *testing.T) { // Model has fetched status once but daemon hadn't reloaded yet - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.statusFetchedOnce = true m.lastConfigReloadCounter = 0 // No reload had occurred @@ -6202,7 +6413,7 @@ func TestTUIConfigReloadFlash(t *testing.T) { }) t.Run("no flash when ConfigReloadCounter unchanged", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.statusFetchedOnce = true m.lastConfigReloadCounter = 1 @@ -6222,8 +6433,9 @@ func TestTUIConfigReloadFlash(t *testing.T) { } func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { + setupTuiTestEnv(t) t.Run("triggers reconnection after 3 consecutive connection errors", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 2 // Already had 2 connection errors // Third connection error should trigger reconnection @@ -6242,7 +6454,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("does not trigger reconnection before 3 errors", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 1 // Only 1 error so far updated, cmd := m.Update(tuiJobsErrMsg{err: mockConnError("connection refused")}) @@ -6260,7 +6472,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("does not count application errors for reconnection", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 2 // 2 connection errors // Application error (404, parse error, etc.) should not increment counter @@ -6279,7 +6491,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("does not count non-connection errors in jobs fetch", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 2 // Non-connection error (like parse error) should not increment @@ -6292,7 +6504,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("pagination errors also trigger reconnection", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 2 // Connection error in pagination should trigger reconnection @@ -6311,7 +6523,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("status/review connection errors trigger reconnection", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 2 // Connection error via tuiErrMsg (from fetchStatus, fetchReview, etc.) @@ -6330,7 +6542,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("status/review application errors do not trigger reconnection", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 2 // Application error via tuiErrMsg should not increment @@ -6349,7 +6561,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("resets error count on successful jobs fetch", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 5 updated, _ := m.Update(tuiJobsMsg{jobs: []storage.ReviewJob{}, hasMore: false}) @@ -6361,7 +6573,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("resets error count on successful status fetch", func(t *testing.T) { - m := newTuiModel("http://localhost:7373") + m := newTuiModel(testServerAddr) m.consecutiveErrors = 5 updated, _ := m.Update(tuiStatusMsg(storage.DaemonStatus{Version: "1.0.0"})) @@ -6373,7 +6585,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("updates server address on successful reconnection", func(t *testing.T) { - m := newTuiModel("http://127.0.0.1:7373") + m := newTuiModel(testServerAddr) m.reconnecting = true updated, cmd := m.Update(tuiReconnectMsg{newAddr: "http://127.0.0.1:7374", version: "2.0.0"}) @@ -6400,15 +6612,15 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("handles reconnection to same address", func(t *testing.T) { - m := newTuiModel("http://127.0.0.1:7373") + m := newTuiModel(testServerAddr) m.reconnecting = true m.consecutiveErrors = 3 // Same address - no change needed - updated, cmd := m.Update(tuiReconnectMsg{newAddr: "http://127.0.0.1:7373"}) + updated, cmd := m.Update(tuiReconnectMsg{newAddr: testServerAddr}) m2 := updated.(tuiModel) - if m2.serverAddr != "http://127.0.0.1:7373" { + if m2.serverAddr != testServerAddr { t.Errorf("Expected serverAddr unchanged, got %s", m2.serverAddr) } if m2.reconnecting { @@ -6421,7 +6633,7 @@ func TestTUIReconnectOnConsecutiveErrors(t *testing.T) { }) t.Run("handles failed reconnection", func(t *testing.T) { - m := newTuiModel("http://127.0.0.1:7373") + m := newTuiModel(testServerAddr) m.reconnecting = true m.consecutiveErrors = 3 diff --git a/internal/storage/reviews_test.go b/internal/storage/reviews_test.go index 7b34820..acd3f99 100644 --- a/internal/storage/reviews_test.go +++ b/internal/storage/reviews_test.go @@ -234,6 +234,54 @@ func TestAddCommentToJobWithNoReview(t *testing.T) { } } +func TestGetReviewByJobIDIncludesModel(t *testing.T) { + db := openReviewsTestDB(t) + defer db.Close() + + repo, err := db.GetOrCreateRepo("/tmp/test-repo") + if err != nil { + t.Fatalf("GetOrCreateRepo failed: %v", err) + } + + tests := []struct { + name string + gitRef string + model string + expectedModel string + }{ + {"model is populated when set", "abc123", "o3", "o3"}, + {"model is empty when not set", "def456", "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + job, err := db.EnqueueRangeJob(repo.ID, tt.gitRef, "codex", tt.model, "thorough") + if err != nil { + t.Fatalf("EnqueueRangeJob failed: %v", err) + } + + // Claim job to move to running, then complete it + db.ClaimJob("test-worker") + err = db.CompleteJob(job.ID, "codex", "test prompt", "Test review output\n\n## Verdict: PASS") + if err != nil { + t.Fatalf("CompleteJob failed: %v", err) + } + + review, err := db.GetReviewByJobID(job.ID) + if err != nil { + t.Fatalf("GetReviewByJobID failed: %v", err) + } + + if review.Job == nil { + t.Fatal("Expected review.Job to be populated") + } + if review.Job.Model != tt.expectedModel { + t.Errorf("Expected model %q, got %q", tt.expectedModel, review.Job.Model) + } + }) + } +} + func openReviewsTestDB(t *testing.T) *DB { tmpDir := t.TempDir() dbPath := filepath.Join(tmpDir, "test.db")