Refresh background workspace git metadata after external checkout#987
Refresh background workspace git metadata after external checkout#987austinywang merged 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds internal background priming for tabs/workspaces: per-tab priming state and policy, background terminal surface startup, Git metadata probe workflow with delayed retries, pending-background-load bookkeeping, UI wiring to trigger priming, and small public API additions to opt into eager terminal loading. Changes
Sequence DiagramsequenceDiagram
participant User
participant TabManager
participant ContentView
participant Workspace
participant TerminalPanel
User->>TabManager: addWorkspace(..., eagerLoadTerminal: true)
TabManager->>TabManager: requestBackgroundWorkspaceLoad(workspaceId)
TabManager->>TabManager: scheduleInitialWorkspaceGitMetadataRefresh(workspaceId)
TabManager-->>ContentView: publish pendingBackgroundWorkspaceLoadIds
ContentView->>ContentView: detect workspace in pending set
ContentView->>ContentView: primeBackgroundWorkspaceIfNeeded(workspaceId)
ContentView->>Workspace: requestBackgroundTerminalSurfaceStartIfNeeded()
Workspace->>TerminalPanel: surface.requestBackgroundSurfaceStartIfNeeded()
TerminalPanel-->>TerminalPanel: initialize surface (async)
TerminalPanel-->>Workspace: surface ready
ContentView->>TabManager: completeBackgroundWorkspaceLoad(workspaceId)
TabManager->>TabManager: remove from pendingBackgroundWorkspaceLoadIds
TabManager-->>ContentView: publish updated pending set
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a bounded background git-metadata refresh pipeline for workspaces that are opened with an explicit Key changes:
The main technical concern is in Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant TC as TerminalController
participant TM as TabManager (MainActor)
participant Q as initialWorkspaceGitProbeQueue
participant Git as git subprocess
participant CV as ContentView (SwiftUI)
TC->>TM: addWorkspace(cwd, eagerLoadTerminal: true)
TM->>TM: scheduleInitialWorkspaceGitMetadataRefresh(workspaceId, panelId, dir)
TM->>TM: requestBackgroundWorkspaceLoad(workspaceId)
TM-->>CV: @Published pendingBackgroundWorkspaceLoadIds updated
CV->>CV: reconcileMountedWorkspaceIds() — pins workspace
CV->>CV: .task(id: tab.id) fires → primeBackgroundWorkspaceIfNeeded()
loop 6 probes at [0, 0.5, 1.5, 3.0, 6.0, 10.0]s
Q->>Git: git -C dir branch --show-current
Git-->>Q: branch name
Q->>Git: git -C dir status --porcelain -uno
Git-->>Q: dirty status
Q->>TM: Task @MainActor applyInitialWorkspaceGitMetadataSnapshot(snapshot)
TM->>TM: updatePanelGitBranch / clearPanelPullRequest
end
loop every 50 ms (up to 2 s)
CV->>CV: stepBackgroundWorkspacePrime(workspaceId)
CV->>CV: requestBackgroundTerminalSurfaceStartIfNeeded()
alt surface loaded
CV->>TM: completeBackgroundWorkspaceLoad(workspaceId)
TM-->>CV: pendingBackgroundWorkspaceLoadIds updated → task cancelled
else timeout
CV->>TM: completeBackgroundWorkspaceLoad(workspaceId)
end
end
Last reviewed commit: 1291776 |
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/TabManager.swift">
<violation number="1" location="Sources/TabManager.swift:991">
P1: `runGitCommand` can deadlock by calling `waitUntilExit()` before reading piped stdout.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/ContentView.swift (1)
2604-2613: Extract priming timing values into named constants.
2.0and50_000_000are magic numbers in a central flow. Pulling them into local policy constants will make tuning/tests clearer and safer.♻️ Suggested refactor
+ private enum BackgroundWorkspacePrimePolicy { + static let timeoutSeconds: TimeInterval = 2.0 + static let pollIntervalNanoseconds: UInt64 = 50_000_000 + } + private func primeBackgroundWorkspaceIfNeeded(workspaceId: UUID) async { @@ - let timeout = Date().addingTimeInterval(2.0) + let timeout = Date().addingTimeInterval(BackgroundWorkspacePrimePolicy.timeoutSeconds) while !Task.isCancelled { @@ - try? await Task.sleep(nanoseconds: 50_000_000) + try? await Task.sleep(nanoseconds: BackgroundWorkspacePrimePolicy.pollIntervalNanoseconds) continue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 2604 - 2613, The loop uses magic numbers for the priming timeout and poll sleep; extract these into named constants (e.g. primingTimeoutSeconds and primingPollSleepNanoseconds) near the surrounding function so the timeout initialization (currently Date().addingTimeInterval(2.0)) and the Task.sleep(nanoseconds: 50_000_000) call reference those constants instead of raw literals; update any related comparisons and comments in the block that calls stepBackgroundWorkspacePrime(workspaceId:) and checks Task.isCancelled to use the new constants for clarity and easier tuning/testing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests_v2/test_cli_new_workspace_external_git_branch_refresh.py`:
- Line 19: SOCKET_PATH currently falls back to the untagged
"/tmp/cmux-debug.sock"; remove the default and require a tagged socket value by
reading CMUX_SOCKET without a fallback and validating it matches the tagged
pattern (starts with "/tmp/cmux-debug-" and contains a tag and ".sock"); update
the SOCKET_PATH assignment in
tests_v2/test_cli_new_workspace_external_git_branch_refresh.py and add a short
validation that raises/prints an error and exits the test if CMUX_SOCKET is
unset or does not match the "/tmp/cmux-debug-<tag>.sock" format so the test
cannot connect to an untagged instance.
---
Nitpick comments:
In `@Sources/ContentView.swift`:
- Around line 2604-2613: The loop uses magic numbers for the priming timeout and
poll sleep; extract these into named constants (e.g. primingTimeoutSeconds and
primingPollSleepNanoseconds) near the surrounding function so the timeout
initialization (currently Date().addingTimeInterval(2.0)) and the
Task.sleep(nanoseconds: 50_000_000) call reference those constants instead of
raw literals; update any related comparisons and comments in the block that
calls stepBackgroundWorkspacePrime(workspaceId:) and checks Task.isCancelled to
use the new constants for clarity and easier tuning/testing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62edac6c-2ed4-4e0c-81a2-142330b3700c
📒 Files selected for processing (5)
Sources/ContentView.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/Workspace.swifttests_v2/test_cli_new_workspace_external_git_branch_refresh.py
Summary
Verification
./scripts/reload.sh --tag issue-915-terminal-not-loadedSummary by cubic
Fixes background workspaces not updating git branch/status after external repo changes so the sidebar stays correct. Addresses Linear issue 915 by eager-loading background terminals and refreshing git metadata without waiting for a new shell prompt.
Written for commit 0285cd1. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests