Show repository disk size in sidebar group headers

[StephaneDelcroix]

Summary

Shows disk size (bare clone + worktrees) for each tracked repository in the sidebar group headers.

Changes

• RepoManager: Added GetRepoDiskSize(), RefreshDiskSizesAsync(), GetDirectorySizeBytes(), and FormatSize() with a ConcurrentDictionary-based cache (5-minute TTL)
• SessionSidebar: Displays disk size as a dim label next to session count in repo group headers, and includes it in the tooltip. Periodic refresh via a Timer (every 5 min), disposed properly on cleanup.
• CSS: Added .group-disk-size style for the size label
• Tests: 16 unit tests covering FormatSize (locale-safe), directory scanning, cache behavior, and non-existent path handling

Design

• Disk sizes computed on background thread via Task.Run — never blocks UI
• Cached with 5-minute TTL to avoid expensive repeated scans
• Fires OnStateChanged after refresh to trigger sidebar re-render
• Handles missing directories gracefully (returns 0)

[StephaneDelcroix]

Multi-Model Re-Review: PR #246 — Show repository disk size

Previous Findings Status

• 🟡 TTL/timer mismatch (cache TTL = timer period → disk size blinks away every 5 min): FIXED ✅ — DiskSizeCacheTtl raised to 7 min in commit 283b543; entries remain valid through the entire 5-min refresh scan.

---

New Findings (5-model consensus, 3+ models required)

🟡 MODERATE — RepoManager.cs:RefreshDiskSizesAsync — No in-flight scan guard (4/5 models)

RefreshDiskSizesAsync launches a new Task.Run unconditionally every time the timer fires. On a machine with many large repos, a recursive stat()-per-file walk can take longer than 5 minutes. When that happens, two scans run concurrently, doubling disk I/O and firing OnStateChanged twice. No data corruption (ConcurrentDictionary is safe), but it's wasteful and causes a redundant re-render burst.

Fix: add a simple reentrance guard at the top of the method:

private int _scanning;
public Task RefreshDiskSizesAsync()
{
    if (Interlocked.CompareExchange(ref _scanning, 1, 0) != 0) return Task.CompletedTask;
    return Task.Run(() => { try { /* existing body */ } finally { _scanning = 0; } });
}

🟢 MINOR — RepoManager.cs:RefreshDiskSizesAsync — OnStateChanged invoked from background thread (3/5 models)

The call OnStateChanged?.Invoke() fires from inside Task.Run(). Today this is safe because the sole subscriber (SessionSidebar.OnRepoStateChanged) wraps its handler in InvokeAsync(StateHasChanged). However, every other service in the codebase (CopilotService, DemoService) marshals OnStateChanged to the UI thread at the source via SyncContext.Post()/InvokeOnUI(). A future subscriber that follows the same producer-marshals convention will get an InvalidOperationException. Low urgency but inconsistent with the established pattern.

🟢 MINOR — RepoManager.cs:GetDirectorySizeBytes — SearchOption.AllDirectories follows symlinks (3/5 models)

Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories) follows directory symlinks on macOS. Worktrees with symlinked node_modules or other vendored directories could be counted in both the bare clone and the worktree scan, inflating the reported size. Fix: use EnumerationOptions { RecurseSubdirectories = true, AttributesToSkip = FileAttributes.ReparsePoint }.

---

No Issues Found (Investigated, Clean)

• ✅ firstRender guard prevents duplicate timer creation on re-renders
• ✅ _diskSizeCache thread-safety: ConcurrentDictionary handles concurrent reads/writes correctly
• ✅ SessionSidebar.Dispose() correctly unsubscribes from OnStateChanged and disposes the timer
• ✅ FormatSize uses InvariantCulture and correct KB/MB/GB thresholds
• ✅ Singleton injection: RepoManager is a DI singleton, so instance _diskSizeCache is shared correctly

---

Test Coverage

16 new tests cover FormatSize, GetDirectorySizeBytes, GetRepoDiskSize cache TTL, and RefreshDiskSizesAsync. The concurrent-scan path and the symlink edge case are not tested.

---

CI

⚠️ Pre-existing failure: Scenario_FullReflectCycleWithScoring (locale decimal separator mismatch, unrelated to this PR).

---

Verdict

⚠️ Request Changes — The MODERATE concurrent-scan issue should be addressed before merge (easy one-liner fix). The two MINOR issues are low-urgency but worth tracking.

[StephaneDelcroix]

✅ Multi-Model Re-Review (Round 2): Ready to Merge

Commit: 409c831

Previous Findings Status

Finding	Status
🟡 TTL/timer mismatch — disk size blinks away every 5min	✅ FIXED (round 1 — TTL raised to 7min)
🟡 No reentrance guard — overlapping scans on slow machines	✅ FIXED (round 2 — Interlocked.CompareExchange guard)
🟢 OnStateChanged from background thread	✅ N/A — subscriber uses InvokeAsync(StateHasChanged), safe
🟢 EnumerateFiles follows symlinks	✅ N/A — accepted known limitation

---

Round 2 Re-Review (5-model consensus, 3+ required to include)

No new CRITICAL or MODERATE issues found. Two pre-known MINOR items noted by 2/5 models:

• 🟢 RepoManager.cs — OnStateChanged fires from background thread: All 5 existing OnStateChanged call sites in RepoManager follow the same pattern (caller-context, subscribers marshal themselves). Consistent with the codebase. Subscriber already uses InvokeAsync. No action needed.

• 🟢 RepoManager.cs — EnumerateFiles(..., SearchOption.AllDirectories) follows dir symlinks: Accepted limitation. Worst case: slightly inflated size display for repos with symlinked node_modules. The outer catch {} prevents crashes.

---

Test Coverage

16 tests covering: FormatSize boundary values, GetDirectorySizeBytes (empty/files/nonexistent), cache TTL logic, worktree inclusion, and missing path handling. CI: ⚠️ pre-existing locale failure (Scenario_FullReflectCycleWithScoring) unrelated to this PR.

---

Verdict: ✅ Approve

Both rounds of fixes applied and verified. All 1632 non-pre-existing tests pass. The implementation is solid — reentrance guard, correct TTL, proper timer lifecycle (firstRender guard + Dispose). Ready to merge.