Unified tree filter with lazy branch loading, search, and cwd prioritization

[nstrayer]

This PR replaces the separate repo/branch filter modals with a unified tree-style filter view. Repos are listed as expandable nodes with branches lazy-loaded on demand. The current working directory repo and branch are prioritized in sort order.

Summary

• Unified tree filter replacing separate repo and branch modals
• Lazy branch loading: branches fetch per-repo on expand (right-arrow) or search
• Search matches unloaded branches by triggering lazy fetches in the background
• Cwd repo sorts to top of filter tree; cwd branch sorts to top within its repo
• b key shortcut opens filter with branches auto-expanded for the most relevant repo
• Left-arrow collapse works during active search (overrides search auto-expansion)
• Search-triggered loads do not permanently expand repos; only user-initiated expansion persists
• Stale in-flight branch responses detected via rootPaths identity check
• Branch fetch errors surface through connection tracking and reconnect logic
• Slow tests moved behind //go:build integration tag; shared RSA key generation via sync.Once
• Dead code removed: old fetchBranches, flatFilterEntry.depth, unused message fields

Details

Unified tree filter (5bb3631): Replaces the two-modal flow (repo list → branch list) with a single tree view. Each repo is a collapsible node; right-arrow expands to show branches, left-arrow collapses. Enter selects a repo or branch filter.

Cwd prioritization (e997569): At TUI launch, the model detects cwdRepoRoot and cwdBranch. The cwd repo sorts to index 0 in the filter tree, and the cwd branch sorts to first position when branches load. Independent of autoFilterRepo config.

Stale message guards (e06e319): tuiRepoBranchesMsg carries rootPaths so the handler can verify the tree node at repoIdx still represents the same repo, preventing races when the filter is closed and reopened during in-flight fetches.

'b' key shortcut (ec18ede, db62b33): Opens the unified filter with branches auto-expanded for the target repo (active filter > cwd > first). Cursor lands on the first branch. filterBranchMode clears on escape, enter, and after branches load.

Filter regressions fixed (49e73f8): h/l keys no longer captured by expand/collapse (arrow-keys only), multi-root-path repos matched correctly via rootPathsMatch(), branch fetch failures clear loading state.

Error handling fixes (5e4d7f1): Branch fetch errors bypass the view/staleness gate so connection tracking and reconnect logic always fire. Errors are no longer silently dropped when the filter view is not active.

Lazy search loading (527f35b): Typing in the filter search triggers fetchUnloadedBranches() for repos that haven't loaded branches yet, so search can match branch names across all repos.

Search expansion fixes (d034bbf, 104a9c9): Search-triggered loads use expandOnLoad: false so repos don't permanently expand. Left-arrow sets userCollapsed to override search auto-expansion. Right-arrow during an in-flight search load preserves user expand intent.

Test speedups (57ace30): TestDaemonRunStartsAndShutdownsCleanly and stream tests moved behind //go:build integration. RSA key generation shared via sync.Once. Analyze test uses direct arg validation instead of cmd.Execute().

Test plan

• Unified tree filter shows repos with expand/collapse via arrow keys
• Lazy branch loading on right-arrow and search triggers
• Search matches branches across all repos (including previously unloaded)
• Search-triggered loads do not permanently expand repos
• Left-arrow collapses repos during active search
• Right-arrow during in-flight search load preserves user intent
• userCollapsed resets when search is cleared
• Cwd repo/branch sort to first position
• Stale branch messages with mismatched rootPaths are dropped
• b key auto-expands target repo and positions cursor on first branch
• Branch fetch errors clear loading state and trigger reconnect
• h/l keys pass through to search; only arrow keys expand/collapse
• Multi-root-path repos matched correctly in b-mode expansion
• go test ./... passes; go vet ./... clean

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review

Verdict: 1 Medium issue found (no Critical/High findings; security reviews reported no vulnerabilities).

Medium

