Fix blank terminal when creating a new workspace - #2835
austinywang wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds an initial render-refresh flow for newly selected workspaces: defers surface display until window attach, exposes a surface display request on the Ghostty surface view, schedules multi-pass geometry/refresh from Workspace when a workspace is selected, and adds a UI test exercising rapid workspace creation. Changes
Sequence Diagram(s)sequenceDiagram
participant TabMgr as TabManager
participant WS as Workspace
participant Panel as TerminalPanel
participant PortalReg as TerminalWindowPortalRegistry
participant Surface as GhosttySurfaceScrollView
TabMgr->>WS: scheduleInitialSelectedTerminalRenderRefresh(reason)
WS->>Panel: reattachView() / scheduleGeometryReconcile()
Note over WS: First refresh pass (delay 0)
WS->>WS: validate selected workspace & focused panel
WS->>PortalReg: synchronizeExternalGeometry(window-aware)
WS->>Panel: reconcileGeometry()
alt Ghostty surface exists
WS->>Panel: forceRefresh(reason)
else No surface yet
WS->>Panel: startBackgroundSurface()
end
WS->>Surface: requestVisibleSurfaceDisplayIfNeeded()
Surface->>Surface: if window==nil set deferral flag and return
Surface->>Surface: else layoutSubtreeIfNeeded() + setNeedsDisplay()
Note over WS: Second refresh pass (delay 0.03)
WS->>WS: validate still selected & focused
WS->>PortalReg: synchronizeExternalGeometry()
WS->>Panel: reconcileGeometry()
WS->>Surface: requestVisibleSurfaceDisplayIfNeeded()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
Greptile SummaryThis PR fixes a blank terminal screen on new workspace creation by scheduling a post-insertion render refresh that mirrors the re-present path: reattaching the portal, synchronizing geometry, forcing a surface refresh, and nudging the selected surface layer to display. A UI regression test validates that each of 10 rapidly-created workspaces presents its first terminal frame before the next is created. Confidence Score: 5/5Safe to merge — fix follows established patterns, guards against stale state, and is covered by a new regression test. All findings are P2 style suggestions (async-redispatch guard vs. assertion, code duplication with scheduleMovedTerminalRefresh). No logic errors, data integrity issues, or threading violations were found. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Socket as Socket Command
participant TM as TabManager.addWorkspace
participant WS as Workspace
participant Portal as TerminalWindowPortalRegistry
participant SV as GhosttySurfaceScrollView
Socket->>TM: new_workspace (select: true)
TM->>TM: selectedTabId = newWorkspace.id
TM->>TM: post .ghosttyDidFocusTab
TM->>WS: scheduleInitialSelectedTerminalRenderRefresh()
WS->>WS: terminalPanel.requestViewReattach()
WS->>WS: scheduleTerminalGeometryReconcile()
Note over WS: Pass 1 — asyncAfter(delay: 0)
WS->>Portal: scheduleExternalGeometrySynchronize(for: window)
WS->>SV: reconcileGeometryNow()
WS->>SV: surface.forceRefresh(reason:)
WS->>SV: requestVisibleSurfaceDisplayIfNeeded()
Note over WS: Pass 2 — asyncAfter(delay: 0.03 s)
WS->>Portal: scheduleExternalGeometrySynchronize(for: window)
WS->>SV: reconcileGeometryNow()
WS->>SV: surface.forceRefresh(reason:)
WS->>SV: requestVisibleSurfaceDisplayIfNeeded()
Reviews (1): Last reviewed commit: "Refresh terminal portal after workspace ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if !Thread.isMainThread { | ||
| DispatchQueue.main.async { [weak self] in | ||
| self?.requestVisibleSurfaceDisplayIfNeeded() | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
The if !Thread.isMainThread branch dispatches asynchronously back to main, but since every call site is already dispatched via DispatchQueue.main.asyncAfter, this path is never exercised. A silent async redispatch here would mask a threading mistake at the call site rather than surface it. An assert or precondition would be more appropriate for a method that is explicitly scoped to structural transitions.
| if !Thread.isMainThread { | |
| DispatchQueue.main.async { [weak self] in | |
| self?.requestVisibleSurfaceDisplayIfNeeded() | |
| } | |
| return | |
| } | |
| assert(Thread.isMainThread, "requestVisibleSurfaceDisplayIfNeeded must be called on the main thread") |
| func scheduleInitialSelectedTerminalRenderRefresh(reason: String = "workspace.create.initialSelection") { | ||
| guard let terminalPanel = focusedTerminalPanel else { return } | ||
| let panelId = terminalPanel.id | ||
|
|
||
| // New-workspace insertion can saturate the current layout turn before the freshly | ||
| // selected terminal gets the same portal sync/reveal path that a tab re-present does. | ||
| terminalPanel.requestViewReattach() | ||
| scheduleTerminalGeometryReconcile() | ||
|
|
||
| let runRefreshPass: (TimeInterval) -> Void = { [weak self] delay in | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + delay) { | ||
| guard let self, | ||
| self.owningTabManager?.selectedTabId == self.id, | ||
| self.focusedPanelId == panelId, | ||
| let panel = self.terminalPanel(for: panelId) else { return } | ||
|
|
||
| if let window = panel.hostedView.window { | ||
| TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window) | ||
| } else { | ||
| TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows() | ||
| } | ||
|
|
||
| panel.hostedView.reconcileGeometryNow() | ||
| if panel.surface.surface != nil { | ||
| panel.surface.forceRefresh(reason: reason) | ||
| } else { | ||
| panel.surface.requestBackgroundSurfaceStartIfNeeded() | ||
| } | ||
| panel.hostedView.requestVisibleSurfaceDisplayIfNeeded() | ||
| } | ||
| } | ||
|
|
||
| runRefreshPass(0) | ||
| runRefreshPass(0.03) | ||
| } |
There was a problem hiding this comment.
runRefreshPass pattern with scheduleMovedTerminalRefresh
scheduleInitialSelectedTerminalRenderRefresh and the private scheduleMovedTerminalRefresh (lines 10890–10913) share the same double-pass asyncAfter structure with near-identical closure bodies. The new method adds portal geometry sync and the requestVisibleSurfaceDisplayIfNeeded() nudge on top, but the overall skeleton is repeated verbatim. Extracting the shared structure into a private helper (e.g. accepting the extra steps as closures or a flags enum) would make the two call sites easier to maintain together when the refresh logic evolves.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/Workspace.swift (1)
10932-10935: Avoid the all-windows geometry sync fallback here.When
panel.hostedView.windowis stillnil, callingscheduleExternalGeometrySynchronizeForAllWindows()adds cross-window portal work on the exact path this bug report says is already main-thread saturated, and it still does not give this panel a concrete window to synchronize against.Sources/GhosttyTerminalView.swift:11952-11981already treats “no window yet” as a defer/retry case. Prefer a panel-local retry once the hosted view is attached instead of fanning out to every window.♻️ Suggested change
- if let window = panel.hostedView.window { - TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window) - } else { - TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows() - } + guard let window = panel.hostedView.window else { + panel.requestViewReattach() + self.scheduleTerminalGeometryReconcile() + return + } + TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 10932 - 10935, The current fallback calling TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows() when panel.hostedView.window is nil should be removed; instead, defer and retry for this specific panel by observing when panel.hostedView gets attached to a window and then call TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window). Concretely: stop invoking scheduleExternalGeometrySynchronizeForAllWindows(), add a panel-local retry mechanism (e.g., attach a one-shot observer/callback on panel.hostedView to detect when hostedView.window becomes non-nil or reuse the same defer/retry approach used in GhosttyTerminalView’s handling), and once you detect a window, call TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window) and remove the observer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Workspace.swift`:
- Around line 10932-10935: The current fallback calling
TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronizeForAllWindows()
when panel.hostedView.window is nil should be removed; instead, defer and retry
for this specific panel by observing when panel.hostedView gets attached to a
window and then call
TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window).
Concretely: stop invoking scheduleExternalGeometrySynchronizeForAllWindows(),
add a panel-local retry mechanism (e.g., attach a one-shot observer/callback on
panel.hostedView to detect when hostedView.window becomes non-nil or reuse the
same defer/retry approach used in GhosttyTerminalView’s handling), and once you
detect a window, call
TerminalWindowPortalRegistry.scheduleExternalGeometrySynchronize(for: window)
and remove the observer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98e7f74a-e192-4e73-b8e1-865a01f74391
📒 Files selected for processing (4)
Sources/GhosttyTerminalView.swiftSources/TabManager.swiftSources/Workspace.swiftcmuxUITests/AutomationSocketUITests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Summary
TabManager.addWorkspace()selects the new workspaceCloses #2555
Note
Medium Risk
Touches terminal rendering/portal synchronization and CoreAnimation display triggers during workspace insertion, which could introduce visual regressions or extra redraw work. Scope is limited to post-selection refresh paths and adds a UI regression test to catch failures.
Overview
Prevents newly created workspaces from showing a blank terminal by scheduling an immediate post-selection render/geometry sync when
TabManager.addWorkspace()selects the new workspace.Adds
Workspace.scheduleInitialSelectedTerminalRenderRefresh()to reattach the focused panel, reconcile portal geometry, force/start the surface, and request a layer display pass via the newGhosttyTerminalView.requestVisibleSurfaceDisplayIfNeeded()(including deferred execution until the view has a window).Adds an automation-socket UI regression test that rapidly creates 10 workspaces and asserts each selected terminal presents its first IOSurface-backed frame using new
render_statspolling helpers and a lightweight UNIX-domain socket client.Reviewed by Cursor Bugbot for commit 0561f15. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #2555: new workspaces now show the selected terminal’s first frame immediately, preventing a blank terminal. Adds a regression UI test that creates 10 workspaces and confirms first-frame rendering without switching tabs.
TabManager.addWorkspace(), schedule an initial render refresh on the newly selected terminal: reattach the view, reconcile geometry and portal sync (per-window when possible), start the surface if needed, force a refresh, and request a visible-surface display with a window-attach fallback; run this pass twice (now and ~30ms later) to cover layout/composition races.render_stats; bypasses onboarding with-cmuxWelcomeShown YES.Written for commit 0561f15. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests