Fix Cmd+W terminal close in terminal+browser split

This commit is contained in:
austinpower1258 2026-02-24 21:54:25 -08:00
parent 65f5b9be6d
commit 17d8956789
6 changed files with 338 additions and 8 deletions

View file

@ -4537,6 +4537,37 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return true return true
} }
// Cmd+W must close the focused panel even if first-responder momentarily lags on a
// browser NSTextView during split focus transitions.
if normalizedFlags == [.command], (chars == "w" || event.keyCode == 13) {
if let targetWindow = event.window ?? NSApp.keyWindow ?? NSApp.mainWindow,
targetWindow.identifier?.rawValue == "cmux.settings" {
targetWindow.performClose(nil)
} else {
let responder = event.window?.firstResponder
?? NSApp.keyWindow?.firstResponder
?? NSApp.mainWindow?.firstResponder
if let ghosttyView = cmuxOwningGhosttyView(for: responder),
let workspaceId = ghosttyView.tabId,
let panelId = ghosttyView.terminalSurface?.id,
let manager = tabManagerFor(tabId: workspaceId) ?? tabManager {
#if DEBUG
dlog(
"shortcut.cmdW route=ghostty workspace=\(workspaceId.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) selected=\(manager.selectedTabId?.uuidString.prefix(5) ?? "nil")"
)
#endif
manager.closePanelWithConfirmation(tabId: workspaceId, surfaceId: panelId)
} else {
#if DEBUG
dlog("shortcut.cmdW route=focusedPanelFallback")
#endif
tabManager?.closeCurrentPanelWithConfirmation()
}
}
return true
}
if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .closeWorkspace)) { if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .closeWorkspace)) {
tabManager?.closeCurrentWorkspaceWithConfirmation() tabManager?.closeCurrentWorkspaceWithConfirmation()
return true return true

View file

@ -137,8 +137,11 @@ final class TerminalPanel: Panel, ObservableObject {
func close() { func close() {
// The surface will be cleaned up by its deinit // The surface will be cleaned up by its deinit
// Just unfocus before closing // Detach from the window portal on real close so stale hosted views
// cannot remain above browser panes after split close.
unfocus() unfocus()
hostedView.setVisibleInUI(false)
TerminalWindowPortalRegistry.detach(hostedView: hostedView)
} }
func requestViewReattach() { func requestViewReattach() {

View file

@ -1116,10 +1116,28 @@ class TabManager: ObservableObject {
} }
private func closePanelWithConfirmation(tab: Workspace, panelId: UUID) { private func closePanelWithConfirmation(tab: Workspace, panelId: UUID) {
let bonsplitTabCount = tab.bonsplitController.allPaneIds.reduce(0) { partial, paneId in
partial + tab.bonsplitController.tabs(inPane: paneId).count
}
let panelKind: String = {
guard let panel = tab.panels[panelId] else { return "missing" }
if panel is TerminalPanel { return "terminal" }
if panel is BrowserPanel { return "browser" }
return String(describing: type(of: panel))
}()
#if DEBUG
dlog(
"surface.close.shortcut.begin tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) kind=\(panelKind) " +
"panelCount=\(tab.panels.count) bonsplitTabs=\(bonsplitTabCount)"
)
#endif
// Cmd+W closes the focused Bonsplit tab (a "tab" in the UI). When the workspace only has // Cmd+W closes the focused Bonsplit tab (a "tab" in the UI). When the workspace only has
// a single tab left, closing it should close the workspace (and possibly the window), // a single tab left, closing it should close the workspace (and possibly the window),
// rather than creating a replacement terminal. // rather than creating a replacement terminal.
let isLastTabInWorkspace = tab.panels.count <= 1 let effectiveSurfaceCount = max(tab.panels.count, bonsplitTabCount)
let isLastTabInWorkspace = effectiveSurfaceCount <= 1
if isLastTabInWorkspace { if isLastTabInWorkspace {
let willCloseWindow = tabs.count <= 1 let willCloseWindow = tabs.count <= 1
let needsConfirm = workspaceNeedsConfirmClose(tab) let needsConfirm = workspaceNeedsConfirmClose(tab)
@ -1127,11 +1145,25 @@ class TabManager: ObservableObject {
let message = willCloseWindow let message = willCloseWindow
? "This will close the last tab and close the window." ? "This will close the last tab and close the window."
: "This will close the last tab and close its workspace." : "This will close the last tab and close its workspace."
#if DEBUG
dlog(
"surface.close.shortcut.confirm tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=lastTab"
)
#endif
guard confirmClose( guard confirmClose(
title: "Close tab?", title: "Close tab?",
message: message, message: message,
acceptCmdD: willCloseWindow acceptCmdD: willCloseWindow
) else { return } ) else {
#if DEBUG
dlog(
"surface.close.shortcut.cancel tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=lastTabConfirmDismissed"
)
#endif
return
}
} }
AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: tab.id) AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: tab.id)
@ -1145,15 +1177,36 @@ class TabManager: ObservableObject {
if let terminalPanel = tab.terminalPanel(for: panelId), if let terminalPanel = tab.terminalPanel(for: panelId),
terminalPanel.needsConfirmClose() { terminalPanel.needsConfirmClose() {
#if DEBUG
dlog(
"surface.close.shortcut.confirm tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=terminalNeedsConfirm"
)
#endif
guard confirmClose( guard confirmClose(
title: "Close tab?", title: "Close tab?",
message: "This will close the current tab.", message: "This will close the current tab.",
acceptCmdD: false acceptCmdD: false
) else { return } ) else {
#if DEBUG
dlog(
"surface.close.shortcut.cancel tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) reason=terminalConfirmDismissed"
)
#endif
return
}
} }
// We already confirmed (if needed); bypass Bonsplit's delegate gating. // We already confirmed (if needed); bypass Bonsplit's delegate gating.
tab.closePanel(panelId, force: true) let closed = tab.closePanel(panelId, force: true)
#if DEBUG
dlog(
"surface.close.shortcut tab=\(tab.id.uuidString.prefix(5)) " +
"panel=\(panelId.uuidString.prefix(5)) closed=\(closed ? 1 : 0) " +
"panelsAfterCall=\(tab.panels.count)"
)
#endif
} }
func closePanelWithConfirmation(tabId: UUID, surfaceId: UUID) { func closePanelWithConfirmation(tabId: UUID, surfaceId: UUID) {

View file

@ -1463,6 +1463,14 @@ enum TerminalWindowPortalRegistry {
portal.hideEntry(forHostedId: hostedId) portal.hideEntry(forHostedId: hostedId)
} }
/// Permanently detach a hosted terminal view from the window-level portal.
/// Use this when a terminal panel is actually closing (not transient SwiftUI dismantle).
static func detach(hostedView: GhosttySurfaceScrollView) {
let hostedId = ObjectIdentifier(hostedView)
guard let windowId = hostedToWindowId.removeValue(forKey: hostedId) else { return }
portalsByWindowId[windowId]?.detachHostedView(withId: hostedId)
}
/// Update the visibleInUI flag on an existing portal entry without rebinding. /// Update the visibleInUI flag on an existing portal entry without rebinding.
/// Called when a bind is deferred (host not yet in window) to prevent stale /// Called when a bind is deferred (host not yet in window) to prevent stale
/// portal syncs from hiding a view that is about to become visible. /// portal syncs from hiding a view that is about to become visible.

View file

@ -1952,17 +1952,38 @@ final class Workspace: Identifiable, ObservableObject {
} }
// Mapping can transiently drift during split-tree mutations. If the target panel is // Mapping can transiently drift during split-tree mutations. If the target panel is
// currently focused, close whichever tab bonsplit marks selected in that focused pane. // currently focused (or is the active terminal first responder), close whichever tab
guard focusedPanelId == panelId, // bonsplit marks selected in that focused pane.
let firstResponderPanelId = cmuxOwningGhosttyView(
for: NSApp.keyWindow?.firstResponder ?? NSApp.mainWindow?.firstResponder
)?.terminalSurface?.id
let targetIsActive = focusedPanelId == panelId || firstResponderPanelId == panelId
guard targetIsActive,
let focusedPane = bonsplitController.focusedPaneId, let focusedPane = bonsplitController.focusedPaneId,
let selected = bonsplitController.selectedTab(inPane: focusedPane) else { let selected = bonsplitController.selectedTab(inPane: focusedPane) else {
#if DEBUG
dlog(
"surface.close.fallback.skip panel=\(panelId.uuidString.prefix(5)) " +
"focusedPanel=\(focusedPanelId?.uuidString.prefix(5) ?? "nil") " +
"firstResponderPanel=\(firstResponderPanelId?.uuidString.prefix(5) ?? "nil") " +
"focusedPane=\(bonsplitController.focusedPaneId?.id.uuidString.prefix(5) ?? "nil")"
)
#endif
return false return false
} }
if force { if force {
forceCloseTabIds.insert(selected.id) forceCloseTabIds.insert(selected.id)
} }
return bonsplitController.closeTab(selected.id) let closed = bonsplitController.closeTab(selected.id)
#if DEBUG
dlog(
"surface.close.fallback panel=\(panelId.uuidString.prefix(5)) " +
"selectedTab=\(String(describing: selected.id).prefix(5)) " +
"closed=\(closed ? 1 : 0)"
)
#endif
return closed
} }
func paneId(forPanelId panelId: UUID) -> PaneID? { func paneId(forPanelId panelId: UUID) -> PaneID? {

View file

@ -0,0 +1,214 @@
#!/usr/bin/env python3
"""
Regression test for issue #464:
Scenario:
- One workspace with exactly two panes:
left: terminal
right: browser (cnn.com)
- Focus the terminal and press Cmd+W.
Expected:
- Terminal closes.
- Browser remains and fills the workspace (no stale terminal content/pane).
This test uses debug socket commands (`simulate_shortcut`, `layout_debug`,
`surface_health`, `drag_hit_chain`).
Run against a Debug app socket (typically with CMUX_SOCKET_MODE=allowAll).
"""
import os
import sys
import time
from pathlib import Path
sys.path.insert(0, str(Path(__file__).parent))
from cmux import cmux, cmuxError
SOCKET_PATH = os.environ.get("CMUX_SOCKET", "/tmp/cmux-debug.sock")
def _wait_until(predicate, timeout_s: float = 5.0, interval_s: float = 0.05) -> bool:
start = time.time()
while time.time() - start < timeout_s:
if predicate():
return True
time.sleep(interval_s)
return False
def _wait_url_contains(client: cmux, panel_id: str, needle: str, timeout_s: float = 20.0) -> None:
def _matches() -> bool:
response = client._send_command(f"get_url {panel_id}").strip().lower()
return not response.startswith("error") and needle.lower() in response
if not _wait_until(_matches, timeout_s=timeout_s, interval_s=0.1):
current = client._send_command(f"get_url {panel_id}")
raise cmuxError(f"Timed out waiting for browser URL containing '{needle}', got: {current}")
def _capture_screenshot(client: cmux, label: str) -> str:
response = client._send_command(f"screenshot {label}").strip()
if not response.startswith("OK "):
return f"<unavailable: {response}>"
parts = response.split(" ", 2)
if len(parts) < 3:
return f"<unavailable: malformed response {response}>"
return parts[2]
def _focused_terminal_ready(client: cmux, panel_id: str) -> bool:
try:
return client.is_terminal_focused(panel_id)
except Exception:
return False
def _drag_hit_chain(client: cmux, nx: float, ny: float) -> str:
return client._send_command(f"drag_hit_chain {nx:.3f} {ny:.3f}").strip()
def _top_hit_view_class(hit_chain: str) -> str:
if not hit_chain or hit_chain == "none" or hit_chain.startswith("ERROR"):
return hit_chain
first = hit_chain.split("->", 1)[0]
return first.split("@", 1)[0]
def main() -> int:
with cmux(SOCKET_PATH) as client:
# Quick sanity check: fail early with actionable info if socket is not in allow mode.
ping_ok = client.ping()
if not ping_ok:
raise cmuxError(
f"Socket ping failed on {SOCKET_PATH}. "
"Launch Debug app with CMUX_SOCKET_MODE=allowAll for this test."
)
workspace_id = client.new_workspace()
try:
client.select_workspace(workspace_id)
time.sleep(0.25)
client.activate_app()
time.sleep(0.15)
browser_id = client.new_pane(
direction="right",
panel_type="browser",
url="https://cnn.com",
)
_wait_url_contains(client, browser_id, "cnn", timeout_s=20.0)
health_before = client.surface_health()
terminal_rows = [row for row in health_before if row.get("type") == "terminal"]
browser_rows = [row for row in health_before if row.get("type") == "browser"]
if len(terminal_rows) != 1 or len(browser_rows) != 1:
raise cmuxError(
f"Expected exactly one terminal and one browser before close; "
f"health={health_before}"
)
terminal_id = terminal_rows[0]["id"]
client.focus_surface(terminal_id)
if not _wait_until(lambda: _focused_terminal_ready(client, terminal_id), timeout_s=4.0):
raise cmuxError(f"Terminal did not become first responder before Cmd+W: {terminal_id}")
before_surfaces = client.list_surfaces()
before_panes = client.list_panes()
before_layout = client.layout_debug()
before_shot = _capture_screenshot(client, "issue464_cmdw_before")
client.simulate_shortcut("cmd+w")
# Give close animations/routing time to settle.
_wait_until(lambda: len(client.list_surfaces()) == 1, timeout_s=4.0, interval_s=0.05)
time.sleep(0.25)
after_surfaces = client.list_surfaces()
after_panes = client.list_panes()
after_health = client.surface_health()
after_layout = client.layout_debug()
after_shot = _capture_screenshot(client, "issue464_cmdw_after")
after_hit_chain = _drag_hit_chain(client, 0.42, 0.50)
after_top_hit_class = _top_hit_view_class(after_hit_chain)
failures: list[str] = []
if len(after_surfaces) != 1:
failures.append(f"Expected 1 surface after Cmd+W, got {len(after_surfaces)}: {after_surfaces}")
if len(after_panes) != 1:
failures.append(f"Expected 1 pane after Cmd+W, got {len(after_panes)}: {after_panes}")
visible_terminals = [
row for row in after_health
if row.get("type") == "terminal" and row.get("in_window") is True
]
if visible_terminals:
failures.append(f"Terminal still visible in_window after Cmd+W: {visible_terminals}")
remaining_browsers = [row for row in after_health if row.get("type") == "browser"]
if len(remaining_browsers) != 1:
failures.append(f"Expected one remaining browser in health, got: {remaining_browsers}")
else:
rb = remaining_browsers[0]
if str(rb.get("id", "")).lower() != browser_id.lower():
failures.append(
f"Remaining browser id mismatch: expected {browser_id}, got {rb.get('id')}"
)
if rb.get("in_window") is not True:
failures.append(f"Remaining browser not in window: {rb}")
selected_panels = after_layout.get("selectedPanels") or []
if len(selected_panels) != 1:
failures.append(f"Expected one selected panel after close, got {selected_panels}")
else:
selected_id = str(selected_panels[0].get("panelId", "")).lower()
if selected_id != browser_id.lower():
failures.append(
f"Selected panel mismatch after close: expected browser {browser_id}, got {selected_id}"
)
if after_top_hit_class == "GhosttyNSView":
failures.append(
"Stale terminal overlay still hit-testable after close "
f"(top_hit={after_top_hit_class}, chain={after_hit_chain})"
)
if failures:
details = [
"Cmd+W close regression reproduced (issue #464).",
f"workspace={workspace_id}",
f"browser={browser_id}",
f"terminal={terminal_id}",
f"before_screenshot={before_shot}",
f"after_screenshot={after_shot}",
f"before_surfaces={before_surfaces}",
f"before_panes={before_panes}",
f"before_layout={before_layout}",
f"after_surfaces={after_surfaces}",
f"after_panes={after_panes}",
f"after_health={after_health}",
f"after_layout={after_layout}",
f"after_hit_chain={after_hit_chain}",
f"after_top_hit_class={after_top_hit_class}",
]
details.extend(f"failure={msg}" for msg in failures)
raise cmuxError("\n".join(details))
print(
"PASS: Cmd+W closed terminal in terminal+browser split and left browser as sole visible pane."
)
print(f"before_screenshot={before_shot}")
print(f"after_screenshot={after_shot}")
return 0
finally:
try:
client.close_workspace(workspace_id)
except Exception:
pass
if __name__ == "__main__":
raise SystemExit(main())