Skip to content

Fix #2347 terminal focus and surface recovery #2354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
austinywang merged 4 commits into from
Mar 30, 2026
Merged

Fix #2347 terminal focus and surface recovery #2354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7932ac7
Add regressions for issue #2347 terminal focus loss
austinywang Mar 30, 2026
87c1de7
Fix issue #2347 terminal focus and surface recovery
austinywang Mar 30, 2026
803dff1
Add regressions for missing-surface recovery review cases
austinywang Mar 30, 2026
5686090
Fix missing-surface recovery lifecycle and focus replay
austinywang Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 74 additions & 2 deletions Sources/GhosttyTerminalView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3394,6 +3394,10 @@ final class TerminalSurface: Identifiable, ObservableObject {
}
}

private func allowsRuntimeSurfaceCreation() -> Bool {
portalLifecycleState == .live
}

func beginPortalCloseLifecycle(reason: String) {
guard portalLifecycleState != .closed else { return }
guard portalLifecycleState != .closing else { return }
Expand Down Expand Up @@ -3459,14 +3463,18 @@ final class TerminalSurface: Identifiable, ObservableObject {
}
}

#if DEBUG
#if DEBUG
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Mismatched #if DEBUG / #endif indentation

The #if DEBUG directive was de-indented from 4 spaces to column 0, but its matching #endif on line 3499 ( #endif) is still indented with 4 spaces. All other #if DEBUG / #endif pairs in this file use consistent column-0 placement. The compiler is fine with mixed indentation, but it's inconsistent with the rest of the file.

Suggested change
#if DEBUG
#if DEBUG

private static let surfaceLogPath = "/tmp/cmux-ghostty-surface.log"
private static let sizeLogPath = "/tmp/cmux-ghostty-size.log"

func debugCurrentPixelSize() -> (width: UInt32, height: UInt32) {
(lastPixelWidth, lastPixelHeight)
}

func debugDesiredFocusState() -> Bool {
desiredFocusState
}