1. Branch node can get stuck in loading state after a fetch error
   • File refs: cmd/roborev/tui_handlers.go:84, cmd/roborev/tui.go:1820, cmd/roborev/tui.go (case tuiErrMsg)
   • Impact: If fetchBranchesForRepo fails once, the repo can remain loading=true, and subsequent expand/retry is blocked (!node.loading gate).
   • Suggested fix: Emit a repo-scoped error (include repoIdx/rootPaths) and clear that node’s loading flag in Update on failure; also clear filterBranchMode when auto-expand fails.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Mostly clean change set, with one Medium UI behavior regression to address before merge.

Medium

• Search lazy-load permanently expands repos (UI regression)
  Files: cmd/roborev/tui.go:1887, cmd/roborev/tui_handlers.go:185
  Issue: Search-triggered lazy branch fetches mark repos as expanded (expanded = true) after fetch success. When search is cleared, many repos remain expanded even if the user never explicitly expanded them.
  Why it matters: This changes persistent UI state unexpectedly and makes the filter view noisy after normal search usage.
  Suggested fix: Track fetch intent (for example, explicit expand vs search preload) in the branch-fetch message and only set expanded = true for explicit user expansion flows (for example, right-arrow / branch-open behavior), not search preloads.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: 2 Medium-severity issues should be addressed before merge.

Medium

1. Terminal escape injection in TUI rendering

   • Location: cmd/roborev/tui.go:2831, cmd/roborev/tui.go:2836 (renderFilterView)
   • Finding: Repository/branch names are rendered directly (node.name, branch.name) without output sanitization, allowing ANSI/control-sequence injection into terminal UI.
   • Impact: UI spoofing/corruption in terminal output.
   • Suggested fix: Sanitize names before rendering (for example, apply existing display sanitization like sanitizeForDisplay(...)).

2. Search-triggered branch loading can cause thundering-herd resource spikes

   • Location: cmd/roborev/tui_handlers.go:203 (fetchUnloadedBranches)
   • Finding: Search input can trigger concurrent fetches for many/all unloaded repos at once.
   • Impact: High local resource usage and degraded responsiveness for large repo sets.
   • Suggested fix: Add debounce/rate limiting and cap concurrent fetches (or otherwise batch/defer loading).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Looks good overall, with one Medium regression risk to address before merge.

Medium

1. Retry can get stuck after branch-load failure
   • Files: cmd/roborev/tui_handlers.go (handleFilterKey, "right" case), cmd/roborev/tui.go (case tuiRepoBranchesMsg, error path)
   • Issue: If a repo is set expanded=true while branch loading is in-flight and that load fails, loading is cleared but expanded remains true with children == nil. Subsequent right-arrow presses are blocked by the if !node.expanded guard, so retry is effectively blocked unless the user first collapses or changes search text.
   • Suggested fix: On fetch error, reset expanded=false when children is nil, or allow right-arrow to fetch when children==nil && !loading even if expanded is already true.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: One Medium correctness issue was found; no High/Critical or security issues were identified.

Medium

• Order-sensitive repo identity comparison can mis-handle multi-path groups
  File: cmd/roborev/tui.go:1494
  Also used at: cmd/roborev/tui.go:1850, cmd/roborev/tui.go:1888, cmd/roborev/tui.go:1899
rootPathsMatch compares paths by index, so equivalent path sets in different orders may be treated as different repos. This can cause incorrect b auto-expand targeting, stale-message misclassification, and loading-state edge-case inconsistencies.
  Suggested fix: compare root paths as sets (or sort copies before comparing).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: One Medium-severity issue should be addressed before merge.

Medium

1. Integration test coverage can silently drop from default CI
   • File: cmd/roborev/daemon_integration_test.go:1, internal/daemon/stream_integration_test.go:1
   • Finding: Adding //go:build integration removes daemon lifecycle/stream tests from default go test ./...; if CI does not explicitly run -tags=integration, regressions in startup/shutdown and stream behavior may go undetected.
   • Suggested fix: Add/require a CI job that runs integration-tagged tests (for example: go test -tags=integration ./cmd/roborev ./internal/daemon).

---

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review

Verdict: Mostly clean change set, but there is 1 Medium regression risk that should be addressed before merge.

Medium

