Fix early split child-exit close race
This commit is contained in:
parent
4fb5da373d
commit
cb0efa3edd
3 changed files with 100 additions and 26 deletions
|
|
@ -1126,11 +1126,13 @@ class GhosttyApp {
|
|||
// "Process exited. Press any key..." into the terminal unless the host
|
||||
// handles this action. For cmux, the correct behavior is to close
|
||||
// the panel immediately (no prompt).
|
||||
let callbackTabId = surfaceView.tabId
|
||||
let callbackSurfaceId = surfaceView.terminalSurface?.id
|
||||
#if DEBUG
|
||||
cmuxWriteChildExitProbe(
|
||||
[
|
||||
"probeShowChildExitedTabId": surfaceView.tabId?.uuidString ?? "",
|
||||
"probeShowChildExitedSurfaceId": surfaceView.terminalSurface?.id.uuidString ?? "",
|
||||
"probeShowChildExitedTabId": callbackTabId?.uuidString ?? "",
|
||||
"probeShowChildExitedSurfaceId": callbackSurfaceId?.uuidString ?? "",
|
||||
],
|
||||
increments: ["probeShowChildExitedCount": 1]
|
||||
)
|
||||
|
|
@ -1139,12 +1141,12 @@ class GhosttyApp {
|
|||
// dispatching this action callback.
|
||||
DispatchQueue.main.async {
|
||||
guard let app = AppDelegate.shared else { return }
|
||||
if let tabId = surfaceView.tabId,
|
||||
let surfaceId = surfaceView.terminalSurface?.id,
|
||||
let manager = app.tabManagerFor(tabId: tabId) ?? app.tabManager,
|
||||
let workspace = manager.tabs.first(where: { $0.id == tabId }),
|
||||
workspace.panels[surfaceId] != nil {
|
||||
manager.closePanelAfterChildExited(tabId: tabId, surfaceId: surfaceId)
|
||||
if let callbackTabId,
|
||||
let callbackSurfaceId,
|
||||
let manager = app.tabManagerFor(tabId: callbackTabId) ?? app.tabManager,
|
||||
let workspace = manager.tabs.first(where: { $0.id == callbackTabId }),
|
||||
workspace.panels[callbackSurfaceId] != nil {
|
||||
manager.closePanelAfterChildExited(tabId: callbackTabId, surfaceId: callbackSurfaceId)
|
||||
}
|
||||
}
|
||||
// Always report handled so Ghostty doesn't print the fallback prompt.
|
||||
|
|
@ -1945,7 +1947,10 @@ final class TerminalSurface: Identifiable, ObservableObject {
|
|||
}
|
||||
|
||||
deinit {
|
||||
if let surface = surface {
|
||||
guard let surface else { return }
|
||||
|
||||
// Defer teardown to the next main-actor turn so close callbacks can unwind first.
|
||||
Task.detached { @MainActor in
|
||||
ghostty_surface_free(surface)
|
||||
}
|
||||
}
|
||||
|
|
@ -4066,7 +4071,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
/// This exercises the same key path as real keyboard input (ghostty_surface_key),
|
||||
/// unlike `sendText`, which bypasses key translation.
|
||||
@discardableResult
|
||||
func sendSyntheticCtrlDForUITest() -> Bool {
|
||||
func sendSyntheticCtrlDForUITest(modifierFlags: NSEvent.ModifierFlags = [.control]) -> Bool {
|
||||
guard let window else { return false }
|
||||
window.makeFirstResponder(surfaceView)
|
||||
|
||||
|
|
@ -4074,7 +4079,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
guard let keyDown = NSEvent.keyEvent(
|
||||
with: .keyDown,
|
||||
location: .zero,
|
||||
modifierFlags: [.control],
|
||||
modifierFlags: modifierFlags,
|
||||
timestamp: timestamp,
|
||||
windowNumber: window.windowNumber,
|
||||
context: nil,
|
||||
|
|
@ -4087,7 +4092,7 @@ final class GhosttySurfaceScrollView: NSView {
|
|||
guard let keyUp = NSEvent.keyEvent(
|
||||
with: .keyUp,
|
||||
location: .zero,
|
||||
modifierFlags: [.control],
|
||||
modifierFlags: modifierFlags,
|
||||
timestamp: timestamp + 0.001,
|
||||
windowNumber: window.windowNumber,
|
||||
context: nil,
|
||||
|
|
|
|||
|
|
@ -2732,6 +2732,8 @@ class TabManager: ObservableObject {
|
|||
let strictKeyOnly = env["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_STRICT"] == "1"
|
||||
let triggerMode = (env["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_TRIGGER_MODE"] ?? "shell_input")
|
||||
.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
let useEarlyCtrlShiftTrigger = triggerMode == "early_ctrl_shift_d"
|
||||
let triggerUsesShift = triggerMode == "ctrl_shift_d" || useEarlyCtrlShiftTrigger
|
||||
let layout = (env["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_LAYOUT"] ?? "lr")
|
||||
.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
let expectedPanelsAfter = max(
|
||||
|
|
@ -2870,7 +2872,9 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
|
||||
tab.focusPanel(exitPanelId)
|
||||
try? await Task.sleep(nanoseconds: 100_000_000)
|
||||
if !useEarlyCtrlShiftTrigger {
|
||||
try? await Task.sleep(nanoseconds: 100_000_000)
|
||||
}
|
||||
|
||||
let focusedPanelBefore = tab.focusedPanelId?.uuidString ?? ""
|
||||
let firstResponderPanelBefore = tab.panels.compactMap { (panelId, panel) -> UUID? in
|
||||
|
|
@ -2974,21 +2978,31 @@ class TabManager: ObservableObject {
|
|||
return
|
||||
}
|
||||
|
||||
// Wait for the target panel to be fully attached after split churn.
|
||||
let readyDeadline = Date().addingTimeInterval(2.0)
|
||||
let triggerModifiers: NSEvent.ModifierFlags = triggerUsesShift
|
||||
? [.control, .shift]
|
||||
: [.control]
|
||||
let shouldWaitForSurface = !useEarlyCtrlShiftTrigger
|
||||
|
||||
var attachedBeforeTrigger = false
|
||||
var hasSurfaceBeforeTrigger = false
|
||||
while Date() < readyDeadline {
|
||||
guard let panel = tab.terminalPanel(for: exitPanelId) else {
|
||||
write(["autoTriggerError": "missingExitPanelBeforeTrigger"])
|
||||
return
|
||||
if shouldWaitForSurface {
|
||||
// Wait for the target panel to be fully attached after split churn.
|
||||
let readyDeadline = Date().addingTimeInterval(2.0)
|
||||
while Date() < readyDeadline {
|
||||
guard let panel = tab.terminalPanel(for: exitPanelId) else {
|
||||
write(["autoTriggerError": "missingExitPanelBeforeTrigger"])
|
||||
return
|
||||
}
|
||||
attachedBeforeTrigger = panel.hostedView.window != nil
|
||||
hasSurfaceBeforeTrigger = panel.surface.surface != nil
|
||||
if attachedBeforeTrigger, hasSurfaceBeforeTrigger {
|
||||
break
|
||||
}
|
||||
try? await Task.sleep(nanoseconds: 50_000_000)
|
||||
}
|
||||
} else if let panel = tab.terminalPanel(for: exitPanelId) {
|
||||
attachedBeforeTrigger = panel.hostedView.window != nil
|
||||
hasSurfaceBeforeTrigger = panel.surface.surface != nil
|
||||
if attachedBeforeTrigger, hasSurfaceBeforeTrigger {
|
||||
break
|
||||
}
|
||||
try? await Task.sleep(nanoseconds: 50_000_000)
|
||||
}
|
||||
write([
|
||||
"exitPanelAttachedBeforeTrigger": attachedBeforeTrigger ? "1" : "0",
|
||||
|
|
@ -3000,7 +3014,7 @@ class TabManager: ObservableObject {
|
|||
return
|
||||
}
|
||||
// Exercise the real key path (ghostty_surface_key for Ctrl+D).
|
||||
if panel.hostedView.sendSyntheticCtrlDForUITest() {
|
||||
if panel.hostedView.sendSyntheticCtrlDForUITest(modifierFlags: triggerModifiers) {
|
||||
write(["autoTriggerSentCtrlDKey1": "1"])
|
||||
} else {
|
||||
write([
|
||||
|
|
@ -3012,13 +3026,19 @@ class TabManager: ObservableObject {
|
|||
|
||||
// In strict mode, never mask routing bugs with fallback writes.
|
||||
if strictKeyOnly {
|
||||
write(["autoTriggerMode": "strict_ctrl_d"])
|
||||
let strictModeLabel: String = {
|
||||
if useEarlyCtrlShiftTrigger { return "strict_early_ctrl_shift_d" }
|
||||
if triggerUsesShift { return "strict_ctrl_shift_d" }
|
||||
return "strict_ctrl_d"
|
||||
}()
|
||||
write(["autoTriggerMode": strictModeLabel])
|
||||
return
|
||||
}
|
||||
|
||||
// Non-strict mode keeps one additional Ctrl+D retry for startup timing variance.
|
||||
try? await Task.sleep(nanoseconds: 450_000_000)
|
||||
if tab.panels[exitPanelId] != nil, panel.hostedView.sendSyntheticCtrlDForUITest() {
|
||||
if tab.panels[exitPanelId] != nil,
|
||||
panel.hostedView.sendSyntheticCtrlDForUITest(modifierFlags: triggerModifiers) {
|
||||
write(["autoTriggerSentCtrlDKey2": "1"])
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -546,6 +546,55 @@ final class CloseWorkspaceCmdDUITests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
func testCtrlShiftDEarlyDuringSplitStartupKeepsWindowOpen() {
|
||||
let attempts = 12
|
||||
for attempt in 1...attempts {
|
||||
let app = XCUIApplication()
|
||||
let dataPath = "/tmp/cmux-ui-test-child-exit-keyboard-lr-early-shift-\(UUID().uuidString).json"
|
||||
try? FileManager.default.removeItem(atPath: dataPath)
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_SETUP"] = "1"
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_PATH"] = dataPath
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_LAYOUT"] = "lr"
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_EXPECTED_PANELS_AFTER"] = "1"
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_AUTO_TRIGGER"] = "1"
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_STRICT"] = "1"
|
||||
app.launchEnvironment["CMUX_UI_TEST_CHILD_EXIT_KEYBOARD_TRIGGER_MODE"] = "early_ctrl_shift_d"
|
||||
app.launch()
|
||||
app.activate()
|
||||
defer { app.terminate() }
|
||||
|
||||
XCTAssertTrue(
|
||||
waitForAnyJSON(atPath: dataPath, timeout: 12.0),
|
||||
"Attempt \(attempt): expected early Ctrl+Shift+D setup data at \(dataPath)"
|
||||
)
|
||||
guard let done = waitForJSONKey("done", equals: "1", atPath: dataPath, timeout: 10.0) else {
|
||||
XCTFail("Attempt \(attempt): timed out waiting for done=1 after early Ctrl+Shift+D. data=\(loadJSON(atPath: dataPath) ?? [:])")
|
||||
return
|
||||
}
|
||||
|
||||
if let setupError = done["setupError"], !setupError.isEmpty {
|
||||
XCTFail("Attempt \(attempt): setup failed: \(setupError)")
|
||||
return
|
||||
}
|
||||
|
||||
let workspaceCountAfter = Int(done["workspaceCountAfter"] ?? "") ?? -1
|
||||
let panelCountAfter = Int(done["panelCountAfter"] ?? "") ?? -1
|
||||
let closedWorkspace = (done["closedWorkspace"] ?? "") == "1"
|
||||
let timedOut = (done["timedOut"] ?? "") == "1"
|
||||
let triggerMode = done["autoTriggerMode"] ?? ""
|
||||
|
||||
XCTAssertFalse(timedOut, "Attempt \(attempt): early Ctrl+Shift+D timed out. data=\(done)")
|
||||
XCTAssertEqual(triggerMode, "strict_early_ctrl_shift_d", "Attempt \(attempt): expected strict early Ctrl+Shift+D trigger mode. data=\(done)")
|
||||
XCTAssertFalse(closedWorkspace, "Attempt \(attempt): workspace/window should stay open after early Ctrl+Shift+D. data=\(done)")
|
||||
XCTAssertEqual(workspaceCountAfter, 1, "Attempt \(attempt): workspace should remain open after early Ctrl+Shift+D. data=\(done)")
|
||||
XCTAssertEqual(panelCountAfter, 1, "Attempt \(attempt): only focused pane should close after early Ctrl+Shift+D. data=\(done)")
|
||||
XCTAssertTrue(
|
||||
waitForWindowCount(app: app, atLeast: 1, timeout: 2.0),
|
||||
"Attempt \(attempt): app window should remain open after early Ctrl+Shift+D. data=\(done)"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private func waitForCloseWorkspaceAlert(app: XCUIApplication, timeout: TimeInterval) -> Bool {
|
||||
let deadline = Date().addingTimeInterval(timeout)
|
||||
while Date() < deadline {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue