From e6eb6e14c9f2d177952cbe41355b634814d7f546 Mon Sep 17 00:00:00 2001
From: Wes McKinney <wesmckinn+git@gmail.com>
Date: Mon, 26 Jan 2026 10:01:22 -0500
Subject: [PATCH 1/5] Allow multiple clones of the same repository (#131)

Remove unique constraint on repos.identity to allow multiple local clones
of the same remote repository. The identity column is for correlating repos
during sync, not enforcing uniqueness.

This fixes the error:
  UNIQUE constraint failed: repos.identity (2067)

Changes:
- Migration now creates a non-unique index on repos.identity
- Migration detects and converts existing unique indexes to non-unique
- Removed duplicate identity check that blocked migration
- Added test for multiple clones with same identity
- Updated migration tests to verify duplicates are now allowed

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 internal/storage/db.go        |  33 +++++---
 internal/storage/db_test.go   |  55 +++++++++++++
 internal/storage/sync_test.go | 146 +++++++++++++++++++++++++++++-----
 3 files changed, 199 insertions(+), 35 deletions(-)

diff --git a/internal/storage/db.go b/internal/storage/db.go
index b4d14d7..c413f27 100644
--- a/internal/storage/db.go
+++ b/internal/storage/db.go
@@ -660,27 +660,34 @@ func (db *DB) migrateSyncColumns() error {
 		}
 	}
 
-	// Create unique index on repos.identity (allows NULL values, only enforces uniqueness on non-NULL)
-	// First normalize empty strings to NULL (treat empty as "unset")
+	// Normalize empty strings to NULL (treat empty as "unset")
 	_, err = db.Exec(`UPDATE repos SET identity = NULL WHERE identity = ''`)
 	if err != nil {
 		return fmt.Errorf("normalize empty identities to NULL: %w", err)
 	}
 
-	// Check for duplicates that would prevent index creation
-	var dupCount int
-	err = db.QueryRow(`SELECT COUNT(*) FROM (
-		SELECT identity FROM repos WHERE identity IS NOT NULL
-		GROUP BY identity HAVING COUNT(*) > 1
-	)`).Scan(&dupCount)
-	if err != nil {
-		return fmt.Errorf("check duplicate identities: %w", err)
+	// Create non-unique index on repos.identity for query performance.
+	// Note: identity is NOT unique because multiple local clones of the same repo
+	// (e.g., ~/project-1 and ~/project-2 both cloned from the same remote)
+	// should be allowed and will share the same identity.
+	// See: https://github.com/roborev-dev/roborev/issues/131
+
+	// Migration: If an old UNIQUE index exists, drop it first
+	var indexSQL sql.NullString
+	err = db.QueryRow(`SELECT sql FROM sqlite_master WHERE type='index' AND name='idx_repos_identity'`).Scan(&indexSQL)
+	if err != nil && err != sql.ErrNoRows {
+		return fmt.Errorf("check existing idx_repos_identity: %w", err)
 	}
-	if dupCount > 0 {
-		return fmt.Errorf("cannot create unique index on repos.identity: %d duplicate non-NULL identities exist; resolve duplicates before upgrading", dupCount)
+	if indexSQL.Valid && strings.Contains(strings.ToUpper(indexSQL.String), "UNIQUE") {
+		// Drop the old unique index
+		_, err = db.Exec(`DROP INDEX idx_repos_identity`)
+		if err != nil {
+			return fmt.Errorf("drop old unique idx_repos_identity: %w", err)
+		}
 	}
 
-	_, err = db.Exec(`CREATE UNIQUE INDEX IF NOT EXISTS idx_repos_identity ON repos(identity) WHERE identity IS NOT NULL`)
+	// Create non-unique index (or recreate after dropping unique)
+	_, err = db.Exec(`CREATE INDEX IF NOT EXISTS idx_repos_identity ON repos(identity) WHERE identity IS NOT NULL`)
 	if err != nil {
 		return fmt.Errorf("create idx_repos_identity: %w", err)
 	}
diff --git a/internal/storage/db_test.go b/internal/storage/db_test.go
index 210f32b..0e18b15 100644
--- a/internal/storage/db_test.go
+++ b/internal/storage/db_test.go
@@ -1906,6 +1906,61 @@ func TestRepoIdentity(t *testing.T) {
 			t.Errorf("Expected identity to remain 'original-identity', got %q", repo2.Identity)
 		}
 	})
