Skip to content

Commit c552730

Browse files
wesmclaude
andauthored
Fix clipboard header to use job ID for consistency (#110)
## Summary - Fix clipboard yank header to use job ID instead of review ID - Queue view and review screen display job IDs, so clipboard should match ## Changes - Always use `job.ID` when `Job` struct is available - Fall back to `review.JobID` when `Job` struct is not populated - Updated tests to reflect job ID usage ## Test plan - [x] All existing clipboard tests pass - [x] `TestFormatClipboardContent` updated to verify job ID is used - [x] `TestTUIFetchReviewAndCopySuccess` and `TestTUIFetchReviewAndCopyJobInjection` updated 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent c976927 commit c552730

File tree

2 files changed

+73
-25
lines changed

2 files changed

+73
-25
lines changed

cmd/roborev/tui.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -553,21 +553,30 @@ func formatClipboardContent(review *storage.Review) string {
553553
}
554554

555555
// Build header: "Review #ID /repo/path abc1234"
556-
// Use job ID if review ID is 0 (e.g., failed jobs with no review record)
556+
// Always use job ID for consistency with queue and review screen display
557+
// ID priority: Job.ID → JobID → review.ID
558+
var id int64
559+
if review.Job != nil && review.Job.ID != 0 {
560+
id = review.Job.ID
561+
} else if review.JobID != 0 {
562+
id = review.JobID
563+
} else {
564+
id = review.ID
565+
}
566+
557567
var header string
558-
if review.Job != nil {
559-
gitRef := review.Job.GitRef
560-
// Truncate SHA to 7 chars if it's a full 40-char hex SHA (not a range or branch name)
561-
if fullSHAPattern.MatchString(gitRef) {
562-
gitRef = gitRef[:7]
563-
}
564-
id := review.ID
565-
if id == 0 {
566-
id = review.Job.ID
567-
}
568-
header = fmt.Sprintf("Review #%d %s %s\n\n", id, review.Job.RepoPath, gitRef)
569-
} else if review.ID != 0 {
570-
header = fmt.Sprintf("Review #%d\n\n", review.ID)
568+
if id != 0 {
569+
if review.Job != nil && review.Job.RepoPath != "" {
570+
// Include repo path and git ref when available
571+
gitRef := review.Job.GitRef
572+
// Truncate SHA to 7 chars if it's a full 40-char hex SHA (not a range or branch name)
573+
if fullSHAPattern.MatchString(gitRef) {
574+
gitRef = gitRef[:7]
575+
}
576+
header = fmt.Sprintf("Review #%d %s %s\n\n", id, review.Job.RepoPath, gitRef)
577+
} else {
578+
header = fmt.Sprintf("Review #%d\n\n", id)
579+
}
571580
}
572581

573582
return header + review.Output

cmd/roborev/tui_test.go

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5278,8 +5278,8 @@ func TestTUIFetchReviewAndCopySuccess(t *testing.T) {
52785278
t.Errorf("Expected no error, got %v", result.err)
52795279
}
52805280

5281-
// Clipboard should contain header + review content
5282-
expectedContent := "Review #1\n\nReview content for clipboard"
5281+
// Clipboard should contain header with JobID + review content
5282+
expectedContent := "Review #123\n\nReview content for clipboard"
52835283
if mock.lastText != expectedContent {
52845284
t.Errorf("Expected clipboard to contain review with header, got %q", mock.lastText)
52855285
}
@@ -5462,8 +5462,8 @@ func TestTUIFetchReviewAndCopyJobInjection(t *testing.T) {
54625462
t.Errorf("Expected no error, got %v", result.err)
54635463
}
54645464

5465-
// Clipboard should contain header with injected job info (truncated SHA)
5466-
expectedContent := "Review #42 /path/to/repo a1b2c3d\n\nReview content"
5465+
// Clipboard should contain header with injected job info (job ID, truncated SHA)
5466+
expectedContent := "Review #123 /path/to/repo a1b2c3d\n\nReview content"
54675467
if mock.lastText != expectedContent {
54685468
t.Errorf("Expected clipboard with injected job info, got %q", mock.lastText)
54695469
}
@@ -5489,17 +5489,28 @@ func TestFormatClipboardContent(t *testing.T) {
54895489
expected: "",
54905490
},
54915491
{
5492-
name: "review with ID only (no job)",
5492+
name: "review with JobID only (no job struct)",
54935493
review: &storage.Review{
5494-
ID: 42,
5494+
ID: 99, // review.ID is different from JobID
5495+
JobID: 42,
54955496
Output: "Content here",
54965497
},
54975498
expected: "Review #42\n\nContent here",
54985499
},
54995500
{
5500-
name: "review with ID 0 and no job (no header)",
5501+
name: "review with JobID 0 but review ID set (legacy fallback)",
5502+
review: &storage.Review{
5503+
ID: 77,
5504+
JobID: 0,
5505+
Output: "Content here",
5506+
},
5507+
expected: "Review #77\n\nContent here",
5508+
},
5509+
{
5510+
name: "review with all IDs 0 and no job struct (no header)",
55015511
review: &storage.Review{
55025512
ID: 0,
5513+
JobID: 0,
55035514
Output: "Content here",
55045515
},
55055516
expected: "Content here",
@@ -5544,17 +5555,45 @@ func TestFormatClipboardContent(t *testing.T) {
55445555
expected: "Review #100 /path/to/repo abc1234..def5678\n\nReview content",
55455556
},
55465557
{
5547-
name: "review ID 0 uses job ID instead",
5558+
name: "always uses job ID from Job struct",
55485559
review: &storage.Review{
5549-
ID: 0,
5550-
Output: "Failed job content",
5560+
ID: 999, // review.ID is ignored when Job is present with valid ID
5561+
Output: "Review content",
55515562
Job: &storage.ReviewJob{
55525563
ID: 555,
55535564
RepoPath: "/repo/path",
55545565
GitRef: "abcdef1234567890abcdef1234567890abcdef12",
55555566
},
55565567
},
5557-
expected: "Review #555 /repo/path abcdef1\n\nFailed job content",
5568+
expected: "Review #555 /repo/path abcdef1\n\nReview content",
5569+
},
5570+
{
5571+
name: "Job present but Job.ID is 0 falls back to JobID with context",
5572+
review: &storage.Review{
5573+
ID: 999,
5574+
JobID: 123,
5575+
Output: "Review content",
5576+
Job: &storage.ReviewJob{
5577+
ID: 0, // zero ID, should fall back to JobID
5578+
RepoPath: "/repo/path",
5579+
GitRef: "abc1234",
5580+
},
5581+
},
5582+
expected: "Review #123 /repo/path abc1234\n\nReview content",
5583+
},
5584+
{
5585+
name: "Job present but Job.ID is 0 falls back to review.ID with context",
5586+
review: &storage.Review{
5587+
ID: 999,
5588+
JobID: 0,
5589+
Output: "Review content",
5590+
Job: &storage.ReviewJob{
5591+
ID: 0, // zero ID, should fall back to review.ID
5592+
RepoPath: "/repo/path",
5593+
GitRef: "abc1234",
5594+
},
5595+
},
5596+
expected: "Review #999 /repo/path abc1234\n\nReview content",
55585597
},
55595598
{
55605599
name: "short git ref not truncated",

0 commit comments

Comments
 (0)