From 8651e42aeb1fa7aaa12d766965c6ea1b6f95a6eb Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Sun, 23 Aug 2026 21:12:57 +0200 Subject: [PATCH 1/4] Switch worktrees by reparenting the terminal surface, not rebuilding it Drop the per-selection and whole-tree `.id`s that tore down and rebuilt the pane hosting chain on every worktree switch and split/close; identity now lives per leaf, so a topology change re-identifies only the branch it changed. Cache each surface's scroll wrapper and reuse it across remounts, so a switch reparents the live surface instead of building a new wrapper at zero size, which made the renderer discard the frames painted for the old bounds. --- .../Views/WorktreeDetailView.swift | 14 +-- .../Features/Terminal/Models/SplitTree.swift | 87 ------------------- .../Terminal/Views/LayoutContentView.swift | 80 ++++++++++------- .../Ghostty/GhosttySurfaceView.swift | 21 +++++ supacodeTests/GhosttySurfaceViewTests.swift | 41 +++++++++ 5 files changed, 122 insertions(+), 121 deletions(-) diff --git a/supacode/Features/Repositories/Views/WorktreeDetailView.swift b/supacode/Features/Repositories/Views/WorktreeDetailView.swift index 0c3357b9c..4a4b4ccd3 100644 --- a/supacode/Features/Repositories/Views/WorktreeDetailView.swift +++ b/supacode/Features/Repositories/Views/WorktreeDetailView.swift @@ -303,6 +303,10 @@ struct WorktreeDetailView: View { } } else if let selectedWorktree { let shouldFocusTerminal = repositories.shouldFocusTerminal(for: selectedWorktree.id) + let pendingTerminalFocus: Worktree.ID? = shouldFocusTerminal ? selectedWorktree.id : nil + // No `.id` on purpose: keeping the view stable across a worktree switch + // lets the live surface reparent its cached wrapper instead of tearing + // the hosting chain down and rebuilding it at zero size. WorktreeLayoutView( worktree: selectedWorktree, manager: terminalManager, @@ -311,13 +315,13 @@ struct WorktreeDetailView: View { forceAutoFocus: shouldFocusTerminal, isLifecycleBusy: selectedSlice?.lifecycle.isBusy ?? false ) - .id(selectedWorktree.id) .frame(maxWidth: .infinity, maxHeight: .infinity) .ignoresSafeArea(.container, edges: .bottom) - .onAppear { - if shouldFocusTerminal { - store.send(.repositories(.consumeTerminalFocus(selectedWorktree.id))) - } + // The subtree is stable across a switch, so `onAppear` fires only once; + // drive the consume from the focus request itself. + .onChange(of: pendingTerminalFocus, initial: true) { _, target in + guard let target else { return } + store.send(.repositories(.consumeTerminalFocus(target))) } } else if !repositories.isInitialLoadComplete { DetailPlaceholderView() diff --git a/supacode/Features/Terminal/Models/SplitTree.swift b/supacode/Features/Terminal/Models/SplitTree.swift index 35c4008d8..9d0d8d5b6 100644 --- a/supacode/Features/Terminal/Models/SplitTree.swift +++ b/supacode/Features/Terminal/Models/SplitTree.swift @@ -359,47 +359,6 @@ nonisolated struct SplitTree: Equatable { return best?.leaf } - var structuralIdentity: StructuralIdentity { - StructuralIdentity(self) - } - - struct StructuralIdentity: Hashable { - private let root: Node? - private let zoomed: Node? - - init(_ tree: SplitTree) { - self.root = tree.root - self.zoomed = tree.zoomed - } - - static func == (lhs: Self, rhs: Self) -> Bool { - areNodesStructurallyEqual(lhs.root, rhs.root) - && areNodesStructurallyEqual(lhs.zoomed, rhs.zoomed) - } - - func hash(into hasher: inout Hasher) { - hasher.combine(0) - if let root { - root.hashStructure(into: &hasher) - } - hasher.combine(1) - if let zoomed { - zoomed.hashStructure(into: &hasher) - } - } - - private static func areNodesStructurallyEqual(_ lhs: Node?, _ rhs: Node?) -> Bool { - switch (lhs, rhs) { - case (nil, nil): - return true - case (let node1?, let node2?): - return node1.isStructurallyEqual(to: node2) - default: - return false - } - } - } - private init(root: Node?, zoomed: Node?) { self.root = root self.zoomed = zoomed @@ -743,52 +702,6 @@ nonisolated extension SplitTree.Node { return slots } } - - var structuralIdentity: StructuralIdentity { - StructuralIdentity(self) - } - - struct StructuralIdentity: Hashable { - private let node: SplitTree.Node - - init(_ node: SplitTree.Node) { - self.node = node - } - - static func == (lhs: Self, rhs: Self) -> Bool { - lhs.node.isStructurallyEqual(to: rhs.node) - } - - func hash(into hasher: inout Hasher) { - node.hashStructure(into: &hasher) - } - } - - fileprivate func isStructurallyEqual(to other: Node) -> Bool { - switch (self, other) { - case (.leaf(let view1), .leaf(let view2)): - return view1 == view2 - case (.split(let split1), .split(let split2)): - return split1.direction == split2.direction - && split1.left.isStructurallyEqual(to: split2.left) - && split1.right.isStructurallyEqual(to: split2.right) - default: - return false - } - } - - fileprivate func hashStructure(into hasher: inout Hasher) { - switch self { - case .leaf(let view): - hasher.combine(0) - hasher.combine(view) - case .split(let split): - hasher.combine(1) - hasher.combine(split.direction) - split.left.hashStructure(into: &hasher) - split.right.hashStructure(into: &hasher) - } - } } nonisolated extension SplitTree.Spatial { diff --git a/supacode/Features/Terminal/Views/LayoutContentView.swift b/supacode/Features/Terminal/Views/LayoutContentView.swift index aa0185d05..fc40428f7 100644 --- a/supacode/Features/Terminal/Views/LayoutContentView.swift +++ b/supacode/Features/Terminal/Views/LayoutContentView.swift @@ -80,14 +80,14 @@ struct LayoutPaneTreeView: View { var body: some View { Group { if let node = store.layout.tree.visibleNode { - // Panes and the windowed set flow down by value from this `.id` - // boundary, so a dismantling copy cannot retarget a content host to - // post-swap state. + // Panes and the windowed set flow down by value, so a dismantling copy + // cannot retarget a content host to post-swap state. Identity lives on + // the leaves instead of the whole tree, so a split or close + // re-identifies only the branch it changed, never the survivor. PaneNodeView( node: node, panes: store.layout.panes, windowedPaneIDs: store.windowedPaneIDs, store: store, renderContext: renderContext ) - .id(store.layout.tree.structuralIdentity) } else { EmptyLayoutView() } @@ -199,8 +199,6 @@ private struct EmptyLayoutView: View { /// One node of the pane tree: a split renders its children with a draggable /// divider, a leaf renders its pane. private struct PaneNodeView: View { - private static let logger = SupaLogger("LayoutContentView") - let node: SplitTree.Node let panes: IdentifiedArrayOf /// Frozen alongside `panes`: a live read here would let a dismantling copy @@ -212,28 +210,13 @@ private struct PaneNodeView: View { var body: some View { switch node { case .leaf(let paneID): - if let pane = panes[id: paneID] { - if windowedPaneIDs.contains(paneID) { - WindowedPanePlaceholderView(paneID: paneID, store: store, showWindow: renderContext.showWindowedPane) - } else { - PaneStripView( - pane: pane, windowedPaneIDs: windowedPaneIDs, store: store, - runtime: renderContext.runtime, - unfocusedOverlay: renderContext.unfocusedOverlay, - surfaceState: renderContext.surfaceState, - isLifecycleBusy: renderContext.isLifecycleBusy, - dragModel: renderContext.dragModel) - } - } else { - // Tree and panes disagree; render an explicit fallback, never a hole. - EmptyTerminalPaneView( - message: "This pane is unavailable.", - hint: Text("Reopen the worktree to rebuild its layout.") - ) - .onAppear { - Self.logger.error("Tree leaf \(paneID.rawValue) has no pane; layout state is inconsistent.") - } - } + // The pane's own id, not the tree's shape: a split or close elsewhere + // leaves this leaf's identity, and so its mounted surface, untouched. + PaneLeafView( + paneID: paneID, panes: panes, windowedPaneIDs: windowedPaneIDs, + store: store, renderContext: renderContext + ) + .id(paneID) case .split(let split): PaneSplitView( node: node, split: split, panes: panes, windowedPaneIDs: windowedPaneIDs, @@ -242,6 +225,43 @@ private struct PaneNodeView: View { } } +/// One leaf of the pane tree: the pane's strip, its windowed-pane placeholder, +/// or an explicit fallback when the tree and panes disagree. +private struct PaneLeafView: View { + private static let logger = SupaLogger("LayoutContentView") + + let paneID: PaneID + let panes: IdentifiedArrayOf + let windowedPaneIDs: Set + let store: StoreOf + let renderContext: PaneRenderContext + + var body: some View { + if let pane = panes[id: paneID] { + if windowedPaneIDs.contains(paneID) { + WindowedPanePlaceholderView(paneID: paneID, store: store, showWindow: renderContext.showWindowedPane) + } else { + PaneStripView( + pane: pane, windowedPaneIDs: windowedPaneIDs, store: store, + runtime: renderContext.runtime, + unfocusedOverlay: renderContext.unfocusedOverlay, + surfaceState: renderContext.surfaceState, + isLifecycleBusy: renderContext.isLifecycleBusy, + dragModel: renderContext.dragModel) + } + } else { + // Tree and panes disagree; render an explicit fallback, never a hole. + EmptyTerminalPaneView( + message: "This pane is unavailable.", + hint: Text("Reopen the worktree to rebuild its layout.") + ) + .onAppear { + Self.logger.error("Tree leaf \(paneID.rawValue) has no pane; layout state is inconsistent.") + } + } + } +} + /// A split of two child nodes; the divider drag resizes and a double-click /// equalizes, both through the reducer. private struct PaneSplitView: View { @@ -801,7 +821,9 @@ private struct ContentHostView: NSViewRepresentable { { return } - hostedView = GhosttySurfaceScrollView(surfaceView: surface) + // Reuse the surface's own wrapper: a remount reparents it rather than + // rebuilding at zero size, so the IOSurface keeps its frames. + hostedView = surface.hostedView() } else { // Already showing the right renderer: nothing to do. if container.subviews.first === renderer { return } diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift index 1d2a586f0..5f5073373 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift @@ -146,6 +146,20 @@ final class GhosttySurfaceView: NSView, Identifiable { } } } + // Strong hold forms a surface<->wrapper cycle, so `closeSurface` must release + // it; `deinit` cannot free the surface until it has. + private var ownedScrollWrapper: GhosttySurfaceScrollView? + + /// The view a content host mounts for this surface: its scroll wrapper, built + /// once and reused across remounts so a rebuild or worktree switch reparents + /// it and the live surface keeps its painted frames, instead of rebuilding at + /// zero size. + func hostedView() -> GhosttySurfaceScrollView { + if let ownedScrollWrapper { return ownedScrollWrapper } + let wrapper = GhosttySurfaceScrollView(surfaceView: self) + ownedScrollWrapper = wrapper + return wrapper + } var onFocusChange: ((Bool) -> Void)? /// Asks the owning state to re-derive activity because user input reached an /// occluded surface, passing the window's fresh key/visibility readings so @@ -292,6 +306,11 @@ final class GhosttySurfaceView: NSView, Identifiable { MainActor.assumeIsolated { SecureInput.shared.removeScoped(id) } + // A live surface here means a teardown path bypassed `closeSurface`; the + // call below still frees it. + if surface != nil { + assertionFailure("GhosttySurfaceView deallocated with a live surface; a teardown path bypassed closeSurface().") + } closeSurface() if let workingDirectoryCString { free(workingDirectoryCString) @@ -322,6 +341,8 @@ final class GhosttySurfaceView: NSView, Identifiable { lastOcclusion = nil lastSurfaceFocus = nil } + // Break the surface<->wrapper cycle; the strong hold otherwise blocks deinit. + ownedScrollWrapper = nil } private func updateScreenObservers() { diff --git a/supacodeTests/GhosttySurfaceViewTests.swift b/supacodeTests/GhosttySurfaceViewTests.swift index 7fbcf8e3c..5a7cbd862 100644 --- a/supacodeTests/GhosttySurfaceViewTests.swift +++ b/supacodeTests/GhosttySurfaceViewTests.swift @@ -513,6 +513,7 @@ struct GhosttySurfaceViewTests { initialGeometry: .fallback, context: GHOSTTY_SURFACE_CONTEXT_TAB ) + defer { surfaceView.closeSurface() } let wrapper = GhosttySurfaceScrollView(surfaceView: surfaceView) #expect(wrapper.safeAreaInsets.top == 0) @@ -520,4 +521,44 @@ struct GhosttySurfaceViewTests { #expect(wrapper.safeAreaInsets.bottom == 0) #expect(wrapper.safeAreaInsets.right == 0) } + + // A remount must reparent the same wrapper, so the live IOSurface keeps its + // frames instead of the renderer rebuilding at zero size on every switch. + @Test func hostedViewReturnsTheSameWrapperAcrossRemounts() { + let surfaceView = GhosttySurfaceView( + id: UUID(), + runtime: GhosttyRuntime(), + workingDirectory: nil, + initialGeometry: .fallback, + context: GHOSTTY_SURFACE_CONTEXT_TAB + ) + defer { surfaceView.closeSurface() } + + let wrapper = surfaceView.hostedView() + #expect(surfaceView.hostedView() === wrapper) + // The reuse guard in the content host keys off `scrollWrapper`; it has to + // point at the very wrapper `hostedView()` vends. + #expect(surfaceView.scrollWrapper === wrapper) + } + + // `closeSurface` must clear the cached wrapper so the surface<->wrapper cycle + // is broken: proven by `hostedView()` vending a fresh instance afterwards. If + // the release is ever dropped, the same wrapper comes back and both leak. + @Test func closeSurfaceClearsTheCachedWrapper() { + let surfaceView = GhosttySurfaceView( + id: UUID(), + runtime: GhosttyRuntime(), + workingDirectory: nil, + initialGeometry: .fallback, + context: GHOSTTY_SURFACE_CONTEXT_TAB + ) + defer { surfaceView.closeSurface() } + + let first = surfaceView.hostedView() + surfaceView.closeSurface() + let second = surfaceView.hostedView() + + #expect(first !== second) + #expect(surfaceView.scrollWrapper === second) + } } From fbc40eff98095ff568d8b05b655ef7a5792eeb0d Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Sun, 23 Aug 2026 22:26:35 +0200 Subject: [PATCH 2/4] Keep per-tab title churn off the root store Agent TUIs rewrite their title several times a second. That report now lands on the content's observable TabChrome and reaches the layout only as one discrete titleCommitted at teardown, so a title storm no longer invalidates the detail view or the sidebar, or pays a per-action reducer log. The strip and window title read the live value off the chrome (TabTitle.resolved); persistence pulls it into the snapshot (TabTitle.stored). Also gate the per-action state-diff logging behind SUPACODE_LOG_ACTIONS, and skip the no-op per-keystroke bell and per-prompt pwd writes. --- AGENTS.md | 2 +- supacode/App/WindowTitle.swift | 4 +- .../BusinessLogic/LayoutPersistence.swift | 6 ++ .../BusinessLogic/LayoutSurfaceConduit.swift | 2 +- .../BusinessLogic/WorktreeContentHost.swift | 31 ++++++++- .../WorktreeTerminalManager.swift | 40 +++++++++-- .../Features/Terminal/Content/TabChrome.swift | 30 +++++++++ .../Terminal/Reducer/LayoutFeature.swift | 33 +++++---- .../Terminal/Reducer/TerminalsFeature.swift | 6 +- .../Terminal/Views/PaneTabStripView.swift | 6 +- .../Terminal/Views/PaneWindowManager.swift | 10 +-- .../Ghostty/GhosttySurfaceBridge.swift | 5 +- .../Ghostty/GhosttySurfaceView.swift | 8 ++- supacode/Support/DebugCaseOutput.swift | 27 +++++++- supacodeTests/LayoutFeatureTests.swift | 16 +++-- supacodeTests/LayoutPersistenceTests.swift | 32 +++++++++ supacodeTests/LogActionsReducerTests.swift | 43 ++++++++++++ supacodeTests/TabTitleTests.swift | 67 +++++++++++++++++++ supacodeTests/WorktreeContentHostTests.swift | 42 ++++++++++++ 19 files changed, 362 insertions(+), 48 deletions(-) create mode 100644 supacodeTests/LogActionsReducerTests.swift create mode 100644 supacodeTests/TabTitleTests.swift diff --git a/AGENTS.md b/AGENTS.md index e207ba402..a199c8150 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,7 +36,7 @@ Supacode is a macOS terminal emulator that for running multiple coding agents in - Avoid top-level free functions. Default to `static` methods, computed properties, or instance methods on a relevant type (enum/struct/extension). Free functions pollute the module namespace, are harder to discover, and easily drift from the inline implementation a consumer ends up writing instead. If the operation is pure and stateless, make it a `static` on a caseless `enum` or the most relevant type, not a top-level `func`. - Closure-typed focused values invalidate the AppKit menu on every body run (closures have no Equatable conformance, so SwiftUI re-publishes every time). Always wrap menu-bar action closures with `FocusedAction` and publish via `.focusedSceneAction(_:enabled:token:perform:)` / `.focusedAction(_:enabled:token:perform:)`. The wrapper dedupes on `(isEnabled, token)`, so AppKit only rebuilds the menu when something the menu actually displays changes. Token rules in `App/Models/FocusedAction.swift`: set `token` to a hashable projection of any captured state that affects behavior; leave it `nil` when the closure captures only the store / `@State` bindings. Consumers should read the action with `@FocusedValue(\.x)` and gate with `action?.isEnabled != true`, not `action == nil`. - Sidebar rows must not fan out invalidation. Per-row state lives in `RepositoriesFeature.State.sidebarItems` so a per-leaf mutation (notification tick, agent activity, running-script update) invalidates only that leaf, not every sibling. The view renders the cached `state.sidebarStructure` (computed in the reducer's post-reduce hook), never reading `sidebarItems[id:]` from a view body; derive per-leaf data in `computeSidebarStructure(...)`, not in the view. -- Never lift content-specific (e.g. terminal-only) mechanics or state into the layout layer. `LayoutFeature` owns topology and strip mechanics only (panes, tabs, selection, focus, zoom, rename identity); anything a specific content kind produces (agent badges, progress, script locks, busyness) lives on the content side, exposed to the strip through the content's observable `TabChrome` (`Features/Terminal/Content/TabChrome.swift`), never as layout reducer state or actions. +- Never lift content-specific (e.g. terminal-only) mechanics or state into the layout layer. `LayoutFeature` owns topology and strip mechanics only (panes, tabs, selection, focus, zoom, rename identity); anything a specific content kind produces (agent badges, progress, script locks, busyness, reported title, pwd) lives on the content side, exposed to the strip through the content's observable `TabChrome` (`Features/Terminal/Content/TabChrome.swift`), never as layout reducer state or actions. High-frequency reported signals (title, pwd, bell) are the sharp edge of this rule: an agent TUI rewrites its title several times a second, so a reported title lands on the content's chrome and reaches the layout only as one discrete commit at teardown (`titleCommitted`), never one reducer action per report. The strip and window title read the live value off `TabChrome` (`TabTitle.resolved`), and persistence pulls it into the snapshot (`TabTitle.stored`). ## UX Standards diff --git a/supacode/App/WindowTitle.swift b/supacode/App/WindowTitle.swift index c2f41e769..0bf28392c 100644 --- a/supacode/App/WindowTitle.swift +++ b/supacode/App/WindowTitle.swift @@ -65,8 +65,10 @@ enum WindowTitle { repository: repository, repositories: repositories ) + // Through the content's chrome: reported titles never enter the reducer, so + // this read is what keeps the window title tracking the terminal. let tabTitle = terminalManager.hostIfExists(for: worktreeID)?.focusedTab.flatMap { tab in - sanitize(tab.customTitle ?? tab.title) + sanitize(TabTitle.resolved(for: tab, runtime: ContentRuntime.liveValue)) } return format(repo: repoTitle, tab: tabTitle) } diff --git a/supacode/Features/Terminal/BusinessLogic/LayoutPersistence.swift b/supacode/Features/Terminal/BusinessLogic/LayoutPersistence.swift index f745acb42..b684b0be4 100644 --- a/supacode/Features/Terminal/BusinessLogic/LayoutPersistence.swift +++ b/supacode/Features/Terminal/BusinessLogic/LayoutPersistence.swift @@ -23,6 +23,12 @@ enum LayoutPersistence { let live = runtime.content(for: contentID) if let live { overlaid.panes[paneIndex].tabs[tabIndex].content = live.snapshot() + // Reported titles never enter the layout reducer, so while the content + // is live the saved record pulls its latest one here. + overlaid.panes[paneIndex].tabs[tabIndex].title = TabTitle.stored( + for: overlaid.panes[paneIndex].tabs[tabIndex], + chrome: live.chrome + ) } guard case .terminal(let state) = overlaid.panes[paneIndex].tabs[tabIndex].content.state else { continue } diff --git a/supacode/Features/Terminal/BusinessLogic/LayoutSurfaceConduit.swift b/supacode/Features/Terminal/BusinessLogic/LayoutSurfaceConduit.swift index 7dcf3bf4c..9ad44b744 100644 --- a/supacode/Features/Terminal/BusinessLogic/LayoutSurfaceConduit.swift +++ b/supacode/Features/Terminal/BusinessLogic/LayoutSurfaceConduit.swift @@ -41,7 +41,7 @@ struct LayoutSurfaceConduit { let host = host view.bridge.onTitleChange = { [weak view] title in guard let view, isLive(view) else { return } - host.sendLayoutAction(.runtime(.titleChanged(id: contentID, title: title))) + host.updateReportedTitle(for: contentID, title: title) } // Layout topology belongs to the app's own chords, menus, and palette; // a Ghostty keybind for it is consumed and ignored so the terminal can diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeContentHost.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeContentHost.swift index 574d2d136..8c5afae78 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeContentHost.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeContentHost.swift @@ -27,6 +27,10 @@ final class WorktreeContentHost { /// Routes a topology mutation into the worktree's `LayoutFeature`. @ObservationIgnored var sendLayoutAction: (LayoutFeature.Action) -> Void = { _ in } @ObservationIgnored var onNotificationReceived: ((UUID, String, String, Bool) -> Void)? + /// A content reported a new title. Nothing in TCA moved (the title lives on + /// the content's chrome), so this exists only to re-arm the persistence + /// debounce, which pulls the title back at snapshot time. + @ObservationIgnored var onReportedTitleChanged: (() -> Void)? @ObservationIgnored var onNotificationIndicatorChanged: (() -> Void)? @ObservationIgnored var onFocusChanged: ((UUID) -> Void)? @ObservationIgnored var onFocusedSurfaceColorChanged: (() -> Void)? @@ -541,7 +545,28 @@ final class WorktreeContentHost { /// The tab's content-owned strip chrome, nil for non-terminal contents. private func terminalChrome(for tabID: TabID) -> TerminalTabChrome? { guard let contentID = tab(withID: tabID)?.content.id else { return nil } - return runtime.content(for: contentID)?.chrome as? TerminalTabChrome + return terminalChrome(for: contentID) + } + + private func terminalChrome(for contentID: ContentID) -> TerminalTabChrome? { + runtime.content(for: contentID)?.chrome as? TerminalTabChrome + } + + /// A live or dormant terminal reported a title. Agent TUIs rewrite it several + /// times a second, so it lands on the content's observable chrome (only that + /// one tab label re-renders) and never as a layout action. The lock is + /// resolved at display and snapshot time, so a script tab's title survives its + /// shell's reports. + func updateReportedTitle(for contentID: ContentID, title: String) { + // Shells clear the title mid-command and reset it at the next prompt; ignore + // the empty report so the tab label holds its last real title instead of + // flashing to the layout's creation-time name. + let trimmed = title.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty, let chrome = terminalChrome(for: contentID), + chrome.reportedTitle != trimmed + else { return } + chrome.reportedTitle = trimmed + onReportedTitleChanged?() } func emitTaskStatusIfChanged() { @@ -693,7 +718,7 @@ final class WorktreeContentHost { if tabID(containing: surfaceID) != nil, let title = liveSurface(surfaceID)?.bridge.state.title, !title.isEmpty { - sendLayoutAction(.runtime(.titleChanged(id: ContentID(rawValue: surfaceID), title: title))) + updateReportedTitle(for: ContentID(rawValue: surfaceID), title: title) } emitFocusChangedIfNeeded(surfaceID) } @@ -1186,7 +1211,7 @@ final class WorktreeContentHost { private func updateDormantTabTitle(surfaceID: UUID, title: String) { let trimmed = title.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty, isDormantSurface(surfaceID) else { return } - sendLayoutAction(.runtime(.titleChanged(id: ContentID(rawValue: surfaceID), title: trimmed))) + updateReportedTitle(for: ContentID(rawValue: surfaceID), title: trimmed) } /// Full teardown on prune or quit: watchers stop, bookkeeping clears. diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 3692b46d8..987e191f8 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -760,6 +760,12 @@ final class WorktreeTerminalManager { self?.emitNotificationIndicatorCountIfNeeded() self?.emitProjection(for: worktree.id) } + // Only the debounce: the title itself is read back off the chrome when the + // snapshot is built, so a title storm costs one coalesced write, not one + // store send per report. + host.onReportedTitleChanged = { [weak self] in + self?.markLayoutDirty(worktreeID: worktree.id) + } host.onFocusChanged = { [weak self] surfaceID in self?.emit(.focusChanged(worktreeID: worktree.id, surfaceID: surfaceID)) self?.refreshFocusedSurfaceBackground() @@ -858,6 +864,17 @@ final class WorktreeTerminalManager { ) } + /// Hands the content's last reported title back to the layout. The title is + /// carried by the content's chrome alone, so every teardown that keeps the tab + /// (a reattach rebuild, quit-time termination) must commit it first or the tab + /// falls back to its creation-time name. + private func commitReportedTitle(of contentID: ContentID, worktreeID: Worktree.ID) { + guard let title = ContentRuntime.liveValue.content(for: contentID)?.chrome?.reportedTitle, + !title.isEmpty + else { return } + sendLayout(worktreeID, .runtime(.titleCommitted(id: contentID, title: title))) + } + /// An unexpected zmx exit: probe the session, then spare, kill, or reattach. func handleUnexpectedZmxClose(_ view: GhosttySurfaceView, worktreeID: Worktree.ID) { let surfaceID = view.id @@ -883,6 +900,7 @@ final class WorktreeTerminalManager { } if session.clients == 0 { // Reattachable: rebuild the same content at its persisted geometry. + self.commitReportedTitle(of: ContentID(rawValue: surfaceID), worktreeID: worktreeID) ContentRuntime.liveValue.remove(ContentID(rawValue: surfaceID), tombstone: false) self.sendLayout(worktreeID, .wakeTab(id: tabID)) return @@ -1737,18 +1755,30 @@ final class WorktreeTerminalManager { /// so "Quit and Terminate" must explicitly sweep orphan sessions or they /// would survive forever. func terminateAllSessions(killBudget: Duration = WorktreeTerminalManager.quitKillBudget) async { - let trackedSurfaceIDs = hosts.values.flatMap(\.allSurfaceIDs) - let trackedSessionIDs = Set(trackedSurfaceIDs.map(ZmxSessionID.make(surfaceID:))) + // Captured before `tearDown`, which clears the hosts' surface tracking. + let trackedByWorktree = hosts.flatMap { worktreeID, host in + host.allSurfaceIDs.map { (worktreeID: worktreeID, surfaceID: $0) } + } + let trackedSessionIDs = Set(trackedByWorktree.map { ZmxSessionID.make(surfaceID: $0.surfaceID) }) // "Quit and Terminate" promises nothing keeps running, so the host-side // sessions of remote worktrees are swept too (best-effort over SSH). let trackedRemoteSessions = Self.remoteSessions(in: Array(hosts.values)) + // Commit reported titles before teardown: the content is still live, so + // this reaches the layout before the snapshot save empties the runtime. A + // commit after `tearDown` would re-run layout lifecycle reconciliation, + // which sees the removed renderers as dormant and restarts the very session + // watchers this teardown just stopped (the process survives a Terminate All). + for entry in trackedByWorktree { + commitReportedTitle(of: ContentID(rawValue: entry.surfaceID), worktreeID: entry.worktreeID) + } for host in hosts.values { host.tearDown() } - for surfaceID in trackedSurfaceIDs { - guard let content = ContentRuntime.liveValue.content(for: ContentID(rawValue: surfaceID)) else { continue } + for entry in trackedByWorktree { + let contentID = ContentID(rawValue: entry.surfaceID) + guard let content = ContentRuntime.liveValue.content(for: contentID) else { continue } content.hibernate() - ContentRuntime.liveValue.remove(content.id, tombstone: false) + ContentRuntime.liveValue.remove(contentID, tombstone: false) } emitHasAnyTerminalSurfaceIfNeeded() // This instance's tracked local sessions are killed. A remote surface's diff --git a/supacode/Features/Terminal/Content/TabChrome.swift b/supacode/Features/Terminal/Content/TabChrome.swift index 648240089..45e6d11c7 100644 --- a/supacode/Features/Terminal/Content/TabChrome.swift +++ b/supacode/Features/Terminal/Content/TabChrome.swift @@ -16,6 +16,35 @@ protocol TabChrome: AnyObject { /// Whether the terminal refuses input (a completed blocking script's parked /// shell). The tab's own `isLocked` drives the visible lock marker. var isReadOnly: Bool { get } + /// The title the content last reported, nil until it reports one (and nil for + /// content kinds that never report one). Agent TUIs rewrite it several times a + /// second, so it lives here rather than as a layout reducer action, and only + /// one tab's label re-renders on a report. + var reportedTitle: String? { get } +} + +/// Resolves what a tab shows, and what the layout should persist for it, from +/// the layout's own title and the content's live reported title. +@MainActor +enum TabTitle { + /// The title the record persists: the content's live report when it has one, + /// else the layout's own. A locked tab owns its title (a shell report never + /// reaches it), and the user override is excluded (it persists in its field). + static func stored(for tab: TabItem, chrome: (any TabChrome)?) -> String { + guard !tab.isLocked, let reported = chrome?.reportedTitle, !reported.isEmpty else { + return tab.title + } + return reported + } + + /// What the tab displays: a user override wins over the reported title. + static func resolved(for tab: TabItem, chrome: (any TabChrome)?) -> String { + tab.customTitle ?? stored(for: tab, chrome: chrome) + } + + static func resolved(for tab: TabItem, runtime: ContentRuntime) -> String { + resolved(for: tab, chrome: runtime.content(for: tab.content.id)?.chrome) + } } /// Terminal chrome, written by the content host and the agent-presence @@ -27,6 +56,7 @@ final class TerminalTabChrome: TabChrome { var isWorking = false var progress: TerminalTabProgressDisplay? var isReadOnly = false + var reportedTitle: String? var accessory: AnyView? { guard !agents.isEmpty else { return nil } diff --git a/supacode/Features/Terminal/Reducer/LayoutFeature.swift b/supacode/Features/Terminal/Reducer/LayoutFeature.swift index 1b35cf6f2..9036db0a2 100644 --- a/supacode/Features/Terminal/Reducer/LayoutFeature.swift +++ b/supacode/Features/Terminal/Reducer/LayoutFeature.swift @@ -130,10 +130,16 @@ struct LayoutFeature { @Presents var alert: AlertState? } - /// Events pushed by the content-runtime plumbing. + /// Events pushed by the content-runtime plumbing. Individual title reports are + /// NOT here: they arrive at keystroke frequency, so they land on the content's + /// own `TabChrome` and reach persistence through the snapshot pull. Only the + /// once-per-content commit below crosses into the layout. nonisolated enum RuntimeEvent: Equatable, Sendable { case killConfirmed(id: ContentID) - case titleChanged(id: ContentID, title: String) + /// The content's last reported title, handed back before the content leaves + /// the runtime; the chrome that carried it dies with it, and the layout's + /// own title is what the tab falls back to afterwards. + case titleCommitted(id: ContentID, title: String) } /// What `focusPane` aims at. One payload instead of two `focusPane` @@ -220,18 +226,19 @@ struct LayoutFeature { private static let logger = SupaLogger("LayoutFeature") - // Ratio drags and title reports arrive at frame rate and cannot alter - // structure; exempt them from the per-action layout walk. + // Ratio drags arrive at frame rate and the inline rename begin/end toggles are + // transient; neither alters structure, so exempt them from the per-action walk. private static func isExemptFromConsistencyCheck(_ action: Action) -> Bool { switch action { - case .resizePane, .runtime(.titleChanged), .beginTabRename, .endTabRename: + case .resizePane, .beginTabRename, .endTabRename: return true case .newTab, .splitPane, .closeTab, .closePane, .selectTab, .renameTab, .focusPane, .moveTab, .moveTabToSplit, .moveTabToSpanningSplit, .enterWindowMode, .exitWindowMode, .equalizePanes, .toggleZoom, .hibernateTab, .wakeTab, .runtime(.killConfirmed), - .contentRequestedClose, .contentRequestedNewTab, .contentRequestedSplit, - .contentRequestedFocus, .contentRequestedFocusSplit, .contentRequestedToggleZoom, - .contentRequestedResize, .contentRequestedGotoTab, .contentRequestedMoveTab, .alert: + .runtime(.titleCommitted), .contentRequestedClose, .contentRequestedNewTab, + .contentRequestedSplit, .contentRequestedFocus, .contentRequestedFocusSplit, + .contentRequestedToggleZoom, .contentRequestedResize, .contentRequestedGotoTab, + .contentRequestedMoveTab, .alert: return false } } @@ -1035,13 +1042,11 @@ extension LayoutFeature { switch event { case .killConfirmed(let contentID): contentRuntime.confirmKill(contentID) - case .titleChanged(let contentID, let title): + case .titleCommitted(let contentID, let title): guard let located = state.layout.tab(containingContent: contentID) else { break } - // A script tab owns its title; shell reports must not overwrite it. - guard !located.tab.isLocked else { break } - // TUIs rewrite their title constantly; skip no-op writes so an unchanged - // title does not re-render the tab strip on every report. - guard located.tab.title != title else { break } + // A script tab owns its title; shell reports must not overwrite it. Skip a + // no-op write so an identical commit does not re-render the tab strip. + guard !located.tab.isLocked, located.tab.title != title else { break } var pane = located.pane pane.tabs[id: located.tab.id]?.title = title state.layout.panes[id: pane.id] = pane diff --git a/supacode/Features/Terminal/Reducer/TerminalsFeature.swift b/supacode/Features/Terminal/Reducer/TerminalsFeature.swift index ac8978aaf..63f26343f 100644 --- a/supacode/Features/Terminal/Reducer/TerminalsFeature.swift +++ b/supacode/Features/Terminal/Reducer/TerminalsFeature.swift @@ -70,11 +70,11 @@ struct TerminalsFeature { private static let logger = SupaLogger("TerminalsFeature") - // Ratio drags and title reports arrive at high frequency and cannot flip - // tab visibility; skip the layout-wide re-diff for them. + // Ratio drags, the inline rename begin/end toggles, and a teardown title + // commit never flip tab visibility, so skip the layout-wide re-diff for them. private static func canAffectVisibility(_ action: LayoutFeature.Action) -> Bool { switch action { - case .resizePane, .runtime(.titleChanged), .beginTabRename, .endTabRename: + case .resizePane, .beginTabRename, .endTabRename, .runtime(.titleCommitted): return false case .newTab, .splitPane, .closeTab, .closePane, .selectTab, .renameTab, .focusPane, .moveTab, .moveTabToSplit, .moveTabToSpanningSplit, .enterWindowMode, .exitWindowMode, diff --git a/supacode/Features/Terminal/Views/PaneTabStripView.swift b/supacode/Features/Terminal/Views/PaneTabStripView.swift index 8a6a6818e..579dbbddd 100644 --- a/supacode/Features/Terminal/Views/PaneTabStripView.swift +++ b/supacode/Features/Terminal/Views/PaneTabStripView.swift @@ -384,6 +384,7 @@ private struct PaneTabView: View { HStack(spacing: TerminalTabBarMetrics.contentSpacing) { PaneTabLabelView( tab: tab, + title: TabTitle.resolved(for: tab, chrome: chrome), isSelected: isSelected, isDormant: isDormant, accessory: chrome?.accessory, @@ -546,7 +547,7 @@ private struct PaneTabView: View { } private var displayTitle: String { - tab.customTitle ?? tab.title + TabTitle.resolved(for: tab, runtime: runtime) } private var contentOpacity: Double { @@ -669,6 +670,7 @@ private struct PaneTabView: View { /// title. private struct PaneTabLabelView: View { let tab: TabItem + let title: String let isSelected: Bool let isDormant: Bool let accessory: AnyView? @@ -703,7 +705,7 @@ private struct PaneTabLabelView: View { .accessibilityHidden(true) } PaneTabTitleLabel( - title: tab.customTitle ?? tab.title, + title: title, isSelected: isSelected, isShimmering: isShimmering ) diff --git a/supacode/Features/Terminal/Views/PaneWindowManager.swift b/supacode/Features/Terminal/Views/PaneWindowManager.swift index 2fb17f625..f601861cd 100644 --- a/supacode/Features/Terminal/Views/PaneWindowManager.swift +++ b/supacode/Features/Terminal/Views/PaneWindowManager.swift @@ -404,7 +404,7 @@ final class PaneWindowManager { window.minSize = NSSize(width: 320, height: 240) window.title = terminalManager.layoutState(for: worktreeID)?.layout.panes[id: paneID] - .flatMap(WindowedPaneRootView.title(for:)) ?? "Terminal" + .map { WindowedPaneRootView.title(for: $0, runtime: ContentRuntime.liveValue) } ?? "Terminal" window.center() cascadePoint = window.cascadeTopLeft(from: cascadePoint) let headerModel = PaneWindowHeaderModel() @@ -573,9 +573,9 @@ private struct WindowedPaneRootView: View { ) } .onAppear { - updateWindowTitle(Self.title(for: pane)) + updateWindowTitle(Self.title(for: pane, runtime: runtime)) } - .onChange(of: Self.title(for: pane)) { _, title in + .onChange(of: Self.title(for: pane, runtime: runtime)) { _, title in updateWindowTitle(title) } .focusedSceneAction( @@ -687,9 +687,9 @@ private struct WindowedPaneRootView: View { action(surface) } - static func title(for pane: Pane) -> String { + static func title(for pane: Pane, runtime: ContentRuntime) -> String { guard let tab = pane.selectedTab else { return "Terminal" } - return tab.customTitle ?? tab.title + return TabTitle.resolved(for: tab, runtime: runtime) } } diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift index add31fbad..d4a78546c 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceBridge.swift @@ -276,7 +276,10 @@ final class GhosttySurfaceBridge { return true case GHOSTTY_ACTION_PWD: - state.pwd = string(from: action.action.pwd.pwd) + // Shells re-emit OSC 7 per prompt; skip the no-op write and a11y posts. + let pwd = string(from: action.action.pwd.pwd) + guard pwd != state.pwd else { return true } + state.pwd = pwd if let surfaceView { NSAccessibility.post(element: surfaceView, notification: .valueChanged) // VoiceOver does not reliably re-read the label on `.valueChanged` alone. diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift index 5f5073373..1156df582 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift @@ -564,7 +564,7 @@ final class GhosttySurfaceView: NSView, Identifiable { guard surface != nil else { return } guard self.focused != focused else { return } self.focused = focused - if focused { + if focused, bridge.state.bellCount != 0 { bridge.state.bellCount = 0 } setSurfaceFocus(focused) @@ -712,7 +712,11 @@ final class GhosttySurfaceView: NSView, Identifiable { interpretKeyEvents([event]) return } - bridge.state.bellCount = 0 + // Guarded: an unconditional write invalidates every observer of the bridge + // state on every keystroke, key repeat included. + if bridge.state.bellCount != 0 { + bridge.state.bellCount = 0 + } let (translationEvent, translationMods) = translationState(event, surface: surface) let action = event.isARepeat ? GHOSTTY_ACTION_REPEAT : GHOSTTY_ACTION_PRESS keyTextAccumulator = [] diff --git a/supacode/Support/DebugCaseOutput.swift b/supacode/Support/DebugCaseOutput.swift index d6e35ee0f..ffda5835c 100644 --- a/supacode/Support/DebugCaseOutput.swift +++ b/supacode/Support/DebugCaseOutput.swift @@ -11,15 +11,35 @@ extension Reducer where State: Equatable { } } +#if DEBUG + private enum ActionLogging { + // Opt-in: the per-action state copy, reflective diff, and print are the + // single largest debug-build cost on the terminal hot path. Enable by + // setting SUPACODE_LOG_ACTIONS to a truthy value (1); a falsy or unset value + // leaves the wrapper a passthrough. + static let isEnabled: Bool = { + guard let value = ProcessInfo.processInfo.environment["SUPACODE_LOG_ACTIONS"] else { + return false + } + return !["", "0", "false", "no"].contains(value.lowercased()) + }() + } +#endif + struct LogActionsReducer: Reducer where Base.State: Equatable { let base: Base - private let logger = SupaLogger("TCA") + #if DEBUG + private let logger = SupaLogger("TCA") + #endif func reduce(into state: inout Base.State, action: Base.Action) -> Effect { - let actionLabel = debugCaseOutput(action) - logger.debug("Action: \(actionLabel)") #if DEBUG + guard ActionLogging.isEnabled else { + return base.reduce(into: &state, action: action) + } + let actionLabel = debugCaseOutput(action) + logger.debug("Action: \(actionLabel)") let previousState = state let effects = base.reduce(into: &state, action: action) if previousState != state, let diff = CustomDump.diff(previousState, state) { @@ -27,6 +47,7 @@ struct LogActionsReducer: Reducer where Base.State: Equatable { } return effects #else + let actionLabel = debugCaseOutput(action) SentrySDK.logger.info("Action: \(actionLabel)") let breadcrumb = Breadcrumb(level: .debug, category: "action") breadcrumb.message = actionLabel diff --git a/supacodeTests/LayoutFeatureTests.swift b/supacodeTests/LayoutFeatureTests.swift index f0f5d47d1..4f6a9b3aa 100644 --- a/supacodeTests/LayoutFeatureTests.swift +++ b/supacodeTests/LayoutFeatureTests.swift @@ -1423,13 +1423,15 @@ struct LayoutFeatureTests { #expect(harness.store.state.layout.isConsistent) } - @Test func titleChangedUpdatesTitleNotCustomTitle() async { + @Test func titleCommittedUpdatesTitleNotCustomTitle() async { let harness = await makeHarness() let paneID = harness.paneID await harness.store.send(.renameTab(id: harness.tabID, title: "Custom")) { $0.layout.panes[id: paneID]?.tabs[id: harness.tabID]?.customTitle = "Custom" } - await harness.store.send(.runtime(.titleChanged(id: harness.contentID, title: "zsh"))) { + // The commit is what survives the content leaving the runtime, so it lands + // on the layout's own title, never on the user's override. + await harness.store.send(.runtime(.titleCommitted(id: harness.contentID, title: "zsh"))) { $0.layout.panes[id: paneID]?.tabs[id: harness.tabID]?.title = "zsh" } #expect(harness.store.state.layout.panes[id: paneID]?.tabs[id: harness.tabID]?.customTitle == "Custom") @@ -1456,10 +1458,10 @@ struct LayoutFeatureTests { #expect(killed.value.first?.worktree == WorktreeID("/tmp/layout-feature")) } - @Test func titleChangedWithTheSameTitleIsANoOp() async { + @Test func titleCommittedWithTheSameTitleIsANoOp() async { let harness = await makeHarness() - // The bootstrap tab is titled "One"; an identical report must not write. - await harness.store.send(.runtime(.titleChanged(id: harness.contentID, title: "One"))) + // The bootstrap tab is titled "One"; an identical commit must not write. + await harness.store.send(.runtime(.titleCommitted(id: harness.contentID, title: "One"))) #expect(harness.store.state.layout.isConsistent) } @@ -1838,7 +1840,7 @@ struct LayoutFeatureTests { #expect(harness.store.state.layout.isConsistent) } - @Test func renameAndTitleReportsRespectTheTitleLock() async { + @Test func renameAndTitleCommitsRespectTheTitleLock() async { let harness = await makeHarness() let paneID = harness.paneID // Lock the bootstrap tab's title, as a script tab would be. @@ -1849,7 +1851,7 @@ struct LayoutFeatureTests { locked.panes[id: paneID]?.tabs[id: harness.tabID]?.isLocked = true let bundle = makeStore(layout: locked) await bundle.store.send(.renameTab(id: harness.tabID, title: "Rejected")) - await bundle.store.send(.runtime(.titleChanged(id: harness.contentID, title: "Shell Report"))) + await bundle.store.send(.runtime(.titleCommitted(id: harness.contentID, title: "Shell Report"))) #expect(bundle.store.state.layout.panes[id: paneID]?.tabs[id: harness.tabID]?.customTitle == "Custom") #expect(bundle.store.state.layout.panes[id: paneID]?.tabs[id: harness.tabID]?.title == "One") } diff --git a/supacodeTests/LayoutPersistenceTests.swift b/supacodeTests/LayoutPersistenceTests.swift index 4ef985331..783782683 100644 --- a/supacodeTests/LayoutPersistenceTests.swift +++ b/supacodeTests/LayoutPersistenceTests.swift @@ -48,6 +48,38 @@ struct LayoutPersistenceTests { ) } + @Test func overlaysTheChromesReportedTitle() { + let paneID = PaneID() + let tabID = TabID() + let contentID = ContentID() + let runtime = ContentRuntime() + let live = ChromeTabContent(id: contentID) + _ = runtime.provision(live, at: .fallback) + live.terminalChrome.reportedTitle = "claude" + + // Reported titles never reach the reducer, so the snapshot pull is their + // only path to disk. + let record = LayoutPersistence.record( + for: layout(paneID: paneID, tabID: tabID, contentID: contentID), + runtime: runtime + ) + #expect(record.layout.panes[id: paneID]?.tabs[id: tabID]?.title == "claude") + } + + @Test func keepsTheStoredTitleForContentThatNeverReported() { + let paneID = PaneID() + let tabID = TabID() + let contentID = ContentID() + let runtime = ContentRuntime() + _ = runtime.provision(ChromeTabContent(id: contentID), at: .fallback) + + let record = LayoutPersistence.record( + for: layout(paneID: paneID, tabID: tabID, contentID: contentID), + runtime: runtime + ) + #expect(record.layout.panes[id: paneID]?.tabs[id: tabID]?.title == "One") + } + @Test func overlaysLiveSnapshotsOverStoredOnes() throws { let paneID = PaneID() let tabID = TabID() diff --git a/supacodeTests/LogActionsReducerTests.swift b/supacodeTests/LogActionsReducerTests.swift new file mode 100644 index 000000000..a2bf14f0a --- /dev/null +++ b/supacodeTests/LogActionsReducerTests.swift @@ -0,0 +1,43 @@ +import ComposableArchitecture +import Testing + +@testable import supacode + +/// A minimal hand-rolled reducer (no `@Reducer` macro) so the test exercises +/// `logActions()` over a real base reduction with both a state write and an +/// effect. +private struct LogActionsCounter: Reducer { + struct State: Equatable { + var count = 0 + var ranEffect = false + } + enum Action: Equatable { + case bump + case effectFired + } + + func reduce(into state: inout State, action: Action) -> Effect { + switch action { + case .bump: + state.count += 1 + return .run { await $0(.effectFired) } + case .effectFired: + state.ranEffect = true + return .none + } + } +} + +/// `.logActions()` is a transparent passthrough when SUPACODE_LOG_ACTIONS is +/// unset, which is the default in every build and in the test process. It must +/// not change what the base reducer does to state or effects. +@MainActor +struct LogActionsReducerTests { + @Test func passthroughPreservesStateMutationAndEffects() async { + let store = TestStore(initialState: LogActionsCounter.State()) { + LogActionsCounter().logActions() + } + await store.send(.bump) { $0.count = 1 } + await store.receive(.effectFired) { $0.ranEffect = true } + } +} diff --git a/supacodeTests/TabTitleTests.swift b/supacodeTests/TabTitleTests.swift new file mode 100644 index 000000000..a97092b23 --- /dev/null +++ b/supacodeTests/TabTitleTests.swift @@ -0,0 +1,67 @@ +import Testing + +@testable import supacode + +/// Locks the title contract now that reported titles live on the content's +/// chrome instead of in `LayoutFeature`: nothing but the chrome carries a +/// terminal's live title, and the layout's own title is the fallback. +@MainActor +struct TabTitleTests { + private func tab( + title: String = "Terminal 1", + customTitle: String? = nil, + isLocked: Bool = false, + contentID: ContentID = ContentID() + ) -> TabItem { + TabItem( + id: TabID(), + title: title, + customTitle: customTitle, + content: ContentSnapshot( + id: contentID, + state: .terminal(TerminalContentState(workingDirectory: nil)) + ), + isLocked: isLocked + ) + } + + private func chrome(reporting title: String?) -> TerminalTabChrome { + let chrome = TerminalTabChrome() + chrome.reportedTitle = title + return chrome + } + + @Test func theReportedTitleWinsOverTheLayoutsOwn() { + #expect(TabTitle.resolved(for: tab(), chrome: chrome(reporting: "zsh")) == "zsh") + #expect(TabTitle.stored(for: tab(), chrome: chrome(reporting: "zsh")) == "zsh") + } + + @Test func anAbsentOrEmptyReportFallsBackToTheLayoutsTitle() { + #expect(TabTitle.resolved(for: tab(), chrome: chrome(reporting: nil)) == "Terminal 1") + #expect(TabTitle.resolved(for: tab(), chrome: chrome(reporting: "")) == "Terminal 1") + #expect(TabTitle.resolved(for: tab(), chrome: nil) == "Terminal 1") + } + + @Test func aUserOverrideWinsOverTheReportButNeverPersists() { + let renamed = tab(customTitle: "Custom") + #expect(TabTitle.resolved(for: renamed, chrome: chrome(reporting: "zsh")) == "Custom") + // The override persists in its own field; folding it into the stored title + // would make clearing the rename restore the override text. + #expect(TabTitle.stored(for: renamed, chrome: chrome(reporting: "zsh")) == "zsh") + } + + @Test func aLockedTabRefusesTheShellsReport() { + let script = tab(title: "Setup", isLocked: true) + #expect(TabTitle.resolved(for: script, chrome: chrome(reporting: "zsh")) == "Setup") + #expect(TabTitle.stored(for: script, chrome: chrome(reporting: "zsh")) == "Setup") + } + + @Test func resolvingThroughTheRuntimeReadsTheRegisteredContentsChrome() { + let contentID = ContentID() + let runtime = ContentRuntime() + let content = ChromeTabContent(id: contentID) + _ = runtime.provision(content, at: .fallback) + content.terminalChrome.reportedTitle = "claude" + #expect(TabTitle.resolved(for: tab(contentID: contentID), runtime: runtime) == "claude") + } +} diff --git a/supacodeTests/WorktreeContentHostTests.swift b/supacodeTests/WorktreeContentHostTests.swift index 365a5664c..3ee13936b 100644 --- a/supacodeTests/WorktreeContentHostTests.swift +++ b/supacodeTests/WorktreeContentHostTests.swift @@ -131,6 +131,48 @@ struct WorktreeContentHostTests { host.trackBlockingScript(kind: .archive, tabID: tabID, launchDirectory: nil) #expect(content.terminalChrome.isReadOnly == false) } + + @Test func aReportedTitleLandsOnTheChromeAndRearmsPersistenceOnce() { + let surfaceID = UUID() + let contentID = ContentID(rawValue: surfaceID) + let runtime = ContentRuntime() + let content = ChromeTabContent(id: contentID) + #expect(runtime.provision(content, at: .fallback)) + let host = makeHost(layout: singleTabLayout(contentID: surfaceID), runtime: runtime) + var sentLayoutActions = 0 + var persistenceRearms = 0 + host.sendLayoutAction = { _ in sentLayoutActions += 1 } + host.onReportedTitleChanged = { persistenceRearms += 1 } + + host.updateReportedTitle(for: contentID, title: "claude") + // An unchanged report is dropped before it can touch the chrome. + host.updateReportedTitle(for: contentID, title: "claude") + + #expect(content.terminalChrome.reportedTitle == "claude") + #expect(persistenceRearms == 1) + // The whole point: a title storm never reaches the store. + #expect(sentLayoutActions == 0) + } + + @Test(.dependencies) func anEmptyReportedTitleIsIgnoredSoTheLabelHoldsItsLastValue() { + let surfaceID = UUID() + let contentID = ContentID(rawValue: surfaceID) + let runtime = ContentRuntime() + let content = ChromeTabContent(id: contentID) + #expect(runtime.provision(content, at: .fallback)) + let host = makeHost(layout: singleTabLayout(contentID: surfaceID), runtime: runtime) + var persistenceRearms = 0 + host.onReportedTitleChanged = { persistenceRearms += 1 } + + host.updateReportedTitle(for: contentID, title: "~/project") + // A shell that clears the title mid-command must not flash the label: the + // empty and whitespace reports are dropped, keeping the last real title. + host.updateReportedTitle(for: contentID, title: "") + host.updateReportedTitle(for: contentID, title: " ") + + #expect(content.terminalChrome.reportedTitle == "~/project") + #expect(persistenceRearms == 1) + } } /// Pins the render-host claim invariants the steal-proof mount depends on. From eb747a03d2e80c6ba7eb23dcaccd6f54c9e90f9c Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Sun, 23 Aug 2026 23:37:01 +0200 Subject: [PATCH 3/4] Keep recently-used worktrees live and reclaim under memory pressure Hibernation was a pure 5-minute wall-clock timer, so flipping back to a worktree you were just in paid a full surface rebuild and zmx replay. Keep the most recently selected worktrees' visible tabs live regardless of idle time (capped at liveWorktreeLimit), so switching among them never rewakes; the clock still hibernates worktrees outside that window, and stacked background tabs as before. Under memory pressure, drop the recency budget to the current selection and hibernate every hidden tab immediately, so the live set never grows unbounded. --- .../AppLifecycle/MemoryPressureClient.swift | 27 ++ .../Features/App/Reducer/AppFeature.swift | 1 + .../Terminal/Reducer/TerminalsFeature.swift | 150 +++++++++- supacodeTests/TerminalsFeatureTests.swift | 266 +++++++++++++++++- 4 files changed, 430 insertions(+), 14 deletions(-) create mode 100644 supacode/Clients/AppLifecycle/MemoryPressureClient.swift diff --git a/supacode/Clients/AppLifecycle/MemoryPressureClient.swift b/supacode/Clients/AppLifecycle/MemoryPressureClient.swift new file mode 100644 index 000000000..f497a105b --- /dev/null +++ b/supacode/Clients/AppLifecycle/MemoryPressureClient.swift @@ -0,0 +1,27 @@ +import ComposableArchitecture +import Foundation + +/// Process-wide memory-pressure warnings, as a stream the hibernation policy +/// subscribes to once. Injected so a test can drive pressure without the kernel. +struct MemoryPressureClient { + var warnings: @Sendable () -> AsyncStream +} + +extension MemoryPressureClient: DependencyKey { + static let liveValue = MemoryPressureClient( + warnings: { + AsyncStream { continuation in + let source = DispatchSource.makeMemoryPressureSource( + eventMask: [.warning, .critical], + queue: .main + ) + source.setEventHandler { continuation.yield() } + continuation.onTermination = { _ in source.cancel() } + source.resume() + } + } + ) + + // Silent by default: only a test that opts in should see pressure. + static let testValue = MemoryPressureClient(warnings: { AsyncStream { $0.finish() } }) +} diff --git a/supacode/Features/App/Reducer/AppFeature.swift b/supacode/Features/App/Reducer/AppFeature.swift index c6fc9d7f8..bac8ce02b 100644 --- a/supacode/Features/App/Reducer/AppFeature.swift +++ b/supacode/Features/App/Reducer/AppFeature.swift @@ -398,6 +398,7 @@ struct AppFeature { refreshInstalledOpenActionsEffect(current: state.installedOpenActions), .send(.repositories(.task)), .send(.settings(.task)), + .send(.terminals(.task)), .run { @MainActor send in guard startupHotkey != nil else { return } if !appLifecycleClient.updateGlobalHotkey(startupHotkey) { diff --git a/supacode/Features/Terminal/Reducer/TerminalsFeature.swift b/supacode/Features/Terminal/Reducer/TerminalsFeature.swift index 63f26343f..75617805d 100644 --- a/supacode/Features/Terminal/Reducer/TerminalsFeature.swift +++ b/supacode/Features/Terminal/Reducer/TerminalsFeature.swift @@ -22,11 +22,21 @@ struct TerminalsFeature { /// Grace window a tab must stay hidden before it hibernates. static let hibernationGraceWindow: Duration = .seconds(5 * 60) + /// How many most-recently-selected worktrees keep their visible tabs live no + /// matter how long they stay deselected, so flipping back among that set never + /// pays a rewake. + static let liveWorktreeLimit = 3 + /// Per-tab cancellation key for the hibernation grace timer. nonisolated enum HibernationTimerID: Hashable, Sendable { case tab(TabID) } + /// Cancellation key for the process-wide memory-pressure subscription. + nonisolated enum CancelID: Hashable, Sendable { + case memoryPressure + } + @ObservableState struct State: Equatable { /// Per-worktree pane and tab topology, hydrated from `layouts.json` v2. @@ -37,6 +47,10 @@ struct TerminalsFeature { /// The selected worktree; only its panes' selected tabs are visible, so /// everything else is a hibernation candidate. var selectedWorktreeID: Worktree.ID? + /// Most-recently-selected worktrees, newest first, capped at + /// `liveWorktreeLimit`. Their visible panes' selected tabs never arm a grace + /// timer, so flipping back among them is instant. + var recentWorktreeIDs: [Worktree.ID] = [] /// Tabs with an armed hibernation grace timer. var hibernationArmedTabs: Set = [] /// Hidden-but-ineligible tabs already logged, so a permanently ineligible @@ -49,6 +63,8 @@ struct TerminalsFeature { enum Action { case layouts(IdentifiedActionOf) + /// Subscribes the memory-pressure source the hibernation policy reacts to. + case task /// The migrated layouts file finished loading. Consistent records become /// `LayoutFeature` states; inconsistent ones fall back to a fresh layout /// on first use. @@ -66,6 +82,9 @@ struct TerminalsFeature { case hibernationPolicyChanged /// A tab's grace timer fired; re-verify and hibernate or re-arm. case hibernationGraceElapsed(worktreeID: Worktree.ID, tabID: TabID) + /// The system reported memory pressure: drop the recency budget to the + /// selection and hibernate the hidden tabs now, skipping the grace window. + case memoryPressureWarning } private static let logger = SupaLogger("TerminalsFeature") @@ -88,6 +107,7 @@ struct TerminalsFeature { @Dependency(ContentRuntime.self) private var contentRuntime @Dependency(LayoutChangeObserver.self) private var layoutChangeObserver + @Dependency(MemoryPressureClient.self) private var memoryPressure @Dependency(\.continuousClock) private var clock var body: some Reducer { @@ -106,6 +126,14 @@ struct TerminalsFeature { case .layouts: return reconcileHibernation(&state) + case .task: + return .run { [memoryPressure] send in + for await _ in memoryPressure.warnings() { + await send(.memoryPressureWarning) + } + } + .cancellable(id: CancelID.memoryPressure, cancelInFlight: true) + case .attachLayout(let worktreeID, let titlePrefix): if state.layouts[id: worktreeID] == nil { state.layouts.append(LayoutFeature.State(id: worktreeID, layout: PaneLayout())) @@ -117,10 +145,12 @@ struct TerminalsFeature { // Bookkeeping is NOT pre-cleared: the reconcile below must still see // the armed entries to emit their timer cancellations. state.layouts.remove(id: worktreeID) + state.recentWorktreeIDs.removeAll { $0 == worktreeID } return reconcileHibernation(&state) case .selectedWorktreeChanged(let worktreeID): state.selectedWorktreeID = worktreeID + Self.recordSelection(worktreeID, in: &state.recentWorktreeIDs) return reconcileHibernation(&state) case .hibernationPolicyChanged: @@ -129,6 +159,9 @@ struct TerminalsFeature { case .hibernationGraceElapsed(let worktreeID, let tabID): return reduceHibernationGraceElapsed(&state, worktreeID: worktreeID, tabID: tabID) + case .memoryPressureWarning: + return reduceMemoryPressureWarning(&state) + case .layoutsHydrated(let file): state.layoutsAreReadOnly = file.schemaVersion > LayoutsFile.currentSchemaVersion // The runtime keys globally by content id and hibernation by tab id, so @@ -188,12 +221,40 @@ extension TerminalsFeature { return layout.id == selectedWorktreeID && visiblePanes.contains(pane.id) } + /// Moves a selection to the front of the recency list, capped at + /// `liveWorktreeLimit`. Deselecting keeps the list, so the worktree just left + /// stays the most recent. + private static func recordSelection(_ worktreeID: Worktree.ID?, in recents: inout [Worktree.ID]) { + guard let worktreeID else { return } + recents.removeAll { $0 == worktreeID } + recents.insert(worktreeID, at: 0) + if recents.count > liveWorktreeLimit { + recents.removeLast(recents.count - liveWorktreeLimit) + } + } + + /// Whether recency alone keeps this hidden tab live: it is the selected tab of + /// a visible pane (so stacked background tabs never qualify) and its worktree + /// is still inside the recency window. + private static func recencyRetains( + _ tab: TabItem, + pane: Pane, + in layout: LayoutFeature.State, + visiblePanes: Set, + recentWorktreeIDs: [Worktree.ID] + ) -> Bool { + guard pane.selectedTabID == tab.id, visiblePanes.contains(pane.id) else { return false } + return recentWorktreeIDs.contains(layout.id) + } + /// Diffs the hidden set against armed timers and wakes newly visible /// hibernated tabs. Cheap enough to run after every layout action. private func reconcileHibernation(_ state: inout State) -> Effect { @Shared(.settingsFile) var settingsFile: SettingsFile let enabled = settingsFile.global.terminalHibernationEnabled - var hidden: Set = [] + // Tabs that should hold an armed grace timer this pass; anything armed and + // absent here is cancelled below. + var keepArmed: Set = [] var allTabs: Set = [] var effects: [Effect] = [] for layout in state.layouts { @@ -209,14 +270,24 @@ extension TerminalsFeature { allTabs.insert(tab.id) let isHidden = Self.isTabHidden(tab, pane: pane, paneShowsContent: showsContent) if isHidden { - hidden.insert(tab.id) state.wakeRequestedTabs.remove(tab.id) - guard enabled, !state.hibernationArmedTabs.contains(tab.id) else { continue } - // Arm only live renderers; hibernated tabs have nothing to tear - // down and re-arm on wake through this same funnel. - guard contentRuntime.content(for: tab.content.id)?.renderer != nil else { continue } - state.hibernationArmedTabs.insert(tab.id) - effects.append(armGraceTimer(worktreeID: layout.id, tabID: tab.id)) + // Recency keeps the top worktrees' visible tabs live, so a flip back + // among them never pays a rewake; they never arm. + if Self.recencyRetains( + tab, pane: pane, in: layout, + visiblePanes: visiblePanes, recentWorktreeIDs: state.recentWorktreeIDs + ) { + continue + } + // Only a hidden, enabled tab with a live renderer keeps a timer; a + // hibernated tab (renderer gone) has nothing left to tear down, so + // it falls out of `keepArmed` and any stale timer is cancelled. + guard enabled, contentRuntime.content(for: tab.content.id)?.renderer != nil else { continue } + keepArmed.insert(tab.id) + if !state.hibernationArmedTabs.contains(tab.id) { + state.hibernationArmedTabs.insert(tab.id) + effects.append(armGraceTimer(worktreeID: layout.id, tabID: tab.id)) + } } else if contentNeedsWake(tab) { // The selection landed on a hibernated tab; wake it at its frozen // geometry, once per visibility spell so a failed wake can't loop. @@ -229,8 +300,9 @@ extension TerminalsFeature { } } } - // Cancel timers for tabs that became visible, vanished, or lost the flag. - for armed in state.hibernationArmedTabs where !enabled || !hidden.contains(armed) { + // Cancel any armed tab no longer eligible: visible, vanished, recency-covered, + // renderer gone (hibernated), or the flag flipped off. + for armed in state.hibernationArmedTabs where !keepArmed.contains(armed) { state.hibernationArmedTabs.remove(armed) state.hibernationDeferralLogged.remove(armed) effects.append(.cancel(id: HibernationTimerID.tab(armed))) @@ -268,16 +340,28 @@ extension TerminalsFeature { guard settingsFile.global.terminalHibernationEnabled else { return .none } guard let layout = state.layouts[id: worktreeID], let pane = layout.layout.pane(containingTab: tabID), - let tab = pane.tabs[id: tabID], + let tab = pane.tabs[id: tabID] + else { return .none } + let visiblePanes = Set(layout.layout.tree.visibleLeaves()) + guard Self.isTabHidden( tab, pane: pane, paneShowsContent: Self.paneShowsContent( pane, in: layout, - visiblePanes: Set(layout.layout.tree.visibleLeaves()), + visiblePanes: visiblePanes, selectedWorktreeID: state.selectedWorktreeID ) + ), + // Recency can cover a tab after its timer armed; the fire-time gate must + // agree with the arm-time one or a protected tab still hibernates. + !Self.recencyRetains( + tab, + pane: pane, + in: layout, + visiblePanes: visiblePanes, + recentWorktreeIDs: state.recentWorktreeIDs ) else { return .none } guard layout.alert == nil else { @@ -285,7 +369,15 @@ extension TerminalsFeature { state.hibernationArmedTabs.insert(tabID) return armGraceTimer(worktreeID: worktreeID, tabID: tabID) } - guard contentRuntime.content(for: tab.content.id)?.isHibernatable == true else { + let content = contentRuntime.content(for: tab.content.id) + guard content?.isHibernatable == true else { + // Nothing left to hibernate (the renderer is already gone, e.g. a + // concurrent pressure sweep hibernated it after this timer fired): settle + // instead of re-arming a dead tab into a forever loop. + guard content?.renderer != nil else { + state.hibernationDeferralLogged.remove(tabID) + return .none + } // Still hidden but momentarily ineligible; re-arm so a later // eligibility flip still hibernates instead of wedging forever. if state.hibernationDeferralLogged.insert(tabID).inserted { @@ -297,4 +389,36 @@ extension TerminalsFeature { state.hibernationDeferralLogged.remove(tabID) return .send(.layouts(.element(id: worktreeID, action: .hibernateTab(id: tabID)))) } + + /// Under pressure the recency budget is the first thing to go: keep only the + /// selection live and hibernate every hidden hibernatable tab now instead of + /// waiting out the grace window. Gated on the hibernation Beta flag. + private func reduceMemoryPressureWarning(_ state: inout State) -> Effect { + @Shared(.settingsFile) var settingsFile: SettingsFile + guard settingsFile.global.terminalHibernationEnabled else { return .none } + state.recentWorktreeIDs = state.selectedWorktreeID.map { [$0] } ?? [] + var effects: [Effect] = [] + for layout in state.layouts { + // A pending close confirmation keeps its worktree's tabs live; the grace + // path already exempts it, so the sweep must too. + guard layout.alert == nil else { continue } + let visiblePanes = Set(layout.layout.tree.visibleLeaves()) + for pane in layout.layout.panes { + let showsContent = Self.paneShowsContent( + pane, in: layout, visiblePanes: visiblePanes, selectedWorktreeID: state.selectedWorktreeID) + for tab in pane.tabs where Self.isTabHidden(tab, pane: pane, paneShowsContent: showsContent) { + guard contentRuntime.content(for: tab.content.id)?.isHibernatable == true else { continue } + // Cancel the pending grace timer and hibernate through the fire-time + // action, which re-checks visibility just before teardown so a tab the + // user selects between this sweep and the hibernate is spared. + if state.hibernationArmedTabs.remove(tab.id) != nil { + effects.append(.cancel(id: HibernationTimerID.tab(tab.id))) + } + state.hibernationDeferralLogged.remove(tab.id) + effects.append(.send(.hibernationGraceElapsed(worktreeID: layout.id, tabID: tab.id))) + } + } + } + return effects.isEmpty ? .none : .merge(effects) + } } diff --git a/supacodeTests/TerminalsFeatureTests.swift b/supacodeTests/TerminalsFeatureTests.swift index bd9ef0dc1..17e8dff2d 100644 --- a/supacodeTests/TerminalsFeatureTests.swift +++ b/supacodeTests/TerminalsFeatureTests.swift @@ -77,6 +77,8 @@ struct TerminalsFeatureTests { let hiddenTab: TabID let selectedContent: HibernatableContent let hiddenContent: HibernatableContent + /// Drives injected memory-pressure warnings; `.task` must be sent to subscribe. + let pressure: AsyncStream.Continuation } /// One worktree, one pane, two tabs; both contents live in the runtime. @@ -121,6 +123,7 @@ struct TerminalsFeatureTests { focusedPaneID: paneID ) let clock = TestClock() + let pressure = AsyncStream.makeStream() let store = TestStore( initialState: TerminalsFeature.State(layouts: [LayoutFeature.State(id: worktreeID, layout: layout)]) ) { @@ -129,6 +132,7 @@ struct TerminalsFeatureTests { $0.continuousClock = clock $0.contentRuntime = runtime $0[ContentSessionKiller.self] = ContentSessionKiller(kill: { _, _ in }) + $0[MemoryPressureClient.self] = MemoryPressureClient(warnings: { pressure.stream }) } return HibernationHarness( store: store, @@ -139,7 +143,8 @@ struct TerminalsFeatureTests { selectedTab: selectedTab, hiddenTab: hiddenTab, selectedContent: selectedContent, - hiddenContent: hiddenContent + hiddenContent: hiddenContent, + pressure: pressure.continuation ) } @@ -149,6 +154,7 @@ struct TerminalsFeatureTests { let harness = makeHibernationHarness() await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] $0.hibernationArmedTabs = [harness.hiddenTab] } await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow) @@ -168,6 +174,7 @@ struct TerminalsFeatureTests { let harness = makeHibernationHarness() await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] $0.hibernationArmedTabs = [harness.hiddenTab] } // Selecting the hidden tab makes it visible and hides the other one. @@ -195,6 +202,7 @@ struct TerminalsFeatureTests { let harness = makeHibernationHarness() await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] $0.hibernationArmedTabs = [harness.hiddenTab] } $settingsFile.withLock { $0.global.terminalHibernationEnabled = false } @@ -212,6 +220,7 @@ struct TerminalsFeatureTests { harness.hiddenContent.claimsHibernation = false await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] $0.hibernationArmedTabs = [harness.hiddenTab] } await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow) @@ -239,6 +248,7 @@ struct TerminalsFeatureTests { harness.selectedContent.hibernate() await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] $0.hibernationArmedTabs = [harness.hiddenTab] $0.wakeRequestedTabs = [harness.selectedTab] } @@ -270,6 +280,7 @@ struct TerminalsFeatureTests { // selection. await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) { $0.selectedWorktreeID = Worktree.ID("/tmp/other") + $0.recentWorktreeIDs = [Worktree.ID("/tmp/other")] } await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow) await harness.store.receive(\.hibernationGraceElapsed) { @@ -294,6 +305,7 @@ struct TerminalsFeatureTests { } await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) { $0.selectedWorktreeID = Worktree.ID("/tmp/other") + $0.recentWorktreeIDs = [Worktree.ID("/tmp/other")] } // Re-attaching withdraws the exemption: the selection is hidden again. await harness.store.send( @@ -397,6 +409,7 @@ struct TerminalsFeatureTests { } await store.send(.selectedWorktreeChanged(worktreeID)) { $0.selectedWorktreeID = worktreeID + $0.recentWorktreeIDs = [worktreeID] // Pane B sits behind the zoom, so its selection is hidden and arms. $0.hibernationArmedTabs = [tabB] } @@ -413,6 +426,7 @@ struct TerminalsFeatureTests { // Selecting another worktree hides both tabs; both arm. await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) { $0.selectedWorktreeID = Worktree.ID("/tmp/other") + $0.recentWorktreeIDs = [Worktree.ID("/tmp/other")] $0.hibernationArmedTabs = [harness.selectedTab, harness.hiddenTab] } await harness.store.send(.detachLayout(worktreeID: harness.worktreeID)) { @@ -424,6 +438,256 @@ struct TerminalsFeatureTests { await harness.store.finish() } + @Test(.dependencies) func aRecentWorktreesSelectionStaysLiveWhileDeselected() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { + $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] + $0.hibernationArmedTabs = [harness.hiddenTab] + } + // Deselecting keeps the worktree inside the recency window, so its visible + // selection is retained (never arms) even though it is now hidden; only the + // stacked tab stays armed. + await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) { + $0.selectedWorktreeID = Worktree.ID("/tmp/other") + $0.recentWorktreeIDs = [Worktree.ID("/tmp/other"), harness.worktreeID] + } + // Advance well past the grace window: a retained selection never arms, so no + // amount of idle time hibernates it, while the stacked tab fires once. + await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow * 2) + await harness.store.receive(\.hibernationGraceElapsed) { + $0.hibernationArmedTabs = [] + } + await harness.store.receive(\.layouts) { + $0.layouts[id: harness.worktreeID]?.renderEpoch = 1 + } + // The stacked tab hibernated; the recency-retained selection did not. + #expect(harness.hiddenContent.renderer == nil) + #expect(harness.selectedContent.renderer != nil) + await harness.store.finish() + } + + @Test(.dependencies) func aWorktreePushedOutOfTheRecencyWindowArmsItsSelection() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { + $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] + $0.hibernationArmedTabs = [harness.hiddenTab] + } + // Visit enough other worktrees to push this one past `liveWorktreeLimit`. + let others = ["/tmp/o1", "/tmp/o2", "/tmp/o3"].map { Worktree.ID($0) } + await harness.store.send(.selectedWorktreeChanged(others[0])) { + $0.selectedWorktreeID = others[0] + $0.recentWorktreeIDs = [others[0], harness.worktreeID] + } + await harness.store.send(.selectedWorktreeChanged(others[1])) { + $0.selectedWorktreeID = others[1] + $0.recentWorktreeIDs = [others[1], others[0], harness.worktreeID] + } + await harness.store.send(.selectedWorktreeChanged(others[2])) { + $0.selectedWorktreeID = others[2] + // The worktree drops out of the window, so its selection loses recency + // cover and arms alongside the stacked tab. + $0.recentWorktreeIDs = [others[2], others[1], others[0]] + $0.hibernationArmedTabs = [harness.hiddenTab, harness.selectedTab] + } + $settingsFile.withLock { $0.global.terminalHibernationEnabled = false } + await harness.store.send(.hibernationPolicyChanged) { + $0.hibernationArmedTabs = [] + } + await harness.store.finish() + } + + @Test(.dependencies) func memoryPressureDropsRecencyAndHibernatesEveryHiddenTab() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + // The sweep fans out one hibernate per hidden tab; assert the outcome, not + // each cascading action. + harness.store.exhaustivity = .off + await harness.store.send(.task) + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) + // Deselect but stay recent: without pressure the selection is retained live. + await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) + #expect(harness.selectedContent.renderer != nil) + + harness.pressure.yield() + await harness.store.receive(\.memoryPressureWarning) + await harness.store.skipReceivedActions() + + // Recency collapses to the current selection, and the deselected worktree's + // retained selection hibernates now instead of waiting out the grace window. + #expect(harness.store.state.recentWorktreeIDs == [Worktree.ID("/tmp/other")]) + #expect(harness.selectedContent.renderer == nil) + #expect(harness.hiddenContent.renderer == nil) + + harness.pressure.finish() + await harness.store.finish() + } + + @Test(.dependencies) func memoryPressureSparesTheVisibleSelection() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + harness.store.exhaustivity = .off + await harness.store.send(.task) + // The worktree stays selected across the pressure event. + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) + + harness.pressure.yield() + await harness.store.receive(\.memoryPressureWarning) + await harness.store.skipReceivedActions() + + // The on-screen selection survives; only the stacked tab hibernates. + #expect(harness.selectedContent.renderer != nil) + #expect(harness.hiddenContent.renderer == nil) + + harness.pressure.finish() + await harness.store.finish() + } + + @Test(.dependencies) func pressureSparesATabReselectedBeforeItsHibernateLands() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + harness.store.exhaustivity = .off + await harness.store.send(.task) + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) + // Deselect so the selection tab is hidden and becomes a pressure target. + await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) + + harness.pressure.yield() + await harness.store.receive(\.memoryPressureWarning) + // Before the queued hibernations land, the user flips back, making the + // selection visible again. Routing pressure through the fire-time action + // means its re-check spares the now-visible tab. + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) + await harness.store.skipReceivedActions() + + #expect(harness.selectedContent.renderer != nil) + + // The reselect re-armed the stacked tab's grace timer; drain it so the + // store finishes with no in-flight effect. + await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow) + await harness.store.skipReceivedActions() + harness.pressure.finish() + await harness.store.finish() + } + + @Test(.dependencies) func memoryPressureRespectsTheHibernationFlag() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = false } + let harness = makeHibernationHarness() + await harness.store.send(.task) + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { + $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] + } + await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) { + $0.selectedWorktreeID = Worktree.ID("/tmp/other") + $0.recentWorktreeIDs = [Worktree.ID("/tmp/other"), harness.worktreeID] + } + + harness.pressure.yield() + // Hibernation disabled: the warning is a no-op. No surface is dropped and + // the recency budget is left intact. + await harness.store.receive(\.memoryPressureWarning) + #expect(harness.selectedContent.renderer != nil) + #expect(harness.hiddenContent.renderer != nil) + #expect(harness.store.state.recentWorktreeIDs == [Worktree.ID("/tmp/other"), harness.worktreeID]) + + harness.pressure.finish() + await harness.store.finish() + } + + @Test(.dependencies) func theFireTimeGateSparesASelectionThatBecameRecent() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { + $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] + $0.hibernationArmedTabs = [harness.hiddenTab] + } + // Deselect but stay recent: the selection is retained, never armed. + await harness.store.send(.selectedWorktreeChanged(Worktree.ID("/tmp/other"))) { + $0.selectedWorktreeID = Worktree.ID("/tmp/other") + $0.recentWorktreeIDs = [Worktree.ID("/tmp/other"), harness.worktreeID] + } + // A grace timer that fired for the now-retained selection (a race the + // arm-time cancel could miss) must not hibernate it: the fire-time gate wins. + await harness.store.send( + .hibernationGraceElapsed(worktreeID: harness.worktreeID, tabID: harness.selectedTab) + ) + #expect(harness.selectedContent.renderer != nil) + // Drain the stacked tab's still-armed timer. + $settingsFile.withLock { $0.global.terminalHibernationEnabled = false } + await harness.store.send(.hibernationPolicyChanged) { + $0.hibernationArmedTabs = [] + } + await harness.store.finish() + } + + @Test(.dependencies) func graceElapsedSettlesInsteadOfReArmingAHibernatedTab() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = true } + let harness = makeHibernationHarness() + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { + $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] + $0.hibernationArmedTabs = [harness.hiddenTab] + } + // The armed tab hibernates out of band (as a concurrent pressure sweep + // would), so when its grace timer fires there is nothing left to hibernate. + harness.hiddenContent.hibernate() + await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow) + await harness.store.receive(\.hibernationGraceElapsed) { + $0.hibernationArmedTabs = [] + } + // Settled, not re-armed: a second window produces no further grace action. + await harness.clock.advance(by: TerminalsFeature.hibernationGraceWindow) + await harness.store.finish() + } + + @Test(.dependencies) func reSelectingARecentWorktreeMovesItToFrontWithoutGrowing() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = false } + let harness = makeHibernationHarness() + let worktreeA = harness.worktreeID + let worktreeB = Worktree.ID("/tmp/b") + await harness.store.send(.selectedWorktreeChanged(worktreeA)) { + $0.selectedWorktreeID = worktreeA + $0.recentWorktreeIDs = [worktreeA] + } + await harness.store.send(.selectedWorktreeChanged(worktreeB)) { + $0.selectedWorktreeID = worktreeB + $0.recentWorktreeIDs = [worktreeB, worktreeA] + } + // Re-selecting the first worktree moves it to front without duplicating or growing the list. + await harness.store.send(.selectedWorktreeChanged(worktreeA)) { + $0.selectedWorktreeID = worktreeA + $0.recentWorktreeIDs = [worktreeA, worktreeB] + } + } + + @Test(.dependencies) func detachLayoutRemovesTheWorktreeFromRecents() async { + @Shared(.settingsFile) var settingsFile + $settingsFile.withLock { $0.global.terminalHibernationEnabled = false } + let harness = makeHibernationHarness() + await harness.store.send(.selectedWorktreeChanged(harness.worktreeID)) { + $0.selectedWorktreeID = harness.worktreeID + $0.recentWorktreeIDs = [harness.worktreeID] + } + await harness.store.send(.detachLayout(worktreeID: harness.worktreeID)) { + $0.layouts = [] + $0.recentWorktreeIDs = [] + } + } + @Test func layoutsHydrationServesConsistentRecordsOnly() async { let paneID = PaneID() let good = Self.layout(paneID: paneID, tabID: TabID(), contentID: ContentID()) From 877b7ff7afc32e8f71e1f869040e9157274b5656 Mon Sep 17 00:00:00 2001 From: Stefano Bertagno Date: Mon, 24 Aug 2026 01:01:43 +0200 Subject: [PATCH 4/4] Cut close latency by deferring the terminal surface free Closing a pane ran ghostty_surface_free synchronously in the reducer, before the split tree collapsed: it joins the surface's search, renderer, and IO threads and tears down the Metal renderer, all on the click. Detach the surface synchronously (unregister it, clear the callback wiring, hide the view so the "[Process exited]" overlay cannot flash while the layout collapses) and free it off the reducer turn on the main queue, retaining the Ghostty app and the callback bridge until the free lands. Reap after the tree collapses so the collapse is the turn's state mutation. Deferring the free keeps the surface's background threads alive briefly after closeSurface returns, so an action one emits can reach the app callback after the surface is freed. Drop a background-thread action whose surface was unregistered (closing or freed) via a live-surface registry cleared synchronously in closeSurface, so a late action lands on nothing instead of freed memory. --- .../Terminal/Reducer/LayoutFeature.swift | 27 ++++++------ .../Ghostty/GhosttyRuntime.swift | 29 ++++++++++--- .../Ghostty/GhosttySurfaceView.swift | 42 +++++++++++++------ supacodeTests/GhosttySurfaceViewTests.swift | 26 ++++++++++++ 4 files changed, 95 insertions(+), 29 deletions(-) diff --git a/supacode/Features/Terminal/Reducer/LayoutFeature.swift b/supacode/Features/Terminal/Reducer/LayoutFeature.swift index 9036db0a2..ccef4dc9f 100644 --- a/supacode/Features/Terminal/Reducer/LayoutFeature.swift +++ b/supacode/Features/Terminal/Reducer/LayoutFeature.swift @@ -573,19 +573,21 @@ extension LayoutFeature { guard var pane = state.layout.pane(containingTab: tabID), let index = pane.tabs.index(id: tabID) else { return .none } - let reaping = reap(pane.tabs[index].content.id, worktree: state.id) + let contentID = pane.tabs[index].content.id releaseTabBookkeeping(&state, tabID: tabID) pane.tabs.remove(at: index) - guard !pane.tabs.isEmpty else { + if pane.tabs.isEmpty { collapse(&state, paneID: pane.id) - return reaping - } - if pane.selectedTabID == tabID { - // Selection retargets to the previous tab, else the first. - pane.selectedTabID = index > 0 ? pane.tabs[index - 1].id : pane.tabs.first?.id + } else { + if pane.selectedTabID == tabID { + // Selection retargets to the previous tab, else the first. + pane.selectedTabID = index > 0 ? pane.tabs[index - 1].id : pane.tabs.first?.id + } + state.layout.panes[id: pane.id] = pane } - state.layout.panes[id: pane.id] = pane - return reaping + // Reap after the tree has collapsed so the collapse is the turn's state + // mutation and the surface teardown runs off it, not before it. + return reap(contentID, worktree: state.id) } private func reduceMoveTab( @@ -916,13 +918,14 @@ extension LayoutFeature { private func reduceClosePane(_ state: inout State, paneID: PaneID) -> Effect { guard let pane = state.layout.panes[id: paneID] else { return .none } - // Merged: one hung kill must not queue the siblings behind it. - let reaping = Effect.merge(pane.tabs.map { reap($0.content.id, worktree: state.id) }) for tab in pane.tabs { releaseTabBookkeeping(&state, tabID: tab.id) } collapse(&state, paneID: paneID) - return reaping + // Reap after the tree has collapsed so the collapse is the turn's state + // mutation and the surface teardown runs off it, not before it. Merged: one + // hung kill must not queue the siblings behind it. + return .merge(pane.tabs.map { reap($0.content.id, worktree: state.id) }) } private func reduceResizePane(_ state: inout State, node: SplitTree.Node, ratio: Double) -> Effect { diff --git a/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift b/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift index bae9a3101..082c40a3b 100644 --- a/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift +++ b/supacode/Infrastructure/Ghostty/GhosttyRuntime.swift @@ -8,13 +8,17 @@ import UniformTypeIdentifiers final class GhosttyRuntime { private static let logger = SupaLogger("Ghostty") - /// Live-pointer registries for C callbacks. A queued main-queue callback - /// (e.g. a wakeup) can fire after its runtime deinit freed the app, so - /// dereferencing the raw userdata/app pointer would be use-after-free; - /// every resolution validates membership first. Registered in init, - /// removed in deinit. + /// Live-pointer registries for C callbacks. A queued main-queue callback can + /// fire after the raw pointer it names was freed, so dereferencing it would be + /// use-after-free; the deferred handler validates membership first. App and + /// userdata are registered in init and removed in deinit; surfaces are + /// registered on `registerSurface` and removed on `unregisterSurface`, which + /// `closeSurface` calls synchronously before the deferred + /// `ghostty_surface_free`, so a background-thread action queued around the + /// close is dropped instead of hitting freed memory. private static var liveUserdataBits: Set = [] private static var liveAppBits: Set = [] + private static var liveSurfaceBits: Set = [] final class SurfaceReference { let surface: ghostty_surface_t @@ -183,6 +187,7 @@ final class GhosttyRuntime { let ref = SurfaceReference(surface) surfaceRefs.append(ref) surfaceRefs = surfaceRefs.filter { $0.isValid } + Self.liveSurfaceBits.insert(UInt(bitPattern: surface)) if let lastColorScheme { ghostty_surface_set_color_scheme(surface, lastColorScheme) } @@ -192,6 +197,7 @@ final class GhosttyRuntime { func unregisterSurface(_ ref: SurfaceReference) { ref.invalidate() surfaceRefs = surfaceRefs.filter { $0.isValid } + Self.liveSurfaceBits.remove(UInt(bitPattern: ref.surface)) } /// Reloads the full app config from disk and re-applies the current color scheme. @@ -315,12 +321,25 @@ final class GhosttyRuntime { guard let app else { return false } let appBits = UInt(bitPattern: app) if Thread.isMainThread { + // Synchronous: the caller is driving this surface right now (e.g. its + // creation, which emits the initial cell-size/title before the surface is + // registered), so it is live by construction. return MainActor.assumeIsolated { handleAction(appBits: appBits, target: target, action: action) } } + // A background surface thread can emit an action just before, or during, the + // surface's deferred free; capture its identity so the main-actor block can + // drop it if the surface was unregistered (closing/freed) in the meantime, + // rather than dereferencing a pointer that is about to be, or already is, + // freed. + let surfaceBits = + target.tag == GHOSTTY_TARGET_SURFACE + ? target.target.surface.map { UInt(bitPattern: $0) } + : nil DispatchQueue.main.async { MainActor.assumeIsolated { + if let surfaceBits, !liveSurfaceBits.contains(surfaceBits) { return } _ = handleAction(appBits: appBits, target: target, action: action) } } diff --git a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift index 1156df582..c2ed76d96 100644 --- a/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift +++ b/supacode/Infrastructure/Ghostty/GhosttySurfaceView.swift @@ -307,7 +307,7 @@ final class GhosttySurfaceView: NSView, Identifiable { SecureInput.shared.removeScoped(id) } // A live surface here means a teardown path bypassed `closeSurface`; the - // call below still frees it. + // call below still frees it, off the turn. if surface != nil { assertionFailure("GhosttySurfaceView deallocated with a live surface; a teardown path bypassed closeSurface().") } @@ -330,19 +330,37 @@ final class GhosttySurfaceView: NSView, Identifiable { func closeSurface() { clearNotificationObservers() - if let surface { - if let surfaceRef { - runtime.unregisterSurface(surfaceRef) - self.surfaceRef = nil - } + // Break the surface<->wrapper cycle; the strong hold otherwise blocks deinit. + defer { ownedScrollWrapper = nil } + guard let surface else { return } + if let surfaceRef { + runtime.unregisterSurface(surfaceRef) + self.surfaceRef = nil + } + self.surface = nil + bridge.surface = nil + lastOcclusion = nil + lastSurfaceFocus = nil + // Hide before the free so the "[Process exited]" overlay can't paint while + // the layout collapses around the closing pane. + isHidden = true + // Free off the current turn on the main queue: `ghostty_surface_free` joins + // the surface's search, renderer, and IO threads and tears down the Metal + // renderer, which would otherwise block the reducer turn. The main queue, + // not a `Task`, runs the free outside the reducer's inherited task-local + // scope, where an isolated-deinit release it triggers can abort as an + // invalid free. Retain the runtime and bridge by hand across the free (a + // Sendable block can't capture them) so the Ghostty app stays alive and a + // synchronous callback during the free still resolves a live bridge; `self` + // is intentionally not captured, as the free never touches the surface's + // nsview. + let retainedRuntime = Unmanaged.passRetained(runtime) + let retainedBridge = Unmanaged.passRetained(bridge) + DispatchQueue.main.async { ghostty_surface_free(surface) - self.surface = nil - bridge.surface = nil - lastOcclusion = nil - lastSurfaceFocus = nil + retainedBridge.release() + retainedRuntime.release() } - // Break the surface<->wrapper cycle; the strong hold otherwise blocks deinit. - ownedScrollWrapper = nil } private func updateScreenObservers() { diff --git a/supacodeTests/GhosttySurfaceViewTests.swift b/supacodeTests/GhosttySurfaceViewTests.swift index 5a7cbd862..e7a81dba8 100644 --- a/supacodeTests/GhosttySurfaceViewTests.swift +++ b/supacodeTests/GhosttySurfaceViewTests.swift @@ -561,4 +561,30 @@ struct GhosttySurfaceViewTests { #expect(first !== second) #expect(surfaceView.scrollWrapper === second) } + + // `closeSurface` detaches synchronously (surface cleared, view hidden so the + // "[Process exited]" overlay can't flash during a collapse) and defers the + // costly `ghostty_surface_free` off the turn, and a second close is a safe + // no-op. Only the hide needs a live renderer, absent when the display sleeps; + // the synchronous clear and the double-close no-op hold on both paths. + @Test func closeSurfaceDetachesSynchronouslyAndHidesTheView() { + let surfaceView = GhosttySurfaceView( + id: UUID(), + runtime: GhosttyRuntime(), + workingDirectory: nil, + initialGeometry: .fallback, + context: GHOSTTY_SURFACE_CONTEXT_TAB + ) + let hadLiveRenderer = surfaceView.surface != nil + + surfaceView.closeSurface() + #expect(surfaceView.surface == nil) + if hadLiveRenderer { + #expect(surfaceView.isHidden) + } + + // Idempotent: the surface is already gone, so this must not double-free. + surfaceView.closeSurface() + #expect(surfaceView.surface == nil) + } }