+
+	t.Run("multiple clones with same identity allowed", func(t *testing.T) {
+		// This tests the fix for https://github.com/roborev-dev/roborev/issues/131
+		// Multiple clones of the same repo (e.g., ~/project-1 and ~/project-2 both
+		// cloned from the same remote) should be allowed and share the same identity.
+		db := openTestDB(t)
+		defer db.Close()
+
+		sharedIdentity := "git@github.com:org/shared-repo.git"
+
+		// Create first clone
+		repo1, err := db.GetOrCreateRepo("/tmp/clone-1", sharedIdentity)
+		if err != nil {
+			t.Fatalf("GetOrCreateRepo for clone-1 failed: %v", err)
+		}
+		if repo1.Identity != sharedIdentity {
+			t.Errorf("Expected identity %q for clone-1, got %q", sharedIdentity, repo1.Identity)
+		}
+
+		// Create second clone with same identity - should succeed (was failing before fix)
+		repo2, err := db.GetOrCreateRepo("/tmp/clone-2", sharedIdentity)
+		if err != nil {
+			t.Fatalf("GetOrCreateRepo for clone-2 failed: %v (multiple clones with same identity should be allowed)", err)
+		}
+		if repo2.Identity != sharedIdentity {
+			t.Errorf("Expected identity %q for clone-2, got %q", sharedIdentity, repo2.Identity)
+		}
+
+		// Verify they are different repos
+		if repo1.ID == repo2.ID {
+			t.Errorf("Expected different repo IDs, but both are %d", repo1.ID)
+		}
+		if repo1.RootPath == repo2.RootPath {
+			t.Errorf("Expected different root paths")
+		}
+
+		// Verify both repos exist and can be retrieved
+		repos, err := db.ListRepos()
+		if err != nil {
+			t.Fatalf("ListRepos failed: %v", err)
+		}
+
+		foundClone1, foundClone2 := false, false
+		for _, r := range repos {
+			if r.ID == repo1.ID {
+				foundClone1 = true
+			}
+			if r.ID == repo2.ID {
+				foundClone2 = true
+			}
+		}
+		if !foundClone1 || !foundClone2 {
+			t.Errorf("Expected both clones to exist in ListRepos, found clone1=%v clone2=%v", foundClone1, foundClone2)
+		}
+	})
 }
 
 func TestDuplicateSHAHandling(t *testing.T) {
diff --git a/internal/storage/sync_test.go b/internal/storage/sync_test.go
index 1c66f83..c007ed5 100644
--- a/internal/storage/sync_test.go
+++ b/internal/storage/sync_test.go
@@ -680,9 +680,10 @@ func openRawDB(dbPath string) (*sql.DB, error) {
 	return sql.Open("sqlite", dbPath+"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)")
 }
 
-func TestDuplicateRepoIdentity_MigrationError(t *testing.T) {
-	// This test verifies that migration fails with a clear error if duplicate
-	// non-NULL repos.identity values exist before creating the unique index.
+func TestDuplicateRepoIdentity_MigrationSuccess(t *testing.T) {
+	// This test verifies that migration succeeds even when duplicate
+	// repos.identity values exist. Multiple clones of the same repo
+	// should be allowed (fix for https://github.com/roborev-dev/roborev/issues/131).
 	dbPath := filepath.Join(t.TempDir(), "test.db")
 
 	rawDB, err := openRawDB(dbPath)
@@ -690,7 +691,7 @@ func TestDuplicateRepoIdentity_MigrationError(t *testing.T) {
 		t.Fatalf("Failed to open raw database: %v", err)
 	}
 
-	// Create schema with identity column but no unique index (simulates partial migration)
+	// Create schema with identity column but no index (simulates partial migration)
 	_, err = rawDB.Exec(`
 		CREATE TABLE repos (
 			id INTEGER PRIMARY KEY,
@@ -751,13 +752,13 @@ func TestDuplicateRepoIdentity_MigrationError(t *testing.T) {
 		t.Fatalf("Failed to create schema: %v", err)
 	}
 
-	// Insert two repos with the same identity (duplicate)
-	_, err = rawDB.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/repo1', 'repo1', 'dup-identity')`)
+	// Insert two repos with the same identity (e.g., two clones of same remote)
+	_, err = rawDB.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/repo1', 'repo1', 'git@github.com:org/repo.git')`)
 	if err != nil {
 		rawDB.Close()
 		t.Fatalf("Failed to insert repo1: %v", err)
 	}
-	_, err = rawDB.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/repo2', 'repo2', 'dup-identity')`)
+	_, err = rawDB.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/repo2', 'repo2', 'git@github.com:org/repo.git')`)
 	if err != nil {
 		rawDB.Close()
 		t.Fatalf("Failed to insert repo2: %v", err)
@@ -765,33 +766,134 @@ func TestDuplicateRepoIdentity_MigrationError(t *testing.T) {
 
 	rawDB.Close()
 
-	// Now open with storage.Open which runs migrations - should fail with clear error
-	_, err = Open(dbPath)
-	if err == nil {
-		t.Fatal("Expected migration to fail due to duplicate identities, but it succeeded")
+	// Now open with storage.Open which runs migrations - should succeed
+	db, err := Open(dbPath)
+	if err != nil {
+		t.Fatalf("Expected migration to succeed with duplicate identities, but got error: %v", err)
+	}
+	defer db.Close()
+
+	// Verify both repos exist
+	repos, err := db.ListRepos()
+	if err != nil {
+		t.Fatalf("ListRepos failed: %v", err)
 	}
-	if !regexp.MustCompile(`duplicate.*identit`).MatchString(err.Error()) {
-		t.Errorf("Expected error about duplicate identities, got: %v", err)
+	if len(repos) != 2 {
+		t.Errorf("Expected 2 repos, got %d", len(repos))
 	}
 }
 
-func TestGetRepoByIdentity_DuplicateError(t *testing.T) {
-	// This test verifies GetRepoByIdentity returns an error if duplicates exist
-	// (which shouldn't happen with the unique index, but tests the code path)
+func TestUniqueIndexMigration(t *testing.T) {
+	// This test verifies that an existing database with the old UNIQUE index
+	// on repos.identity is properly migrated to a non-unique index.
+	// See: https://github.com/roborev-dev/roborev/issues/131
 	dbPath := filepath.Join(t.TempDir(), "test.db")
+
+	rawDB, err := openRawDB(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to open raw database: %v", err)
+	}
+
+	// Create schema with identity column AND the old unique index
+	_, err = rawDB.Exec(`
+		CREATE TABLE repos (
+			id INTEGER PRIMARY KEY,
+			root_path TEXT UNIQUE NOT NULL,
+			name TEXT NOT NULL,
+			created_at TEXT NOT NULL DEFAULT (datetime('now')),
+			identity TEXT
+		);
+		CREATE UNIQUE INDEX idx_repos_identity ON repos(identity) WHERE identity IS NOT NULL;
+		CREATE TABLE commits (
+			id INTEGER PRIMARY KEY,
+			repo_id INTEGER NOT NULL REFERENCES repos(id),
+			sha TEXT NOT NULL,
+			author TEXT NOT NULL,
+			subject TEXT NOT NULL,
+			timestamp TEXT NOT NULL,
+			created_at TEXT NOT NULL DEFAULT (datetime('now')),
+			UNIQUE(repo_id, sha)
+		);
+		CREATE TABLE review_jobs (
+			id INTEGER PRIMARY KEY,
+			repo_id INTEGER NOT NULL REFERENCES repos(id),
+			commit_id INTEGER REFERENCES commits(id),
+			git_ref TEXT NOT NULL,
+			agent TEXT NOT NULL DEFAULT 'codex',
+			reasoning TEXT NOT NULL DEFAULT 'thorough',
+			status TEXT NOT NULL CHECK(status IN ('queued','running','done','failed','canceled')) DEFAULT 'queued',
+			enqueued_at TEXT NOT NULL DEFAULT (datetime('now')),
+			started_at TEXT,
+			finished_at TEXT,
+			worker_id TEXT,
+			error TEXT,
+			prompt TEXT,
+			retry_count INTEGER NOT NULL DEFAULT 0,
+			diff_content TEXT,
+			agentic INTEGER NOT NULL DEFAULT 0
+		);
+		CREATE TABLE reviews (
+			id INTEGER PRIMARY KEY,
+			job_id INTEGER UNIQUE NOT NULL REFERENCES review_jobs(id),
+			agent TEXT NOT NULL,
+			prompt TEXT NOT NULL,
+			output TEXT NOT NULL,
+			created_at TEXT NOT NULL DEFAULT (datetime('now')),
+			addressed INTEGER NOT NULL DEFAULT 0
+		);
+		CREATE TABLE responses (
+			id INTEGER PRIMARY KEY,
+			commit_id INTEGER REFERENCES commits(id),
+			job_id INTEGER REFERENCES review_jobs(id),
+			responder TEXT NOT NULL,
+			response TEXT NOT NULL,
+			created_at TEXT NOT NULL DEFAULT (datetime('now'))
+		);
+		CREATE INDEX idx_commits_sha ON commits(sha);
+	`)
+	if err != nil {
+		rawDB.Close()
+		t.Fatalf("Failed to create schema: %v", err)
+	}
+
+	// Insert one repo
+	_, err = rawDB.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/repo1', 'repo1', 'git@github.com:org/repo.git')`)
+	if err != nil {
+		rawDB.Close()
+		t.Fatalf("Failed to insert repo1: %v", err)
+	}
+
+	rawDB.Close()
+
+	// Open with storage.Open which runs migrations
 	db, err := Open(dbPath)
 	if err != nil {
-		t.Fatalf("Failed to open database: %v", err)
+		t.Fatalf("Migration failed: %v", err)
 	}
-	defer db.Close()
 
-	// Create two repos with different paths but we'll manually set same identity
-	// bypassing the unique constraint by using raw SQL after dropping the index
-	_, err = db.Exec(`DROP INDEX IF EXISTS idx_repos_identity`)
+	// Verify we can now insert a second repo with the same identity
+	// (this would fail if the unique index wasn't converted to non-unique)
+	_, err = db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/repo2', 'repo2', 'git@github.com:org/repo.git')`)
 	if err != nil {
-		t.Fatalf("Failed to drop index: %v", err)
+		db.Close()
+		t.Fatalf("Inserting second repo with same identity should succeed after migration, but got: %v", err)
 	}
 
+	db.Close()
+}
+
+func TestGetRepoByIdentity_DuplicateError(t *testing.T) {
+	// This test verifies GetRepoByIdentity returns an error if duplicates exist.
+	// Multiple repos can share the same identity (e.g., multiple clones of the same remote),
+	// but GetRepoByIdentity should return an error when asked to find a unique repo.
+	dbPath := filepath.Join(t.TempDir(), "test.db")
+	db, err := Open(dbPath)
+	if err != nil {
+		t.Fatalf("Failed to open database: %v", err)
+	}
+	defer db.Close()
+
+	// Create two repos with same identity (simulates two clones of the same remote)
 	_, err = db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES ('/path1', 'repo1', 'same-id')`)
 	if err != nil {
 		t.Fatalf("Failed to insert repo1: %v", err)

From c4bbaec788e59e34bb920a53144692244a0a4c08 Mon Sep 17 00:00:00 2001
From: Wes McKinney <wesmckinn+git@gmail.com>
Date: Mon, 26 Jan 2026 10:12:38 -0500
Subject: [PATCH 2/5] Fix daemon client to send job_id instead of review_id

The daemon client's MarkReviewAddressed method was still sending
review_id after the server was updated to require job_id. This would
cause all call sites (e.g., refine command) to fail with 400/404.

Changes:
- Update Client interface and HTTPClient to accept job_id
- Update all callers in refine.go to pass review.JobID
- Update client_test.go to verify job_id is sent
- Update refine_test.go mock and assertions for job IDs
- Add tests for MarkReviewAddressedByJobID storage method

Addresses review #2649.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 cmd/roborev/refine.go          | 16 ++++-----
 cmd/roborev/refine_test.go     | 54 +++++++++++++++---------------
 internal/daemon/client.go      |  8 ++---
 internal/daemon/client_test.go |  4 +--
 internal/storage/db_test.go    | 61 ++++++++++++++++++++++++++++++++++
 5 files changed, 102 insertions(+), 41 deletions(-)

diff --git a/cmd/roborev/refine.go b/cmd/roborev/refine.go
index 413f7c1..030700b 100644
--- a/cmd/roborev/refine.go
+++ b/cmd/roborev/refine.go
@@ -284,8 +284,8 @@ func runRefine(agentName, modelStr, reasoningStr string, maxIterations int, quie
 				if verdict == "F" && !review.Addressed {
 					currentFailedReview = review
 				} else if verdict == "P" {
-					if err := client.MarkReviewAddressed(review.ID); err != nil {
-						fmt.Printf("Warning: failed to mark review %d as addressed: %v\n", review.ID, err)
+					if err := client.MarkReviewAddressed(review.JobID); err != nil {
+						fmt.Printf("Warning: failed to mark review (job %d) as addressed: %v\n", review.JobID, err)
 					}
 					continue // Loop back to check for more
 				}
@@ -484,8 +484,8 @@ func runRefine(agentName, modelStr, reasoningStr string, maxIterations int, quie
 		client.AddComment(currentFailedReview.JobID, "roborev-refine", responseText)
 
 		// Mark old review as addressed
-		if err := client.MarkReviewAddressed(currentFailedReview.ID); err != nil {
-			fmt.Printf("Warning: failed to mark review %d as addressed: %v\n", currentFailedReview.ID, err)
+		if err := client.MarkReviewAddressed(currentFailedReview.JobID); err != nil {
+			fmt.Printf("Warning: failed to mark review (job %d) as addressed: %v\n", currentFailedReview.JobID, err)
 		}
 
 		// Wait for new commit to be reviewed (if post-commit hook triggers it)
@@ -511,8 +511,8 @@ func runRefine(agentName, modelStr, reasoningStr string, maxIterations int, quie
 		verdict := storage.ParseVerdict(review.Output)
 		if verdict == "P" {
 			fmt.Println("New commit passed review!")
-			if err := client.MarkReviewAddressed(review.ID); err != nil {
-				fmt.Printf("Warning: failed to mark review %d as addressed: %v\n", review.ID, err)
+			if err := client.MarkReviewAddressed(review.JobID); err != nil {
+				fmt.Printf("Warning: failed to mark review (job %d) as addressed: %v\n", review.JobID, err)
 			}
 			currentFailedReview = nil // Move on to next oldest failed commit
 		} else {
@@ -573,8 +573,8 @@ func findFailedReviewForBranch(client daemon.Client, commits []string, skip map[
 
 		// Mark passing reviews as addressed so they don't need to be checked again
 		if verdict == "P" {
-			if err := client.MarkReviewAddressed(review.ID); err != nil {
-				return nil, fmt.Errorf("marking review %d as addressed: %w", review.ID, err)
+			if err := client.MarkReviewAddressed(review.JobID); err != nil {
+				return nil, fmt.Errorf("marking review (job %d) as addressed: %w", review.JobID, err)
 			}
 		}
 	}
diff --git a/cmd/roborev/refine_test.go b/cmd/roborev/refine_test.go
index ecdb0c1..670b84b 100644
--- a/cmd/roborev/refine_test.go
+++ b/cmd/roborev/refine_test.go
@@ -22,7 +22,7 @@ type mockDaemonClient struct {
 	responses map[int64][]storage.Response
 
 	// Track calls for assertions
-	addressedReviews []int64
+	addressedJobIDs []int64
 	addedComments   []addedComment
 	enqueuedReviews  []enqueuedReview
 
@@ -70,11 +70,11 @@ func (m *mockDaemonClient) GetReviewByJobID(jobID int64) (*storage.Review, error
 	return m.reviews[job.GitRef], nil
 }
 
-func (m *mockDaemonClient) MarkReviewAddressed(reviewID int64) error {
+func (m *mockDaemonClient) MarkReviewAddressed(jobID int64) error {
 	if m.markAddressedErr != nil {
 		return m.markAddressedErr
 	}
-	m.addressedReviews = append(m.addressedReviews, reviewID)
+	m.addressedJobIDs = append(m.addressedJobIDs, jobID)
 	return nil
 }
 
@@ -497,18 +497,18 @@ func TestFindFailedReviewForBranch_MarksPassingAsAddressed(t *testing.T) {
 		t.Errorf("expected no failed reviews, got job %d", found.JobID)
 	}
 
-	// Both passing reviews should be marked as addressed
-	if len(client.addressedReviews) != 2 {
-		t.Errorf("expected 2 reviews to be marked addressed, got %d", len(client.addressedReviews))
+	// Both passing reviews should be marked as addressed (by job ID)
+	if len(client.addressedJobIDs) != 2 {
+		t.Errorf("expected 2 reviews to be marked addressed, got %d", len(client.addressedJobIDs))
 	}
 
-	// Verify the specific review IDs were marked
+	// Verify the specific job IDs were marked
 	addressed := make(map[int64]bool)
-	for _, id := range client.addressedReviews {
+	for _, id := range client.addressedJobIDs {
 		addressed[id] = true
 	}
-	if !addressed[1] || !addressed[2] {
-		t.Errorf("expected reviews 1 and 2 to be marked addressed, got %v", client.addressedReviews)
+	if !addressed[100] || !addressed[200] {
+		t.Errorf("expected jobs 100 and 200 to be marked addressed, got %v", client.addressedJobIDs)
 	}
 }
 
@@ -533,12 +533,12 @@ func TestFindFailedReviewForBranch_MarksPassingBeforeFailure(t *testing.T) {
 		t.Errorf("expected failed review (job 200), got %v", found)
 	}
 
-	// Passing review before the failure should be marked addressed
-	if len(client.addressedReviews) != 1 {
-		t.Errorf("expected 1 review to be marked addressed, got %d", len(client.addressedReviews))
+	// Passing review before the failure should be marked addressed (by job ID)
+	if len(client.addressedJobIDs) != 1 {
+		t.Errorf("expected 1 review to be marked addressed, got %d", len(client.addressedJobIDs))
 	}
-	if len(client.addressedReviews) > 0 && client.addressedReviews[0] != 1 {
-		t.Errorf("expected review 1 to be marked addressed, got %v", client.addressedReviews)
+	if len(client.addressedJobIDs) > 0 && client.addressedJobIDs[0] != 100 {
+		t.Errorf("expected job 100 to be marked addressed, got %v", client.addressedJobIDs)
 	}
 }
 
@@ -563,8 +563,8 @@ func TestFindFailedReviewForBranch_DoesNotMarkAlreadyAddressed(t *testing.T) {
 	}
 
 	// Already-addressed review should NOT be marked again
-	if len(client.addressedReviews) != 0 {
-		t.Errorf("expected no reviews to be marked addressed (already addressed), got %v", client.addressedReviews)
+	if len(client.addressedJobIDs) != 0 {
+		t.Errorf("expected no reviews to be marked addressed (already addressed), got %v", client.addressedJobIDs)
 	}
 }
 
@@ -598,16 +598,16 @@ func TestFindFailedReviewForBranch_MixedScenario(t *testing.T) {
 	}
 
 	// commit1 and commit4 should be marked as addressed (unaddressed passing reviews)
-	if len(client.addressedReviews) != 2 {
-		t.Errorf("expected 2 reviews to be marked addressed, got %d: %v", len(client.addressedReviews), client.addressedReviews)
+	if len(client.addressedJobIDs) != 2 {
+		t.Errorf("expected 2 reviews to be marked addressed, got %d: %v", len(client.addressedJobIDs), client.addressedJobIDs)
 	}
 
 	addressed := make(map[int64]bool)
-	for _, id := range client.addressedReviews {
+	for _, id := range client.addressedJobIDs {
 		addressed[id] = true
 	}
-	if !addressed[1] || !addressed[4] {
-		t.Errorf("expected reviews 1 and 4 to be marked addressed, got %v", client.addressedReviews)
+	if !addressed[100] || !addressed[400] {
+		t.Errorf("expected jobs 100 and 400 to be marked addressed, got %v", client.addressedJobIDs)
 	}
 }
 
@@ -635,8 +635,8 @@ func TestFindFailedReviewForBranch_StopsAtFirstFailure(t *testing.T) {
 	}
 
 	// No reviews should be marked as addressed (we stopped at first failure)
-	if len(client.addressedReviews) != 0 {
-		t.Errorf("expected no reviews to be marked addressed, got %v", client.addressedReviews)
+	if len(client.addressedJobIDs) != 0 {
+		t.Errorf("expected no reviews to be marked addressed, got %v", client.addressedJobIDs)
 	}
 }
 
@@ -663,10 +663,10 @@ func TestFindFailedReviewForBranch_MarkAddressedError(t *testing.T) {
 		t.Errorf("expected nil review when error occurs, got job %d", found.JobID)
 	}
 
-	// Error message should indicate which review failed
-	expectedMsg := "marking review 1 as addressed"
+	// Error message should indicate which job failed
+	expectedMsg := "marking review (job 100) as addressed"
 	if !strings.Contains(err.Error(), expectedMsg) {
-		t.Errorf("error should mention review ID, got: %v", err)
+		t.Errorf("error should mention job ID, got: %v", err)
 	}
 }
 
diff --git a/internal/daemon/client.go b/internal/daemon/client.go
index 8a93239..53b97e8 100644
--- a/internal/daemon/client.go
+++ b/internal/daemon/client.go
@@ -23,8 +23,8 @@ type Client interface {
 	// GetReviewByJobID retrieves a review by job ID
 	GetReviewByJobID(jobID int64) (*storage.Review, error)
 
-	// MarkReviewAddressed marks a review as addressed
-	MarkReviewAddressed(reviewID int64) error
+	// MarkReviewAddressed marks a review as addressed by job ID
+	MarkReviewAddressed(jobID int64) error
 
 	// AddComment adds a comment to a job
 	AddComment(jobID int64, commenter, comment string) error
@@ -130,9 +130,9 @@ func (c *HTTPClient) GetReviewByJobID(jobID int64) (*storage.Review, error) {
 	return &review, nil
 }
 
-func (c *HTTPClient) MarkReviewAddressed(reviewID int64) error {
+func (c *HTTPClient) MarkReviewAddressed(jobID int64) error {
 	reqBody, _ := json.Marshal(map[string]interface{}{
-		"review_id": reviewID,
+		"job_id":    jobID,
 		"addressed": true,
 	})
 
diff --git a/internal/daemon/client_test.go b/internal/daemon/client_test.go
index fafbc8c..aa3e404 100644
--- a/internal/daemon/client_test.go
+++ b/internal/daemon/client_test.go
@@ -68,8 +68,8 @@ func TestHTTPClientMarkReviewAddressed(t *testing.T) {
 		t.Fatalf("MarkReviewAddressed failed: %v", err)
 	}
 
-	if received["review_id"].(float64) != 99 {
-		t.Errorf("expected review_id 99, got %v", received["review_id"])
+	if received["job_id"].(float64) != 99 {
+		t.Errorf("expected job_id 99, got %v", received["job_id"])
 	}
 	if received["addressed"] != true {
 		t.Errorf("expected addressed true, got %v", received["addressed"])
diff --git a/internal/storage/db_test.go b/internal/storage/db_test.go
index 0e18b15..15f3cdf 100644
--- a/internal/storage/db_test.go
+++ b/internal/storage/db_test.go
@@ -393,6 +393,67 @@ func TestMarkReviewAddressedNotFound(t *testing.T) {
 	}
 }
 
+func TestMarkReviewAddressedByJobID(t *testing.T) {
+	db := openTestDB(t)
+	defer db.Close()
+
+	repo, _ := db.GetOrCreateRepo("/tmp/test-repo")
+	commit, _ := db.GetOrCreateCommit(repo.ID, "jobaddr123", "Author", "Subject", time.Now())
+	job, _ := db.EnqueueJob(repo.ID, commit.ID, "jobaddr123", "codex", "", "")
+	db.ClaimJob("worker-1")
+	db.CompleteJob(job.ID, "codex", "prompt", "output")
+
+	// Get the review to verify initial state
+	review, err := db.GetReviewByJobID(job.ID)
+	if err != nil {
+		t.Fatalf("GetReviewByJobID failed: %v", err)
+	}
+
+	// Initially not addressed
+	if review.Addressed {
+		t.Error("Review should not be addressed initially")
+	}
+
+	// Mark as addressed using job ID
+	err = db.MarkReviewAddressedByJobID(job.ID, true)
+	if err != nil {
+		t.Fatalf("MarkReviewAddressedByJobID failed: %v", err)
+	}
+
+	// Verify it's addressed
+	updated, _ := db.GetReviewByJobID(job.ID)
+	if !updated.Addressed {
+		t.Error("Review should be addressed after MarkReviewAddressedByJobID(true)")
+	}
+
+	// Mark as unaddressed using job ID
+	err = db.MarkReviewAddressedByJobID(job.ID, false)
+	if err != nil {
+		t.Fatalf("MarkReviewAddressedByJobID(false) failed: %v", err)
+	}
+
+	updated2, _ := db.GetReviewByJobID(job.ID)
+	if updated2.Addressed {
+		t.Error("Review should not be addressed after MarkReviewAddressedByJobID(false)")
+	}
+}
+
+func TestMarkReviewAddressedByJobIDNotFound(t *testing.T) {
+	db := openTestDB(t)
+	defer db.Close()
+
+	// Try to mark a non-existent job
+	err := db.MarkReviewAddressedByJobID(999999, true)
+	if err == nil {
+		t.Fatal("Expected error for non-existent job")
+	}
+
+	// Should be sql.ErrNoRows
+	if !errors.Is(err, sql.ErrNoRows) {
+		t.Errorf("Expected sql.ErrNoRows, got: %v", err)
+	}
+}
+
 func TestJobCounts(t *testing.T) {
 	db := openTestDB(t)
 	defer db.Close()

From 69d2b39fc32b8b766ec6eb58fdb2bc8b94df23f5 Mon Sep 17 00:00:00 2001
From: Wes McKinney <wesmckinn+git@gmail.com>
Date: Mon, 26 Jan 2026 10:20:50 -0500
Subject: [PATCH 3/5] Handle duplicate identities in sync with placeholder
 repos

When syncing from PostgreSQL, multiple local clones can share the same
identity (e.g., two clones of the same remote). Previously this caused
sync to fail because GetRepoByIdentity returns an error for duplicates.

This change introduces placeholder repos for sync:
- Placeholder repos have root_path == identity (not a real directory)
- GetOrCreateRepoByIdentity now creates/uses placeholder repos
- Synced jobs attach to the placeholder, not ambiguous local clones
- Local reviews stay with their actual local clone repos

Also adds:
- ExtractRepoNameFromIdentity for human-readable display names
  (e.g., "git@github.com:org/repo.git" -> "repo")
- Config support for custom repo name overrides via sync.repo_names
- Comprehensive tests for duplicate identity handling

Addresses review #2650.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 internal/config/config.go     | 13 +++++
 internal/storage/sync.go      | 56 ++++++++++++++++----
 internal/storage/sync_test.go | 98 ++++++++++++++++++++++++++++++++---
 3 files changed, 150 insertions(+), 17 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index 7545c35..0ba3061 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -50,6 +50,10 @@ type SyncConfig struct {
 
 	// ConnectTimeout is the connection timeout (e.g., "5s"). Default: 5s
 	ConnectTimeout string `toml:"connect_timeout"`
+
+	// RepoNames provides custom display names for synced repos by identity.
+	// Example: {"git@github.com:org/repo.git": "my-project"}
+	RepoNames map[string]string `toml:"repo_names"`
 }
 
 // PostgresURLExpanded returns the PostgreSQL URL with environment variables expanded.
@@ -61,6 +65,15 @@ func (c *SyncConfig) PostgresURLExpanded() string {
 	return os.ExpandEnv(c.PostgresURL)
 }
 
+// GetRepoDisplayName returns the configured display name for a repo identity,
+// or empty string if no override is configured.
+func (c *SyncConfig) GetRepoDisplayName(identity string) string {
+	if c.RepoNames == nil {
+		return ""
+	}
+	return c.RepoNames[identity]
+}
+
 // Validate checks the sync configuration for common issues.
 // Returns a list of warnings (non-fatal issues).
 func (c *SyncConfig) Validate() []string {
diff --git a/internal/storage/sync.go b/internal/storage/sync.go
index 5d8d8f8..417d3cf 100644
--- a/internal/storage/sync.go
+++ b/internal/storage/sync.go
@@ -621,29 +621,63 @@ func (db *DB) GetKnownJobUUIDs() ([]string, error) {
 	return uuids, rows.Err()
 }
 
-// GetOrCreateRepoByIdentity finds or creates a repo by identity.
-// If the repo doesn't exist, creates it with a placeholder path.
+// GetOrCreateRepoByIdentity finds or creates a placeholder repo for syncing.
+// A placeholder repo has root_path == identity and is used for synced data.
+// This allows multiple local clones to coexist with a single sync placeholder.
 func (db *DB) GetOrCreateRepoByIdentity(identity string) (int64, error) {
-	// Try to find existing repo
-	repo, err := db.GetRepoByIdentity(identity)
-	if err != nil {
-		return 0, err
+	// First, look for an existing placeholder repo (root_path == identity)
+	var placeholderID int64
+	err := db.QueryRow(`SELECT id FROM repos WHERE root_path = ? AND identity = ?`, identity, identity).Scan(&placeholderID)
+	if err == nil {
+		return placeholderID, nil
 	}
-	if repo != nil {
-		return repo.ID, nil
+	if err != sql.ErrNoRows {
+		return 0, fmt.Errorf("find placeholder repo: %w", err)
 	}
 
-	// Create a placeholder repo - path is identity since we don't know the local path
+	// No placeholder exists - create one
+	// Use extracted repo name for display, but root_path stays as identity to mark it as a placeholder
+	displayName := ExtractRepoNameFromIdentity(identity)
 	result, err := db.Exec(`
 		INSERT INTO repos (root_path, name, identity)
 		VALUES (?, ?, ?)
-	`, identity, identity, identity)
+	`, identity, displayName, identity)
 	if err != nil {
-		return 0, fmt.Errorf("create repo: %w", err)
+		return 0, fmt.Errorf("create placeholder repo: %w", err)
 	}
 	return result.LastInsertId()
 }
 
+// ExtractRepoNameFromIdentity extracts a human-readable name from a git identity.
+// Examples:
+//   - "git@github.com:org/repo.git" -> "repo"
+//   - "https://github.com/org/my-project.git" -> "my-project"
+//   - "https://github.com/org/repo" -> "repo"
+func ExtractRepoNameFromIdentity(identity string) string {
+	// Remove trailing .git if present
+	name := strings.TrimSuffix(identity, ".git")
+
+	// Find the last path component
+	// Handle both SSH (git@host:path) and HTTPS (https://host/path) formats
+	if idx := strings.LastIndex(name, "/"); idx >= 0 {
+		name = name[idx+1:]
+	} else if idx := strings.LastIndex(name, ":"); idx >= 0 {
+		// SSH format like git@github.com:org/repo - get part after last /
+		afterColon := name[idx+1:]
+		if slashIdx := strings.LastIndex(afterColon, "/"); slashIdx >= 0 {
+			name = afterColon[slashIdx+1:]
+		} else {
+			name = afterColon
+		}
+	}
+
+	// If we ended up with empty string, use the identity as-is
+	if name == "" {
+		return identity
+	}
+	return name
+}
+
 // GetOrCreateCommitByRepoAndSHA finds or creates a commit.
 func (db *DB) GetOrCreateCommitByRepoAndSHA(repoID int64, sha, author, subject string, timestamp time.Time) (int64, error) {
 	// Try to find existing
diff --git a/internal/storage/sync_test.go b/internal/storage/sync_test.go
index c007ed5..465497e 100644
--- a/internal/storage/sync_test.go
+++ b/internal/storage/sync_test.go
@@ -367,10 +367,11 @@ func TestGetOrCreateRepoByIdentity(t *testing.T) {
 			t.Fatalf("Query repo failed: %v", err)
 		}
 		if rootPath != localIdentity {
-			t.Errorf("Expected root_path %q, got %q", localIdentity, rootPath)
+			t.Errorf("Expected root_path %q (placeholder), got %q", localIdentity, rootPath)
 		}
-		if name != localIdentity {
-			t.Errorf("Expected name %q, got %q", localIdentity, name)
+		// Name is extracted from identity
+		if name != "my-local-project" {
+			t.Errorf("Expected name 'my-local-project' (extracted), got %q", name)
 		}
 		if identity != localIdentity {
 			t.Errorf("Expected identity %q, got %q", localIdentity, identity)
@@ -419,18 +420,103 @@ func TestGetOrCreateRepoByIdentity(t *testing.T) {
 			t.Fatalf("GetOrCreateRepoByIdentity failed: %v", err)
 		}
 
-		// Verify identity is set correctly
-		var identity string
-		err = db.QueryRow(`SELECT identity FROM repos WHERE id = ?`, repoID).Scan(&identity)
+		// Verify identity is set correctly and name is extracted
+		var identity, name string
+		err = db.QueryRow(`SELECT identity, name FROM repos WHERE id = ?`, repoID).Scan(&identity, &name)
 		if err != nil {
 			t.Fatalf("Query repo failed: %v", err)
 		}
 		if identity != gitIdentity {
 			t.Errorf("Expected identity %q, got %q", gitIdentity, identity)
 		}
+		if name != "repo" {
+			t.Errorf("Expected name 'repo' (extracted from URL), got %q", name)
+		}
+	})
+
+	t.Run("creates placeholder even when local clones exist", func(t *testing.T) {
+		// This tests the fix for https://github.com/roborev-dev/roborev/issues/131
+		// When multiple local clones have the same identity, GetOrCreateRepoByIdentity
+		// should create/use a dedicated placeholder repo (root_path == identity).
+		sharedIdentity := "git@github.com:org/shared-repo.git"
+
+		// Create two local clones with the same identity (simulates real scenario)
+		_, err := db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES (?, ?, ?)`,
+			"/home/user/clone-1", "clone-1", sharedIdentity)
+		if err != nil {
+			t.Fatalf("Insert clone-1 failed: %v", err)
+		}
+		_, err = db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES (?, ?, ?)`,
+			"/home/user/clone-2", "clone-2", sharedIdentity)
+		if err != nil {
+			t.Fatalf("Insert clone-2 failed: %v", err)
+		}
+
+		// GetOrCreateRepoByIdentity should create a placeholder, not fail
+		placeholderID, err := db.GetOrCreateRepoByIdentity(sharedIdentity)
+		if err != nil {
+			t.Fatalf("GetOrCreateRepoByIdentity should succeed with duplicates, got: %v", err)
+		}
+
+		// Verify it created a placeholder (root_path == identity)
+		var rootPath, name string
+		err = db.QueryRow(`SELECT root_path, name FROM repos WHERE id = ?`, placeholderID).Scan(&rootPath, &name)
+		if err != nil {
+			t.Fatalf("Query placeholder failed: %v", err)
+		}
+		if rootPath != sharedIdentity {
+			t.Errorf("Expected placeholder root_path %q, got %q", sharedIdentity, rootPath)
+		}
+		if name != "shared-repo" {
+			t.Errorf("Expected placeholder name 'shared-repo' (extracted), got %q", name)
+		}
+
+		// Subsequent calls should return the same placeholder
+		placeholderID2, err := db.GetOrCreateRepoByIdentity(sharedIdentity)
+		if err != nil {
+			t.Fatalf("Second GetOrCreateRepoByIdentity failed: %v", err)
+		}
+		if placeholderID != placeholderID2 {
+			t.Errorf("Expected same placeholder ID, got %d and %d", placeholderID, placeholderID2)
+		}
 	})
 }
 
+func TestExtractRepoNameFromIdentity(t *testing.T) {
+	tests := []struct {
+		identity string
+		expected string
+	}{
+		// SSH format
+		{"git@github.com:org/repo.git", "repo"},
+		{"git@github.com:user/my-project.git", "my-project"},
+		{"git@gitlab.com:group/subgroup/repo.git", "repo"},
+
+		// HTTPS format
+		{"https://github.com/org/repo.git", "repo"},
+		{"https://github.com/org/repo", "repo"},
+		{"https://gitlab.com/group/subgroup/project.git", "project"},
+
+		// Local format
+		{"local:my-project", "my-project"},
+		{"local:another-repo", "another-repo"},
+
+		// Edge cases
+		{"repo.git", "repo"},
+		{"repo", "repo"},
+		{"", ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.identity, func(t *testing.T) {
+			got := ExtractRepoNameFromIdentity(tt.identity)
+			if got != tt.expected {
+				t.Errorf("ExtractRepoNameFromIdentity(%q) = %q, want %q", tt.identity, got, tt.expected)
+			}
+		})
+	}
+}
+
 func TestSetRepoIdentity(t *testing.T) {
 	dbPath := filepath.Join(t.TempDir(), "test.db")
 	db, err := Open(dbPath)

From 96adb97bad6dc60e2e9294d964544c3d68c99275 Mon Sep 17 00:00:00 2001
From: Wes McKinney <wesmckinn+git@gmail.com>
Date: Mon, 26 Jan 2026 10:44:13 -0500
Subject: [PATCH 4/5] Address review findings for sync and config

Fixes from reviews #2655, #2656, #2657:

1. Fix nil receiver in GetRepoDisplayName (Medium)
   - Added nil receiver check to prevent panic

2. Add test coverage for GetRepoDisplayName (Medium)
   - Tests for nil receiver, nil map, missing key, and configured name

3. Handle empty identity in ExtractRepoNameFromIdentity (Medium)
   - Returns "unknown" instead of empty string to avoid empty repo names

4. Reuse local repo when exactly one exists (Medium, #2655)
   - GetOrCreateRepoByIdentity now returns the single local repo directly
   - Only creates placeholder when 0 or 2+ local repos have the identity
   - This ensures synced jobs attach to the local repo when unambiguous

5. Added test for single-clone reuse scenario

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 internal/config/config.go      |  2 +-
 internal/config/config_test.go | 39 ++++++++++++++++++++++++++++
 internal/storage/sync.go       | 46 +++++++++++++++++++++++++++++++---
 internal/storage/sync_test.go  | 44 ++++++++++++++++++++++++++------
 4 files changed, 119 insertions(+), 12 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index 0ba3061..0b1d1ae 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -68,7 +68,7 @@ func (c *SyncConfig) PostgresURLExpanded() string {
 // GetRepoDisplayName returns the configured display name for a repo identity,
 // or empty string if no override is configured.
 func (c *SyncConfig) GetRepoDisplayName(identity string) string {
-	if c.RepoNames == nil {
+	if c == nil || c.RepoNames == nil {
 		return ""
 	}
 	return c.RepoNames[identity]
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 3dc88a1..526f31e 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -570,6 +570,45 @@ func TestSyncConfigPostgresURLExpanded(t *testing.T) {
 	})
 }
 
+func TestSyncConfigGetRepoDisplayName(t *testing.T) {
+	t.Run("nil receiver returns empty", func(t *testing.T) {
+		var cfg *SyncConfig
+		if got := cfg.GetRepoDisplayName("any"); got != "" {
+			t.Errorf("Expected empty string for nil receiver, got %q", got)
+		}
+	})
+
+	t.Run("nil map returns empty", func(t *testing.T) {
+		cfg := &SyncConfig{}
+		if got := cfg.GetRepoDisplayName("any"); got != "" {
+			t.Errorf("Expected empty string for nil map, got %q", got)
+		}
+	})
+
+	t.Run("missing key returns empty", func(t *testing.T) {
+		cfg := &SyncConfig{
+			RepoNames: map[string]string{
+				"git@github.com:org/repo.git": "my-repo",
+			},
+		}
+		if got := cfg.GetRepoDisplayName("unknown"); got != "" {
+			t.Errorf("Expected empty string for missing key, got %q", got)
+		}
+	})
+
+	t.Run("returns configured name", func(t *testing.T) {
+		cfg := &SyncConfig{
+			RepoNames: map[string]string{
+				"git@github.com:org/repo.git": "my-custom-name",
+			},
+		}
+		expected := "my-custom-name"
+		if got := cfg.GetRepoDisplayName("git@github.com:org/repo.git"); got != expected {
+			t.Errorf("Expected %q, got %q", expected, got)
+		}
+	})
+}
+
 func TestSyncConfigValidate(t *testing.T) {
 	t.Run("disabled returns no warnings", func(t *testing.T) {
 		cfg := SyncConfig{Enabled: false}
diff --git a/internal/storage/sync.go b/internal/storage/sync.go
index 417d3cf..7146fa8 100644
--- a/internal/storage/sync.go
+++ b/internal/storage/sync.go
@@ -621,9 +621,16 @@ func (db *DB) GetKnownJobUUIDs() ([]string, error) {
 	return uuids, rows.Err()
 }
 
-// GetOrCreateRepoByIdentity finds or creates a placeholder repo for syncing.
-// A placeholder repo has root_path == identity and is used for synced data.
-// This allows multiple local clones to coexist with a single sync placeholder.
+// GetOrCreateRepoByIdentity finds or creates a repo for syncing by identity.
+// The logic is:
+//  1. If a placeholder repo exists (root_path == identity), use it
+//  2. If exactly one local repo has this identity, use it (no placeholder needed)
+//  3. If 0 or 2+ local repos have this identity, create/use a placeholder
+//
+// This ensures synced jobs attach to the right repo:
+//   - Single clone: jobs attach directly to the local repo
+//   - Multiple clones: jobs attach to a neutral placeholder
+//   - No local clone: placeholder serves as a sync-only repo
 func (db *DB) GetOrCreateRepoByIdentity(identity string) (int64, error) {
 	// First, look for an existing placeholder repo (root_path == identity)
 	var placeholderID int64
@@ -635,7 +642,32 @@ func (db *DB) GetOrCreateRepoByIdentity(identity string) (int64, error) {
 		return 0, fmt.Errorf("find placeholder repo: %w", err)
 	}
 
-	// No placeholder exists - create one
+	// No placeholder exists - check for local repos with this identity
+	// (excluding the placeholder pattern where root_path == identity)
+	rows, err := db.Query(`SELECT id FROM repos WHERE identity = ? AND root_path != ?`, identity, identity)
+	if err != nil {
+		return 0, fmt.Errorf("find repos by identity: %w", err)
+	}
+	defer rows.Close()
+
+	var repoIDs []int64
+	for rows.Next() {
+		var id int64
+		if err := rows.Scan(&id); err != nil {
+			return 0, fmt.Errorf("scan repo id: %w", err)
+		}
+		repoIDs = append(repoIDs, id)
+	}
+	if err := rows.Err(); err != nil {
+		return 0, fmt.Errorf("iterate repos: %w", err)
+	}
+
+	// If exactly one local repo exists, use it directly
+	if len(repoIDs) == 1 {
+		return repoIDs[0], nil
+	}
+
+	// 0 or 2+ local repos - create a placeholder
 	// Use extracted repo name for display, but root_path stays as identity to mark it as a placeholder
 	displayName := ExtractRepoNameFromIdentity(identity)
 	result, err := db.Exec(`
@@ -653,7 +685,13 @@ func (db *DB) GetOrCreateRepoByIdentity(identity string) (int64, error) {
 //   - "git@github.com:org/repo.git" -> "repo"
 //   - "https://github.com/org/my-project.git" -> "my-project"
 //   - "https://github.com/org/repo" -> "repo"
+//   - "" -> "unknown"
 func ExtractRepoNameFromIdentity(identity string) string {
+	// Handle empty identity
+	if identity == "" {
+		return "unknown"
+	}
+
 	// Remove trailing .git if present
 	name := strings.TrimSuffix(identity, ".git")
 
diff --git a/internal/storage/sync_test.go b/internal/storage/sync_test.go
index 465497e..1319b7e 100644
--- a/internal/storage/sync_test.go
+++ b/internal/storage/sync_test.go
@@ -434,13 +434,43 @@ func TestGetOrCreateRepoByIdentity(t *testing.T) {
 		}
 	})
 
-	t.Run("creates placeholder even when local clones exist", func(t *testing.T) {
-		// This tests the fix for https://github.com/roborev-dev/roborev/issues/131
-		// When multiple local clones have the same identity, GetOrCreateRepoByIdentity
-		// should create/use a dedicated placeholder repo (root_path == identity).
+	t.Run("reuses single local repo when one exists", func(t *testing.T) {
+		// When exactly one local repo has the identity, use it directly (no placeholder)
+		singleIdentity := "git@github.com:org/single-clone-repo.git"
+
+		// Create one local clone with the identity
+		result, err := db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES (?, ?, ?)`,
+			"/home/user/single-clone", "single-clone", singleIdentity)
+		if err != nil {
+			t.Fatalf("Insert single clone failed: %v", err)
+		}
+		localRepoID, _ := result.LastInsertId()
+
+		// GetOrCreateRepoByIdentity should return the existing local repo, not create a placeholder
+		gotID, err := db.GetOrCreateRepoByIdentity(singleIdentity)
+		if err != nil {
+			t.Fatalf("GetOrCreateRepoByIdentity failed: %v", err)
+		}
+		if gotID != localRepoID {
+			t.Errorf("Expected to reuse local repo ID %d, got %d", localRepoID, gotID)
+		}
+
+		// Verify no placeholder was created
+		var count int
+		err = db.QueryRow(`SELECT COUNT(*) FROM repos WHERE root_path = ?`, singleIdentity).Scan(&count)
+		if err != nil {
+			t.Fatalf("Count query failed: %v", err)
+		}
+		if count != 0 {
+			t.Errorf("Expected no placeholder to be created, found %d", count)
+		}
+	})
+
+	t.Run("creates placeholder when multiple local clones exist", func(t *testing.T) {
+		// When multiple local clones have the same identity, create a placeholder
 		sharedIdentity := "git@github.com:org/shared-repo.git"
 
-		// Create two local clones with the same identity (simulates real scenario)
+		// Create two local clones with the same identity
 		_, err := db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES (?, ?, ?)`,
 			"/home/user/clone-1", "clone-1", sharedIdentity)
 		if err != nil {
@@ -452,7 +482,7 @@ func TestGetOrCreateRepoByIdentity(t *testing.T) {
 			t.Fatalf("Insert clone-2 failed: %v", err)
 		}
 
-		// GetOrCreateRepoByIdentity should create a placeholder, not fail
+		// GetOrCreateRepoByIdentity should create a placeholder
 		placeholderID, err := db.GetOrCreateRepoByIdentity(sharedIdentity)
 		if err != nil {
 			t.Fatalf("GetOrCreateRepoByIdentity should succeed with duplicates, got: %v", err)
@@ -504,7 +534,7 @@ func TestExtractRepoNameFromIdentity(t *testing.T) {
 		// Edge cases
 		{"repo.git", "repo"},
 		{"repo", "repo"},
-		{"", ""},
+		{"", "unknown"},
 	}
 
 	for _, tt := range tests {

From d3ba7b515eef43ec24df9cae1598fe8af9b3fb53 Mon Sep 17 00:00:00 2001
From: Wes McKinney <wesmckinn+git@gmail.com>
Date: Mon, 26 Jan 2026 10:53:28 -0500
Subject: [PATCH 5/5] Prefer single local repo over existing placeholder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix bug where placeholders were always returned if they existed, even
when exactly one local repo now has the identity. The comment said we
prefer single local repos, but the code checked for placeholder first.

Reordered the logic:
1. Check for local repos (excluding placeholders)
2. If exactly 1 → return it (always preferred)
3. Look for placeholder → if exists, return it
4. If 0 or 2+ → create placeholder

This ensures that if a user:
1. Syncs with no local clone (placeholder created)
2. Clones the repo (now has 1 local clone)
3. Syncs again → local repo is used (not placeholder)

Added test: "prefers single local repo over existing placeholder"

Addresses review #2658.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 internal/storage/sync.go      | 37 +++++++++++++++++++----------------
 internal/storage/sync_test.go | 31 +++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/internal/storage/sync.go b/internal/storage/sync.go
index 7146fa8..cebc3b2 100644
--- a/internal/storage/sync.go
+++ b/internal/storage/sync.go
@@ -623,27 +623,20 @@ func (db *DB) GetKnownJobUUIDs() ([]string, error) {
 
 // GetOrCreateRepoByIdentity finds or creates a repo for syncing by identity.
 // The logic is:
-//  1. If a placeholder repo exists (root_path == identity), use it
-//  2. If exactly one local repo has this identity, use it (no placeholder needed)
-//  3. If 0 or 2+ local repos have this identity, create/use a placeholder
+//  1. If exactly one local repo has this identity, use it (always preferred)
+//  2. If a placeholder repo exists (root_path == identity), use it
+//  3. If 0 or 2+ local repos have this identity, create a placeholder
 //
 // This ensures synced jobs attach to the right repo:
 //   - Single clone: jobs attach directly to the local repo
 //   - Multiple clones: jobs attach to a neutral placeholder
 //   - No local clone: placeholder serves as a sync-only repo
+//
+// Note: Single local repos are always preferred, even if a placeholder exists
+// from a previous sync (e.g., when there were 0 or 2+ clones before).
 func (db *DB) GetOrCreateRepoByIdentity(identity string) (int64, error) {
-	// First, look for an existing placeholder repo (root_path == identity)
-	var placeholderID int64
-	err := db.QueryRow(`SELECT id FROM repos WHERE root_path = ? AND identity = ?`, identity, identity).Scan(&placeholderID)
-	if err == nil {
-		return placeholderID, nil
-	}
-	if err != sql.ErrNoRows {
-		return 0, fmt.Errorf("find placeholder repo: %w", err)
-	}
-
-	// No placeholder exists - check for local repos with this identity
-	// (excluding the placeholder pattern where root_path == identity)
+	// First, check for local repos with this identity
+	// (excluding placeholders where root_path == identity)
 	rows, err := db.Query(`SELECT id FROM repos WHERE identity = ? AND root_path != ?`, identity, identity)
 	if err != nil {
 		return 0, fmt.Errorf("find repos by identity: %w", err)
@@ -662,12 +655,22 @@ func (db *DB) GetOrCreateRepoByIdentity(identity string) (int64, error) {
 		return 0, fmt.Errorf("iterate repos: %w", err)
 	}
 
-	// If exactly one local repo exists, use it directly
+	// If exactly one local repo exists, always use it (even if placeholder exists)
 	if len(repoIDs) == 1 {
 		return repoIDs[0], nil
 	}
 
-	// 0 or 2+ local repos - create a placeholder
+	// 0 or 2+ local repos - look for existing placeholder
+	var placeholderID int64
+	err = db.QueryRow(`SELECT id FROM repos WHERE root_path = ? AND identity = ?`, identity, identity).Scan(&placeholderID)
+	if err == nil {
+		return placeholderID, nil
+	}
+	if err != sql.ErrNoRows {
+		return 0, fmt.Errorf("find placeholder repo: %w", err)
+	}
+
+	// No placeholder exists - create one
 	// Use extracted repo name for display, but root_path stays as identity to mark it as a placeholder
 	displayName := ExtractRepoNameFromIdentity(identity)
 	result, err := db.Exec(`
diff --git a/internal/storage/sync_test.go b/internal/storage/sync_test.go
index 1319b7e..8a0e22f 100644
--- a/internal/storage/sync_test.go
+++ b/internal/storage/sync_test.go
@@ -510,6 +510,37 @@ func TestGetOrCreateRepoByIdentity(t *testing.T) {
 			t.Errorf("Expected same placeholder ID, got %d and %d", placeholderID, placeholderID2)
 		}
 	})
+
+	t.Run("prefers single local repo over existing placeholder", func(t *testing.T) {
+		// This tests the fix for review #2658: when a placeholder exists from
+		// a previous sync (e.g., when there were 0 clones), but now there's
+		// exactly one local repo, we should prefer the local repo.
+		placeholderIdentity := "git@github.com:org/placeholder-then-clone.git"
+
+		// First, create a placeholder (simulates sync with no local clone)
+		_, err := db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES (?, ?, ?)`,
+			placeholderIdentity, "placeholder-then-clone", placeholderIdentity)
+		if err != nil {
+			t.Fatalf("Insert placeholder failed: %v", err)
+		}
+
+		// Now create a single local clone (user cloned the repo after syncing)
+		result, err := db.Exec(`INSERT INTO repos (root_path, name, identity) VALUES (?, ?, ?)`,
+			"/home/user/new-clone", "new-clone", placeholderIdentity)
+		if err != nil {
+			t.Fatalf("Insert local clone failed: %v", err)
+		}
+		localRepoID, _ := result.LastInsertId()
+
+		// GetOrCreateRepoByIdentity should return the local repo, not the placeholder
+		gotID, err := db.GetOrCreateRepoByIdentity(placeholderIdentity)
+		if err != nil {
+			t.Fatalf("GetOrCreateRepoByIdentity failed: %v", err)
+		}
+		if gotID != localRepoID {
+			t.Errorf("Expected to prefer local repo ID %d over placeholder, got %d", localRepoID, gotID)
+		}
+	})
 }
 
 func TestExtractRepoNameFromIdentity(t *testing.T) {