Fix sidebar live refresh for branch and PR state - #2331
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis change introduces a dedicated, faster polling mechanism (5-second interval) for the currently selected workspace's focused panel Git metadata. It replaces branch-based polling guards with simpler candidate panel selection logic that always includes the focused panel, removing two helper methods that consulted branch information for polling decisions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes two related bugs: (1) the background git-metadata poll was silently skipping workspaces on Confidence Score: 5/5Safe to merge; the only open item is a P2 suggestion to add a testing shim for the new fast-path function. All findings are P2 style/coverage suggestions. The logic change is correct: removing the main/master filter is precisely the intended fix, the PR-lookup guard in workspacePullRequestSnapshot is left intact preventing GitHub API regressions, and the two-commit regression structure matches CLAUDE.md policy. Sources/TabManager.swift — the new refreshSelectedWorkspaceGitMetadata function lacks a testing accessor, but its underlying logic is covered transitively by the existing tests. Important Files Changed
Sequence DiagramsequenceDiagram
participant ST as selectedWorkspaceGitMetadataPollTimer (5s)
participant BT as workspaceGitMetadataPollTimer (30s)
participant TM as TabManager (main thread)
participant WS as Workspace (selected)
participant Git as git process
Note over ST,BT: Both timers fire on .global(qos: .utility), dispatch to main
ST->>TM: refreshSelectedWorkspaceGitMetadata()
TM->>TM: trackedWorkspaceGitMetadataPollCandidatePanelIds(selectedWorkspace)
TM->>TM: guard candidatePanelIds.contains(focusedPanelId)
TM->>TM: scheduleWorkspaceGitMetadataRefreshIfPossible(focusedPanelId)
TM->>Git: git branch --show-current
TM->>Git: git status --porcelain -uno
Git-->>TM: branch / dirty state
TM->>WS: updatePanelGitBranch(focusedPanelId, branch, isDirty)
Note right of WS: also sets gitBranch when panelId == focusedPanelId
BT->>TM: refreshTrackedWorkspaceGitMetadata()
TM->>TM: iterate all workspace tabs
TM->>TM: scheduleWorkspaceGitMetadataRefreshIfPossible (skipped if probe active)
Reviews (1): Last reviewed commit: "Refresh sidebar git metadata on active w..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private func refreshSelectedWorkspaceGitMetadata() { | ||
| guard let workspace = selectedWorkspace, | ||
| let focusedPanelId = workspace.focusedPanelId else { | ||
| return | ||
| } | ||
|
|
||
| let activeProbeKeys = Set(workspaceGitProbeGenerationByKey.keys) | ||
| let candidatePanelIds = trackedWorkspaceGitMetadataPollCandidatePanelIds( | ||
| in: workspace, | ||
| activeProbeKeys: activeProbeKeys | ||
| ) | ||
| guard candidatePanelIds.contains(focusedPanelId) else { return } | ||
|
|
||
| scheduleWorkspaceGitMetadataRefreshIfPossible( | ||
| workspaceId: workspace.id, | ||
| panelId: focusedPanelId, | ||
| reason: "selectedPeriodicPoll" | ||
| ) | ||
| } |
There was a problem hiding this comment.
refreshSelectedWorkspaceGitMetadata contains its own guard logic (specifically the candidatePanelIds.contains(focusedPanelId) check) that isn't exercised by any test. All new tests invoke refreshTrackedWorkspaceGitMetadataForTesting(), which goes through the background-sweep path.
The existing convention in the file is to expose a ForTesting wrapper for functions that need unit coverage. Consider adding:
func refreshSelectedWorkspaceGitMetadataForTesting() {
refreshSelectedWorkspaceGitMetadata()
}This would let a test verify that a workspace whose focused panel is not yet in panelGitBranches is correctly skipped by the fast path, and that a refresh is scheduled once the panel becomes a candidate.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmuxTests/TabManagerUnitTests.swift (1)
499-504: Wait on both branch projections in the same predicate.The test only blocks until
workspace.panelGitBranches[panelId]flips, then immediately readsworkspace.gitBranch. If those two updates land on adjacent async turns, this becomes unnecessarily timing-sensitive in CI.♻️ Suggested test hardening
- XCTAssertTrue( - waitForCondition { - workspace.panelGitBranches[panelId]?.branch == "feature/sidebar-live-refresh" - } - ) - XCTAssertEqual(workspace.gitBranch?.branch, "feature/sidebar-live-refresh") + XCTAssertTrue( + waitForCondition { + workspace.panelGitBranches[panelId]?.branch == "feature/sidebar-live-refresh" + && workspace.gitBranch?.branch == "feature/sidebar-live-refresh" + } + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/TabManagerUnitTests.swift` around lines 499 - 504, The test currently waits only for workspace.panelGitBranches[panelId]?.branch to become "feature/sidebar-live-refresh" then immediately asserts workspace.gitBranch?.branch, which can race; update the waitForCondition predicate (the closure passed to waitForCondition) to check both workspace.panelGitBranches[panelId]?.branch and workspace.gitBranch?.branch equal "feature/sidebar-live-refresh" in the same predicate so the test blocks until both projections are updated (referencing waitForCondition, workspace.panelGitBranches[panelId], workspace.gitBranch, and panelId).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TabManager.swift`:
- Line 702: The selected-workspace loop currently driven by
selectedWorkspaceGitMetadataPollInterval (5s) runs the full metadata probe
including the gh pr list / gh pr checks calls; change this so the 5s loop only
performs local checks (branch name, working-tree dirty state) and move remote
PR/GitHub CLI calls into a separate, lower-frequency path. Concretely: in the
selected-workspace loop (the code that reads
selectedWorkspaceGitMetadataPollInterval) remove or short-circuit calls to the
functions that invoke the GitHub CLI (e.g., any
fetchPRMetadata/probeRemotePRs/performGitHubCliChecks helpers) and either (a)
add a separate timer/worker for remote probes with its own TTL/backoff, or (b)
gate remote calls with a lastPRCheckTime + TTL check so remote probes run far
less often. Ensure you reference selectedWorkspaceGitMetadataPollInterval and
the PR-check helper functions when applying the change so only local checkout
detection remains on the 5s interval.
---
Nitpick comments:
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 499-504: The test currently waits only for
workspace.panelGitBranches[panelId]?.branch to become
"feature/sidebar-live-refresh" then immediately asserts
workspace.gitBranch?.branch, which can race; update the waitForCondition
predicate (the closure passed to waitForCondition) to check both
workspace.panelGitBranches[panelId]?.branch and workspace.gitBranch?.branch
equal "feature/sidebar-live-refresh" in the same predicate so the test blocks
until both projections are updated (referencing waitForCondition,
workspace.panelGitBranches[panelId], workspace.gitBranch, and panelId).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 482b51f5-d775-4980-b655-cdb44466d3ec
📒 Files selected for processing (2)
Sources/TabManager.swiftcmuxTests/TabManagerUnitTests.swift
| private static var nextPortOrdinal: Int = 0 | ||
| private static let initialWorkspaceGitProbeDelays: [TimeInterval] = [0, 0.5, 1.5, 3.0, 6.0, 10.0] | ||
| private static let workspaceGitMetadataPollInterval: TimeInterval = 30 | ||
| private static let selectedWorkspaceGitMetadataPollInterval: TimeInterval = 5 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="Sources/TabManager.swift"
printf '== New fast polling loop ==\n'
sed -n '700,1015p' "$file"
printf '\n== Full metadata probe path used by that loop ==\n'
sed -n '1582,1856p' "$file"
printf '\n== Worst-case selected-workspace gh call volume ==\n'
python - <<'PY'
selected_interval_seconds = 5
gh_calls_per_poll_when_pr_open = 2 # gh pr list + gh pr checks
polls_per_hour = 3600 // selected_interval_seconds
print({
"selected_workspace_polls_per_hour": polls_per_hour,
"worst_case_gh_calls_per_hour_per_window": polls_per_hour * gh_calls_per_poll_when_pr_open
})
PYRepository: manaflow-ai/cmux
Length of output: 23150
Throttle PR/network probes separately from the new 5s branch poll.
The new selected-workspace loop runs the full metadata probe path, which includes gh pr list and gh pr checks calls on every 5-second cycle when a PR is open on a non-main branch. This drives ~1,440 GitHub CLI calls per hour per window—a significant reliability and performance cost for code that only needs fast local checkout detection.
Keep the 5-second loop local-only (branch/dirtiness) or add a separate PR TTL/backoff to decouple remote polling from the high-frequency timer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TabManager.swift` at line 702, The selected-workspace loop currently
driven by selectedWorkspaceGitMetadataPollInterval (5s) runs the full metadata
probe including the gh pr list / gh pr checks calls; change this so the 5s loop
only performs local checks (branch name, working-tree dirty state) and move
remote PR/GitHub CLI calls into a separate, lower-frequency path. Concretely: in
the selected-workspace loop (the code that reads
selectedWorkspaceGitMetadataPollInterval) remove or short-circuit calls to the
functions that invoke the GitHub CLI (e.g., any
fetchPRMetadata/probeRemotePRs/performGitHubCliChecks helpers) and either (a)
add a separate timer/worker for remote probes with its own TTL/backoff, or (b)
gate remote calls with a lastPRCheckTime + TTL check so remote probes run far
less often. Ensure you reference selectedWorkspaceGitMetadataPollInterval and
the PR-check helper functions when applying the change so only local checkout
detection remains on the 5s interval.
* Add regression coverage for sidebar live refresh * Refresh sidebar git metadata on active workspaces
This branch was successfully deployed
1 participant
Summary
Closes #2329
Testing
Summary by cubic
Fixes sidebar live refresh so branch checkouts and PR state update within ~5s, even on
main/master(closes #2329). Adds a fast poll for the selected workspace and keeps git metadata polling active for all tracked workspaces.main/master; all tracked panels are now polled unless a probe is active.main/masterinclusion and for updating after checking out a feature branch.Written for commit 5ab3ee1. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Tests