private static func surfaceLog(_ message: String) {
let timestamp = ISO8601DateFormatter().string(from: Date())
let line = "[\(timestamp)] \(message)\n"
Expand Down Expand Up @@ -3564,6 +3572,15 @@ final class TerminalSurface: Identifiable, ObservableObject {
// If surface doesn't exist yet, create it once the view is in a real window so
// content scale and pixel geometry are derived from the actual backing context.
if surface == nil {
guard allowsRuntimeSurfaceCreation() else {
#if DEBUG
dlog(
"surface.attach.skip surface=\(id.uuidString.prefix(5)) " +
"reason=lifecycle.\(portalLifecycleState.rawValue)"
)
#endif
return
}
guard view.window != nil else {
#if DEBUG
dlog(
Expand Down Expand Up @@ -3593,6 +3610,18 @@ final class TerminalSurface: Identifiable, ObservableObject {
}

private func createSurface(for view: GhosttyNSView) {
guard allowsRuntimeSurfaceCreation() else {
#if DEBUG
dlog(
"surface.create.skip surface=\(id.uuidString.prefix(5)) " +
"reason=lifecycle.\(portalLifecycleState.rawValue)"
)
Self.surfaceLog(
"createSurface SKIPPED surface=\(id.uuidString) tab=\(tabId.uuidString) lifecycle=\(portalLifecycleState.rawValue)"
)
#endif
return
}
#if DEBUG
let resourcesDir = getenv("GHOSTTY_RESOURCES_DIR").flatMap { String(cString: $0) } ?? "(unset)"
let terminfo = getenv("TERMINFO").flatMap { String(cString: $0) } ?? "(unset)"
Expand Down Expand Up @@ -4140,13 +4169,15 @@ final class TerminalSurface: Identifiable, ObservableObject {
return
}

guard allowsRuntimeSurfaceCreation() else { return }
guard surface == nil, attachedView != nil else { return }
guard !backgroundSurfaceStartQueued else { return }
backgroundSurfaceStartQueued = true

DispatchQueue.main.async { [weak self] in
guard let self else { return }
self.backgroundSurfaceStartQueued = false
guard self.allowsRuntimeSurfaceCreation() else { return }
guard self.surface == nil, let view = self.attachedView else { return }
#if DEBUG
let startedAt = ProcessInfo.processInfo.systemUptime
Expand Down Expand Up @@ -5030,6 +5061,16 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
return surface
}

private func requestInputRecoveryAfterSurfaceMiss(reason: String) {
terminalSurface?.requestBackgroundSurfaceStartIfNeeded()
#if DEBUG
dlog(
"focus.input_recovery surface=\(terminalSurface?.id.uuidString.prefix(5) ?? "nil") " +
"reason=\(reason) inWindow=\(window != nil ? 1 : 0)"
)
#endif
}
Comment on lines +5064 to +5072
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Propagate focus intent on the missing-surface key path.

This helper only queues recreation, but createSurface(for:) initializes the new runtime surface from TerminalSurface.desiredFocusState, not GhosttyNSView.desiredFocus. If first-responder acquisition happened while the runtime surface was nil, that state is still stale here, so the recreated surface can come back unfocused and the next key still goes nowhere. Mirror the setFocus(true) behavior that reassertTerminalSurfaceFocus(reason:) already uses.

💡 Proposed fix 
     private func requestInputRecoveryAfterSurfaceMiss(reason: String) {
+        desiredFocus = true
+        terminalSurface?.setFocus(true)
         terminalSurface?.requestBackgroundSurfaceStartIfNeeded()
 `#if` DEBUG
         dlog(
             "focus.input_recovery surface=\(terminalSurface?.id.uuidString.prefix(5) ?? "nil") " +

Also applies to: 5732-5738

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 5064 - 5072,
requestInputRecoveryAfterSurfaceMiss(reason:) currently only queues surface
recreation but doesn't propagate the intended focus state, so
createSurface(for:) will initialize the new TerminalSurface from
TerminalSurface.desiredFocusState (which may be stale) instead of
GhosttyNSView.desiredFocus; update requestInputRecoveryAfterSurfaceMiss(reason:)
to mirror setFocus(true) used by reassertTerminalSurfaceFocus(reason:) by
explicitly propagating the view's desired focus intent to the recreated surface
(e.g., call the same focus propagation path or set the new surface's desired
focus to GhosttyNSView.desiredFocus / call setFocus(true) logic) so the
recreated surface receives first-responder intent; apply the same change to the
second occurrence of this helper (the block also present around the 5732-5738
area).


func performBindingAction(_ action: String) -> Bool {
guard let surface = surface else { return false }
return action.withCString { cString in
Expand Down Expand Up @@ -5460,12 +5501,12 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
let result = super.resignFirstResponder()
if result {
desiredFocus = false
terminalSurface?.recordExternalFocusState(false)
}
if result, let surface = surface {
let now = CACurrentMediaTime()
let deltaMs = (now - lastScrollEventTime) * 1000
Self.focusLog("resignFirstResponder: surface=\(terminalSurface?.id.uuidString ?? "nil") deltaSinceScrollMs=\(String(format: "%.2f", deltaMs))")
terminalSurface?.recordExternalFocusState(false)
ghostty_surface_set_focus(surface, false)
}
return result
Expand Down Expand Up @@ -5689,6 +5730,7 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
let ensureSurfaceStart = ProcessInfo.processInfo.systemUptime
#endif
guard let surface = ensureSurfaceReadyForInput() else {
requestInputRecoveryAfterSurfaceMiss(reason: "keyDown.missingSurface")
#if DEBUG
ensureSurfaceMs = (ProcessInfo.processInfo.systemUptime - ensureSurfaceStart) * 1000.0
#endif
Expand Down Expand Up @@ -8865,6 +8907,33 @@ final class GhosttySurfaceScrollView: NSView {

func clearSuppressReparentFocus() {
surfaceView.suppressingReparentFocus = false
let hasUsablePortalGeometry: Bool = {
let size = bounds.size
return size.width > 1 && size.height > 1
}()
let isHiddenForFocus = isHiddenOrHasHiddenAncestor || surfaceView.isHiddenOrHasHiddenAncestor
let surfaceShort = surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil"
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 surfaceShort computed before early-return guards

surfaceShort is only consumed inside #if DEBUG blocks further down, yet it is computed eagerly here — before the four guard statements that will short-circuit the function in the common "nothing to do" case. In release builds this is a dead allocation on every non-matching call.

Consider wrapping the declaration:

#if DEBUG
let surfaceShort = surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil"
#endif

hasUsablePortalGeometry and isHiddenForFocus are used in the non-debug guard so they need to stay where they are — only surfaceShort is affected.


guard surfaceView.desiredFocus else { return }
guard isSurfaceViewFirstResponder() else { return }
guard isActive else { return }
guard surfaceView.isVisibleInUI else { return }
guard let window, window.isKeyWindow else { return }
guard !isHiddenForFocus, hasUsablePortalGeometry else {
#if DEBUG
dlog(
"focus.reparent.resume.defer surface=\(surfaceShort) " +
"reason=hidden_or_tiny hidden=\(isHiddenForFocus ? 1 : 0) " +
"frame=\(String(format: "%.1fx%.1f", bounds.width, bounds.height))"
)
#endif
scheduleAutomaticFirstResponderApply(reason: "clearSuppressReparentFocus.hiddenOrTiny")
return
}
#if DEBUG
dlog("focus.reparent.resume surface=\(surfaceShort) firstResponder=\(String(describing: window.firstResponder))")
#endif
reassertTerminalSurfaceFocus(reason: "clearSuppressReparentFocus")
Comment on lines 8909 to +8936
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This path restores Ghostty focus but not pane focus state.

If suppression masked the original becomeFirstResponder(), onFocus?() never ran. Reasserting only the Ghostty surface here can leave Bonsplit/current-panel state pointing at the pane chosen by focusPanel(...) while AppKit first responder and terminal input stay on this surface.

💡 Suggested fix 
         guard !isHiddenForFocus, hasUsablePortalGeometry else {
 `#if` DEBUG
             dlog(
                 "focus.reparent.resume.defer surface=\(surfaceShort) " +
                 "reason=hidden_or_tiny hidden=\(isHiddenForFocus ? 1 : 0) " +
                 "frame=\(String(format: "%.1fx%.1f", bounds.width, bounds.height))"
             )
 `#endif`
             scheduleAutomaticFirstResponderApply(reason: "clearSuppressReparentFocus.hiddenOrTiny")
             return
         }
+        surfaceView.onFocus?()
 `#if` DEBUG
         dlog("focus.reparent.resume surface=\(surfaceShort) firstResponder=\(String(describing: window.firstResponder))")
 `#endif`
         reassertTerminalSurfaceFocus(reason: "clearSuppressReparentFocus")
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 8884 - 8911, The current path
clears suppressingReparentFocus and reasserts only the terminal surface
(reassertTerminalSurfaceFocus) which restores AppKit first responder but does
not restore Bonsplit/pane focus state or invoke the surfaceView's onFocus
callback; update the code that runs when suppression is removed (the block after
the guards where reassertTerminalSurfaceFocus is called) to also restore pane
focus state by calling the same method used elsewhere to set
Bonsplit/current-panel (e.g. focusPanel(...) or the internal function that
updates Bonsplit state) and invoke surfaceView.onFocus?() (or the equivalent
onFocus handler) so the pane-level focus state and any on-focus side effects run
in addition to reassertTerminalSurfaceFocus.

}

/// Returns true if the terminal's actual Ghostty surface view is (or contains) the window first responder.
Expand All @@ -8891,6 +8960,9 @@ final class GhosttySurfaceScrollView: NSView {

private func reassertTerminalSurfaceFocus(reason: String) {
guard let terminalSurface = surfaceView.terminalSurface else { return }
if terminalSurface.surface == nil {
terminalSurface.requestBackgroundSurfaceStartIfNeeded()
}
#if DEBUG
dlog("focus.surface.reassert surface=\(terminalSurface.id.uuidString.prefix(5)) reason=\(reason)")
#endif
Expand Down
205 changes: 205 additions & 0 deletions cmuxTests/TerminalAndGhosttyTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,10 @@ final class TerminalDirectoryOpenTargetAvailabilityTests: XCTestCase {

@MainActor
final class TerminalNotificationDirectInteractionTests: XCTestCase {
private final class FocusProbeView: NSView {
override var acceptsFirstResponder: Bool { true }
}

private func makeWindow() -> NSWindow {
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 480, height: 320),
Expand Down Expand Up @@ -1497,6 +1501,207 @@ final class TerminalNotificationDirectInteractionTests: XCTestCase {
XCTAssertFalse(store.hasUnreadNotification(forTabId: workspace.id, surfaceId: terminalPanel.id))
XCTAssertEqual(GhosttySurfaceScrollView.flashCount(for: terminalPanel.id), 1)
}

func testKeyDownRecoversReleasedSurfaceWhileHostedViewIsDetached() throws {
#if DEBUG
let window = makeWindow()
defer { window.orderOut(nil) }

guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}

let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
hostedView.frame = contentView.bounds
hostedView.autoresizingMask = [.width, .height]
contentView.addSubview(hostedView)

window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()
contentView.layoutSubtreeIfNeeded()
hostedView.layoutSubtreeIfNeeded()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))

guard let surfaceView = surfaceView(in: hostedView) as? GhosttyNSView else {
XCTFail("Expected terminal surface view")
return
}
XCTAssertNotNil(surface.surface, "Expected runtime surface before simulating the detach race")

surface.releaseSurfaceForTesting()
XCTAssertNil(surface.surface, "Expected runtime surface to be released for the regression setup")

hostedView.removeFromSuperview()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertNil(surfaceView.window, "Expected hosted terminal view to be detached from any window")

let event = makeKeyEvent(characters: "a", keyCode: 0, window: window)
surfaceView.keyDown(with: event)

let recovered = XCTNSPredicateExpectation(
predicate: NSPredicate { _, _ in
surface.surface != nil
},
object: NSObject()
)
wait(for: [recovered], timeout: 1.0)

XCTAssertNotNil(
surface.surface,
"Missing-surface keyDown should request background surface recreation instead of leaving terminal input dead"
)
#else
throw XCTSkip("Debug-only regression test")
#endif
}

func testKeyDownRecoveryDoesNotReplayFocusAfterResponderMovesAway() throws {
#if DEBUG
let window = makeWindow()
defer { window.orderOut(nil) }

guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}

let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
hostedView.frame = contentView.bounds
hostedView.autoresizingMask = [.width, .height]
contentView.addSubview(hostedView)

let otherResponder = FocusProbeView(frame: NSRect(x: 0, y: 0, width: 40, height: 40))
contentView.addSubview(otherResponder)

window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()
contentView.layoutSubtreeIfNeeded()
hostedView.layoutSubtreeIfNeeded()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))

guard let surfaceView = surfaceView(in: hostedView) as? GhosttyNSView else {
XCTFail("Expected terminal surface view")
return
}

XCTAssertTrue(window.makeFirstResponder(surfaceView))
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertTrue(surface.debugDesiredFocusState(), "Focused terminal should start with desired Ghostty focus")

surface.releaseSurfaceForTesting()
XCTAssertNil(surface.surface, "Expected runtime surface to be released for the regression setup")

hostedView.removeFromSuperview()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertNil(surfaceView.window, "Expected hosted terminal view to be detached from any window")
XCTAssertTrue(
(window.firstResponder as? NSView) === surfaceView,
"Expected the detached Ghostty view to remain the stale first responder during the regression setup"
)

let event = makeKeyEvent(characters: "a", keyCode: 0, window: window)
surfaceView.keyDown(with: event)

XCTAssertTrue(window.makeFirstResponder(otherResponder))
RunLoop.current.run(until: Date().addingTimeInterval(0.05))

XCTAssertTrue(
(window.firstResponder as? NSView) === otherResponder,
"Expected focus to move to the replacement responder"
)
XCTAssertFalse(
surface.debugDesiredFocusState(),
"Responder loss after a missing-surface keyDown should clear desired Ghostty focus before recovery completes"
)

let recovered = XCTNSPredicateExpectation(
predicate: NSPredicate { _, _ in
surface.surface != nil
},
object: NSObject()
)
wait(for: [recovered], timeout: 1.0)

XCTAssertNotNil(surface.surface, "Expected missing-surface recovery to still recreate the runtime surface")
XCTAssertFalse(
surface.debugDesiredFocusState(),
"Recovered runtime surface should not restore focus after the pane already lost first responder"
)
#else
throw XCTSkip("Debug-only regression test")
#endif
}

func testKeyDownRecoveryDoesNotRecreateClosedSurface() throws {
#if DEBUG
let window = makeWindow()
defer { window.orderOut(nil) }

guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}

let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
hostedView.frame = contentView.bounds
hostedView.autoresizingMask = [.width, .height]
contentView.addSubview(hostedView)

window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()
contentView.layoutSubtreeIfNeeded()
hostedView.layoutSubtreeIfNeeded()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))

guard let surfaceView = surfaceView(in: hostedView) as? GhosttyNSView else {
XCTFail("Expected terminal surface view")
return
}
XCTAssertNotNil(surface.surface, "Expected runtime surface before simulating close lifecycle teardown")

surface.beginPortalCloseLifecycle(reason: "test.close")
surface.teardownSurface()
XCTAssertNil(surface.surface, "Teardown should release the runtime surface")
XCTAssertEqual(surface.portalBindingStateLabel(), "closed")

hostedView.removeFromSuperview()
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertNil(surfaceView.window, "Expected hosted terminal view to be detached from any window")

let event = makeKeyEvent(characters: "a", keyCode: 0, window: window)
surfaceView.keyDown(with: event)

let drained = expectation(description: "background recovery drained")
DispatchQueue.main.async { drained.fulfill() }
wait(for: [drained], timeout: 1.0)

XCTAssertNil(
surface.surface,
"Missing-surface keyDown should not recreate a Ghostty runtime surface after close lifecycle teardown"
)
#else
throw XCTSkip("Debug-only regression test")
#endif
}
}


Expand Down
Loading
Loading