1. Incomplete branch search results when many repos are tracked
   • Files: cmd/roborev/tui_handlers.go:173, cmd/roborev/tui_handlers.go:218, cmd/roborev/tui.go:1900
   • Issue: Search-triggered branch loading is capped per keystroke, but completion of a fetch does not schedule subsequent batches. If typing pauses, repos beyond early batches may never load, so valid branch matches can be missing from results.
   • Suggested fix: When handling tuiRepoBranchesMsg and filterSearch != "", keep scheduling fetchUnloadedBranches() until all unloaded repos are processed, with explicit in-flight tracking to enforce a global concurrency cap.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Mostly solid refactor, but there is 1 Medium-severity correctness issue that should be fixed before merge.

Medium

1. consecutiveErrors is not reset after successful branch fetches
   • File: cmd/roborev/tui.go (handler for tuiRepoBranchesMsg, around @@ -1714,43 +1812,139)
   • Issue: On successful per-repo branch fetches, m.consecutiveErrors is not reset.
   • Impact: Reconnect/failure-threshold logic can trigger on effectively non-consecutive failures (e.g., fail -> success -> fail -> fail still counts as consecutive).
   • Suggested fix: Reset m.consecutiveErrors on successful tuiRepoBranchesMsg handling (optionally clear m.err as done in other successful fetch handlers).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Mostly clean, with one Medium functional regression to address before merge.

Medium

1. Branch search can stay empty if user types before repos load
   • Location: cmd/roborev/tui.go:1837
   • Issue: In case tuiReposMsg, the tree is rebuilt but branch prefetch is not triggered when filterSearch was entered before repositories finished loading.
   • Impact: Branch-only searches may appear empty until another keypress triggers fetchUnloadedBranches().
   • Suggested fix: After m.rebuildFilterFlatList() in tuiReposMsg, if m.filterSearch != "", invoke m.fetchUnloadedBranches() (or equivalent batched branch-fetch logic).

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: Mostly clean, with one Medium regression risk to address before merge.

Critical

None.

High

None.

Medium

1. fetchFailed can persist after reconnect and block branch-search recovery
   • cmd/roborev/tui.go:1902
   • cmd/roborev/tui_handlers.go:224
   • cmd/roborev/tui_handlers.go:1281
   • When search-triggered branch fetches fail, fetchFailed=true is set and later skipped by fetchUnloadedBranches(). After a successful reconnect, this state is not cleared, so branch search may stay incomplete until search is cleared or repos are manually retried.
   • Recommended fix: clear fetchFailed on successful reconnect (or only set it for non-connection errors), then trigger a top-up fetch when filterSearch != "".

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[roborev-ci]

roborev: Combined Review

Verdict: One medium-severity regression risk identified; no critical/high issues and no new security vulnerabilities reported.

Medium

1. fetchFailed may persist across non-empty search edits, blocking future auto-loads
   • Files: cmd/roborev/tui.go (in rebuildFilterFlatList), cmd/roborev/tui_handlers.go (in fetchUnloadedBranches)
   • Issue: fetchFailed is reset only when search becomes exactly empty. If fetch fails for search "a" and user changes to "ab" (still non-empty), that repo can remain skipped for auto-fetch, hiding branch matches until manual retry/reconnect/full clear.
   • Suggested fix: Reset failure state on any filterSearchSeq change (not only empty search), or scope failures to a search generation key (for example fetchFailedSeq). Add a test for non-empty → non-empty search transitions after failure.

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)

[wesm]

Gosh, so many darn edge cases! I'm going to merge this once the build is green

[roborev-ci]

roborev: Combined Review

Verdict: One Medium finding remains; security reviews reported no vulnerabilities.

Medium

1. Integration tests may no longer run in default CI (testing gap)
   • Files: cmd/roborev/daemon_integration_test.go:1, internal/daemon/stream_integration_test.go:1
   • Issue: These tests are now behind //go:build integration, so they are skipped by default go test ./.... If CI does not explicitly run integration-tagged tests, regressions may be missed.
   • Suggested fix: Add/verify a CI step that runs go test -tags=integration ./....

---

Synthesized from 4 reviews (agents: codex, gemini | types: security, default)