Skip to content

Commit 980b749

Browse files
wesmclaude
wesm
and
claude
authored
Add 'y' hotkey to copy review content to clipboard (#90)
## Summary - Press `y` in review view or queue view to copy review content to clipboard - Shows "Copied to clipboard" flash message on success - Cross-platform support via `atotto/clipboard` (no CGO required) ## Changes - Add clipboard interface with mockable implementation for testing - Add flash message system for user feedback - Update help text in queue and review views - Comprehensive test coverage including failure scenarios ## CI Improvements - Add macOS and Windows to test matrix - Add CGO_ENABLED=0 build step to verify release compatibility - Move coverage to separate Ubuntu-only job ## Test plan - [x] Unit tests for copy from review view - [x] Unit tests for copy from queue view (fetch + copy) - [x] Tests for clipboard write failures - [x] Tests for 404 and empty output handling - [x] Flash message rendering tests - [x] All tests pass on Linux - [x] CI validates macOS and Windows 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1a220ce commit 980b749

File tree

17 files changed

+731
-73
lines changed

17 files changed

+731
-73
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ on:
88

99
jobs:
1010
test:
11-
runs-on: ubuntu-latest
11+
strategy:
12+
matrix:
13+
os: [ubuntu-latest, macos-latest, windows-latest]
14+
runs-on: ${{ matrix.os }}
1215
steps:
1316
- uses: actions/checkout@v4
1417

@@ -19,7 +22,29 @@ jobs:
1922
- name: Build
2023
run: go build ./...
2124

22-
- name: Test with race detection
25+
- name: Test with race detection (Linux/macOS)
26+
if: matrix.os != 'windows-latest'
27+
run: go test -race ./...
28+
29+
- name: Test (Windows)
30+
if: matrix.os == 'windows-latest'
31+
run: go test ./...
32+
33+
- name: Build with CGO disabled
34+
run: go build ./...
35+
env:
36+
CGO_ENABLED: 0
37+
38+
coverage:
39+
runs-on: ubuntu-latest
40+
steps:
41+
- uses: actions/checkout@v4
42+
43+
- uses: actions/setup-go@v5
44+
with:
45+
go-version: '1.24'
46+
47+
- name: Test with coverage
2348
run: go test -race -coverprofile=coverage.out ./...
2449

2550
- name: Upload coverage

‎cmd/roborev/client_test.go‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,11 @@ func TestWaitForReview(t *testing.T) {
176176

177177
func TestFindJobForCommit(t *testing.T) {
178178
t.Run("finds matching job", func(t *testing.T) {
179+
// Use a real temp dir so path normalization works cross-platform
180+
repoDir := t.TempDir()
179181
allJobs := []storage.ReviewJob{
180-
{ID: 1, GitRef: "abc123", RepoPath: "/test/repo"},
181-
{ID: 2, GitRef: "def456", RepoPath: "/test/repo"},
182+
{ID: 1, GitRef: "abc123", RepoPath: repoDir},
183+
{ID: 2, GitRef: "def456", RepoPath: repoDir},
182184
}
183185
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
184186
if r.URL.Path != "/api/jobs" || r.Method != "GET" {
@@ -201,7 +203,7 @@ func TestFindJobForCommit(t *testing.T) {
201203
}))
202204
defer cleanup()
203205

204-
job, err := findJobForCommit("/test/repo", "def456")
206+
job, err := findJobForCommit(repoDir, "def456")
205207
if err != nil {
206208
t.Fatalf("unexpected error: %v", err)
207209
}
@@ -214,8 +216,9 @@ func TestFindJobForCommit(t *testing.T) {
214216
})
215217

216218
t.Run("returns nil when not found", func(t *testing.T) {
219+
repoDir := t.TempDir()
217220
allJobs := []storage.ReviewJob{
218-
{ID: 1, GitRef: "abc123", RepoPath: "/test/repo"},
221+
{ID: 1, GitRef: "abc123", RepoPath: repoDir},
219222
}
220223
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
221224
if r.URL.Path != "/api/jobs" || r.Method != "GET" {
@@ -238,7 +241,7 @@ func TestFindJobForCommit(t *testing.T) {
238241
}))
239242
defer cleanup()
240243

241-
job, err := findJobForCommit("/test/repo", "notfound")
244+
job, err := findJobForCommit(repoDir, "notfound")
242245
if err != nil {
243246
t.Fatalf("unexpected error: %v", err)
244247
}
@@ -249,9 +252,12 @@ func TestFindJobForCommit(t *testing.T) {
249252

250253
t.Run("fallback skips jobs from different repo", func(t *testing.T) {
251254
// Primary query returns empty (repo mismatch), fallback returns job from different repo
255+
otherRepo := t.TempDir()
256+
anotherRepo := t.TempDir()
257+
queryRepo := t.TempDir()
252258
allJobs := []storage.ReviewJob{
253-
{ID: 1, GitRef: "abc123", RepoPath: "/other/repo"},
254-
{ID: 2, GitRef: "abc123", RepoPath: "/another/repo"},
259+
{ID: 1, GitRef: "abc123", RepoPath: otherRepo},
260+
{ID: 2, GitRef: "abc123", RepoPath: anotherRepo},
255261
}
256262
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
257263
if r.URL.Path != "/api/jobs" || r.Method != "GET" {
@@ -273,8 +279,8 @@ func TestFindJobForCommit(t *testing.T) {
273279
}))
274280
defer cleanup()
275281

276-
// Request job for /test/repo, but all jobs are for different repos
277-
job, err := findJobForCommit("/test/repo", "abc123")
282+
// Request job for queryRepo, but all jobs are for different repos
283+
job, err := findJobForCommit(queryRepo, "abc123")
278284
if err != nil {
279285
t.Fatalf("unexpected error: %v", err)
280286
}
@@ -347,13 +353,14 @@ func TestFindJobForCommit(t *testing.T) {
347353
})
348354

349355
t.Run("returns error on invalid JSON", func(t *testing.T) {
356+
repoDir := t.TempDir()
350357
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
351358
w.WriteHeader(http.StatusOK)
352359
w.Write([]byte("not json"))
353360
}))
354361
defer cleanup()
355362

356-
_, err := findJobForCommit("/test/repo", "abc123")
363+
_, err := findJobForCommit(repoDir, "abc123")
357364
if err == nil {
358365
t.Fatal("expected error on invalid JSON")
359366
}
@@ -366,10 +373,11 @@ func TestFindJobForCommit(t *testing.T) {
366373
// Jobs with empty or relative paths should be skipped in fallback to avoid
367374
// false matches from cwd resolution. Job 3 has correct SHA but path stored
368375
// differently (simulating path normalization mismatch).
376+
repoDir := t.TempDir()
369377
allJobs := []storage.ReviewJob{
370378
{ID: 1, GitRef: "abc123", RepoPath: ""}, // Empty path - skip
371379
{ID: 2, GitRef: "abc123", RepoPath: "relative/path"}, // Relative path - skip
372-
{ID: 3, GitRef: "abc123", RepoPath: "/test/repo"}, // Absolute path - match via fallback
380+
{ID: 3, GitRef: "abc123", RepoPath: repoDir}, // Absolute path - match via fallback
373381
}
374382
requestCount := 0
375383
_, cleanup := setupMockDaemon(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -405,9 +413,9 @@ func TestFindJobForCommit(t *testing.T) {
405413
}))
406414
defer cleanup()
407415

408-
// Request job for /test/repo - primary query returns empty, fallback should
416+
// Request job for repoDir - primary query returns empty, fallback should
409417
// skip empty/relative paths and find job ID 3 via path normalization
410-
job, err := findJobForCommit("/test/repo", "abc123")
418+
job, err := findJobForCommit(repoDir, "abc123")
411419
if err != nil {
412420
t.Fatalf("unexpected error: %v", err)
413421
}

‎cmd/roborev/main_test.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"os"
1818
"os/exec"
1919
"path/filepath"
20+
"runtime"
2021
"strings"
2122
"sync"
2223
"sync/atomic"
@@ -40,23 +41,23 @@ func setupMockDaemon(t *testing.T, handler http.Handler) (*httptest.Server, func
4041

4142
ts := httptest.NewServer(handler)
4243

43-
// Override HOME
44-
tmpHome := t.TempDir()
45-
origHome := os.Getenv("HOME")
46-
os.Setenv("HOME", tmpHome)
44+
// Use ROBOREV_DATA_DIR to override data directory (works cross-platform)
45+
// On Windows, HOME doesn't work since os.UserHomeDir() uses USERPROFILE
46+
tmpDir := t.TempDir()
47+
origDataDir := os.Getenv("ROBOREV_DATA_DIR")
48+
os.Setenv("ROBOREV_DATA_DIR", tmpDir)
4749

4850
// Write fake daemon.json
49-
roborevDir := filepath.Join(tmpHome, ".roborev")
50-
if err := os.MkdirAll(roborevDir, 0755); err != nil {
51-
t.Fatalf("failed to create roborev dir: %v", err)
51+
if err := os.MkdirAll(tmpDir, 0755); err != nil {
52+
t.Fatalf("failed to create data dir: %v", err)
5253
}
5354
mockAddr := ts.URL[7:] // strip "http://"
5455
daemonInfo := daemon.RuntimeInfo{Addr: mockAddr, PID: os.Getpid(), Version: version.Version}
5556
data, err := json.Marshal(daemonInfo)
5657
if err != nil {
5758
t.Fatalf("failed to marshal daemon.json: %v", err)
5859
}
59-
if err := os.WriteFile(filepath.Join(roborevDir, "daemon.json"), data, 0644); err != nil {
60+
if err := os.WriteFile(filepath.Join(tmpDir, "daemon.json"), data, 0644); err != nil {
6061
t.Fatalf("failed to write daemon.json: %v", err)
6162
}
6263

@@ -66,7 +67,11 @@ func setupMockDaemon(t *testing.T, handler http.Handler) (*httptest.Server, func
6667

6768
cleanup := func() {
6869
ts.Close()
69-
os.Setenv("HOME", origHome)
70+
if origDataDir != "" {
71+
os.Setenv("ROBOREV_DATA_DIR", origDataDir)
72+
} else {
73+
os.Unsetenv("ROBOREV_DATA_DIR")
74+
}
7075
serverAddr = origServerAddr
7176
}
7277

@@ -1426,6 +1431,10 @@ func TestShowJobFlagRequiresArgument(t *testing.T) {
14261431
// ============================================================================
14271432

14281433
func TestDaemonRunStartsAndShutdownsCleanly(t *testing.T) {
1434+
if runtime.GOOS == "windows" {
1435+
t.Skip("skipping daemon integration test on Windows due to file locking differences")
1436+
}
1437+
14291438
// Use temp directories for isolation
14301439
tmpDir := t.TempDir()
14311440
dbPath := filepath.Join(tmpDir, "test.db")

‎cmd/roborev/refine_test.go‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"os/exec"
77
"path/filepath"
8+
"runtime"
89
"strings"
910
"testing"
1011

@@ -217,9 +218,18 @@ func TestResolveAllowUnsafeAgents(t *testing.T) {
217218

218219
func TestSelectRefineAgentCodexUsesRequestedReasoning(t *testing.T) {
219220
tmpDir := t.TempDir()
220-
codexPath := filepath.Join(tmpDir, "codex")
221-
if err := os.WriteFile(codexPath, []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil {
222-
t.Fatalf("write codex stub: %v", err)
221+
var codexPath string
222+
if runtime.GOOS == "windows" {
223+
// On Windows, create a batch file that exits successfully
224+
codexPath = filepath.Join(tmpDir, "codex.bat")
225+
if err := os.WriteFile(codexPath, []byte("@exit /b 0\r\n"), 0755); err != nil {
226+
t.Fatalf("write codex stub: %v", err)
227+
}
228+
} else {
229+
codexPath = filepath.Join(tmpDir, "codex")
230+
if err := os.WriteFile(codexPath, []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil {
231+
t.Fatalf("write codex stub: %v", err)
232+
}
223233
}
224234

225235
t.Setenv("PATH", tmpDir)
@@ -240,9 +250,18 @@ func TestSelectRefineAgentCodexUsesRequestedReasoning(t *testing.T) {
240250

241251
func TestSelectRefineAgentCodexFallbackUsesRequestedReasoning(t *testing.T) {
242252
tmpDir := t.TempDir()
243-
codexPath := filepath.Join(tmpDir, "codex")
244-
if err := os.WriteFile(codexPath, []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil {
245-
t.Fatalf("write codex stub: %v", err)
253+
var codexPath string
254+
if runtime.GOOS == "windows" {
255+
// On Windows, create a batch file that exits successfully
256+
codexPath = filepath.Join(tmpDir, "codex.bat")
257+
if err := os.WriteFile(codexPath, []byte("@exit /b 0\r\n"), 0755); err != nil {
258+
t.Fatalf("write codex stub: %v", err)
259+
}
260+
} else {
261+
codexPath = filepath.Join(tmpDir, "codex")
262+
if err := os.WriteFile(codexPath, []byte("#!/bin/sh\nexit 0\n"), 0755); err != nil {
263+
t.Fatalf("write codex stub: %v", err)
264+
}
246265
}
247266

248267
t.Setenv("PATH", tmpDir)

0 commit comments

Comments
 (0)