Sort repository dropdown by most recently used

[StephaneDelcroix]

Problem

The repository dropdown in the "New Session" form listed repos in insertion order, not by most recently used. Users with multiple repos had to scroll to find the one they just worked with.

Fix

• Added LastUsedAt nullable property to RepositoryInfo (backward-compatible — missing field deserializes as null)
• RepoManager.Repositories now returns repos sorted by LastUsedAt descending, falling back to AddedAt for legacy entries
• Added TouchRepository() method that stamps LastUsedAt = UtcNow and persists
• Called TouchRepository() in CreateSessionWithWorktreeAsync (covers all session creation paths: new branch, PR checkout, and existing worktree)
• Also stamps LastUsedAt inline in CreateWorktreeAsync and CreateWorktreeFromPrAsync

Tests

6 new tests in RepoSortOrderTests.cs:

• Sort by LastUsedAt descending
• Null LastUsedAt falls back to AddedAt
• TouchRepository updates timestamp
• TouchRepository on nonexistent repo doesn't throw
• LastUsedAt round-trips through JSON serialization
• Legacy repos.json without LastUsedAt deserializes gracefully

All 2675 tests pass.

[PureWeen]

PR Review: Sort repository dropdown by most recently used

CI Status: ⚠️ No CI checks reported on this branch.
Prior Reviews: None.
Consensus Model: Issues flagged by 2+ of 5 models (claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex).

---

🟡 MODERATE — Unconditional Save() on nonexistent repo in TouchRepository

PolyPilot/Services/RepoManager.cs ~line 311

lock (_stateLock)
{
    var repo = _state.Repositories.FirstOrDefault(r => r.Id == repoId);
    if (repo != null)
        repo.LastUsedAt = DateTime.UtcNow;
}
Save(); // ← called even when repo == null, no state changed

Save() is called unconditionally after the lock, even when repo == null (the repo ID wasn't found). This serializes and rewrites repos.json to disk for every call with an unrecognized repo ID — with zero state change. Save() should move inside the if (repo != null) block.

Flagged by: opus-4.6 ×2, sonnet-4.6, gemini-3-pro (4/5 models)

---

🟡 MODERATE — Redundant double-stamp and double Save() on the CreateWorktreeAsync path

PolyPilot/Services/CopilotService.cs ~line 2244
PolyPilot/Services/RepoManager.cs ~lines 579, 683

When a new session is created via CreateSessionWithWorktreeAsync:

1. CreateWorktreeAsync (or CreateWorktreeFromPrAsync) sets repo.LastUsedAt = DateTime.UtcNow inside _stateLock and calls Save().
2. Immediately after, CreateSessionWithWorktreeAsync calls TouchRepository(wt.RepoId), which re-acquires _stateLock, sets a later UtcNow, and calls Save() a second time.

This results in two disk writes per session creation, and a brief window where the in-memory LastUsedAt differs from what was just persisted. Since TouchRepository already handles stamping correctly on its own, the inline stamps in CreateWorktreeAsync / CreateWorktreeFromPrAsync are redundant and should be removed.

Flagged by: opus-4.6, sonnet-4.6, gemini-3-pro (3/5 models)

---

🟢 MINOR — LINQ sort executes under _stateLock in Repositories getter

PolyPilot/Services/RepoManager.cs ~line 38

lock (_stateLock) return _state.Repositories.OrderByDescending(r => r.LastUsedAt ?? r.AddedAt).ToList().AsReadOnly();

The O(N log N) sort and ToList() allocation happen while _stateLock is held, blocking concurrent readers/writers for the duration of the sort. The list snapshot should be taken under the lock, and the sort performed after releasing it.

Flagged by: sonnet-4.6, gemini-3-pro (2/5 models)

---

Test Coverage

The 6 new tests in RepoSortOrderTests.cs are well-structured and cover the key scenarios. One missing case: TouchRepository when repo exists but is called while CreateWorktreeAsync is simultaneously in progress — i.e., the double-stamp scenario above has no test that asserts final LastUsedAt is correct after both saves. This would help detect the issue and validate any fix.

---

Recommended Action: ⚠️ Request changes

Two MODERATE issues worth fixing before merge:

1. Move Save() inside the if (repo != null) block in TouchRepository.
2. Remove the inline repo.LastUsedAt = DateTime.UtcNow stamps inside CreateWorktreeAsync and CreateWorktreeFromPrAsync — let TouchRepository be the single stamp point.

The overall approach is solid and the backward-compatibility story (nullable LastUsedAt falling back to AddedAt) is correct.

[PureWeen]

Re-Review: Sort repository dropdown by most recently used

CI Status: ⚠️ No CI checks reported on this branch.
Previous review: #394 (comment)
Models: claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex (5/5)

---

Previous Findings — All Fixed ✅

#	Finding	Status
1	🟡 TouchRepository() called Save() unconditionally even when repo not found	✅ FIXED — else { return; } exits before Save()
2	🟡 Double-stamp + double Save() on CreateWorktreeAsync path	✅ FIXED — TouchRepository moved inside the existing-worktree branch only; CreateWorktreeAsync/CreateWorktreeFromPrAsync keep their inline stamps for non-CopilotService callers
3	🟢 Repositories getter ran O(N log N) sort inside _stateLock	✅ FIXED — snapshot taken under lock, sort runs outside

All 5 models confirmed all 3 fixes. No new issue reached the 2+ model consensus threshold.

2708/2709 tests pass (1 pre-existing failure in RenderThrottleTests unrelated to this PR, present on main before these changes).

---

Recommended Action: ✅ Approve