diff --git a/Packages/CrowTerminal/Sources/CrowTerminal/TerminalManager.swift b/Packages/CrowTerminal/Sources/CrowTerminal/TerminalManager.swift index 6a423df..b1f543f 100644 --- a/Packages/CrowTerminal/Sources/CrowTerminal/TerminalManager.swift +++ b/Packages/CrowTerminal/Sources/CrowTerminal/TerminalManager.swift @@ -82,6 +82,7 @@ public final class TerminalManager { return existing } NSLog("[TerminalManager] surface(for: \(id)) — cached view has nil surface, discarding") + existing.removeFromSuperview() existing.destroy() surfaces.removeValue(forKey: id) } @@ -102,14 +103,20 @@ public final class TerminalManager { public func existingSurface(for id: UUID) -> GhosttySurfaceView? { surfaces[id] } public func destroy(id: UUID) { - if let view = surfaces.removeValue(forKey: id) { view.destroy() } + if let view = surfaces.removeValue(forKey: id) { + view.removeFromSuperview() + view.destroy() + } } /// Discard the cached view for `id` (if any) and re-run preInitialize. /// Used by the UI's "Retry" affordance after a permanent surface-creation failure. public func retry(id: UUID, workingDirectory: String, command: String? = nil) { NSLog("[TerminalManager] retry(\(id)) — destroying broken surface and re-preInitializing") - if let view = surfaces.removeValue(forKey: id) { view.destroy() } + if let view = surfaces.removeValue(forKey: id) { + view.removeFromSuperview() + view.destroy() + } // Re-arm readiness tracking; surfaceDidFail removed the id from the set. monitoredTerminals.insert(id) preInitialize(id: id, workingDirectory: workingDirectory, command: command) diff --git a/Packages/CrowTerminal/Sources/CrowTerminal/TerminalSurfaceView.swift b/Packages/CrowTerminal/Sources/CrowTerminal/TerminalSurfaceView.swift index 02e18b2..3ba470f 100644 --- a/Packages/CrowTerminal/Sources/CrowTerminal/TerminalSurfaceView.swift +++ b/Packages/CrowTerminal/Sources/CrowTerminal/TerminalSurfaceView.swift @@ -36,33 +36,17 @@ public struct TerminalSurfaceView: NSViewRepresentable { public func makeNSView(context: Context) -> NSView { let surface = surfaceForBackend() let container = NSView() - container.addSubview(surface) - surface.translatesAutoresizingMaskIntoConstraints = false - NSLayoutConstraint.activate([ - surface.leadingAnchor.constraint(equalTo: container.leadingAnchor), - surface.trailingAnchor.constraint(equalTo: container.trailingAnchor), - surface.topAnchor.constraint(equalTo: container.topAnchor), - surface.bottomAnchor.constraint(equalTo: container.bottomAnchor), - ]) - // Resize after layout settles - DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { - let size = container.bounds.size - if size.width > 0 && size.height > 0 { - surface.setFrameSize(size) - } - surface.window?.makeFirstResponder(surface) - } - // For tmux backends, switch to this terminal's window now that it's - // about to be visible. - if backend == .tmux { - try? TmuxBackend.shared.makeActive(id: terminalID) - } + attach(surface: surface, to: container) + // makeActive is fired from updateNSView — issuing it here too can + // double-fire `tmux select-window` on the same tab activation when + // SwiftUI calls update right after make on .id() recreation. return container } - /// Re-parent the surface if SwiftUI replaced the container, and resize. - /// For tmux backends, also fire makeActive — this is the "tab switched - /// to a different tmux terminal" hook in the shared-surface model. + /// Re-parent the surface if SwiftUI replaced the container, and acquire + /// first responder once the view is in a window. For tmux backends, also + /// fire makeActive — this is the "tab switched to a different tmux + /// terminal" hook in the shared-surface model. @MainActor public func updateNSView(_ nsView: NSView, context: Context) { guard let surface = existingSurfaceForBackend() else { return } @@ -72,23 +56,41 @@ public struct TerminalSurfaceView: NSViewRepresentable { } if surface.superview !== nsView { - surface.removeFromSuperview() - nsView.addSubview(surface) - surface.translatesAutoresizingMaskIntoConstraints = false - NSLayoutConstraint.activate([ - surface.leadingAnchor.constraint(equalTo: nsView.leadingAnchor), - surface.trailingAnchor.constraint(equalTo: nsView.trailingAnchor), - surface.topAnchor.constraint(equalTo: nsView.topAnchor), - surface.bottomAnchor.constraint(equalTo: nsView.bottomAnchor), - ]) - DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) { - let size = nsView.bounds.size - if size.width > 0 && size.height > 0 { - surface.setFrameSize(size) - } - surface.window?.makeFirstResponder(surface) - } + // addSubview re-parents atomically — no need for an explicit + // removeFromSuperview, which would trigger an extra + // viewDidMoveToWindow(nil) round-trip and a redundant + // ghostty_surface_set_focus(false) on the shared surface. + attach(surface: surface, to: nsView) } + + // Acquire first responder after AppKit has had a chance to add the + // container to the window hierarchy. Scheduling via Task @MainActor + // hops to the next main-actor execution without an arbitrary delay, + // so rapid tab switches don't stack stale closures against the + // (shared, in tmux mode) surface. + Task { @MainActor [weak surface] in + guard let surface, + let window = nsView.window, + surface.superview === nsView, + surface.window === window else { return } + window.makeFirstResponder(surface) + } + } + + /// Re-parent `surface` into `container` and pin to its edges. Idempotent + /// constraint setup: relies on `addSubview` to atomically re-parent and + /// on autolayout to drive subsequent `setFrameSize` calls — manual + /// `setFrameSize` here races with autolayout and is unnecessary. + @MainActor + private func attach(surface: GhosttySurfaceView, to container: NSView) { + container.addSubview(surface) + surface.translatesAutoresizingMaskIntoConstraints = false + NSLayoutConstraint.activate([ + surface.leadingAnchor.constraint(equalTo: container.leadingAnchor), + surface.trailingAnchor.constraint(equalTo: container.trailingAnchor), + surface.topAnchor.constraint(equalTo: container.topAnchor), + surface.bottomAnchor.constraint(equalTo: container.bottomAnchor), + ]) } @MainActor diff --git a/Sources/Crow/App/AppDelegate.swift b/Sources/Crow/App/AppDelegate.swift index 1988fa2..09636cc 100644 --- a/Sources/Crow/App/AppDelegate.swift +++ b/Sources/Crow/App/AppDelegate.swift @@ -26,6 +26,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate { private var appConfig: AppConfig? func applicationDidFinishLaunching(_ notification: Notification) { + installUncaughtExceptionHandler() + // Check for devRoot pointer if let root = ConfigStore.loadDevRoot() { devRoot = root @@ -35,6 +37,23 @@ final class AppDelegate: NSObject, NSApplicationDelegate { } } + /// Capture ObjC exceptions that would otherwise tear the app down + /// silently. The macOS crash reporter handles Mach exceptions / pure + /// SIGSEGV on its own, but ObjC exceptions thrown out of AppKit or + /// libghostty wrappers can `abort()` without producing a useful .ips + /// file. Logging name + reason + symbolicated stack to NSLog routes + /// them into Console.app and the unified log so the next reproduction + /// of issue #240 (and similar) is debuggable. + private func installUncaughtExceptionHandler() { + NSSetUncaughtExceptionHandler { exception in + let symbols = exception.callStackSymbols.joined(separator: "\n") + NSLog( + "[CrowCrash] uncaught NSException name=\(exception.name.rawValue) " + + "reason=\(exception.reason ?? "")\n\(symbols)" + ) + } + } + // MARK: - tmux watchdog alert /// Suppress repeated alerts while one is already on screen. Each alert diff --git a/Tests/CrowTests/FeatureFlagsTests.swift b/Tests/CrowTests/FeatureFlagsTests.swift index 028f14b..9f58273 100644 --- a/Tests/CrowTests/FeatureFlagsTests.swift +++ b/Tests/CrowTests/FeatureFlagsTests.swift @@ -6,7 +6,13 @@ import Testing /// flip the live environment from tests, so this exercises the same shape /// directly via a free function. The intent is to lock in which spellings /// of "true" we accept so the rollout's flag flips are predictable. -@Suite("FeatureFlags env parsing") +/// +/// `.serialized` because `tmuxBackendDefaultIsOff` and +/// `tmuxBackendOnWhenConfigOverrideOn` both mutate the global static +/// `FeatureFlags.tmuxBackendConfigOverride`. Without serialization the two +/// tests race on that shared state and the suite fails intermittently +/// (one test's `= false` reset lands between the other's set + read). +@Suite("FeatureFlags env parsing", .serialized) struct FeatureFlagsTests { /// Mirror of the parsing rule in `FeatureFlags.boolFlag`. If the