diff --git a/AGENTS.md b/AGENTS.md index 58fee190d..1935bd13f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,6 +107,7 @@ Reducer ← .repositories(.worktreeInfoEvent(Event)) ← AsyncStream - Prefer `@Shared` directly in reducers for app storage and shared settings; do not introduce new dependency clients solely to wrap `@Shared`. - Use `SupaLogger` for all logging. Never use `print()` or `os.Logger` directly. `SupaLogger` prints in DEBUG and uses `os.Logger` in release. - 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`. ### Formatting & Linting diff --git a/supacode/App/ContentView.swift b/supacode/App/ContentView.swift index e77df2d5a..850fda5c1 100644 --- a/supacode/App/ContentView.swift +++ b/supacode/App/ContentView.swift @@ -10,6 +10,10 @@ import SupacodeSettingsShared import SwiftUI import UniformTypeIdentifiers +#if DEBUG + private nonisolated let contentRenderLogger = SupaLogger("DetailRender") +#endif + struct ContentView: View { @Bindable var store: StoreOf @Bindable var repositoriesStore: StoreOf @@ -25,7 +29,10 @@ struct ContentView: View { } var body: some View { - NavigationSplitView(columnVisibility: $leftSidebarVisibility) { + #if DEBUG + let _ = contentRenderLogger.info("ContentView.body re-rendered") + #endif + return NavigationSplitView(columnVisibility: $leftSidebarVisibility) { SidebarView(store: repositoriesStore, terminalManager: terminalManager) .navigationSplitViewColumnWidth(min: 220, ideal: 260, max: 320) .safeAreaInset(edge: .bottom, spacing: 0) { @@ -35,7 +42,7 @@ struct ContentView: View { WorktreeDetailView(store: store, terminalManager: terminalManager) } .navigationSplitViewStyle(.automatic) - .disabled(!store.repositories.isInitialLoadComplete) + .disabled(!repositoriesStore.isInitialLoadComplete) .onChange(of: scenePhase) { _, newValue in store.send(.scenePhaseChanged(newValue)) } @@ -78,45 +85,80 @@ struct ContentView: View { ) { customizationStore in RepositoryCustomizationView(store: customizationStore) } - .focusedSceneValue(\.toggleLeftSidebarAction, toggleLeftSidebar) - .focusedSceneValue(\.revealInSidebarAction, revealInSidebarAction) + .focusedSceneAction(\.toggleLeftSidebarAction, enabled: true) { + withAnimation(.easeOut(duration: 0.2)) { + leftSidebarVisibility = leftSidebarVisibility == .detailOnly ? .all : .detailOnly + } + } + .focusedSceneAction( + \.revealInSidebarAction, + enabled: repositoriesStore.selectedWorktreeID != nil + ) { + withAnimation(.easeOut(duration: 0.2)) { + leftSidebarVisibility = .all + } + store.send(.repositories(.revealSelectedWorktreeInSidebar)) + } .overlay { - CommandPaletteOverlayView( - store: store.scope(state: \.commandPalette, action: \.commandPalette), - items: CommandPaletteFeature.commandPaletteItems( - from: store.repositories, - ghosttyCommands: ghosttyShortcuts.commandPaletteEntries, - scripts: store.allScripts, - runningScriptIDs: store.runningScriptIDs - ) + CommandPaletteOverlayHost( + store: store, + repositoriesStore: repositoriesStore, + ghosttyShortcuts: ghosttyShortcuts ) } .background(WindowTabbingDisabler()) .background(WindowChromeObserver(runtime: terminalManager.ghosttyRuntime)) - .navigationTitle( - WindowTitle.compute( - repositories: store.repositories, + .background( + WindowTitleHost( + repositoriesStore: repositoriesStore, terminalManager: terminalManager ) ) } +} - private func toggleLeftSidebar() { - withAnimation(.easeOut(duration: 0.2)) { - leftSidebarVisibility = leftSidebarVisibility == .detailOnly ? .all : .detailOnly - } - } +/// Hosts the command palette overlay so the items build runs in this view's +/// body instead of `ContentView.body`. Per-row sidebar mutations only +/// invalidate this host, leaving ContentView's focused-value closures stable. +private struct CommandPaletteOverlayHost: View { + let store: StoreOf + let repositoriesStore: StoreOf + let ghosttyShortcuts: GhosttyShortcutManager - private var revealInSidebarAction: (() -> Void)? { - guard store.repositories.selectedWorktreeID != nil else { return nil } - return { revealInSidebar() } + var body: some View { + #if DEBUG + let _ = contentRenderLogger.info("CommandPaletteOverlayHost.body re-rendered") + #endif + return CommandPaletteOverlayView( + store: store.scope(state: \.commandPalette, action: \.commandPalette), + items: CommandPaletteFeature.commandPaletteItems( + from: repositoriesStore.state, + ghosttyCommands: ghosttyShortcuts.commandPaletteEntries, + scripts: store.allScripts, + runningScriptIDs: store.runningScriptIDs + ) + ) } +} - private func revealInSidebar() { - withAnimation(.easeOut(duration: 0.2)) { - leftSidebarVisibility = .all - } - store.send(.repositories(.revealSelectedWorktreeInSidebar)) - } +/// Hosts the `.navigationTitle` modifier so the title computation runs in +/// this view's body. `WindowTitle.compute` reads selection / sidebar.sections +/// fields. Confining the reads here keeps ContentView immune to title-only +/// invalidations from tab renames or section title edits. +private struct WindowTitleHost: View { + let repositoriesStore: StoreOf + let terminalManager: WorktreeTerminalManager + var body: some View { + #if DEBUG + let _ = contentRenderLogger.info("WindowTitleHost.body re-rendered") + #endif + return Color.clear + .navigationTitle( + WindowTitle.compute( + repositories: repositoriesStore.state, + terminalManager: terminalManager + ) + ) + } } diff --git a/supacode/App/Models/FocusedAction.swift b/supacode/App/Models/FocusedAction.swift new file mode 100644 index 000000000..867719b95 --- /dev/null +++ b/supacode/App/Models/FocusedAction.swift @@ -0,0 +1,76 @@ +import SwiftUI + +/// Equatable wrapper around a focused-value action closure. +/// +/// SwiftUI's `focusedSceneValue` / `focusedValue` re-publishes whenever the +/// stored value's identity changes. A bare `() -> Void` closure has no +/// Equatable conformance, so every publisher-view body run looks like a +/// "value changed" event to AppKit, which then rebuilds the system menu and +/// drops open-submenu / hover state (#289). Wrapping the closure in this +/// Equatable adapter keeps the focused value stable across no-op body runs. +/// +/// **Contract**: `token` must hash any captured state that affects the +/// closure's behavior. If the closure captures only stable references +/// (the store, projected `@State` bindings), `token` can stay `nil`. If it +/// captures a list of targets, an alert payload, etc., set `token` to a +/// hashable projection of those values so a real change triggers a republish. +struct FocusedAction: Equatable { + let isEnabled: Bool + let token: AnyHashable? + private let perform: (Input) -> Void + + init( + isEnabled: Bool, + token: AnyHashable? = nil, + perform: @escaping (Input) -> Void + ) { + self.isEnabled = isEnabled + self.token = token + self.perform = perform + } + + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.isEnabled == rhs.isEnabled && lhs.token == rhs.token + } + + func callAsFunction(_ input: Input) { + guard isEnabled else { return } + perform(input) + } +} + +extension FocusedAction where Input == Void { + func callAsFunction() { + callAsFunction(()) + } +} + +extension View { + /// Publishes a stable `FocusedAction` through `focusedSceneValue`. + /// Prefer this over a raw closure: AppKit only sees a "value changed" + /// event when `enabled` or `token` flip, instead of on every body run. + func focusedSceneAction( + _ keyPath: WritableKeyPath?>, + enabled: Bool, + token: AnyHashable? = nil, + perform: @escaping (Input) -> Void + ) -> some View { + focusedSceneValue( + keyPath, + FocusedAction(isEnabled: enabled, token: token, perform: perform) + ) + } + + /// `focusedValue` variant. Same contract as `focusedSceneAction`. + func focusedAction( + _ keyPath: WritableKeyPath?>, + enabled: Bool, + token: AnyHashable? = nil, + perform: @escaping (Input) -> Void + ) -> some View { + focusedValue( + keyPath, + FocusedAction(isEnabled: enabled, token: token, perform: perform) + ) + } +} diff --git a/supacode/Clients/Terminal/TerminalClient.swift b/supacode/Clients/Terminal/TerminalClient.swift index 01876bd33..2f7c022c5 100644 --- a/supacode/Clients/Terminal/TerminalClient.swift +++ b/supacode/Clients/Terminal/TerminalClient.swift @@ -55,6 +55,20 @@ struct TerminalClient { /// Per-worktree projection emitted when surfaces / task-running / unseen / notifications drift. /// Routed by the parent into the matching `SidebarItemFeature` via the row's id. case worktreeProjectionChanged(Worktree.ID, WorktreeRowProjection) + /// Per-tab projection emitted when a tab's surfaces, focused pane, or unread + /// count drifts. Routed into the matching `TerminalTabFeature.State` via tab id. + case tabProjectionChanged(worktreeID: Worktree.ID, WorktreeTabProjection) + /// A tab was destroyed in the worktree state. Parent removes the matching + /// `TerminalTabFeature.State` from `terminalTabs`. + case tabRemoved(worktreeID: Worktree.ID, tabID: TerminalTabID) + /// The entire `WorktreeTerminalState` was torn down (worktree pruned). + /// Parent drops any orphan `terminalTabs` entries and removed-tab FIFO + /// records owned by this worktree so a fresh re-attach starts clean. + case worktreeStateTornDown(worktreeID: Worktree.ID) + /// A tab's stripe-progress display flipped. Routed into the matching + /// `TerminalTabFeature.State.progressDisplay` so the stripe recolors. + case tabProgressDisplayChanged( + worktreeID: Worktree.ID, tabID: TerminalTabID, display: TerminalTabProgressDisplay?) /// Forwarded from the terminal manager when surfaces close (single or bulk). /// `AppFeature` translates this into `agentPresence(.surfaceClosed/surfacesClosed)`. case surfacesClosed(Set) diff --git a/supacode/Commands/SidebarCommands.swift b/supacode/Commands/SidebarCommands.swift index 3bd037f60..0a0b8ec38 100644 --- a/supacode/Commands/SidebarCommands.swift +++ b/supacode/Commands/SidebarCommands.swift @@ -76,13 +76,13 @@ struct SidebarCommands: Commands { } .appKeyboardShortcut(toggleLeftSidebar) .help("Toggle Left Sidebar (\(toggleLeftSidebar?.display ?? "none"))") - .disabled(toggleLeftSidebarAction == nil) + .disabled(toggleLeftSidebarAction?.isEnabled != true) Button("Reveal in Sidebar") { revealInSidebarAction?() } .appKeyboardShortcut(revealInSidebar) .help("Reveal in Sidebar (\(revealInSidebar?.display ?? "none"))") - .disabled(revealInSidebarAction == nil) + .disabled(revealInSidebarAction?.isEnabled != true) Section { Menu("Group Relevant Sidebar Rows") { Toggle("Group Pinned Rows", isOn: groupPinnedRowsToggle) @@ -96,20 +96,20 @@ struct SidebarCommands: Commands { } private struct ToggleLeftSidebarActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } private struct RevealInSidebarActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var toggleLeftSidebarAction: (() -> Void)? { + var toggleLeftSidebarAction: FocusedAction? { get { self[ToggleLeftSidebarActionKey.self] } set { self[ToggleLeftSidebarActionKey.self] = newValue } } - var revealInSidebarAction: (() -> Void)? { + var revealInSidebarAction: FocusedAction? { get { self[RevealInSidebarActionKey.self] } set { self[RevealInSidebarActionKey.self] = newValue } } diff --git a/supacode/Commands/TerminalCommands.swift b/supacode/Commands/TerminalCommands.swift index 79c916b63..f222ca334 100644 --- a/supacode/Commands/TerminalCommands.swift +++ b/supacode/Commands/TerminalCommands.swift @@ -17,7 +17,7 @@ struct TerminalCommands: Commands { newTerminalAction?() } .ghosttyKeyboardShortcut("new_tab", in: ghosttyShortcuts) - .disabled(newTerminalAction == nil) + .disabled(newTerminalAction?.isEnabled != true) Divider() @@ -26,7 +26,7 @@ struct TerminalCommands: Commands { splitTerminalAction?(direction) } .ghosttyKeyboardShortcut(direction.ghosttyBinding, in: ghosttyShortcuts) - .disabled(splitTerminalAction == nil) + .disabled(splitTerminalAction?.isEnabled != true) } } CommandGroup(after: .textEditing) { @@ -34,19 +34,19 @@ struct TerminalCommands: Commands { startSearchAction?() } .ghosttyKeyboardShortcut("start_search", in: ghosttyShortcuts) - .disabled(startSearchAction == nil) + .disabled(startSearchAction?.isEnabled != true) Button("Find Next") { navigateSearchNextAction?() } .ghosttyKeyboardShortcut("navigate_search:next", in: ghosttyShortcuts) - .disabled(navigateSearchNextAction == nil) + .disabled(navigateSearchNextAction?.isEnabled != true) Button("Find Previous") { navigateSearchPreviousAction?() } .ghosttyKeyboardShortcut("navigate_search:previous", in: ghosttyShortcuts) - .disabled(navigateSearchPreviousAction == nil) + .disabled(navigateSearchPreviousAction?.isEnabled != true) Divider() @@ -54,7 +54,7 @@ struct TerminalCommands: Commands { endSearchAction?() } .ghosttyKeyboardShortcut("end_search", in: ghosttyShortcuts) - .disabled(endSearchAction == nil) + .disabled(endSearchAction?.isEnabled != true) Divider() @@ -62,105 +62,105 @@ struct TerminalCommands: Commands { searchSelectionAction?() } .ghosttyKeyboardShortcut("search_selection", in: ghosttyShortcuts) - .disabled(searchSelectionAction == nil) + .disabled(searchSelectionAction?.isEnabled != true) } } } private struct NewTerminalActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var newTerminalAction: (() -> Void)? { + var newTerminalAction: FocusedAction? { get { self[NewTerminalActionKey.self] } set { self[NewTerminalActionKey.self] = newValue } } } private struct SplitTerminalActionKey: FocusedValueKey { - typealias Value = (TerminalSplitMenuDirection) -> Void + typealias Value = FocusedAction } extension FocusedValues { - var splitTerminalAction: ((TerminalSplitMenuDirection) -> Void)? { + var splitTerminalAction: FocusedAction? { get { self[SplitTerminalActionKey.self] } set { self[SplitTerminalActionKey.self] = newValue } } } private struct CloseSurfaceActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var closeSurfaceAction: (() -> Void)? { + var closeSurfaceAction: FocusedAction? { get { self[CloseSurfaceActionKey.self] } set { self[CloseSurfaceActionKey.self] = newValue } } } private struct CloseTabActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var closeTabAction: (() -> Void)? { + var closeTabAction: FocusedAction? { get { self[CloseTabActionKey.self] } set { self[CloseTabActionKey.self] = newValue } } } private struct StartSearchActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var startSearchAction: (() -> Void)? { + var startSearchAction: FocusedAction? { get { self[StartSearchActionKey.self] } set { self[StartSearchActionKey.self] = newValue } } } private struct SearchSelectionActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var searchSelectionAction: (() -> Void)? { + var searchSelectionAction: FocusedAction? { get { self[SearchSelectionActionKey.self] } set { self[SearchSelectionActionKey.self] = newValue } } } private struct NavigateSearchNextActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var navigateSearchNextAction: (() -> Void)? { + var navigateSearchNextAction: FocusedAction? { get { self[NavigateSearchNextActionKey.self] } set { self[NavigateSearchNextActionKey.self] = newValue } } } private struct NavigateSearchPreviousActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var navigateSearchPreviousAction: (() -> Void)? { + var navigateSearchPreviousAction: FocusedAction? { get { self[NavigateSearchPreviousActionKey.self] } set { self[NavigateSearchPreviousActionKey.self] = newValue } } } private struct EndSearchActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var endSearchAction: (() -> Void)? { + var endSearchAction: FocusedAction? { get { self[EndSearchActionKey.self] } set { self[EndSearchActionKey.self] = newValue } } diff --git a/supacode/Commands/WindowCommands.swift b/supacode/Commands/WindowCommands.swift index 14365725c..ee7c0083f 100644 --- a/supacode/Commands/WindowCommands.swift +++ b/supacode/Commands/WindowCommands.swift @@ -9,24 +9,25 @@ struct WindowCommands: Commands { let closeSurfaceHotkey = ghosttyShortcuts.keyboardShortcut(for: "close_surface") let isCloseSurfaceOverlapping = closeSurfaceHotkey?.key == "w" && closeSurfaceHotkey?.modifiers == .command + let closeSurfaceEnabled = closeSurfaceAction?.isEnabled == true CommandGroup(replacing: .saveItem) { Button("Close Terminal", systemImage: "xmark") { closeSurfaceAction?() } // Suppress the Ghostty shortcut when the close-surface action is unavailable so Close Window can claim ⌘W. - .keyboardShortcut(closeSurfaceAction == nil ? nil : ghosttyShortcuts.keyboardShortcut(for: "close_surface")) - .disabled(closeSurfaceAction == nil) + .keyboardShortcut(closeSurfaceEnabled ? ghosttyShortcuts.keyboardShortcut(for: "close_surface") : nil) + .disabled(!closeSurfaceEnabled) Button("Close Terminal Tab") { closeTabAction?() } .ghosttyKeyboardShortcut("close_tab", in: ghosttyShortcuts) - .disabled(closeTabAction == nil) + .disabled(closeTabAction?.isEnabled != true) Button("Close Window") { NSApplication.shared.keyWindow?.performClose(nil) } - .keyboardShortcut(!isCloseSurfaceOverlapping || closeSurfaceAction == nil ? .init("w") : nil) + .keyboardShortcut(!isCloseSurfaceOverlapping || !closeSurfaceEnabled ? .init("w") : nil) } } } diff --git a/supacode/Commands/WorktreeCommands.swift b/supacode/Commands/WorktreeCommands.swift index 5f3422893..71ed2a681 100644 --- a/supacode/Commands/WorktreeCommands.swift +++ b/supacode/Commands/WorktreeCommands.swift @@ -5,38 +5,49 @@ import SupacodeSettingsFeature import SupacodeSettingsShared import SwiftUI +#if DEBUG + private nonisolated let commandsRenderLogger = SupaLogger("DetailRender") +#endif + +/// Umbrella that wires the worktree-related menu-bar contributions. Each +/// child is its own `Commands` struct so SwiftUI re-renders only the one +/// whose observed inputs changed; e.g. the static Select Worktree submenu +/// never re-runs during agent storms even when the main menu's snapshot +/// fields tick. struct WorktreeCommands: Commands { @Bindable var store: StoreOf + + var body: some Commands { + WorktreeMainMenu(store: store) + WorktreeFileMenu(store: store) + } +} + +/// The "Worktrees" `CommandMenu`. Re-renders when the snapshot or any read +/// focused value changes; the inner Select-Worktree submenu is its own +/// struct so its static 10-item rendering doesn't churn with the rest. +private struct WorktreeMainMenu: Commands { + @Bindable var store: StoreOf @FocusedValue(\.openSelectedWorktreeAction) private var openSelectedWorktreeAction @FocusedValue(\.revealInFinderAction) private var revealInFinderAction @FocusedValue(\.openActionSelection) private var openActionSelection - @FocusedValue(\.confirmWorktreeAction) private var confirmWorktreeAction @FocusedValue(\.archiveWorktreeAction) private var archiveWorktreeAction @FocusedValue(\.deleteWorktreeAction) private var deleteWorktreeAction @FocusedValue(\.runScriptAction) private var runScriptAction @FocusedValue(\.stopRunScriptAction) private var stopRunScriptAction - @FocusedValue(\.visibleHotkeyWorktreeRows) private var visibleHotkeyWorktreeRows - - init(store: StoreOf) { - self.store = store - } var body: some Commands { - let overrides = store.settings.shortcutOverrides - let repositories = store.repositories - let orderedRows = visibleHotkeyWorktreeRows ?? repositories.hotkeyWorktreeSlots() - let pullRequestURL = selectedPullRequestURL - let githubIntegrationEnabled = store.settings.githubIntegrationEnabled + #if DEBUG + let _: Void = commandsRenderLogger.info("WorktreeMainMenu.body re-rendered") + #endif + let snapshot = store.worktreeMenuSnapshot + let overrides = snapshot.shortcutOverrides let selectNext = AppShortcuts.selectNextWorktree.effective(from: overrides) let selectPrevious = AppShortcuts.selectPreviousWorktree.effective(from: overrides) let historyBack = AppShortcuts.worktreeHistoryBack.effective(from: overrides) let historyForward = AppShortcuts.worktreeHistoryForward.effective(from: overrides) - let canGoBack = repositories.canNavigateWorktreeHistoryBackward - let canGoForward = repositories.canNavigateWorktreeHistoryForward let archive = AppShortcuts.archiveWorktree.effective(from: overrides) let deleteWt = AppShortcuts.deleteWorktree.effective(from: overrides) - let confirm = AppShortcuts.confirmWorktreeAction.effective(from: overrides) - let openRepo = AppShortcuts.openRepository.effective(from: overrides) let openWorktree = AppShortcuts.openWorktree.effective(from: overrides) let revealInFinder = AppShortcuts.revealInFinder.effective(from: overrides) let openPR = AppShortcuts.openPullRequest.effective(from: overrides) @@ -47,13 +58,12 @@ struct WorktreeCommands: Commands { let stop = AppShortcuts.stopRunScript.effective(from: overrides) let jumpToLatestUnread = AppShortcuts.jumpToLatestUnread.effective(from: overrides) CommandMenu("Worktrees") { - // Creation and opening. Button("New Worktree…", systemImage: "plus") { store.send(.repositories(.createRandomWorktree)) } .appKeyboardShortcut(newWt) .help("New Worktree (\(newWt?.display ?? "none"))") - .disabled(!repositories.canCreateWorktree) + .disabled(!snapshot.canCreateWorktree) Divider() let openLabel = openActionSelection.map { "Open in \($0.labelTitle)" } ?? "Open" Button(openLabel, systemImage: "arrow.up.right.square") { @@ -61,107 +71,130 @@ struct WorktreeCommands: Commands { } .appKeyboardShortcut(openWorktree) .help("\(openLabel) (\(openWorktree?.display ?? "none"))") - .disabled(openSelectedWorktreeAction == nil) + .disabled(openSelectedWorktreeAction?.isEnabled != true) Button("Reveal in Finder", systemImage: "folder") { revealInFinderAction?() } .appKeyboardShortcut(revealInFinder) .help("Reveal in Finder (\(revealInFinder?.display ?? "none"))") - .disabled(revealInFinderAction == nil) + .disabled(revealInFinderAction?.isEnabled != true) Button("Open Pull Request", systemImage: "arrow.up.forward") { - if let pullRequestURL { - NSWorkspace.shared.open(pullRequestURL) + if let url = snapshot.selectedPullRequestURL { + NSWorkspace.shared.open(url) } } .appKeyboardShortcut(openPR) .help("Open Pull Request (\(openPR?.display ?? "none"))") - .disabled(pullRequestURL == nil || !githubIntegrationEnabled) + .disabled(snapshot.selectedPullRequestURL == nil || !snapshot.githubIntegrationEnabled) Divider() - // Lifecycle. Button("Refresh Worktrees", systemImage: "arrow.clockwise") { store.send(.repositories(.refreshWorktrees)) } .appKeyboardShortcut(refresh) .help("Refresh (\(refresh?.display ?? "none"))") - .disabled(!repositories.isInitialLoadComplete) + .disabled(!snapshot.isInitialLoadComplete) Button("Archived Worktrees", systemImage: "archivebox") { store.send(.repositories(.selectArchivedWorktrees)) } .appKeyboardShortcut(archived) .help("Archived Worktrees (\(archived?.display ?? "none"))") - .disabled(!repositories.isInitialLoadComplete) + .disabled(!snapshot.isInitialLoadComplete) Divider() - // Commands. Button("Archive Worktree…", systemImage: "archivebox") { archiveWorktreeAction?() } .appKeyboardShortcut(archive) .help("Archive Worktree (\(archive?.display ?? "none"))") - .disabled(archiveWorktreeAction == nil) + .disabled(archiveWorktreeAction?.isEnabled != true) Button("Delete Worktree…", systemImage: "trash") { deleteWorktreeAction?() } .appKeyboardShortcut(deleteWt) .help("Delete Worktree (\(deleteWt?.display ?? "none"))") - .disabled(deleteWorktreeAction == nil) + .disabled(deleteWorktreeAction?.isEnabled != true) Divider() - // Scripts. Button("Run Script", systemImage: ScriptKind.run.defaultSystemImage) { runScriptAction?() } .appKeyboardShortcut(run) .help("Run Script (\(run?.display ?? "none"))") - .disabled(runScriptAction == nil) + .disabled(runScriptAction?.isEnabled != true) Button("Stop Script", systemImage: "stop") { stopRunScriptAction?() } .appKeyboardShortcut(stop) .help("Stop Script (\(stop?.display ?? "none"))") - .disabled(stopRunScriptAction == nil) + .disabled(stopRunScriptAction?.isEnabled != true) Button("Jump to Latest Unread", systemImage: "bell.badge") { store.send(.jumpToLatestUnread) } .appKeyboardShortcut(jumpToLatestUnread) .help("Jump to Latest Unread Notification (\(jumpToLatestUnread?.display ?? "none"))") - .disabled(store.notificationIndicatorCount == 0) + .disabled(snapshot.notificationIndicatorCount == 0) Divider() - // Navigation. + // Always-enabled; the reducer beeps when there's no worktree to move to. Button("Select Next", systemImage: "chevron.down") { store.send(.repositories(.selectNextWorktree)) } .appKeyboardShortcut(selectNext) .help("Select Next (\(selectNext?.display ?? "none"))") - .disabled(orderedRows.isEmpty) Button("Select Previous", systemImage: "chevron.up") { store.send(.repositories(.selectPreviousWorktree)) } .appKeyboardShortcut(selectPrevious) .help("Select Previous (\(selectPrevious?.display ?? "none"))") - .disabled(orderedRows.isEmpty) Button("Back in Worktree History", systemImage: "chevron.left") { store.send(.repositories(.worktreeHistoryBack)) } .appKeyboardShortcut(historyBack) .help("Back in Worktree History (\(historyBack?.display ?? "none"))") - .disabled(!canGoBack) + .disabled(!snapshot.canNavigateBackward) Button("Forward in Worktree History", systemImage: "chevron.right") { store.send(.repositories(.worktreeHistoryForward)) } .appKeyboardShortcut(historyForward) .help("Forward in Worktree History (\(historyForward?.display ?? "none"))") - .disabled(!canGoForward) - // Skip inactive slots so SwiftUI never emits a Button whose stale keyEquivalent misroutes keys. - let slots = AppShortcuts.activeWorktreeSelectionSlots( - overrides: overrides, - orderedRowsCount: orderedRows.count - ) - .map { WorktreeSelectionSlot(shortcut: $0.shortcut, row: orderedRows[$0.index]) } + .disabled(!snapshot.canNavigateForward) Menu("Select Worktree") { - ForEach(slots, id: \.shortcut.id) { slot in - WorktreeShortcutButton(slot: slot, store: store) - } + SelectWorktreeSubmenuItems(store: store, overrides: overrides) + } + } + } +} + +/// Static 10-item submenu. Labels and per-item actions never change, so the +/// menu bar doesn't rebuild during agent storms. Out-of-range slots beep at +/// fire time (handled in the reducer) so we don't need a disabled state here. +private struct SelectWorktreeSubmenuItems: View { + let store: StoreOf + let overrides: [AppShortcutID: AppShortcutOverride] + + var body: some View { + ForEach(0.. + @FocusedValue(\.confirmWorktreeAction) private var confirmWorktreeAction + + var body: some Commands { + #if DEBUG + let _: Void = commandsRenderLogger.info("WorktreeFileMenu.body re-rendered") + #endif + let overrides = store.worktreeMenuSnapshot.shortcutOverrides + let openRepo = AppShortcuts.openRepository.effective(from: overrides) + let confirm = AppShortcuts.confirmWorktreeAction.effective(from: overrides) CommandGroup(replacing: .newItem) { Button("Add Repository or Folder...", systemImage: "folder.badge.plus") { store.send(.repositories(.setOpenPanelPresented(true))) @@ -173,61 +206,31 @@ struct WorktreeCommands: Commands { } .appKeyboardShortcut(confirm) .help("Confirm Action (\(confirm?.display ?? "none"))") - .disabled(confirmWorktreeAction == nil) + .disabled(confirmWorktreeAction?.isEnabled != true) } } - - private var selectedPullRequestURL: URL? { - let repositories = store.repositories - guard let selectedWorktreeID = repositories.selectedWorktreeID else { return nil } - let pullRequest = repositories.sidebarItems[id: selectedWorktreeID]?.pullRequest - return pullRequest.flatMap { URL(string: $0.url) } - } - } -/// Stable projection published through `focusedSceneValue(\.visibleHotkeyWorktreeRows)`. -/// Carries only fields the menu actually needs so per-row PR / lifecycle ticks -/// dedupe and don't churn the menu bar (which rebuilds open submenus and drops hover). +/// Stable projection used by the sidebar's slot-to-row resolution. +/// `repositoryName` is carried for any UI that wants to render a repo-aware +/// label without re-pulling whole `repositories` substate observation. struct HotkeyWorktreeSlot: Equatable, Hashable, Identifiable, Sendable { let id: Worktree.ID let name: String let repositoryID: Repository.ID -} - -private struct WorktreeSelectionSlot { - let shortcut: AppShortcut - let row: HotkeyWorktreeSlot -} - -private struct WorktreeShortcutButton: View { - let slot: WorktreeSelectionSlot - let store: StoreOf - - private var title: String { - let repositoryName = store.repositories.repositoryName(for: slot.row.repositoryID) ?? "Repository" - return "\(repositoryName) · \(slot.row.name)" - } - - var body: some View { - Button(title) { - store.send(.repositories(.selectWorktree(slot.row.id))) - } - .appKeyboardShortcut(slot.shortcut) - .help("Switch to \(title) (\(slot.shortcut.display))") - } + let repositoryName: String } private struct ArchiveWorktreeActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } private struct OpenSelectedWorktreeActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } private struct RevealInFinderActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } private struct OpenActionSelectionKey: FocusedValueKey { @@ -235,20 +238,20 @@ private struct OpenActionSelectionKey: FocusedValueKey { } private struct DeleteWorktreeActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } private struct ConfirmWorktreeActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } extension FocusedValues { - var openSelectedWorktreeAction: (() -> Void)? { + var openSelectedWorktreeAction: FocusedAction? { get { self[OpenSelectedWorktreeActionKey.self] } set { self[OpenSelectedWorktreeActionKey.self] = newValue } } - var revealInFinderAction: (() -> Void)? { + var revealInFinderAction: FocusedAction? { get { self[RevealInFinderActionKey.self] } set { self[RevealInFinderActionKey.self] = newValue } } @@ -258,45 +261,36 @@ extension FocusedValues { set { self[OpenActionSelectionKey.self] = newValue } } - var confirmWorktreeAction: (() -> Void)? { + var confirmWorktreeAction: FocusedAction? { get { self[ConfirmWorktreeActionKey.self] } set { self[ConfirmWorktreeActionKey.self] = newValue } } - var archiveWorktreeAction: (() -> Void)? { + var archiveWorktreeAction: FocusedAction? { get { self[ArchiveWorktreeActionKey.self] } set { self[ArchiveWorktreeActionKey.self] = newValue } } - var deleteWorktreeAction: (() -> Void)? { + var deleteWorktreeAction: FocusedAction? { get { self[DeleteWorktreeActionKey.self] } set { self[DeleteWorktreeActionKey.self] = newValue } } - var runScriptAction: (() -> Void)? { + var runScriptAction: FocusedAction? { get { self[RunScriptActionKey.self] } set { self[RunScriptActionKey.self] = newValue } } - var stopRunScriptAction: (() -> Void)? { + var stopRunScriptAction: FocusedAction? { get { self[StopRunScriptActionKey.self] } set { self[StopRunScriptActionKey.self] = newValue } } - - var visibleHotkeyWorktreeRows: [HotkeyWorktreeSlot]? { - get { self[VisibleHotkeyWorktreeRowsKey.self] } - set { self[VisibleHotkeyWorktreeRowsKey.self] = newValue } - } } private struct RunScriptActionKey: FocusedValueKey { - typealias Value = () -> Void + typealias Value = FocusedAction } private struct StopRunScriptActionKey: FocusedValueKey { - typealias Value = () -> Void -} - -private struct VisibleHotkeyWorktreeRowsKey: FocusedValueKey { - typealias Value = [HotkeyWorktreeSlot] + typealias Value = FocusedAction } diff --git a/supacode/Features/App/Models/WorktreeMenuSnapshot.swift b/supacode/Features/App/Models/WorktreeMenuSnapshot.swift new file mode 100644 index 000000000..c1f0c5123 --- /dev/null +++ b/supacode/Features/App/Models/WorktreeMenuSnapshot.swift @@ -0,0 +1,132 @@ +import Foundation +import SupacodeSettingsFeature +import SupacodeSettingsShared + +#if DEBUG + private nonisolated let menuSnapshotLogger = SupaLogger("DetailRender") +#endif + +/// Frozen view of every primitive the `WorktreeCommands` menu-bar body reads. +/// `WorktreeCommands.body` observes only this single Equatable field; mutations +/// fire only when a value the menu actually displays changes. +struct WorktreeMenuSnapshot: Equatable { + var shortcutOverrides: [AppShortcutID: AppShortcutOverride] = [:] + var githubIntegrationEnabled: Bool = true + var canCreateWorktree: Bool = false + var canNavigateBackward: Bool = false + var canNavigateForward: Bool = false + var isInitialLoadComplete: Bool = false + var selectedPullRequestURL: URL? + var notificationIndicatorCount: Int = 0 +} + +extension AppFeature.State { + /// Compose the current snapshot from substate fields. Called from the + /// post-reduce hook on the root reducer; Equatable diff suppresses no-op + /// writes so SwiftUI only invalidates when something the menu reads changed. + func computeWorktreeMenuSnapshot() -> WorktreeMenuSnapshot { + let pullRequestURL = repositories.selectedWorktreeSlice?.pullRequest + .flatMap { URL(string: $0.url) } + return WorktreeMenuSnapshot( + shortcutOverrides: settings.shortcutOverrides, + githubIntegrationEnabled: settings.githubIntegrationEnabled, + canCreateWorktree: repositories.canCreateWorktree, + canNavigateBackward: repositories.canNavigateWorktreeHistoryBackward, + canNavigateForward: repositories.canNavigateWorktreeHistoryForward, + isInitialLoadComplete: repositories.isInitialLoadComplete, + selectedPullRequestURL: pullRequestURL, + notificationIndicatorCount: notificationIndicatorCount + ) + } + + mutating func recomputeWorktreeMenuSnapshotIfChanged() { + let new = computeWorktreeMenuSnapshot() + if new != worktreeMenuSnapshot { + #if DEBUG + diffSnapshotFields(old: worktreeMenuSnapshot, new: new) + #endif + worktreeMenuSnapshot = new + } + } + + #if DEBUG + private func diffSnapshotFields(old: WorktreeMenuSnapshot, new: WorktreeMenuSnapshot) { + var diffs: [String] = [] + if old.shortcutOverrides != new.shortcutOverrides { diffs.append("shortcutOverrides") } + if old.githubIntegrationEnabled != new.githubIntegrationEnabled { + diffs.append("githubIntegrationEnabled") + } + if old.canCreateWorktree != new.canCreateWorktree { diffs.append("canCreateWorktree") } + if old.canNavigateBackward != new.canNavigateBackward { + diffs.append("canNavigateBackward") + } + if old.canNavigateForward != new.canNavigateForward { + diffs.append("canNavigateForward") + } + if old.isInitialLoadComplete != new.isInitialLoadComplete { + diffs.append("isInitialLoadComplete") + } + if old.selectedPullRequestURL != new.selectedPullRequestURL { + diffs.append("selectedPullRequestURL") + } + if old.notificationIndicatorCount != new.notificationIndicatorCount { + diffs.append("notificationIndicatorCount") + } + menuSnapshotLogger.info("MenuSnapshot mutated. Fields: \(diffs.joined(separator: ", "))") + } + #endif +} + +extension AppFeature.Action { + /// Exhaustive gate for the `WorktreeMenuSnapshot` post-reduce recompute. + /// A `default` arm would silently classify any new action as "no recompute" + /// and risk a stale snapshot; the explicit switch forces classification at + /// compile time. The Equatable diff inside + /// `recomputeWorktreeMenuSnapshotIfChanged` still catches no-op recomputes; + /// this gate avoids the recompute itself on the action volumes #289 cares + /// about (agent-presence ticks, per-tab projection storms). + var affectsWorktreeMenuSnapshot: Bool { + switch self { + // Repository actions: the existing cache-invalidation map is the source + // of truth. Every snapshot input that lives on `repositories` (canCreate, + // canNavigate*, isInitialLoadComplete, selectedWorktreeSlice.pullRequest) + // changes via an action that already invalidates at least one cache. + case .repositories(let inner): + return !inner.cacheInvalidations.isEmpty + // Settings can change `shortcutOverrides` or `githubIntegrationEnabled`. + case .settings: + return true + // Only `notificationIndicatorChanged` writes the snapshot's count field. + case .terminalEvent(let event): + switch event { + case .notificationIndicatorChanged: + return true + case .notificationReceived, .tabCreated, .tabClosed, .focusChanged, + .taskStatusChanged, .blockingScriptCompleted, .commandPaletteToggleRequested, + .setupScriptConsumed, .worktreeProjectionChanged, .tabProjectionChanged, + .tabRemoved, .worktreeStateTornDown, .tabProgressDisplayChanged, + .surfacesClosed, .agentHookEventReceived: + return false + } + // Hot agent-storm paths: per-tab churn never mutates snapshot inputs. + // `.terminals` is safe because it owns only per-tab feature state; any + // change that DOES affect a snapshot input flows back through a separate + // `.terminalEvent.notificationIndicatorChanged` (counted above) or a + // `.repositories` cache invalidation (the cacheInvalidations gate above). + case .agentPresence, .terminals, .commandPalette, .updates: + return false + // Lifecycle / UI / effect-dispatch actions never write snapshot inputs + // directly; any downstream mutation flows back through a classified arm. + case .appLaunched, .scenePhaseChanged, .openActionSelectionChanged, + .worktreeSettingsLoaded, .openSelectedWorktree, .revealInFinder, + .openWorktree, .openWorktreeFailed, .requestQuit, .newTerminal, + .splitTerminal, .jumpToLatestUnread, .runScript, .runNamedScript, + .stopScript, .stopRunScripts, .closeTab, .closeSurface, + .startSearch, .searchSelection, .navigateSearchNext, + .navigateSearchPrevious, .endSearch, + .systemNotificationsPermissionFailed, .deeplinkReceived, + .deeplink, .deeplinkReferenceOpened, .alert, .deeplinkInputConfirmation: + return false + } + } +} diff --git a/supacode/Features/App/Reducer/AppFeature.swift b/supacode/Features/App/Reducer/AppFeature.swift index 255f89e61..71817caa0 100644 --- a/supacode/Features/App/Reducer/AppFeature.swift +++ b/supacode/Features/App/Reducer/AppFeature.swift @@ -1,6 +1,7 @@ import AppKit import ComposableArchitecture import Foundation +import OrderedCollections import PostHog import SupacodeSettingsFeature import SupacodeSettingsShared @@ -24,6 +25,10 @@ struct AppFeature { var settings: SettingsFeature.State var updates = UpdatesFeature.State() var commandPalette = CommandPaletteFeature.State() + /// Terminal-orchestration state. Owns the per-tab feature collection so + /// tab-bar views scope through `\.terminals` (narrow) instead of the full + /// app store. Mirrors sidebar's `RepositoriesFeature` ownership pattern. + var terminals = TerminalsFeature.State() var openActionSelection: OpenWorktreeAction = .finder var repoScripts: [ScriptDefinition] = [] var globalScripts: [ScriptDefinition] = [] @@ -32,6 +37,12 @@ struct AppFeature { var lastKnownAgentPresenceBadgesEnabled: Bool var pendingDeeplinks: [Deeplink] = [] var isDeeplinkReferenceRequested = false + /// Cached projection of every primitive the menu-bar `WorktreeCommands` + /// body reads. The menu observes ONE Equatable field instead of pulling + /// `\.repositories` / `\.settings` (whole-substate) observation through + /// `_modify`, which previously made every per-row mutation rebuild the + /// system menu and drop hover state (#289). + var worktreeMenuSnapshot: WorktreeMenuSnapshot = .init() @Presents var alert: AlertState? @Presents var deeplinkInputConfirmation: DeeplinkInputConfirmationFeature.State? @@ -48,6 +59,9 @@ struct AppFeature { // so deselection (line below in `selectedWorktreeChanged(nil)`) // intentionally does not clear them. globalScripts = settings.globalScripts + // Warm the cache so the first state mutation doesn't churn the snapshot + // and trip every TestStore expectation that omits a state-change closure. + worktreeMenuSnapshot = computeWorktreeMenuSnapshot() } /// Repo scripts followed by global scripts; repo wins on ID collisions. @@ -66,13 +80,11 @@ struct AppFeature { allScripts.primaryScript } - /// Running script IDs for the currently selected worktree. + /// Running script IDs for the currently selected worktree. Sourced from + /// the cached slice so an agent storm on the focused row doesn't pull + /// observation through `sidebarItems[id:]`. var runningScriptIDs: Set { - guard - let worktreeID = repositories.selectedWorktreeID, - let scripts = repositories.sidebarItems[id: worktreeID]?.runningScripts - else { return [] } - return Set(scripts.ids) + Set(repositories.selectedWorktreeSlice?.runningScripts.ids ?? []) } /// Whether any `.run`-kind script is currently running in the selected worktree. @@ -83,6 +95,7 @@ struct AppFeature { enum Action { case agentPresence(AgentPresenceFeature.Action) + case terminals(TerminalsFeature.Action) case appLaunched case scenePhaseChanged(ScenePhase) case repositories(RepositoriesFeature.Action) @@ -958,6 +971,23 @@ struct AppFeature { ) ) + case .terminalEvent(.tabProjectionChanged(let worktreeID, let projection)): + return .send(.terminals(.tabProjectionChanged(worktreeID: worktreeID, projection: projection))) + + case .terminalEvent(.tabRemoved(let worktreeID, let tabID)): + return .send(.terminals(.tabRemoved(worktreeID: worktreeID, tabID: tabID))) + + case .terminalEvent(.worktreeStateTornDown(let worktreeID)): + return .send(.terminals(.worktreeStateTornDown(worktreeID: worktreeID))) + + case .terminalEvent(.tabProgressDisplayChanged(_, let tabID, let display)): + return .send( + .terminals(.terminalTabs(.element(id: tabID, action: .progressDisplayChanged(display)))) + ) + + case .terminals: + return .none + case .terminalEvent(.surfacesClosed(let ids)): guard !ids.isEmpty else { return .none } if ids.count == 1, let id = ids.first { @@ -973,6 +1003,9 @@ struct AppFeature { } } core + Scope(state: \.terminals, action: \.terminals) { + TerminalsFeature() + } Scope(state: \.agentPresence, action: \.agentPresence) { AgentPresenceFeature() } @@ -991,6 +1024,16 @@ struct AppFeature { .ifLet(\.$deeplinkInputConfirmation, action: \.deeplinkInputConfirmation) { DeeplinkInputConfirmationFeature() } + Reduce { state, action in + // Cold-path gate. Without this, an agent storm fires + // `recomputeWorktreeMenuSnapshotIfChanged` hundreds of times per second + // (URL flatMap + 8-field Equatable diff each) only for the Equatable + // diff to find a no-op. The gate skips the recompute itself for + // actions that demonstrably can't change a snapshot input (#289). + guard action.affectsWorktreeMenuSnapshot else { return .none } + state.recomputeWorktreeMenuSnapshotIfChanged() + return .none + } } // MARK: - Agent presence fan-out. @@ -1035,17 +1078,32 @@ struct AppFeature { badgesEnabled: Bool ) -> Effect { let presence = state.agentPresence - let effects: [Effect] = rowIDs.compactMap { rowID in - guard let row = state.repositories.sidebarItems[id: rowID] else { return nil } + var effects: [Effect] = [] + var affectedSurfaces: Set = [] + for rowID in rowIDs { + guard let row = state.repositories.sidebarItems[id: rowID] else { continue } let agents = presence.agents(across: row.surfaceIDs, badgesEnabled: badgesEnabled) let hasActivity = presence.hasActivity(in: row.surfaceIDs) - return .send( - .repositories( - .sidebarItems( - .element(id: rowID, action: .agentSnapshotChanged(agents, hasActivity: hasActivity)) + effects.append( + .send( + .repositories( + .sidebarItems( + .element(id: rowID, action: .agentSnapshotChanged(agents, hasActivity: hasActivity)) + ) ) ) ) + affectedSurfaces.formUnion(row.surfaceIDs) + } + // Per-tab fanout: any tab containing an affected surface re-projects its + // agent snapshot. Tab leaves observe `state.agents` directly so per-tab + // mutations don't invalidate sibling tab leaves. + for tab in state.terminals.terminalTabs + where tab.surfaceIDs.contains(where: affectedSurfaces.contains) { + let agents = presence.agents(across: tab.surfaceIDs, badgesEnabled: badgesEnabled) + effects.append( + .send(.terminals(.terminalTabs(.element(id: tab.id, action: .agentSnapshotChanged(agents))))) + ) } return .merge(effects) } diff --git a/supacode/Features/Repositories/BusinessLogic/SidebarStructure.swift b/supacode/Features/Repositories/BusinessLogic/SidebarStructure.swift index 6d594c9d0..8807d6d9c 100644 --- a/supacode/Features/Repositories/BusinessLogic/SidebarStructure.swift +++ b/supacode/Features/Repositories/BusinessLogic/SidebarStructure.swift @@ -257,72 +257,181 @@ extension RepositoriesFeature.State { sidebarStructure = new } } + + /// Refreshes the cached `selectedWorktreeSlice` from the focused row, using + /// an Equatable diff so observation only invalidates on a real change. + /// Mirrors `recomputeSidebarStructureIfChanged()` for slice-affecting + /// actions; per-leaf reads on `sidebarItems[id:]` happen here, not in views. + mutating func recomputeSelectedWorktreeSliceIfChanged() { + let new = selectedRow(for: selectedWorktreeID).map { SelectedWorktreeSlice($0) } + if new != selectedWorktreeSlice { + selectedWorktreeSlice = new + } + } + + /// Equatable-diffs the toolbar notification snapshot against the cache so a + /// per-row notification append only invalidates SwiftUI when the toolbar + /// projection actually changes. + mutating func recomputeToolbarNotificationGroupsIfChanged() { + let new = computeToolbarNotificationGroups() + if new != toolbarNotificationGroupsCache { + toolbarNotificationGroupsCache = new + } + } +} + +/// Per-cache invalidation flag set returned by every reducer action. Exhaustive +/// switches over the action enums force every new case to declare which +/// post-reduce caches it touches; a missing case is a compile error rather +/// than a silent "skip the recompute". +struct CacheInvalidations: OptionSet { + let rawValue: UInt8 + static let sidebarStructure = CacheInvalidations(rawValue: 1 << 0) + static let selectedWorktreeSlice = CacheInvalidations(rawValue: 1 << 1) + static let toolbarNotificationGroups = CacheInvalidations(rawValue: 1 << 2) + static let all: CacheInvalidations = [ + .sidebarStructure, .selectedWorktreeSlice, .toolbarNotificationGroups, + ] +} + +extension SidebarItemFeature.Action { + var cacheInvalidations: CacheInvalidations { + switch self { + case .lifecycleChanged, .runningScriptStarted, .runningScriptStopped: + return [.sidebarStructure, .selectedWorktreeSlice] + case .agentSnapshotChanged: + return .sidebarStructure + case .terminalProjectionChanged: + return [.sidebarStructure, .toolbarNotificationGroups] + case .pullRequestChanged: + return .selectedWorktreeSlice + case .diffStatsChanged, .pullRequestQueryStarted, + .shortcutHintChanged, .dragSessionChanged, + .focusTerminalRequested, .focusTerminalConsumed: + return [] + } + } } extension RepositoriesFeature.Action { - /// Coarse predicate naming the actions whose handlers touch - /// `sidebarItems` / `sidebar` buckets / `repositories` / `expandedRepositoryIDs` - /// or any other input `SidebarStructure` reads from. Actions absent here - /// skip the post-reduce recompute entirely (user requirement: don't - /// rebuild on actions that can't affect the visible sidebar). - var affectsSidebarStructure: Bool { + /// Exhaustive cache-invalidation map. Update this alongside every new + /// `RepositoriesFeature.Action` case. Adding a case without listing it here + /// is a compile error (no `default`), so we never silently regress the + /// "post-reduce skips the recompute" path. + var cacheInvalidations: CacheInvalidations { switch self { - // Only the per-leaf actions that mutate fields the structure reads. case .sidebarItems(.element(id: _, action: let inner)): - return inner.affectsSidebarStructure - case .sidebarGroupingTogglesChanged, .sidebarNestByBranchChanged: - return true + return inner.cacheInvalidations + case .sidebarItems: + return [] + + // Sidebar layout toggles only. + case .sidebarGroupingTogglesChanged, .sidebarNestByBranchChanged, + .repositoryExpansionChanged, .branchNestExpansionChanged, + .repositoriesMoved, .pinnedWorktreesMoved, .unpinnedWorktreesMoved, + .worktreeNotificationReceived, .worktreeLineChangesLoaded, + .consumeTerminalFocus: + return .sidebarStructure + + // Bulk repository / worktree set changes that touch all caches. case .repositoriesLoaded, .openRepositoriesFinished, .repositoryRemovalCompleted, .repositoriesRemoved, - .removeFailedRepository: - return true - case .repositoryExpansionChanged, .branchNestExpansionChanged, - .repositoriesMoved, .pinnedWorktreesMoved, .unpinnedWorktreesMoved: - return true - case .pinWorktree, .unpinWorktree: - return true - case .archiveWorktreeApply, .unarchiveWorktree, + .removeFailedRepository, + .archiveWorktreeApply, .unarchiveWorktree, .deleteWorktreeApply, .worktreeDeleted, .createWorktreeInRepository, .createRandomWorktreeInRepository, - .createRandomWorktreeSucceeded, .createRandomWorktreeFailed, + .autoDeleteExpiredArchivedWorktrees: + return .all + + // `worktreeInfoEvent` is a pure effect-launcher (HEAD watcher tick): the + // arm only spawns `.run { ... await send(.branchNameLoaded(...)) }` etc. + // and never mutates `state`. The downstream `.worktreeBranchNameLoaded` / + // `.repositoryPullRequestsLoaded` arms declare their own invalidations. + case .worktreeInfoEvent: + return [] + + // `worktreeBranchNameLoaded` mutates `worktree.name` via `updateWorktreeName`, + // which feeds `computeToolbarNotificationGroups()` (notification group title). + // Without `.toolbarNotificationGroups` the popover would show the old name + // until an unrelated bulk action recomputed the cache. + case .worktreeBranchNameLoaded: + return .all + + // Layout + slice but not the notification snapshot (no notification touch). + case .createRandomWorktreeSucceeded, .createRandomWorktreeFailed, .pendingWorktreeProgressUpdated, .archiveScriptCompleted, .deleteScriptCompleted, .scriptCompleted, - .consumeSetupScript, .consumeTerminalFocus, - .autoDeleteExpiredArchivedWorktrees: - return true - case .worktreeBranchNameLoaded, .worktreeLineChangesLoaded, - .worktreeNotificationReceived, .worktreeInfoEvent, + .consumeSetupScript, + .pinWorktree, .unpinWorktree, .repositoryPullRequestsLoaded: - return true - // Repo customization save mutates `sidebar.sections[id].title/color`, - // which the highlight-row tag and per-repo color cache both read. + return [.sidebarStructure, .selectedWorktreeSlice] + + // Selection changes only refresh the slice. + case .selectionChanged, .selectWorktree, .selectArchivedWorktrees, + .selectNextWorktree, .selectPreviousWorktree, .selectWorktreeAtHotkeySlot, + .worktreeHistoryBack, .worktreeHistoryForward: + return .selectedWorktreeSlice + + // Repo customization save mutates the section title / color, which flow + // into the sidebar layout's highlight tag and the notification group name. case .repositoryCustomization(.presented(.delegate(.save))): - return true - default: - return false + return .all + case .repositoryCustomization: + return [] + + // Everything else is UI / effects / transient state, no cache touched. + case .task, .setOpenPanelPresented, .loadPersistedRepositories, + .refreshWorktrees, .reloadRepositories, + .setSidebarSelectedWorktreeIDs, + .openRepositories, + .revealSelectedWorktreeInSidebar, .consumePendingSidebarReveal, + .createRandomWorktree, + .promptedWorktreeCreationDataLoaded, .startPromptedWorktreeCreation, + .promptedWorktreeCreationChecked, + .requestArchiveWorktree, .requestArchiveWorktrees, + .archiveWorktreeConfirmed, + .requestDeleteSidebarItems, .deleteSidebarItemConfirmed, + .deleteWorktreeFailed, + .requestDeleteRepository, + .presentAlert, + .refreshGithubIntegrationAvailability, + .githubIntegrationAvailabilityUpdated, + .repositoryPullRequestRefreshCompleted, + .setGithubIntegrationEnabled, + .setMergedWorktreeAction, + .setAutoDeleteArchivedWorktreesAfterDays, + .setMoveNotifiedWorktreeToTop, + .pullRequestAction, + .showToast, .dismissToast, + .delayedPullRequestRefresh, + .openRepositorySettings, .requestCustomizeRepository, + .contextMenuOpenWorktree, + .worktreeCreationPrompt, + .alert, + .delegate: + return [] } } } -extension SidebarItemFeature.Action { - /// Subset of per-leaf actions that mutate fields `SidebarStructure` reads - /// (`lifecycle`, `runningScripts`, `agents`, `hasUnseenNotifications`). - /// Display-only mutations (diff stats, PR refresh, drag/focus/hint flags) - /// don't trigger a recompute. - var affectsSidebarStructure: Bool { - switch self { - case .lifecycleChanged, .runningScriptStarted, .runningScriptStopped, - .agentSnapshotChanged, .terminalProjectionChanged: - return true - case .diffStatsChanged, .pullRequestQueryStarted, .pullRequestChanged, - .shortcutHintChanged, .dragSessionChanged, - .focusTerminalRequested, .focusTerminalConsumed: - return false +extension RepositoriesFeature.State { + /// Single source of truth for the post-reduce cache recompute. The + /// production hook in `RepositoriesFeature.body` and the test mirror in + /// `RepositoriesSidebarTestHelpers` both call this so a fourth cache lands + /// in one place instead of needing two coordinated updates. + @MainActor + mutating func applyCacheRecomputes(_ invalidations: CacheInvalidations) { + if invalidations.contains(.sidebarStructure) { + recomputeSidebarStructureIfChanged() + } + if invalidations.contains(.selectedWorktreeSlice) { + recomputeSelectedWorktreeSliceIfChanged() + } + if invalidations.contains(.toolbarNotificationGroups) { + recomputeToolbarNotificationGroupsIfChanged() } } -} -extension RepositoriesFeature.State { /// Pinned worktree IDs across every repository in the user's repo order. /// Git main worktrees are excluded (they belong to the per-repo main slot, /// not the user-curated pinned list). Folders seed into `.unpinned` by diff --git a/supacode/Features/Repositories/Models/ToolbarNotificationGroup.swift b/supacode/Features/Repositories/Models/ToolbarNotificationGroup.swift index e0258cafd..4ac274908 100644 --- a/supacode/Features/Repositories/Models/ToolbarNotificationGroup.swift +++ b/supacode/Features/Repositories/Models/ToolbarNotificationGroup.swift @@ -1,4 +1,5 @@ import Foundation +import IdentifiedCollections struct ToolbarNotificationRepositoryGroup: Identifiable, Equatable { let id: Repository.ID @@ -26,9 +27,11 @@ struct ToolbarNotificationWorktreeGroup: Identifiable, Equatable { } extension RepositoriesFeature.State { - func toolbarNotificationGroups( - terminalManager: WorktreeTerminalManager - ) -> [ToolbarNotificationRepositoryGroup] { + /// Reads notification data off the per-row `SidebarItemFeature.State` + /// (populated via `terminalProjectionChanged`) instead of the live + /// `WorktreeTerminalManager`, so this is a pure reducer-state computation. + /// Cached on `toolbarNotificationGroupsCache`; views read the cache. + func computeToolbarNotificationGroups() -> [ToolbarNotificationRepositoryGroup] { let repositoriesByID = Dictionary(uniqueKeysWithValues: repositories.map { ($0.id, $0) }) var groups: [ToolbarNotificationRepositoryGroup] = [] @@ -39,14 +42,14 @@ extension RepositoriesFeature.State { let worktreeGroups: [ToolbarNotificationWorktreeGroup] = orderedWorktrees(in: repository).compactMap { worktree -> ToolbarNotificationWorktreeGroup? in - guard let state = terminalManager.stateIfExists(for: worktree.id), !state.notifications.isEmpty else { + guard let row = sidebarItems[id: worktree.id], !row.notifications.isEmpty else { return nil } return ToolbarNotificationWorktreeGroup( id: worktree.id, name: worktree.name, - notifications: state.notifications, - hasUnseenNotifications: terminalManager.hasUnseenNotifications(for: worktree.id) + notifications: Array(row.notifications), + hasUnseenNotifications: row.hasUnseenNotifications ) } diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature+Removal.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature+Removal.swift index d7fe0c85f..8effdbe8f 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature+Removal.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature+Removal.swift @@ -27,7 +27,7 @@ extension RepositoriesFeature { /// optionally its branch) and is per-worktree; the other three /// are repo-level and drop the whole section from Supacode, with /// `.folderTrash` additionally moving the folder to the Trash. - enum DeleteDisposition: Equatable, Sendable { + enum DeleteDisposition: Hashable, Sendable { case gitWorktreeDelete case gitRepositoryUnlink case folderUnlink diff --git a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift index 9631dc1a5..bd3b7b5dd 100644 --- a/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift +++ b/supacode/Features/Repositories/Reducer/RepositoriesFeature.swift @@ -154,6 +154,16 @@ struct RepositoriesFeature { /// in the recompute helper keeps a no-op rebuild from invalidating /// SwiftUI when the user-visible layout didn't actually change. var sidebarStructure: SidebarStructure = .placeholder + /// Cached projection of the focused row's display fields. The detail body + /// reads this directly instead of `sidebarItems[id: id]` so per-leaf agent + /// / notification mutations on the focused row don't invalidate the + /// detail tree. Recomputed via `recomputeSelectedWorktreeSliceIfChanged()`. + var selectedWorktreeSlice: SelectedWorktreeSlice? + /// Cached toolbar notification snapshot. Detail body reads this instead of + /// iterating `sidebarItems` (which would observe every per-row notification + /// mutation across all worktrees). Recomputed via + /// `recomputeToolbarNotificationGroupsIfChanged()`. + var toolbarNotificationGroupsCache: [ToolbarNotificationRepositoryGroup] = [] @Presents var worktreeCreationPrompt: WorktreeCreationPromptFeature.State? @Presents var repositoryCustomization: RepositoryCustomizationFeature.State? @Presents var alert: AlertState? @@ -245,6 +255,7 @@ struct RepositoriesFeature { roots: [URL] ) case selectWorktree(Worktree.ID?, focusTerminal: Bool = false) + case selectWorktreeAtHotkeySlot(Int) case selectNextWorktree case selectPreviousWorktree case worktreeHistoryBack @@ -377,12 +388,12 @@ struct RepositoriesFeature { let message: String } - struct DeleteWorktreeTarget: Equatable { + struct DeleteWorktreeTarget: Hashable { let worktreeID: Worktree.ID let repositoryID: Repository.ID } - struct ArchiveWorktreeTarget: Equatable { + struct ArchiveWorktreeTarget: Hashable { let worktreeID: Worktree.ID let repositoryID: Repository.ID } @@ -396,7 +407,7 @@ struct RepositoriesFeature { case success(String) } - enum Alert: Equatable { + enum Alert: Hashable { case confirmArchiveWorktree(Worktree.ID, Repository.ID) case confirmArchiveWorktrees([ArchiveWorktreeTarget]) case confirmDeleteSidebarItems([DeleteWorktreeTarget], disposition: DeleteDisposition) @@ -714,12 +725,26 @@ struct RepositoriesFeature { } return .merge(effects) + case .selectWorktreeAtHotkeySlot(let index): + // Snapshot-driven menu items capture only the slot index, so the + // current `hotkeySlots` lookup happens here at action time. Out-of-range + // slots beep so the user gets feedback that the shortcut hit nothing. + let slots = state.sidebarStructure.hotkeySlots + guard slots.indices.contains(index) else { + return .run { _ in NSSound.beep() } + } + return .send(.selectWorktree(slots[index].id)) + case .selectNextWorktree: - guard let id = state.worktreeID(byOffset: 1) else { return .none } + guard let id = state.worktreeID(byOffset: 1) else { + return .run { _ in NSSound.beep() } + } return .send(.selectWorktree(id)) case .selectPreviousWorktree: - guard let id = state.worktreeID(byOffset: -1) else { return .none } + guard let id = state.worktreeID(byOffset: -1) else { + return .run { _ in NSSound.beep() } + } return .send(.selectWorktree(id)) case .worktreeHistoryBack: @@ -3302,9 +3327,8 @@ struct RepositoriesFeature { // `withDependencies`. Reduce { state, action in @Dependency(\.sidebarStructureAutoRecompute) var autoRecompute - if autoRecompute, action.affectsSidebarStructure { - state.recomputeSidebarStructureIfChanged() - } + guard autoRecompute else { return .none } + state.applyCacheRecomputes(action.cacheInvalidations) return .none } } @@ -4174,9 +4198,15 @@ extension RepositoriesFeature.State { /// has composed an order the reducer can't derive on its own (e.g. highlight /// sections hoisted above per-repo rows). func hotkeyWorktreeSlots(for ids: [Worktree.ID]) -> [HotkeyWorktreeSlot] { - ids.compactMap { id in + let nameByRepoID = Dictionary(uniqueKeysWithValues: repositories.map { ($0.id, $0.name) }) + return ids.compactMap { id in guard let item = sidebarItems[id: id] else { return nil } - return HotkeyWorktreeSlot(id: item.id, name: item.name, repositoryID: item.repositoryID) + return HotkeyWorktreeSlot( + id: item.id, + name: item.name, + repositoryID: item.repositoryID, + repositoryName: nameByRepoID[item.repositoryID] ?? "" + ) } } } diff --git a/supacode/Features/Repositories/Reducer/SidebarItemFeature.swift b/supacode/Features/Repositories/Reducer/SidebarItemFeature.swift index bdf14ccf4..70f707d23 100644 --- a/supacode/Features/Repositories/Reducer/SidebarItemFeature.swift +++ b/supacode/Features/Repositories/Reducer/SidebarItemFeature.swift @@ -8,6 +8,12 @@ enum WorktreeAccent: Hashable, Sendable { case main case pinned + static func derive(isMainWorktree: Bool, isPinned: Bool) -> WorktreeAccent { + if isMainWorktree { return .main } + if isPinned { return .pinned } + return .default + } + func shapeStyle(emphasized: Bool) -> AnyShapeStyle { guard !emphasized else { return AnyShapeStyle(.secondary) } return switch self { @@ -202,6 +208,26 @@ extension SidebarItemFeature.State { /// Cascade: nil for main worktrees, then the row id's last path component, /// then the subtitle's last path component, then `branchName`. var sidebarDisplayName: String? { + SidebarDisplayName.compute( + isMainWorktree: isMainWorktree, id: id, subtitle: subtitle, branchName: branchName + ) + } + var accent: WorktreeAccent { WorktreeAccent.derive(isMainWorktree: isMainWorktree, isPinned: isPinned) } + /// True iff any tracked agent on this row is awaiting user input. + /// Drives the Active section's classification ("agent awaiting input"). + var hasAgentAwaitingInput: Bool { agents.contains(where: \.awaitingInput) } +} + +/// Shared cascade used by both `SidebarItemFeature.State` (row) and +/// `SelectedWorktreeSlice` (cached projection). Centralising here keeps the +/// row and the detail title in lock-step; an edge case fix lands once. +enum SidebarDisplayName { + static func compute( + isMainWorktree: Bool, + id: SidebarItemID, + subtitle: String?, + branchName: String + ) -> String? { guard !isMainWorktree else { return nil } if id.contains("/") { let pathName = URL(fileURLWithPath: id).lastPathComponent @@ -213,14 +239,6 @@ extension SidebarItemFeature.State { } return branchName } - var accent: WorktreeAccent { - if isMainWorktree { return .main } - if isPinned { return .pinned } - return .default - } - /// True iff any tracked agent on this row is awaiting user input. - /// Drives the Active section's classification ("agent awaiting input"). - var hasAgentAwaitingInput: Bool { agents.contains(where: \.awaitingInput) } } extension SidebarItemFeature.State.Lifecycle { @@ -239,3 +257,45 @@ struct WorktreeRowProjection: Equatable, Sendable { let hasUnseenNotifications: Bool let notifications: IdentifiedArrayOf } + +/// Value-typed projection of the focused row's display fields, cached on +/// `RepositoriesFeature.State.selectedWorktreeSlice`. Excludes `agents` / +/// `hasAgentActivity` / `surfaceIDs` / `notifications` so per-leaf storms on +/// the focused row don't invalidate the detail body's observation surface. +struct SelectedWorktreeSlice: Equatable, Sendable { + let id: SidebarItemID + let repositoryID: Repository.ID + let kind: SidebarItemFeature.State.Kind + let name: String + let branchName: String + let subtitle: String? + let isMainWorktree: Bool + let isPinned: Bool + let lifecycle: SidebarItemFeature.State.Lifecycle + let pullRequest: GithubPullRequest? + let runningScripts: IdentifiedArrayOf + + init(_ row: SidebarItemFeature.State) { + self.id = row.id + self.repositoryID = row.repositoryID + self.kind = row.kind + self.name = row.name + self.branchName = row.branchName + self.subtitle = row.subtitle + self.isMainWorktree = row.isMainWorktree + self.isPinned = row.isPinned + self.lifecycle = row.lifecycle + self.pullRequest = row.pullRequest + self.runningScripts = row.runningScripts + } + + var sidebarDisplayName: String? { + SidebarDisplayName.compute( + isMainWorktree: isMainWorktree, id: id, subtitle: subtitle, branchName: branchName + ) + } + + var accent: WorktreeAccent { WorktreeAccent.derive(isMainWorktree: isMainWorktree, isPinned: isPinned) } + + var isFolder: Bool { kind == .folder } +} diff --git a/supacode/Features/Repositories/Views/ArchivedWorktreesDetailView.swift b/supacode/Features/Repositories/Views/ArchivedWorktreesDetailView.swift index 6edbd947c..356adff31 100644 --- a/supacode/Features/Repositories/Views/ArchivedWorktreesDetailView.swift +++ b/supacode/Features/Repositories/Views/ArchivedWorktreesDetailView.swift @@ -27,18 +27,7 @@ struct ArchivedWorktreesDetailView: View { repositoryID: repositoryID ) } - let deleteWorktreeAction: (() -> Void)? = { - guard !selectedTargets.isEmpty else { return nil } - return { - store.send(.requestDeleteSidebarItems(selectedTargets)) - } - }() - let confirmWorktreeAction: (() -> Void)? = { - guard let alert = store.state.confirmWorktreeAlert else { return nil } - return { - store.send(.alert(.presented(alert))) - } - }() + let confirmAlert = store.state.confirmWorktreeAlert if groups.isEmpty { ContentUnavailableView( "Archived Worktrees", @@ -94,15 +83,30 @@ struct ArchivedWorktreesDetailView: View { selectedArchivedWorktreeIDs = selectedArchivedWorktreeIDs.intersection(newValue) } .animation(.easeOut(duration: 0.2), value: archivedRowIDs) - .focusedValue(\.deleteWorktreeAction, deleteWorktreeAction) - .focusedSceneValue(\.confirmWorktreeAction, confirmWorktreeAction) + .focusedAction( + \.deleteWorktreeAction, + enabled: !selectedTargets.isEmpty, + token: selectedTargets + ) { + store.send(.requestDeleteSidebarItems(selectedTargets)) + } + .focusedSceneAction( + \.confirmWorktreeAction, + enabled: confirmAlert != nil, + token: confirmAlert + ) { + if let alert = confirmAlert { + store.send(.alert(.presented(alert))) + } + } .toolbar { let deleteShortcut = KeyboardShortcut(.delete, modifiers: [.command, .shift]).display Button("Delete Selected", systemImage: "trash", role: .destructive) { - deleteWorktreeAction?() + guard !selectedTargets.isEmpty else { return } + store.send(.requestDeleteSidebarItems(selectedTargets)) } .help("Delete Selected (\(deleteShortcut))") - .disabled(deleteWorktreeAction == nil) + .disabled(selectedTargets.isEmpty) } } } diff --git a/supacode/Features/Repositories/Views/SidebarItemsView.swift b/supacode/Features/Repositories/Views/SidebarItemsView.swift index 3f5bef96e..227ac4dd6 100644 --- a/supacode/Features/Repositories/Views/SidebarItemsView.swift +++ b/supacode/Features/Repositories/Views/SidebarItemsView.swift @@ -507,11 +507,11 @@ private struct SidebarItemBody: View { .environment(\.focusNotificationAction) { notification in guard let terminalState = terminalManager.stateIfExists(for: rowID) else { notificationLogger.warning( - "No terminal state for worktree \(rowID) when focusing notification \(notification.surfaceId).") + "No terminal state for worktree \(rowID) when focusing notification \(notification.surfaceID).") return } - if !terminalState.focusSurface(id: notification.surfaceId) { - notificationLogger.warning("Failed to focus surface \(notification.surfaceId) for worktree \(rowID).") + if !terminalState.focusSurface(id: notification.surfaceID) { + notificationLogger.warning("Failed to focus surface \(notification.surfaceID) for worktree \(rowID).") } } .tag(SidebarSelection.worktree(rowID)) diff --git a/supacode/Features/Repositories/Views/SidebarListView.swift b/supacode/Features/Repositories/Views/SidebarListView.swift index cb52b0a9f..079cbc608 100644 --- a/supacode/Features/Repositories/Views/SidebarListView.swift +++ b/supacode/Features/Repositories/Views/SidebarListView.swift @@ -68,7 +68,6 @@ struct SidebarListView: View { .listStyle(.sidebar) .focused($isSidebarFocused) .frame(minWidth: 220) - .focusedSceneValue(\.visibleHotkeyWorktreeRows, structure.hotkeySlots) .onChange(of: groupPinnedRows, initial: false) { _, _ in store.send(.sidebarGroupingTogglesChanged) } diff --git a/supacode/Features/Repositories/Views/SidebarView.swift b/supacode/Features/Repositories/Views/SidebarView.swift index 74471d3e8..b5311da3b 100644 --- a/supacode/Features/Repositories/Views/SidebarView.swift +++ b/supacode/Features/Repositories/Views/SidebarView.swift @@ -11,9 +11,25 @@ struct SidebarView: View { var body: some View { let state = store.state let effectiveSelectedRows = state.effectiveSidebarSelectedRows - let confirmWorktreeAction = makeConfirmWorktreeAction(state: state) - let archiveWorktreeAction = makeArchiveWorktreeAction(rows: effectiveSelectedRows) - let deleteWorktreeAction = makeDeleteWorktreeAction(rows: effectiveSelectedRows) + let confirmAlert = state.confirmWorktreeAlert + let archiveTargets = + effectiveSelectedRows + .filter { $0.lifecycle == .idle && !$0.isMainWorktree } + .map { + RepositoriesFeature.ArchiveWorktreeTarget( + worktreeID: $0.id, + repositoryID: $0.repositoryID + ) + } + let deleteTargets = + effectiveSelectedRows + .filter { $0.lifecycle == .idle } + .map { + RepositoriesFeature.DeleteWorktreeTarget( + worktreeID: $0.id, + repositoryID: $0.repositoryID + ) + } let openRepo = AppShortcuts.openRepository.effective(from: settingsFile.global.shortcutOverrides) return SidebarListView( @@ -37,57 +53,32 @@ struct SidebarView: View { .help("Add Repository or Folder (\(openRepo?.display ?? "none"))") } } - .focusedSceneValue(\.confirmWorktreeAction, confirmWorktreeAction) - .focusedValue(\.archiveWorktreeAction, archiveWorktreeAction) - .focusedValue(\.deleteWorktreeAction, deleteWorktreeAction) - } - - private func makeConfirmWorktreeAction( - state: RepositoriesFeature.State - ) -> (() -> Void)? { - guard let alert = state.confirmWorktreeAlert else { return nil } - return { - store.send(.alert(.presented(alert))) - } - } - - private func makeArchiveWorktreeAction( - rows: [SidebarItemFeature.State] - ) -> (() -> Void)? { - let targets = - rows - .filter { $0.lifecycle == .idle && !$0.isMainWorktree } - .map { - RepositoriesFeature.ArchiveWorktreeTarget( - worktreeID: $0.id, - repositoryID: $0.repositoryID - ) + .focusedSceneAction( + \.confirmWorktreeAction, + enabled: confirmAlert != nil, + token: confirmAlert + ) { + if let alert = confirmAlert { + store.send(.alert(.presented(alert))) } - guard !targets.isEmpty else { return nil } - return { - if targets.count == 1, let target = targets.first { + } + .focusedAction( + \.archiveWorktreeAction, + enabled: !archiveTargets.isEmpty, + token: archiveTargets + ) { + if archiveTargets.count == 1, let target = archiveTargets.first { store.send(.requestArchiveWorktree(target.worktreeID, target.repositoryID)) } else { - store.send(.requestArchiveWorktrees(targets)) + store.send(.requestArchiveWorktrees(archiveTargets)) } } - } - - private func makeDeleteWorktreeAction( - rows: [SidebarItemFeature.State] - ) -> (() -> Void)? { - let targets = - rows - .filter { $0.lifecycle == .idle } - .map { - RepositoriesFeature.DeleteWorktreeTarget( - worktreeID: $0.id, - repositoryID: $0.repositoryID - ) - } - guard !targets.isEmpty else { return nil } - return { - store.send(.requestDeleteSidebarItems(targets)) + .focusedAction( + \.deleteWorktreeAction, + enabled: !deleteTargets.isEmpty, + token: deleteTargets + ) { + store.send(.requestDeleteSidebarItems(deleteTargets)) } } } diff --git a/supacode/Features/Repositories/Views/WorktreeDetailTitleView.swift b/supacode/Features/Repositories/Views/WorktreeDetailTitleView.swift index 34d14689d..a5ccb6887 100644 --- a/supacode/Features/Repositories/Views/WorktreeDetailTitleView.swift +++ b/supacode/Features/Repositories/Views/WorktreeDetailTitleView.swift @@ -3,6 +3,10 @@ import Kingfisher import SupacodeSettingsShared import SwiftUI +#if DEBUG + private nonisolated let titleRenderLogger = SupaLogger("DetailRender") +#endif + enum WorktreeToolbarTitleContent: Hashable, Sendable { case git(GitPayload) case folder(name: String) @@ -17,11 +21,46 @@ enum WorktreeToolbarTitleContent: Hashable, Sendable { } } +/// Chrome-aware wrapper that re-resolves `\.colorScheme` against the +/// terminal background so the toolbar title stays readable when the Ghostty +/// theme diverges from the system appearance. struct WorktreeToolbarTitleView: View { let content: WorktreeToolbarTitleContent + let terminalManager: WorktreeTerminalManager + + @Environment(\.colorScheme) private var systemColorScheme + @State private var configReloadCounter = 0 + + var body: some View { + #if DEBUG + Self._printChanges() + titleRenderLogger.info("WorktreeToolbarTitleView.body re-rendered") + #endif + _ = configReloadCounter + _ = systemColorScheme + let tintScheme = terminalManager.surfaceBackgroundColorScheme() + let appearance = SurfaceChromeAppearance( + colorScheme: tintScheme, + systemColorScheme: systemColorScheme + ) + return WorktreeToolbarTitleBody(content: content) + .environment(\.surfaceChromeAppearance, appearance) + .environment(\.colorScheme, tintScheme) + .onReceive(NotificationCenter.default.publisher(for: .ghosttyRuntimeConfigDidChange)) { _ in + configReloadCounter &+= 1 + } + } +} + +private struct WorktreeToolbarTitleBody: View { + let content: WorktreeToolbarTitleContent var body: some View { - HStack(spacing: 8) { + #if DEBUG + let _ = Self._printChanges() + titleRenderLogger.info("WorktreeToolbarTitleBody.body re-rendered") + #endif + return HStack(spacing: 8) { Group { switch content { case .folder: @@ -123,7 +162,7 @@ enum GitHubOwnerAvatar { Text("").toolbar { ToolbarItem { - WorktreeToolbarTitleView( + WorktreeToolbarTitleBody( content: .git( .init( branchName: "sbertix/319-toolbar-details", @@ -142,7 +181,7 @@ enum GitHubOwnerAvatar { #Preview("Main worktree") { Text("").toolbar { ToolbarItem { - WorktreeToolbarTitleView( + WorktreeToolbarTitleBody( content: .git( .init( branchName: "main", @@ -161,7 +200,7 @@ enum GitHubOwnerAvatar { #Preview("Folder") { Text("").toolbar { ToolbarItem { - WorktreeToolbarTitleView(content: .folder(name: "Documents")) + WorktreeToolbarTitleBody(content: .folder(name: "Documents")) } }.frame(width: 600, height: 600) } diff --git a/supacode/Features/Repositories/Views/WorktreeDetailView.swift b/supacode/Features/Repositories/Views/WorktreeDetailView.swift index eb453e694..c73e5c7c6 100644 --- a/supacode/Features/Repositories/Views/WorktreeDetailView.swift +++ b/supacode/Features/Repositories/Views/WorktreeDetailView.swift @@ -6,6 +6,10 @@ import SupacodeSettingsFeature import SupacodeSettingsShared import SwiftUI +#if DEBUG + private nonisolated let detailRenderLogger = SupaLogger("DetailRender") +#endif + struct WorktreeDetailView: View { @Bindable var store: StoreOf let terminalManager: WorktreeTerminalManager @@ -16,12 +20,18 @@ struct WorktreeDetailView: View { private var agentBadgesEnabled: Bool { settingsFile.global.agentPresenceBadgesEnabled } var body: some View { - detailBody(state: store.state) + #if DEBUG + let _ = Self._printChanges() + detailRenderLogger.info("WorktreeDetailView.body re-rendered") + #endif + return detailBody(state: store.state) } private func detailBody(state: AppFeature.State) -> some View { let repositories = state.repositories - let selectedRow = repositories.selectedRow(for: repositories.selectedWorktreeID) + // Reads the cached slice instead of `sidebarItems[id:]` so per-leaf agent + // / notification churn on the focused row doesn't invalidate this body. + let selectedRow = repositories.selectedWorktreeSlice let selectedWorktree = repositories.worktree(for: repositories.selectedWorktreeID) let selectedWorktreeSummaries = selectedWorktreeSummaries(from: repositories) let showsMultiSelectionSummary = shouldShowMultiSelectionSummary( @@ -46,8 +56,11 @@ struct WorktreeDetailView: View { let openActionSelection = state.openActionSelection let repoScripts = state.repoScripts let globalScripts = state.globalScripts - let runningScriptIDs = state.runningScriptIDs - let notificationGroups = repositories.toolbarNotificationGroups(terminalManager: terminalManager) + // Source `runningScriptIDs` from the slice instead of `state.runningScriptIDs` + // so an unrelated `sidebarItems[id:].agents` mutation on the focused row + // doesn't re-publish this. Same field, observed through the projected slice. + let runningScriptIDs = Set(selectedRow?.runningScripts.ids ?? []) + let notificationGroups = repositories.toolbarNotificationGroupsCache let unseenNotificationWorktreeCount = notificationGroups.reduce(0) { count, repository in count + repository.unseenWorktreeCount } @@ -55,6 +68,7 @@ struct WorktreeDetailView: View { repositories: repositories, loadingInfo: loadingInfo, selectedWorktree: selectedWorktree, + selectedSlice: selectedRow, selectedWorktreeSummaries: selectedWorktreeSummaries ) .toolbar(removing: .title) @@ -72,7 +86,7 @@ struct WorktreeDetailView: View { let toolbarState = WorktreeToolbarState( titleContent: titleContent, rootURL: selectedWorktree.repositoryRootURL, - kind: toolbarKind(for: selectedWorktree, repositories: repositories), + kind: toolbarKind(for: selectedWorktree, selectedRow: selectedRow), statusToast: repositories.statusToast, notificationGroups: notificationGroups, unseenNotificationWorktreeCount: unseenNotificationWorktreeCount, @@ -84,6 +98,7 @@ struct WorktreeDetailView: View { ) WorktreeToolbarContent( toolbarState: toolbarState, + terminalManager: terminalManager, onOpenWorktree: { action in store.send(.openWorktree(action)) }, @@ -110,11 +125,14 @@ struct WorktreeDetailView: View { } } let hasRunningRunScript = state.hasRunningRunScript - let actions = makeFocusedActions( + let resolvedSelection: OpenWorktreeAction? = + hasActiveWorktree ? OpenWorktreeAction.availableSelection(store.openActionSelection) : nil + return applyFocusedActions( + content: content, hasActiveWorktree: hasActiveWorktree, - hasRunningRunScript: hasRunningRunScript + hasRunningRunScript: hasRunningRunScript, + resolvedSelection: resolvedSelection ) - return applyFocusedActions(content: content, actions: actions) } private func selectedWorktreeSummaries( @@ -184,6 +202,7 @@ struct WorktreeDetailView: View { repositories: RepositoriesFeature.State, loadingInfo: WorktreeLoadingInfo?, selectedWorktree: Worktree?, + selectedSlice: SelectedWorktreeSlice?, selectedWorktreeSummaries: [MultiSelectedWorktreeSummary] ) -> some View { Group { @@ -199,16 +218,15 @@ struct WorktreeDetailView: View { } else if let loadingInfo { WorktreeLoadingView(info: loadingInfo) } else if let selectedWorktree { - let shouldRunSetupScript = repositories.sidebarItems[id: selectedWorktree.id]?.lifecycle == .pending + let shouldRunSetupScript = selectedSlice?.lifecycle == .pending let shouldFocusTerminal = repositories.shouldFocusTerminal(for: selectedWorktree.id) WorktreeTerminalTabsView( worktree: selectedWorktree, manager: terminalManager, + terminalsStore: store.scope(state: \.terminals, action: \.terminals), shouldRunSetupScript: shouldRunSetupScript, forceAutoFocus: shouldFocusTerminal, - createTab: { store.send(.newTerminal) }, - agentPresence: store.state.agentPresence, - agentBadgesEnabled: agentBadgesEnabled + createTab: { store.send(.newTerminal) } ) .id(selectedWorktree.id) .frame(maxWidth: .infinity, maxHeight: .infinity) @@ -229,53 +247,51 @@ struct WorktreeDetailView: View { private func applyFocusedActions( content: Content, - actions: FocusedActions + hasActiveWorktree: Bool, + hasRunningRunScript: Bool, + resolvedSelection: OpenWorktreeAction? ) -> some View { - let resolvedSelection: OpenWorktreeAction? = - actions.openSelectedWorktree != nil - ? OpenWorktreeAction.availableSelection(store.openActionSelection) : nil - return - content - .focusedSceneValue(\.openSelectedWorktreeAction, actions.openSelectedWorktree) - .focusedSceneValue(\.revealInFinderAction, actions.revealInFinder) + content + .focusedSceneAction(\.openSelectedWorktreeAction, enabled: hasActiveWorktree) { + store.send(.openSelectedWorktree) + } + .focusedSceneAction(\.revealInFinderAction, enabled: hasActiveWorktree) { + store.send(.revealInFinder) + } .focusedSceneValue(\.openActionSelection, resolvedSelection) - .focusedSceneValue(\.newTerminalAction, actions.newTerminal) - .focusedValue(\.splitTerminalAction, actions.splitTerminal) - .focusedValue(\.closeTabAction, actions.closeTab) - .focusedValue(\.closeSurfaceAction, actions.closeSurface) - .focusedSceneValue(\.startSearchAction, actions.startSearch) - .focusedSceneValue(\.searchSelectionAction, actions.searchSelection) - .focusedSceneValue(\.navigateSearchNextAction, actions.navigateSearchNext) - .focusedSceneValue(\.navigateSearchPreviousAction, actions.navigateSearchPrevious) - .focusedSceneValue(\.endSearchAction, actions.endSearch) - .focusedSceneValue(\.runScriptAction, actions.runScript) - .focusedSceneValue(\.stopRunScriptAction, actions.stopRunScript) - } - - private func makeFocusedActions( - hasActiveWorktree: Bool, - hasRunningRunScript: Bool - ) -> FocusedActions { - func action(_ appAction: AppFeature.Action) -> (() -> Void)? { - hasActiveWorktree ? { store.send(appAction) } : nil - } - let splitTerminal: ((TerminalSplitMenuDirection) -> Void)? = - hasActiveWorktree ? { direction in store.send(.splitTerminal(direction)) } : nil - return FocusedActions( - openSelectedWorktree: action(.openSelectedWorktree), - revealInFinder: action(.revealInFinder), - newTerminal: action(.newTerminal), - splitTerminal: splitTerminal, - closeTab: action(.closeTab), - closeSurface: action(.closeSurface), - startSearch: action(.startSearch), - searchSelection: action(.searchSelection), - navigateSearchNext: action(.navigateSearchNext), - navigateSearchPrevious: action(.navigateSearchPrevious), - endSearch: action(.endSearch), - runScript: hasActiveWorktree ? { store.send(.runScript) } : nil, - stopRunScript: hasRunningRunScript ? { store.send(.stopRunScripts) } : nil, - ) + .focusedSceneAction(\.newTerminalAction, enabled: hasActiveWorktree) { + store.send(.newTerminal) + } + .focusedAction(\.splitTerminalAction, enabled: hasActiveWorktree) { direction in + store.send(.splitTerminal(direction)) + } + .focusedAction(\.closeTabAction, enabled: hasActiveWorktree) { + store.send(.closeTab) + } + .focusedAction(\.closeSurfaceAction, enabled: hasActiveWorktree) { + store.send(.closeSurface) + } + .focusedSceneAction(\.startSearchAction, enabled: hasActiveWorktree) { + store.send(.startSearch) + } + .focusedSceneAction(\.searchSelectionAction, enabled: hasActiveWorktree) { + store.send(.searchSelection) + } + .focusedSceneAction(\.navigateSearchNextAction, enabled: hasActiveWorktree) { + store.send(.navigateSearchNext) + } + .focusedSceneAction(\.navigateSearchPreviousAction, enabled: hasActiveWorktree) { + store.send(.navigateSearchPrevious) + } + .focusedSceneAction(\.endSearchAction, enabled: hasActiveWorktree) { + store.send(.endSearch) + } + .focusedSceneAction(\.runScriptAction, enabled: hasActiveWorktree) { + store.send(.runScript) + } + .focusedSceneAction(\.stopRunScriptAction, enabled: hasRunningRunScript) { + store.send(.stopRunScripts) + } } private func selectToolbarNotification( @@ -284,7 +300,7 @@ struct WorktreeDetailView: View { ) { store.send(.repositories(.selectWorktree(worktreeID))) if let terminalState = terminalManager.stateIfExists(for: worktreeID) { - _ = terminalState.focusSurface(id: notification.surfaceId) + _ = terminalState.focusSurface(id: notification.surfaceID) } } @@ -296,22 +312,6 @@ struct WorktreeDetailView: View { } } - private struct FocusedActions { - let openSelectedWorktree: (() -> Void)? - let revealInFinder: (() -> Void)? - let newTerminal: (() -> Void)? - let splitTerminal: ((TerminalSplitMenuDirection) -> Void)? - let closeTab: (() -> Void)? - let closeSurface: (() -> Void)? - let startSearch: (() -> Void)? - let searchSelection: (() -> Void)? - let navigateSearchNext: (() -> Void)? - let navigateSearchPrevious: (() -> Void)? - let endSearch: (() -> Void)? - let runScript: (() -> Void)? - let stopRunScript: (() -> Void)? - } - fileprivate struct ScriptMenuIdentity: Hashable { let rootURL: URL let repoFingerprints: [ScriptFingerprint] @@ -409,6 +409,7 @@ struct WorktreeDetailView: View { fileprivate struct WorktreeToolbarContent: ToolbarContent { let toolbarState: WorktreeToolbarState + let terminalManager: WorktreeTerminalManager let onOpenWorktree: (OpenWorktreeAction) -> Void let onOpenActionSelectionChanged: (OpenWorktreeAction) -> Void let onRevealInFinder: () -> Void @@ -423,7 +424,10 @@ struct WorktreeDetailView: View { var body: some ToolbarContent { ToolbarItem(placement: .navigation) { - WorktreeToolbarTitleView(content: toolbarState.titleContent) + WorktreeToolbarTitleView( + content: toolbarState.titleContent, + terminalManager: terminalManager + ) } .sharedBackgroundVisibility(.hidden) @@ -516,7 +520,7 @@ struct WorktreeDetailView: View { static func makeToolbarTitleContent( selectedWorktree: Worktree, - selectedRow: SidebarItemFeature.State?, + selectedRow: SelectedWorktreeSlice?, repositories: RepositoriesFeature.State, hideSubtitleOnMatch: Bool ) -> WorktreeToolbarTitleContent { @@ -562,22 +566,21 @@ struct WorktreeDetailView: View { private func toolbarKind( for selectedWorktree: Worktree, - repositories: RepositoriesFeature.State + selectedRow: SelectedWorktreeSlice? ) -> WorktreeToolbarState.Kind { - let selectedRow = repositories.selectedRow(for: selectedWorktree.id) guard selectedRow?.isFolder != true else { return .folder } - guard let pullRequest = repositories.sidebarItems[id: selectedWorktree.id]?.pullRequest else { + guard let pullRequest = selectedRow?.pullRequest else { return .git(pullRequest: nil) } // Only surface the PR when its head branch matches the current - // worktree — otherwise stale info sticks around after a rename + // worktree, otherwise stale info sticks around after a rename // or branch switch. let matches = pullRequest.headRefName == nil || pullRequest.headRefName == selectedWorktree.name return .git(pullRequest: matches ? pullRequest : nil) } private func loadingInfo( - for selectedRow: SidebarItemFeature.State?, + for selectedRow: SelectedWorktreeSlice?, selectedWorktreeID: Worktree.ID?, repositories: RepositoriesFeature.State ) -> WorktreeLoadingInfo? { @@ -1034,6 +1037,7 @@ private struct WorktreeToolbarPreview: View { .toolbar { WorktreeDetailView.WorktreeToolbarContent( toolbarState: toolbarState, + terminalManager: WorktreeTerminalManager(runtime: GhosttyRuntime()), onOpenWorktree: { _ in }, onOpenActionSelectionChanged: { _ in }, onRevealInFinder: {}, diff --git a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift index 7d9e5050e..ae5b55ff9 100644 --- a/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift +++ b/supacode/Features/Terminal/BusinessLogic/WorktreeTerminalManager.swift @@ -326,6 +326,19 @@ final class WorktreeTerminalManager { // next mutation). lastEmittedProjections.removeAll() for id in states.keys { emitProjection(for: id) } + // Replay per-tab projections / stripe-progress displays for the same reason: + // a new subscriber needs the existing `terminalTabs[id:]` rows seeded so + // tab-bar leaves don't render empty until the next per-tab mutation. + for (worktreeID, state) in states { + for projection in state.currentTabProjections() { + continuation.yield(.tabProjectionChanged(worktreeID: worktreeID, projection)) + } + for (tabID, display) in state.currentTabProgressDisplays() { + continuation.yield( + .tabProgressDisplayChanged(worktreeID: worktreeID, tabID: tabID, display: display) + ) + } + } return stream } @@ -403,6 +416,15 @@ final class WorktreeTerminalManager { state.onSetupScriptConsumed = { [weak self] in self?.emit(.setupScriptConsumed(worktreeID: worktree.id)) } + state.onTabProjectionChanged = { [weak self] projection in + self?.emit(.tabProjectionChanged(worktreeID: worktree.id, projection)) + } + state.onTabRemoved = { [weak self] tabID in + self?.emit(.tabRemoved(worktreeID: worktree.id, tabID: tabID)) + } + state.onTabProgressDisplayChanged = { [weak self] tabID, display in + self?.emit(.tabProgressDisplayChanged(worktreeID: worktree.id, tabID: tabID, display: display)) + } states[worktree.id] = state terminalLogger.info("Created terminal state for worktree \(worktree.id)") return state @@ -447,13 +469,16 @@ final class WorktreeTerminalManager { for (id, state) in removed { saveLayoutSnapshot?(id, state.captureLayoutSnapshot()) state.closeAllSurfaces() + // Signals the reducer to drop any orphan `terminalTabs` entries and + // recently-removed-tab records for this worktree so a same-session + // restore (snapshot reuses persisted tab UUIDs) starts clean. + emit(.worktreeStateTornDown(worktreeID: id)) } if !removed.isEmpty { terminalLogger.info("Pruned \(removed.count) terminal state(s)") } states = states.filter { worktreeIDs.contains($0.key) } cancelPendingIdleHooks(forSurfaceIDs: prunedSurfaceIDs) - // Drop projection cache for pruned worktrees so a future re-add starts clean. for (id, _) in removed { lastEmittedProjections.removeValue(forKey: id) } emitNotificationIndicatorCountIfNeeded() } @@ -515,10 +540,10 @@ final class WorktreeTerminalManager { for (worktreeID, state) in states { for notification in state.unreadNotifications() { if let bestCreatedAt, bestCreatedAt >= notification.createdAt { break } - guard let tabID = state.tabID(containing: notification.surfaceId) else { + guard let tabID = state.tabID(containing: notification.surfaceID) else { skippedClosedSurface = true terminalLogger.debug( - "latestUnreadNotificationLocation: skipping closed surface \(notification.surfaceId) " + "latestUnreadNotificationLocation: skipping closed surface \(notification.surfaceID) " + "in \(worktreeID); trying older unread." ) continue @@ -526,7 +551,7 @@ final class WorktreeTerminalManager { best = NotificationLocation( worktreeID: worktreeID, tabID: tabID, - surfaceID: notification.surfaceId, + surfaceID: notification.surfaceID, notificationID: notification.id, ) bestCreatedAt = notification.createdAt diff --git a/supacode/Features/Terminal/Models/TerminalTabID.swift b/supacode/Features/Terminal/Models/TerminalTabID.swift index 05da70e2d..b4bccfe91 100644 --- a/supacode/Features/Terminal/Models/TerminalTabID.swift +++ b/supacode/Features/Terminal/Models/TerminalTabID.swift @@ -1,6 +1,6 @@ import Foundation -struct TerminalTabID: Hashable, Identifiable, Codable, Sendable { +nonisolated struct TerminalTabID: Hashable, Identifiable, Codable, Sendable { let rawValue: UUID init() { diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalNotification.swift b/supacode/Features/Terminal/Models/WorktreeTerminalNotification.swift index bfbad3015..2d45927ba 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalNotification.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalNotification.swift @@ -2,7 +2,7 @@ import Foundation struct WorktreeTerminalNotification: Identifiable, Equatable, Sendable { let id: UUID - let surfaceId: UUID + let surfaceID: UUID let title: String let body: String let createdAt: Date @@ -10,14 +10,14 @@ struct WorktreeTerminalNotification: Identifiable, Equatable, Sendable { init( id: UUID = UUID(), - surfaceId: UUID, + surfaceID: UUID, title: String, body: String, createdAt: Date, isRead: Bool = false ) { self.id = id - self.surfaceId = surfaceId + self.surfaceID = surfaceID self.title = title self.body = body self.createdAt = createdAt diff --git a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift index e03ebc809..dd1f3794e 100644 --- a/supacode/Features/Terminal/Models/WorktreeTerminalState.swift +++ b/supacode/Features/Terminal/Models/WorktreeTerminalState.swift @@ -12,6 +12,17 @@ private let blockingScriptLogger = SupaLogger("BlockingScript") private let layoutLogger = SupaLogger("Layout") private let terminalStateLogger = SupaLogger("Terminal") +/// Per-tab projection emitted by `WorktreeTerminalState` whenever a tab's +/// surfaces, focus, unread count, or progress display drifts. The parent +/// reducer applies this to the matching `TerminalTabFeature.State` so the +/// tab-bar leaf observes a per-tab store instead of worktree-wide state. +struct WorktreeTabProjection: Equatable, Sendable { + let tabID: TerminalTabID + let surfaceIDs: [UUID] + let activeSurfaceID: UUID? + let unseenNotificationCount: Int +} + @MainActor @Observable final class WorktreeTerminalState { @@ -26,9 +37,17 @@ final class WorktreeTerminalState { private let worktree: Worktree @ObservationIgnored @SharedReader private var repositorySettings: RepositorySettings - private var trees: [TerminalTabID: SplitTree] = [:] - private var surfaces: [UUID: GhosttySurfaceView] = [:] - private var focusedSurfaceIdByTab: [TerminalTabID: UUID] = [:] + @ObservationIgnored private var trees: [TerminalTabID: SplitTree] = [:] + @ObservationIgnored private var surfaces: [UUID: GhosttySurfaceView] = [:] + @ObservationIgnored private var focusedSurfaceIdByTab: [TerminalTabID: UUID] = [:] + /// Per-tab projection cache. `WorktreeTerminalState` recomputes from `trees` + /// / `notifications` / `focusedSurfaceIdByTab`, compares to the cached value, + /// and fires `onTabProjectionChanged` only on diff. The manager forwards the + /// projection upstream so `TerminalTabFeature.State` mirrors it. + @ObservationIgnored private var lastTabProjections: [TerminalTabID: WorktreeTabProjection] = [:] + /// Per-tab progress-display cache. Tracks the focused-surface or worst-of + /// aggregate so `onTabProgressDisplayChanged` only fires on diff. + @ObservationIgnored private var lastTabProgressDisplays: [TerminalTabID: TerminalTabProgressDisplay?] = [:] var socketPath: String? private(set) var shouldHideTabBar = false private var blockingScripts: [TerminalTabID: BlockingScriptKind] = [:] @@ -41,7 +60,10 @@ final class WorktreeTerminalState { private var lastEmittedFocusSurfaceId: UUID? private var lastWindowIsKey: Bool? private var lastWindowIsVisible: Bool? - var notifications: [WorktreeTerminalNotification] = [] + /// Raw notification log. `@ObservationIgnored` so per-tab notification ticks + /// flow through `TerminalTabState.unseenNotificationCount` projections instead + /// of invalidating every leaf in the worktree. + @ObservationIgnored private(set) var notifications: [WorktreeTerminalNotification] = [] var notificationsEnabled = true @ObservationIgnored @Dependency(\.date.now) private var now private var recentHookBySurfaceID: [UUID: (text: String, recordedAt: Date)] = [:] @@ -50,13 +72,13 @@ final class WorktreeTerminalState { } func hasUnseenNotification(forSurfaceID surfaceID: UUID) -> Bool { - notifications.contains { !$0.isRead && $0.surfaceId == surfaceID } + notifications.contains { !$0.isRead && $0.surfaceID == surfaceID } } func hasUnseenNotification(forTabID tabID: TerminalTabID) -> Bool { guard let tree = trees[tabID] else { return false } let surfaceIDs = Set(tree.leaves().map(\.id)) - return notifications.contains { !$0.isRead && surfaceIDs.contains($0.surfaceId) } + return notifications.contains { !$0.isRead && surfaceIDs.contains($0.surfaceID) } } /// Returns the most recent unread notification in this worktree, or nil. @@ -86,6 +108,17 @@ final class WorktreeTerminalState { var onSetupScriptConsumed: (() -> Void)? /// Forwarded to the manager so it can emit a `surfacesClosed` event into TCA. var onSurfacesClosed: ((Set) -> Void)? + /// Fires when a tab's per-tab projection (surfaces / focus / unseen count) + /// drifts. Manager forwards into `TerminalTabFeature.State` via + /// `tabProjectionChanged` so the leaf observes a per-tab store. + var onTabProjectionChanged: ((WorktreeTabProjection) -> Void)? + /// Fires when a tab is fully removed (closeTab, closeAll). Manager forwards + /// so the parent reducer drops the corresponding `TerminalTabFeature.State`. + var onTabRemoved: ((TerminalTabID) -> Void)? + /// Fires when a tab's stripe-progress display drifts. Computed off the + /// active surface (selected tab) or worst-of-all (unselected tabs) so the + /// stripe stays in lock-step with focus and OSC-9 progress mutations. + var onTabProgressDisplayChanged: ((TerminalTabID, TerminalTabProgressDisplay?) -> Void)? init( runtime: GhosttyRuntime, @@ -373,14 +406,14 @@ final class WorktreeTerminalState { trees.values.flatMap { $0.leaves().map(\.id) } } - func hasSurface(_ surfaceId: UUID, in tabId: TerminalTabID) -> Bool { + func hasSurface(_ surfaceID: UUID, in tabId: TerminalTabID) -> Bool { guard let tree = trees[tabId] else { return false } - return tree.find(id: surfaceId) != nil + return tree.find(id: surfaceID) != nil } /// Checks whether a surface UUID exists anywhere in the worktree (across all tabs). - func hasSurfaceAnywhere(_ surfaceId: UUID) -> Bool { - surfaces[surfaceId] != nil + func hasSurfaceAnywhere(_ surfaceID: UUID) -> Bool { + surfaces[surfaceID] != nil } func selectTab(_ tabId: TerminalTabID) { @@ -388,8 +421,15 @@ final class WorktreeTerminalState { terminalStateLogger.warning("selectTab: tab \(tabId.rawValue) not found in worktree \(worktree.id).") return } + let previousSelectedTabId = tabManager.selectedTabId tabManager.selectTab(tabId) focusSurface(in: tabId) + // Re-emit the stripe progress for both old and new selected tabs: their + // "focused vs aggregate" branch just flipped. + if let previousSelectedTabId, previousSelectedTabId != tabId { + emitTabProgressDisplay(for: previousSelectedTabId) + } + emitTabProgressDisplay(for: tabId) emitTaskStatusIfChanged() } @@ -591,29 +631,29 @@ final class WorktreeTerminalState { surfaceID: surfaceID ) let tree = SplitTree(view: surface) - trees[tabId] = tree - focusedSurfaceIdByTab[tabId] = surface.id + setTree(tree, for: tabId) + setFocusedSurface(surface.id, for: tabId) return tree } func performSplitAction( _ action: GhosttySplitAction, - for surfaceId: UUID, + for surfaceID: UUID, newSurfaceID: UUID? = nil, initialInput: String? = nil ) -> Bool { - guard let tabId = tabID(containing: surfaceId), var tree = trees[tabId] else { + guard let tabId = tabID(containing: surfaceID), var tree = trees[tabId] else { return false } - guard let targetNode = tree.find(id: surfaceId) else { return false } - guard let targetSurface = surfaces[surfaceId] else { return false } + guard let targetNode = tree.find(id: surfaceID) else { return false } + guard let targetSurface = surfaces[surfaceID] else { return false } switch action { case .newSplit(let direction): let newSurface = createSurface( tabId: tabId, initialInput: initialInput, - inheritingFromSurfaceId: surfaceId, + inheritingFromSurfaceId: surfaceID, context: GHOSTTY_SURFACE_CONTEXT_SPLIT, surfaceID: newSurfaceID, ) @@ -628,7 +668,7 @@ final class WorktreeTerminalState { return true } catch { terminalStateLogger.warning( - "performSplitAction: failed to insert split for surface \(surfaceId) in tab \(tabId.rawValue): \(error)") + "performSplitAction: failed to insert split for surface \(surfaceID) in tab \(tabId.rawValue): \(error)") newSurface.closeSurface() surfaces.removeValue(forKey: newSurface.id) return false @@ -734,8 +774,6 @@ final class WorktreeTerminalState { trees.removeAll() focusedSurfaceIdByTab.removeAll() onSurfacesClosed?(Set(closingSurfaceIDs)) - // Agent busy state lives on GhosttySurfaceState and is cleaned up - // when surfaces are removed. let pendingKinds = Set(blockingScripts.values) blockingScripts.removeAll() lastBlockingScriptTabByKind.removeAll() @@ -744,6 +782,14 @@ final class WorktreeTerminalState { onBlockingScriptCompleted?(kind, nil, nil) } tabManager.closeAll() + // Drain per-tab caches and notify so `TerminalsFeature.State.terminalTabs` + // entries don't leak for tabs in a torn-down worktree (#289 follow-up). + let removedTabIDs = Array(lastTabProjections.keys) + lastTabProjections.removeAll() + lastTabProgressDisplays.removeAll() + for tabID in removedTabIDs { + onTabRemoved?(tabID) + } } func setNotificationsEnabled(_ enabled: Bool) { @@ -762,14 +808,18 @@ final class WorktreeTerminalState { for index in notifications.indices { notifications[index].isRead = true } + emitAllTabProjections() emitNotificationIndicatorIfNeeded(previousHasUnseen: previousHasUnseen) } func markNotificationsRead(forSurfaceID surfaceID: UUID) { let previousHasUnseen = hasUnseenNotification - for index in notifications.indices where notifications[index].surfaceId == surfaceID { + for index in notifications.indices where notifications[index].surfaceID == surfaceID { notifications[index].isRead = true } + if let tabId = tabID(containing: surfaceID) { + emitTabProjection(for: tabId) + } emitNotificationIndicatorIfNeeded(previousHasUnseen: previousHasUnseen) } @@ -778,19 +828,28 @@ final class WorktreeTerminalState { let previousHasUnseen = hasUnseenNotification guard let index = notifications.firstIndex(where: { $0.id == id }) else { return } guard !notifications[index].isRead else { return } + let surfaceID = notifications[index].surfaceID notifications[index].isRead = true + if let tabId = tabID(containing: surfaceID) { + emitTabProjection(for: tabId) + } emitNotificationIndicatorIfNeeded(previousHasUnseen: previousHasUnseen) } func dismissNotification(_ notificationID: WorktreeTerminalNotification.ID) { let previousHasUnseen = hasUnseenNotification + let affectedSurface = notifications.first(where: { $0.id == notificationID })?.surfaceID notifications.removeAll { $0.id == notificationID } + if let affectedSurface, let tabId = tabID(containing: affectedSurface) { + emitTabProjection(for: tabId) + } emitNotificationIndicatorIfNeeded(previousHasUnseen: previousHasUnseen) } func dismissAllNotifications() { let previousHasUnseen = hasUnseenNotification notifications.removeAll() + emitAllTabProjections() emitNotificationIndicatorIfNeeded(previousHasUnseen: previousHasUnseen) } @@ -890,8 +949,8 @@ final class WorktreeTerminalState { surfaceID: tabSnapshot.layout.firstLeaf.id, ) let tree = SplitTree(view: surface) - trees[tabId] = tree - focusedSurfaceIdByTab[tabId] = surface.id + setTree(tree, for: tabId) + setFocusedSurface(surface.id, for: tabId) // Recursively restore splits. restoreLayoutNode(tabSnapshot.layout, anchor: surface, tabId: tabId) @@ -908,7 +967,7 @@ final class WorktreeTerminalState { // Focus the correct leaf. let focusedIndex = max(0, min(tabSnapshot.focusedLeafIndex, leaves.count - 1)) if focusedIndex < leaves.count { - focusedSurfaceIdByTab[tabId] = leaves[focusedIndex].id + setFocusedSurface(leaves[focusedIndex].id, for: tabId) } onTabCreated?() @@ -976,7 +1035,7 @@ final class WorktreeTerminalState { ) do { tree = try tree.inserting(view: newSurface, at: anchor, direction: direction, ratio: ratio) - trees[tabId] = tree + setTree(tree, for: tabId) return newSurface } catch { layoutLogger.warning("Failed to restore split for tab \(tabId.rawValue): \(error)") @@ -1198,7 +1257,7 @@ final class WorktreeTerminalState { } view.bridge.onDesktopNotification = { [weak self, weak view] title, body in guard let self, let view else { return } - self.appendNotification(title: title, body: body, surfaceId: view.id) + self.appendNotification(title: title, body: body, surfaceID: view.id) } view.bridge.onCloseRequest = { [weak self, weak view] processAlive in guard let self, let view else { return } @@ -1219,11 +1278,11 @@ final class WorktreeTerminalState { } private func inheritedSurfaceConfig( - fromSurfaceId surfaceId: UUID?, + fromSurfaceId surfaceID: UUID?, context: ghostty_surface_context_e ) -> InheritedSurfaceConfig { - guard let surfaceId, - let view = surfaces[surfaceId], + guard let surfaceID, + let view = surfaces[surfaceID], let sourceSurface = view.surface else { return InheritedSurfaceConfig(workingDirectory: nil, fontSize: nil) @@ -1277,7 +1336,7 @@ final class WorktreeTerminalState { // from explicit focus paths (programmatic focus, split navigation, zoom) // and from AppKit responder changes when the user clicks a pane. private func recordActiveSurface(_ surface: GhosttySurfaceView, in tabId: TerminalTabID) { - focusedSurfaceIdByTab[tabId] = surface.id + setFocusedSurface(surface.id, for: tabId) markNotificationsRead(forSurfaceID: surface.id) updateTabTitle(for: tabId) emitFocusChangedIfNeeded(surface.id) @@ -1299,13 +1358,13 @@ final class WorktreeTerminalState { if let normalized = Self.normalizedText("\(title) \(body)") { recentHookBySurfaceID[surfaceID] = (text: normalized, recordedAt: now) } - appendNotification(title: title, body: body, surfaceId: surfaceID, fromHook: true) + appendNotification(title: title, body: body, surfaceID: surfaceID, fromHook: true) } private func appendNotification( title: String, body: String, - surfaceId: UUID, + surfaceID: UUID, fromHook: Bool = false ) { let trimmedTitle = title.trimmingCharacters(in: .whitespacesAndNewlines) @@ -1313,10 +1372,10 @@ final class WorktreeTerminalState { guard !(trimmedTitle.isEmpty && trimmedBody.isEmpty) else { return } if notificationsEnabled { let previousHasUnseen = hasUnseenNotification - let isRead = isSelected() && isFocusedSurface(surfaceId) + let isRead = isSelected() && isFocusedSurface(surfaceID) notifications.insert( WorktreeTerminalNotification( - surfaceId: surfaceId, + surfaceID: surfaceID, title: trimmedTitle, body: trimmedBody, createdAt: now, @@ -1324,13 +1383,16 @@ final class WorktreeTerminalState { ), at: 0 ) + if let tabId = tabID(containing: surfaceID) { + emitTabProjection(for: tabId) + } emitNotificationIndicatorIfNeeded(previousHasUnseen: previousHasUnseen) } // Suppress OSC 9 system notifications that duplicate a recent hook notification. - if !fromHook, shouldSuppressDesktopNotification(title: trimmedTitle, body: trimmedBody, surfaceId: surfaceId) { + if !fromHook, shouldSuppressDesktopNotification(title: trimmedTitle, body: trimmedBody, surfaceID: surfaceID) { return } - onNotificationReceived?(surfaceId, trimmedTitle, trimmedBody) + onNotificationReceived?(surfaceID, trimmedTitle, trimmedBody) } // MARK: - Notification deduplication (matches supaterm's approach). @@ -1343,10 +1405,10 @@ final class WorktreeTerminalState { "turn complete", ] - private func shouldSuppressDesktopNotification(title: String, body: String, surfaceId: UUID) -> Bool { + private func shouldSuppressDesktopNotification(title: String, body: String, surfaceID: UUID) -> Bool { guard let terminalText = Self.normalizedText("\(title) \(body)"), - let recent = recentHookBySurfaceID[surfaceId], + let recent = recentHookBySurfaceID[surfaceID], now.timeIntervalSince(recent.recordedAt) <= Self.notificationCoalescingWindow else { return false @@ -1380,28 +1442,73 @@ final class WorktreeTerminalState { cleanupSurfaceState(for: surface.id) } focusedSurfaceIdByTab.removeValue(forKey: tabId) + if lastTabProjections.removeValue(forKey: tabId) != nil { + onTabRemoved?(tabId) + } } - func tabID(containing surfaceId: UUID) -> TerminalTabID? { - for (tabId, tree) in trees where tree.find(id: surfaceId) != nil { + func tabID(containing surfaceID: UUID) -> TerminalTabID? { + for (tabId, tree) in trees where tree.find(id: surfaceID) != nil { return tabId } return nil } - private func isFocusedSurface(_ surfaceId: UUID) -> Bool { + private func isFocusedSurface(_ surfaceID: UUID) -> Bool { guard let selectedTabId = tabManager.selectedTabId else { return false } - return focusedSurfaceIdByTab[selectedTabId] == surfaceId + return focusedSurfaceIdByTab[selectedTabId] == surfaceID } private func updateRunningState(for tabId: TerminalTabID) { guard trees[tabId] != nil else { return } tabManager.updateDirty(tabId, isDirty: isTabBusy(tabId)) + emitTabProgressDisplay(for: tabId) emitTaskStatusIfChanged() } + /// Compute the per-tab stripe progress payload off `trees[tabId]`'s surfaces. + /// Selected tab → focused-surface state; unselected tab → worst-of-all + /// (ERROR > PAUSE > determinate > indeterminate > none). + private func computeTabProgressDisplay(for tabId: TerminalTabID) -> TerminalTabProgressDisplay? { + guard let tree = trees[tabId] else { return nil } + let leaves = tree.leaves() + if tabManager.selectedTabId == tabId, + let focusedID = focusedSurfaceIdByTab[tabId], + let focused = leaves.first(where: { $0.id == focusedID }) + { + return TerminalTabProgressDisplay.make( + progressState: focused.bridge.state.progressState, + progressValue: focused.bridge.state.progressValue + ) + } + var worst: TerminalTabProgressDisplay? + for surface in leaves { + guard + let candidate = TerminalTabProgressDisplay.make( + progressState: surface.bridge.state.progressState, + progressValue: surface.bridge.state.progressValue + ) + else { continue } + if worst == nil || candidate.severity > worst!.severity { + worst = candidate + } + } + return worst + } + + /// Recompute and emit the tab's progress display when it differs from the + /// cached value. Idempotent so OSC-9 ticks that don't move the stripe state + /// don't fire the callback. + private func emitTabProgressDisplay(for tabId: TerminalTabID) { + let newDisplay = computeTabProgressDisplay(for: tabId) + if lastTabProgressDisplays[tabId] != newDisplay { + lastTabProgressDisplays[tabId] = newDisplay + onTabProgressDisplayChanged?(tabId, newDisplay) + } + } + private func emitTaskStatusIfChanged() { let newStatus = taskStatus if newStatus != lastReportedTaskStatus { @@ -1410,10 +1517,10 @@ final class WorktreeTerminalState { } } - private func emitFocusChangedIfNeeded(_ surfaceId: UUID) { - guard surfaceId != lastEmittedFocusSurfaceId else { return } - lastEmittedFocusSurfaceId = surfaceId - onFocusChanged?(surfaceId) + private func emitFocusChangedIfNeeded(_ surfaceID: UUID) { + guard surfaceID != lastEmittedFocusSurfaceId else { return } + lastEmittedFocusSurfaceId = surfaceID + onFocusChanged?(surfaceID) } private func emitNotificationIndicatorIfNeeded(previousHasUnseen: Bool) { @@ -1428,10 +1535,81 @@ final class WorktreeTerminalState { } private func updateTree(_ tree: SplitTree, for tabId: TerminalTabID) { - trees[tabId] = tree + setTree(tree, for: tabId) syncFocusIfNeeded() } + /// Single mutation point for `trees[tabId]`. Recomputes and emits the per-tab + /// projection so `TerminalTabFeature.State` mirrors `trees[tabId]`'s leaves + /// + the tab's unread count + focus without observing worktree-wide state. + private func setTree(_ tree: SplitTree, for tabId: TerminalTabID) { + trees[tabId] = tree + emitTabProjection(for: tabId) + } + + /// Single mutation point for `focusedSurfaceIdByTab[tabId]`. Mirrors into the + /// per-tab projection so the stripe-progress leaf observes the focus change + /// per-tab instead of through the worktree-wide dictionary. + private func setFocusedSurface(_ surfaceID: UUID?, for tabId: TerminalTabID) { + if let surfaceID { + focusedSurfaceIdByTab[tabId] = surfaceID + } else { + focusedSurfaceIdByTab.removeValue(forKey: tabId) + } + emitTabProjection(for: tabId) + } + + /// Recompute the per-tab projection and emit `onTabProjectionChanged` when + /// the value differs from the cached one. Idempotent: a no-op rebuild + /// (e.g. a notification arrived on a surface that's already counted) does + /// not fire the callback. + private func emitTabProjection(for tabId: TerminalTabID) { + guard let tree = trees[tabId] else { + if lastTabProjections.removeValue(forKey: tabId) != nil { + onTabRemoved?(tabId) + } + return + } + let surfaceIDs = tree.leaves().map(\.id) + let surfaceIDSet = Set(surfaceIDs) + let unseenCount = notifications.reduce(into: 0) { partial, notification in + if !notification.isRead, surfaceIDSet.contains(notification.surfaceID) { + partial += 1 + } + } + let projection = WorktreeTabProjection( + tabID: tabId, + surfaceIDs: surfaceIDs, + activeSurfaceID: focusedSurfaceIdByTab[tabId], + unseenNotificationCount: unseenCount + ) + guard lastTabProjections[tabId] != projection else { return } + lastTabProjections[tabId] = projection + onTabProjectionChanged?(projection) + } + + /// Recompute every tab's projection. Used after notification-list mutations + /// that may span multiple tabs (mark-all-read, dismiss-all). + private func emitAllTabProjections() { + for tabId in trees.keys { + emitTabProjection(for: tabId) + } + } + + /// Snapshot all current tab projections. Manager replays this on every fresh + /// event-stream subscriber so `terminalTabs[id:]` reconstructs without + /// waiting for the next per-tab mutation. + func currentTabProjections() -> [WorktreeTabProjection] { + Array(lastTabProjections.values) + } + + /// Snapshot all current per-tab stripe-progress displays. Replayed alongside + /// `currentTabProjections()` so the stripe paints the right state on the + /// first frame after re-subscribe. + func currentTabProgressDisplays() -> [TerminalTabID: TerminalTabProgressDisplay?] { + lastTabProgressDisplays + } + private func isRunningProgressState(_ state: ghostty_action_progress_report_state_e?) -> Bool { switch state { case .some(GHOSTTY_PROGRESS_STATE_SET), @@ -1595,6 +1773,19 @@ final class WorktreeTerminalState { } return maxIndex + 1 } + + #if DEBUG + /// Test-only seam for bulk-assigning the notifications log. Fans + /// `emitAllTabProjections()` so `lastTabProjections` stays in sync with + /// the raw log; production code must go through the per-event helpers + /// (`appendNotification`, `markNotificationsRead`, etc.) which already + /// emit. Gated `#if DEBUG` so release builds genuinely can't reach the + /// projection-bypass path. + func setNotificationsForTesting(_ list: [WorktreeTerminalNotification]) { + notifications = list + emitAllTabProjections() + } + #endif } nonisolated func makeCommandInput( diff --git a/supacode/Features/Terminal/Reducer/TerminalTabFeature.swift b/supacode/Features/Terminal/Reducer/TerminalTabFeature.swift new file mode 100644 index 000000000..f771d6d68 --- /dev/null +++ b/supacode/Features/Terminal/Reducer/TerminalTabFeature.swift @@ -0,0 +1,124 @@ +import ComposableArchitecture +import Foundation +import GhosttyKit + +/// Per-tab observable state mirroring the sidebar's per-row `SidebarItemFeature`. +/// Leaves scope through `store.scope(state: \.terminalTabs[id:], action: \.terminalTabs[id:])` +/// so a mutation on tab B only invalidates tab B's leaf. +@Reducer +struct TerminalTabFeature { + @ObservableState + struct State: Identifiable, Equatable, Sendable { + /// Typed `TerminalTabID` so the nominal-type wall against an unrelated + /// raw `UUID` reaches every scoping site. `IdentifiedArrayOf` keys by + /// this id directly. + let id: TerminalTabID + let worktreeID: Worktree.ID + + /// Surface IDs in this tab in split-tree order. Mirrored from + /// `WorktreeTerminalState`'s `onTabProjectionChanged`. + var surfaceIDs: [UUID] = [] + /// Focused pane in this tab. Drives the stripe-progress's per-tab source + /// (focused tab → focused surface; non-focused → worst-of aggregate). + var activeSurfaceID: UUID? + /// Count of unread notifications scoped to this tab's surfaces. + var unseenNotificationCount: Int = 0 + /// Per-tab agent snapshot pushed by `AppFeature.agentPresenceFanOutEffect`. + /// Leaf reads `state.agents` instead of iterating worktree-wide presence on + /// every storm tick. + var agents: [AgentPresenceFeature.AgentInstance] = [] + /// Stripe-progress summary. Computed from the active surface's + /// `GhosttySurfaceState.progressState` (focused tabs) or worst-of-all + /// surfaces (unfocused tabs). Nil = no progress to display. + var progressDisplay: TerminalTabProgressDisplay? + + var hasUnseenNotifications: Bool { unseenNotificationCount > 0 } + } + + enum Action: Equatable, Sendable { + case projectionChanged(WorktreeTabProjection) + case agentSnapshotChanged([AgentPresenceFeature.AgentInstance]) + case progressDisplayChanged(TerminalTabProgressDisplay?) + } + + var body: some Reducer { + Reduce { state, action in + switch action { + case .projectionChanged(let projection): + if state.surfaceIDs != projection.surfaceIDs { state.surfaceIDs = projection.surfaceIDs } + if state.activeSurfaceID != projection.activeSurfaceID { + state.activeSurfaceID = projection.activeSurfaceID + } + if state.unseenNotificationCount != projection.unseenNotificationCount { + state.unseenNotificationCount = projection.unseenNotificationCount + } + return .none + + case .agentSnapshotChanged(let agents): + guard state.agents != agents else { return .none } + state.agents = agents + return .none + + case .progressDisplayChanged(let display): + guard state.progressDisplay != display else { return .none } + state.progressDisplay = display + return .none + } + } + } +} + +/// Stripe-progress visualization payload. Per-tab summary of underlying +/// `GhosttySurfaceState.progressState` so the tab-bar stripe stays in lock-step +/// with the tab's focus state. +struct TerminalTabProgressDisplay: Equatable, Sendable { + enum Style: Equatable, Sendable { + case error + case paused + case indeterminate + case determinate(percent: Int) + } + + let style: Style + /// Accessibility value spoken alongside the tab title ("Busy", "Errored", + /// "Paused", "47 percent complete"). Read from `TerminalTabView.accessibilityValue`. + var accessibilityValue: String { + switch style { + case .error: return "Errored" + case .paused: return "Paused" + case .indeterminate: return "Busy" + case .determinate(let percent): return "\(percent) percent complete" + } + } +} + +extension TerminalTabProgressDisplay { + /// Project a Ghostty per-surface progress payload into the per-tab style. + /// Returns nil for the REMOVE state and for nil input (no progress in flight). + static func make( + progressState: ghostty_action_progress_report_state_e?, + progressValue: Int? + ) -> TerminalTabProgressDisplay? { + guard let progressState, progressState != GHOSTTY_PROGRESS_STATE_REMOVE else { return nil } + let style: Style + switch progressState { + case GHOSTTY_PROGRESS_STATE_ERROR: style = .error + case GHOSTTY_PROGRESS_STATE_PAUSE: style = .paused + case GHOSTTY_PROGRESS_STATE_INDETERMINATE: style = .indeterminate + default: + if let percent = progressValue { style = .determinate(percent: percent) } else { style = .indeterminate } + } + return TerminalTabProgressDisplay(style: style) + } + + /// Worst-of priority for aggregating across surfaces in an unfocused tab. + /// Higher rank wins. error > paused > determinate > indeterminate > none. + var severity: Int { + switch style { + case .error: return 4 + case .paused: return 3 + case .determinate: return 2 + case .indeterminate: return 1 + } + } +} diff --git a/supacode/Features/Terminal/Reducer/TerminalsFeature.swift b/supacode/Features/Terminal/Reducer/TerminalsFeature.swift new file mode 100644 index 000000000..1b780f5d0 --- /dev/null +++ b/supacode/Features/Terminal/Reducer/TerminalsFeature.swift @@ -0,0 +1,94 @@ +import ComposableArchitecture +import Foundation + +/// Owns the collection of per-tab `TerminalTabFeature` states. Mirrors the +/// sidebar's `RepositoriesFeature` ownership of `sidebarItems`. Views scope +/// through `store.scope(state: \.terminals, action: \.terminals)` so tab-bar +/// surface area stays bounded to terminal state instead of the whole app. +@Reducer +struct TerminalsFeature { + /// Bounded recent-removal memory. A late `tabProjectionChanged` emit landing + /// after `tabRemoved` would otherwise re-insert a phantom tab; tracking the + /// most recent removals lets the reducer drop those stragglers. + static let recentlyRemovedTabLimit = 128 + + /// Removed-tab record keyed by `(worktreeID, tabID)`. Same-session + /// snapshot-restore reuses the persisted `tabSnapshot.id`, so scoping the + /// dedup by worktree lets the FIFO drain when its owning worktree's state + /// is torn down without shadowing a legitimate re-add. + struct RecentlyRemovedTab: Equatable, Sendable { + let worktreeID: Worktree.ID + let tabID: TerminalTabID + } + + @ObservableState + struct State: Equatable { + /// Per-tab feature instances keyed by `TerminalTabID`. Tab-bar leaves + /// scope through `\.terminalTabs[id:]` for per-tab observation isolation + /// during agent storms. + var terminalTabs: IdentifiedArrayOf = [] + /// FIFO of recently-removed tabs scoped by `(worktreeID, tabID)`. Insert + /// order = removal order; oldest entry is dropped when the cap is hit. + var recentlyRemovedTabIDs: [RecentlyRemovedTab] = [] + } + + enum Action { + case terminalTabs(IdentifiedActionOf) + /// Tab projection arrived from `WorktreeTerminalState`. Inserts a new + /// per-tab state if missing, then forwards to the tab's reducer. + case tabProjectionChanged(worktreeID: Worktree.ID, projection: WorktreeTabProjection) + /// Tab destroyed in the worktree state. Drops the matching feature state. + case tabRemoved(worktreeID: Worktree.ID, tabID: TerminalTabID) + /// Worktree's entire terminal state was torn down (prune path). Drops any + /// orphan `terminalTabs` rows and removed-tab FIFO records for this + /// worktree so a same-session re-attach starts clean. + case worktreeStateTornDown(worktreeID: Worktree.ID) + } + + var body: some Reducer { + Reduce { state, action in + switch action { + case .terminalTabs: + return .none + + case .tabProjectionChanged(let worktreeID, let projection): + let tabID = projection.tabID + if state.terminalTabs[id: tabID] == nil { + // Drop stale projections arriving after the tab was removed in this + // worktree. Matching by (worktreeID, tabID) so a snapshot-restore + // under a different worktree wouldn't be shadowed; the per-worktree + // drain on teardown covers the same-worktree restore case. + guard + !state.recentlyRemovedTabIDs.contains(where: { + $0.worktreeID == worktreeID && $0.tabID == tabID + }) + else { return .none } + state.terminalTabs.append( + TerminalTabFeature.State(id: tabID, worktreeID: worktreeID) + ) + } + return .send(.terminalTabs(.element(id: tabID, action: .projectionChanged(projection)))) + + case .tabRemoved(let worktreeID, let tabID): + state.terminalTabs.remove(id: tabID) + state.recentlyRemovedTabIDs.append( + RecentlyRemovedTab(worktreeID: worktreeID, tabID: tabID) + ) + if state.recentlyRemovedTabIDs.count > Self.recentlyRemovedTabLimit { + state.recentlyRemovedTabIDs.removeFirst( + state.recentlyRemovedTabIDs.count - Self.recentlyRemovedTabLimit + ) + } + return .none + + case .worktreeStateTornDown(let worktreeID): + state.recentlyRemovedTabIDs.removeAll { $0.worktreeID == worktreeID } + state.terminalTabs.removeAll { $0.worktreeID == worktreeID } + return .none + } + } + .forEach(\.terminalTabs, action: \.terminalTabs) { + TerminalTabFeature() + } + } +} diff --git a/supacode/Features/Terminal/TabBar/TerminalTabBarColors.swift b/supacode/Features/Terminal/TabBar/TerminalTabBarColors.swift index 95b4d5419..95deb8f9a 100644 --- a/supacode/Features/Terminal/TabBar/TerminalTabBarColors.swift +++ b/supacode/Features/Terminal/TabBar/TerminalTabBarColors.swift @@ -32,6 +32,13 @@ struct SurfaceChromeAppearance: Equatable { var separatorOpacity: Double { colorScheme == .dark ? 0.22 : 0.14 } + + /// Opacity for the "secondary" stripe paint used on tabs that carry no + /// custom tint and no progress state. Tuned so the active tab's top stripe + /// stays visible against the chrome without competing with tinted tabs. + var secondaryAccentOpacity: Double { + colorScheme == .dark ? 0.45 : 0.35 + } } private struct SurfaceChromeAppearanceKey: EnvironmentKey { diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabBackground.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabBackground.swift index 36ee41b15..57c448d44 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabBackground.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabBackground.swift @@ -1,41 +1,28 @@ import SupacodeSettingsShared import SwiftUI +/// Background fill + inactive-tab bottom separator. The top stripe is moved +/// to a `TerminalTabView` overlay so it can paint over adjacent dividers +/// without being clipped by the tab's `clipShape`. struct TerminalTabBackground: View { var isActive: Bool var isHovering: Bool var isPressing: Bool var isDragging: Bool - var tintColor: RepositoryColor? @Environment(\.surfaceChromeAppearance) private var chromeAppearance + @Environment(\.pixelLength) + private var pixelLength var body: some View { Color.clear - .overlay(alignment: .top) { - Rectangle() - .fill(tintColor?.color ?? .accentColor) - .frame(height: TerminalTabBarMetrics.activeIndicatorHeight) - .opacity(stripeOpacity) - } .overlay(alignment: .bottom) { if !isActive { Rectangle() .fill(chromeAppearance.overlayTint.opacity(chromeAppearance.separatorOpacity)) - .frame(height: 1) + .frame(height: pixelLength) } } } - - private var stripeOpacity: Double { - guard !isActive else { return 1 } - guard tintColor != nil else { return 0 } - // Mirror `TerminalTabView.contentOpacity` so a press/drag on a tinted - // inactive tab snaps the stripe to full at the same time as the content. - if isPressing || isDragging { return 1 } - return isHovering - ? TerminalTabBarMetrics.inactiveContentOpacityHover - : TerminalTabBarMetrics.inactiveContentOpacityIdle - } } diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabBarView.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabBarView.swift index 857456adc..2d1f21086 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabBarView.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabBarView.swift @@ -1,8 +1,11 @@ +import ComposableArchitecture import SupacodeSettingsShared import SwiftUI struct TerminalTabBarView: View { @Bindable var manager: TerminalTabManager + let terminalState: WorktreeTerminalState + let terminalsStore: StoreOf let createTab: () -> Void let split: (TerminalSplitMenuDirection) -> Void let canSplit: Bool @@ -11,8 +14,6 @@ struct TerminalTabBarView: View { let closeToRight: (TerminalTabID) -> Void let closeAll: () -> Void let renameTab: (TerminalTabID, String) -> Void - let hasNotification: (TerminalTabID) -> Bool - let agentsForTab: (TerminalTabID) -> [AgentPresenceFeature.AgentInstance] @Environment(\.controlActiveState) private var controlActiveState @@ -20,13 +21,13 @@ struct TerminalTabBarView: View { HStack(spacing: 0) { TerminalTabsView( manager: manager, + terminalState: terminalState, + terminalsStore: terminalsStore, closeTab: closeTab, closeOthers: closeOthers, closeToRight: closeToRight, closeAll: closeAll, renameTab: renameTab, - hasNotification: hasNotification, - agentsForTab: agentsForTab, ) Spacer(minLength: 0) TerminalTabBarTrailingAccessories( diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabDivider.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabDivider.swift index fda85db7f..83e1c9853 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabDivider.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabDivider.swift @@ -3,11 +3,13 @@ import SwiftUI struct TerminalTabDivider: View { @Environment(\.surfaceChromeAppearance) private var chromeAppearance + @Environment(\.pixelLength) + private var pixelLength var body: some View { Rectangle() - .frame(width: 1) + .fill(chromeAppearance.overlayTint.opacity(chromeAppearance.separatorOpacity)) + .frame(width: pixelLength) .frame(height: TerminalTabBarMetrics.tabHeight) - .foregroundStyle(chromeAppearance.overlayTint.opacity(chromeAppearance.separatorOpacity)) } } diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabLabelView.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabLabelView.swift index 699a110fb..7e10ab4e2 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabLabelView.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabLabelView.swift @@ -1,3 +1,4 @@ +import ComposableArchitecture import SupacodeSettingsShared import SwiftUI @@ -8,14 +9,14 @@ struct TerminalTabLabelView: View { let isHoveringClose: Bool let shortcutHint: String? let showsShortcutHint: Bool - let agents: [AgentPresenceFeature.AgentInstance] + /// Per-tab scoped store. The badge subview observes `state.agents` here + /// instead of iterating worktree-wide presence, so an agent storm on tab B + /// doesn't invalidate tab A's label body. + let tabStore: StoreOf var body: some View { HStack(spacing: TerminalTabBarMetrics.contentSpacing) { - if !agents.isEmpty { - AgentAvatarGroupView(instances: agents, size: 14) - .padding(.trailing, 2) - } + TerminalTabAgentBadge(tabStore: tabStore) if let icon = tab.icon { Image(systemName: icon) .imageScale(.small) @@ -33,11 +34,6 @@ struct TerminalTabLabelView: View { .foregroundStyle(TerminalTabBarColors.activeText) .shimmer(isActive: tab.isDirty) Spacer(minLength: TerminalTabBarMetrics.contentTrailingSpacing) - ZStack { - if showsShortcutHint, let shortcutHint { - ShortcutHintView(text: shortcutHint, color: TerminalTabBarColors.inactiveText) - } - } } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .leading) .contentShape(.rect) @@ -45,3 +41,27 @@ struct TerminalTabLabelView: View { .padding(.trailing, TerminalTabBarMetrics.closeButtonSize + TerminalTabBarMetrics.contentSpacing) } } + +/// Reads agent presence off the per-tab scoped store, so an agent storm on +/// tab B invalidates only tab B's badge leaf. Mirrors sidebar's +/// `RunningAgentsBadgeContent` pattern with the inner Equatable wrapper. +private struct TerminalTabAgentBadge: View { + let tabStore: StoreOf + + var body: some View { + let agents = tabStore.state.agents + if !agents.isEmpty { + TerminalTabAgentBadgeContent(agents: agents) + .equatable() + } + } +} + +private struct TerminalTabAgentBadgeContent: View, Equatable { + let agents: [AgentPresenceFeature.AgentInstance] + + var body: some View { + AgentAvatarGroupView(instances: agents, size: 14) + .padding(.trailing, 2) + } +} diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabProgressStripe.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabProgressStripe.swift new file mode 100644 index 000000000..698c02236 --- /dev/null +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabProgressStripe.swift @@ -0,0 +1,113 @@ +import ComposableArchitecture +import SupacodeSettingsShared +import SwiftUI + +/// Top-of-tab colored stripe carrying both the tint indicator and the OSC-9 +/// progress signal. Rendered as an overlay above the tab's `clipShape` so it +/// can paint over the adjacent `TerminalTabDivider`s via `-pixelLength` +/// horizontal padding, making the active tab read continuous across boundaries. +struct TerminalTabProgressStripe: View { + let isActive: Bool + let isHovering: Bool + let isPressing: Bool + let isDragging: Bool + let tintColor: RepositoryColor? + let tabStore: StoreOf + + @Environment(\.pixelLength) private var pixelLength + @Environment(\.surfaceChromeAppearance) private var chromeAppearance + + var body: some View { + let progressDisplay = tabStore.state.progressDisplay + let color = strokeColor(progressDisplay: progressDisplay) + StripeBody( + color: color, + progressDisplay: progressDisplay, + opacity: stripeOpacity(progressDisplay: progressDisplay), + pixelLength: pixelLength + ) + } + + /// True when the stripe carries no progress signal and no tab tint, in + /// which case it paints a chrome-aware secondary tone instead of accent. + private var isSecondaryFallback: Bool { + tintColor == nil + } + + private func stripeOpacity(progressDisplay: TerminalTabProgressDisplay?) -> Double { + let hasProgress = progressDisplay != nil + // Secondary-fallback active tab paints at the chrome-aware opacity so it + // doesn't compete with tinted tabs. Tinted or progress-carrying tabs + // still paint at full opacity when active. + if isActive { + if isSecondaryFallback, !hasProgress { + return chromeAppearance.secondaryAccentOpacity + } + return 1 + } + // Inactive untinted tabs with no progress signal stay hidden. + guard tintColor != nil || hasProgress else { return 0 } + if isPressing || isDragging { return 1 } + return isHovering + ? TerminalTabBarMetrics.inactiveContentOpacityHover + : TerminalTabBarMetrics.inactiveContentOpacityIdle + } + + /// Resolves the stripe's primary color. Progress states override the tab + /// tint; the no-tint / no-progress fallback paints the chrome overlay tint + /// (white on dark chrome, black on light chrome) so the active tab's + /// indicator stays visible without an accent-color flash. + private func strokeColor(progressDisplay: TerminalTabProgressDisplay?) -> Color { + switch progressDisplay?.style { + case .error: return .red + case .paused: return .orange + case .indeterminate, .determinate: + return tintColor?.color ?? .accentColor + case .none: + return tintColor?.color ?? chromeAppearance.overlayTint + } + } +} + +private struct StripeBody: View { + let color: Color + let progressDisplay: TerminalTabProgressDisplay? + let opacity: Double + let pixelLength: CGFloat + + var body: some View { + GeometryReader { proxy in + ZStack(alignment: .leading) { + StripeBase(progressDisplay: progressDisplay, color: color) + if case .determinate(let percent) = progressDisplay?.style { + Rectangle() + .fill(color) + .frame(width: proxy.size.width * CGFloat(max(0, min(percent, 100))) / 100) + .animation(.easeInOut(duration: 0.2), value: percent) + } + } + } + .frame(height: TerminalTabBarMetrics.activeIndicatorHeight) + .padding(.horizontal, -pixelLength) + .opacity(opacity) + .allowsHitTesting(false) + } +} + +/// Background of the stripe. ERROR / PAUSE / INDETERMINATE paint the color +/// directly; the determinate variant paints a faded base behind the percent +/// fill so the partial-fill bar reads against a backdrop. +private struct StripeBase: View { + let progressDisplay: TerminalTabProgressDisplay? + let color: Color + + var body: some View { + Rectangle() + .fill(isDeterminate ? color.opacity(0.3) : color) + } + + private var isDeterminate: Bool { + if case .determinate = progressDisplay?.style { return true } + return false + } +} diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabView.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabView.swift index ace3396ec..89dc26f63 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabView.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabView.swift @@ -1,4 +1,5 @@ import AppKit +import ComposableArchitecture import SupacodeSettingsShared import SwiftUI @@ -8,8 +9,10 @@ struct TerminalTabView: View { let isDragging: Bool let tabIndex: Int let fixedWidth: CGFloat? - let hasNotification: Bool - let agents: [AgentPresenceFeature.AgentInstance] + /// Per-tab scoped store. The view body reads exclusively from this for + /// observation-tracked fields (agents, unseen count, progress display) so + /// per-tab mutations invalidate only this leaf, not sibling tabs. + let tabStore: StoreOf let onSelect: () -> Void let onClose: () -> Void let onRename: (String) -> Void @@ -38,7 +41,7 @@ struct TerminalTabView: View { isHoveringClose: isHoveringClose, shortcutHint: shortcutHint, showsShortcutHint: showsShortcutHint, - agents: agents, + tabStore: tabStore, ) } .buttonStyle(TerminalTabButtonStyle(isPressing: $isPressing)) @@ -52,28 +55,36 @@ struct TerminalTabView: View { .contentShape(.rect) .help("Open tab \(tab.displayTitle)") .accessibilityLabel(tab.displayTitle) + .accessibilityValue(accessibilityValue) .allowsHitTesting(!isEditing) .opacity(isEditing ? 0 : 1) - // The dot and close X share the same trailing slot: the dot is - // visible when the tab is idle and has unread notifications, the - // close button replaces it on hover. `TerminalTabCloseButton` already - // owns the hover visibility — we mirror it inverted here. + // Trailing slot: dot OR hotkey hint OR close button. + // Priority: hover wins (close button beats ⌘ hint); + // ⌘ pressed without hover shows the hint; otherwise the notification dot. + // `.fontWeight(.regular)` is explicit because the tab bar lacks the sidebar's + // List/vibrancy context, where `.font(.caption)` would otherwise render heavier. ZStack { - TabNotificationDot() - .opacity(isShowingNotificationDot ? 1 : 0) - .allowsHitTesting(false) + TerminalTabNotificationIndicator( + tabStore: tabStore, + suppress: isHovering || isHoveringClose || isDragging || isShowingHint + ) + if isShowingHint, let shortcutHint { + Text(shortcutHint) + .font(.caption) + .fontWeight(.regular) + .foregroundStyle(.secondary) + } TerminalTabCloseButton( isHoveringTab: isHovering, isDragging: isDragging, - isShowingShortcutHint: showsShortcutHint, + isShowingShortcutHint: isShowingHint, closeAction: onClose, closeButtonGestureActive: $closeButtonGestureActive, isHoveringClose: $isHoveringClose ) } .animation(.easeInOut(duration: TerminalTabBarMetrics.hoverAnimationDuration), value: isHovering) - .animation(.easeInOut(duration: 0.2), value: hasNotification) .padding(.trailing, TerminalTabBarMetrics.tabHorizontalPadding) .opacity(isEditing ? 0 : 1) .allowsHitTesting(!isEditing) @@ -110,8 +121,7 @@ struct TerminalTabView: View { isActive: isActive, isHovering: isHovering, isPressing: isPressing, - isDragging: isDragging, - tintColor: tab.tintColor + isDragging: isDragging ) } // Below `.background` so the stripe's opacity animates in lockstep with @@ -129,6 +139,19 @@ struct TerminalTabView: View { .padding(.bottom, isActive ? TerminalTabBarMetrics.activeTabBottomPadding : 0) .offset(y: isActive ? TerminalTabBarMetrics.activeTabOffset : 0) .clipShape(.rect(cornerRadius: TerminalTabBarMetrics.tabCornerRadius)) + // Stripe overlay sits AFTER `clipShape` with negative horizontal padding + // so the tint paints over adjacent dividers; clipping otherwise leaves a + // 1px gray notch at each side. + .overlay(alignment: .top) { + TerminalTabProgressStripe( + isActive: isActive, + isHovering: isHovering, + isPressing: isPressing, + isDragging: isDragging, + tintColor: tab.tintColor, + tabStore: tabStore + ) + } .contentShape(.rect) .onHover { hovering in isHovering = hovering @@ -198,8 +221,34 @@ struct TerminalTabView: View { commandKeyObserver.isPressed && shortcutHint != nil } - private var isShowingNotificationDot: Bool { - hasNotification && !isHovering && !isHoveringClose && !isDragging && !showsShortcutHint + /// True when the cmd-pressed hotkey hint should occupy the trailing slot. + /// Hover wins: when the user is over the tab the close button takes the + /// slot regardless of whether ⌘ is also pressed. + private var isShowingHint: Bool { + showsShortcutHint && !isHovering && !isDragging + } + + /// State-aware accessibility value for VoiceOver. Restores the OSC-9 progress + /// signal lost when `GhosttySurfaceProgressBar` was deleted: announces + /// "Errored", "Paused", "Busy", or "47 percent complete" on the busy tab. + private var accessibilityValue: String { + tabStore.state.progressDisplay?.accessibilityValue ?? "" + } +} + +/// Reads the tab's unread notification count off the per-tab scoped store. +/// `suppress` short-circuits the count check when the dot would be hidden +/// anyway (hover, drag, shortcut hint). +private struct TerminalTabNotificationIndicator: View { + let tabStore: StoreOf + let suppress: Bool + + var body: some View { + let isShowing = !suppress && tabStore.state.hasUnseenNotifications + TabNotificationDot() + .opacity(isShowing ? 1 : 0) + .allowsHitTesting(false) + .animation(.easeInOut(duration: 0.2), value: isShowing) } } diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift index 758e4cde7..c62023d35 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabsRowView.swift @@ -1,8 +1,11 @@ +import ComposableArchitecture import SupacodeSettingsShared import SwiftUI struct TerminalTabsRowView: View { @Bindable var manager: TerminalTabManager + let terminalState: WorktreeTerminalState + let terminalsStore: StoreOf @Binding var openedTabs: [TerminalTabID] @Binding var tabLocations: [TerminalTabID: CGRect] @Binding var draggingTabId: TerminalTabID? @@ -14,8 +17,6 @@ struct TerminalTabsRowView: View { let closeToRight: (TerminalTabID) -> Void let closeAll: () -> Void let renameTab: (TerminalTabID, String) -> Void - let hasNotification: (TerminalTabID) -> Bool - let agentsForTab: (TerminalTabID) -> [AgentPresenceFeature.AgentInstance] let scrollReader: ScrollViewProxy @State private var dropTargetIndex: Int? @@ -25,15 +26,19 @@ struct TerminalTabsRowView: View { ZStack(alignment: .topLeading) { HStack(alignment: .center, spacing: TerminalTabBarMetrics.tabSpacing) { ForEach(Array(openedTabs.enumerated()), id: \.element) { index, id in - if let item = manager.tabs.first(where: { $0.id == id }) { + if let item = manager.tabs.first(where: { $0.id == id }), + let tabStore = terminalsStore.scope( + state: \.terminalTabs[id: id], + action: \.terminalTabs[id: id] + ) + { TerminalTabView( tab: item, isActive: manager.selectedTabId == id, isDragging: draggingTabId == id, tabIndex: index, fixedWidth: fixedTabWidth, - hasNotification: hasNotification(id), - agents: agentsForTab(id), + tabStore: tabStore, onSelect: { manager.selectTab(id) }, diff --git a/supacode/Features/Terminal/TabBar/Views/TerminalTabsView.swift b/supacode/Features/Terminal/TabBar/Views/TerminalTabsView.swift index 9ebbecd50..520b39e23 100644 --- a/supacode/Features/Terminal/TabBar/Views/TerminalTabsView.swift +++ b/supacode/Features/Terminal/TabBar/Views/TerminalTabsView.swift @@ -1,15 +1,16 @@ +import ComposableArchitecture import SupacodeSettingsShared import SwiftUI struct TerminalTabsView: View { @Bindable var manager: TerminalTabManager + let terminalState: WorktreeTerminalState + let terminalsStore: StoreOf let closeTab: (TerminalTabID) -> Void let closeOthers: (TerminalTabID) -> Void let closeToRight: (TerminalTabID) -> Void let closeAll: () -> Void let renameTab: (TerminalTabID, String) -> Void - let hasNotification: (TerminalTabID) -> Bool - let agentsForTab: (TerminalTabID) -> [AgentPresenceFeature.AgentInstance] @State private var draggingTabId: TerminalTabID? @State private var draggingStartLocation: CGFloat? @@ -27,6 +28,8 @@ struct TerminalTabsView: View { ScrollView(.horizontal) { TerminalTabsRowView( manager: manager, + terminalState: terminalState, + terminalsStore: terminalsStore, openedTabs: $openedTabs, tabLocations: $tabLocations, draggingTabId: $draggingTabId, @@ -38,8 +41,6 @@ struct TerminalTabsView: View { closeToRight: closeToRight, closeAll: closeAll, renameTab: renameTab, - hasNotification: hasNotification, - agentsForTab: agentsForTab, scrollReader: scrollReader ) .padding(.horizontal, TerminalTabBarMetrics.barPadding) diff --git a/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift b/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift deleted file mode 100644 index 1d0e699ef..000000000 --- a/supacode/Features/Terminal/Views/GhosttySurfaceProgressBar.swift +++ /dev/null @@ -1,70 +0,0 @@ -import GhosttyKit -import SwiftUI - -struct GhosttySurfaceProgressBar: View { - let progressState: ghostty_action_progress_report_state_e - let progressValue: Int? - - var body: some View { - let color: Color = - switch progressState { - case GHOSTTY_PROGRESS_STATE_ERROR: .red - case GHOSTTY_PROGRESS_STATE_PAUSE: .orange - default: .accentColor - } - let progress: Int? = - progressValue ?? (progressState == GHOSTTY_PROGRESS_STATE_PAUSE ? 100 : nil) - let accessibilityLabel: String = - switch progressState { - case GHOSTTY_PROGRESS_STATE_ERROR: "Terminal progress - Error" - case GHOSTTY_PROGRESS_STATE_PAUSE: "Terminal progress - Paused" - case GHOSTTY_PROGRESS_STATE_INDETERMINATE: "Terminal progress - In progress" - default: "Terminal progress" - } - let accessibilityValue: String = - if let progress { - "\(progress) percent complete" - } else { - switch progressState { - case GHOSTTY_PROGRESS_STATE_ERROR: "Operation failed" - case GHOSTTY_PROGRESS_STATE_PAUSE: "Operation paused at completion" - case GHOSTTY_PROGRESS_STATE_INDETERMINATE: "Operation in progress" - default: "Indeterminate progress" - } - } - - GeometryReader { geometry in - ZStack(alignment: .leading) { - if let progress { - Rectangle() - .fill(color) - .frame( - width: geometry.size.width * CGFloat(progress) / 100, - height: geometry.size.height - ) - .animation(.easeInOut(duration: 0.2), value: progress) - } else { - ZStack(alignment: .leading) { - Rectangle() - .fill(color.opacity(0.3)) - Rectangle() - .fill(color) - .frame(width: geometry.size.width * 0.25, height: geometry.size.height) - .phaseAnimator([false, true]) { content, moved in - content.offset(x: moved ? geometry.size.width * 0.75 : 0) - } animation: { _ in - .easeInOut(duration: 1.2) - } - } - } - } - } - .frame(height: 2) - .clipped() - .allowsHitTesting(false) - .accessibilityElement(children: .ignore) - .accessibilityAddTraits(.updatesFrequently) - .accessibilityLabel(accessibilityLabel) - .accessibilityValue(accessibilityValue) - } -} diff --git a/supacode/Features/Terminal/Views/GhosttySurfaceProgressOverlay.swift b/supacode/Features/Terminal/Views/GhosttySurfaceProgressOverlay.swift deleted file mode 100644 index 1027f5931..000000000 --- a/supacode/Features/Terminal/Views/GhosttySurfaceProgressOverlay.swift +++ /dev/null @@ -1,20 +0,0 @@ -import GhosttyKit -import SwiftUI - -struct GhosttySurfaceProgressOverlay: View { - @Bindable var state: GhosttySurfaceState - - init(state: GhosttySurfaceState) { - self._state = Bindable(state) - } - - var body: some View { - if let progressState = state.progressState, progressState != GHOSTTY_PROGRESS_STATE_REMOVE { - GhosttySurfaceProgressBar( - progressState: progressState, - progressValue: state.progressValue - ) - .transition(.opacity) - } - } -} diff --git a/supacode/Features/Terminal/Views/TerminalSplitTreeView.swift b/supacode/Features/Terminal/Views/TerminalSplitTreeView.swift index 2f95c453c..64d3a0197 100644 --- a/supacode/Features/Terminal/Views/TerminalSplitTreeView.swift +++ b/supacode/Features/Terminal/Views/TerminalSplitTreeView.swift @@ -142,9 +142,6 @@ struct TerminalSplitTreeView: View { .allowsHitTesting(false) } } - .overlay(alignment: .top) { - GhosttySurfaceProgressOverlay(state: surfaceView.bridge.state) - } .overlay(alignment: .topTrailing) { if surfaceView.bridge.state.searchNeedle != nil { GhosttySurfaceSearchOverlay(surfaceView: surfaceView) diff --git a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift index 25517ccf5..1c32c5d1e 100644 --- a/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift +++ b/supacode/Features/Terminal/Views/WorktreeTerminalTabsView.swift @@ -5,11 +5,13 @@ import SwiftUI struct WorktreeTerminalTabsView: View { let worktree: Worktree let manager: WorktreeTerminalManager + /// Narrowed terminal-orchestration store. The tab bar scopes per-tab + /// `TerminalTabFeature` stores via `\.terminalTabs[id:]` from here, so the + /// tab-bar surface area stays bounded to terminal state. + let terminalsStore: StoreOf let shouldRunSetupScript: Bool let forceAutoFocus: Bool let createTab: () -> Void - let agentPresence: AgentPresenceFeature.State - let agentBadgesEnabled: Bool @State private var windowActivity = WindowActivityState.inactive // Reading the chrome appearance env makes SwiftUI invalidate this body when // `WindowTintColorScheme` republishes after a Ghostty config reload, so the @@ -24,6 +26,8 @@ struct WorktreeTerminalTabsView: View { if !state.shouldHideTabBar { TerminalTabBarView( manager: state.tabManager, + terminalState: state, + terminalsStore: terminalsStore, createTab: createTab, split: { direction in _ = state.performBindingActionOnFocusedSurface(direction.ghosttyBinding) @@ -44,15 +48,6 @@ struct WorktreeTerminalTabsView: View { renameTab: { tabId, newTitle in state.tabManager.setCustomTitle(tabId, title: newTitle) }, - hasNotification: { tabId in - state.hasUnseenNotification(forTabID: tabId) - }, - agentsForTab: { tabId in - agentPresence.agents( - across: state.surfaceIDs(inTab: tabId), - badgesEnabled: agentBadgesEnabled, - ) - }, ) .transition(.move(edge: .top).combined(with: .opacity)) } diff --git a/supacodeTests/AppFeatureRunScriptTests.swift b/supacodeTests/AppFeatureRunScriptTests.swift index 6b3b3f8cf..cf4a7c502 100644 --- a/supacodeTests/AppFeatureRunScriptTests.swift +++ b/supacodeTests/AppFeatureRunScriptTests.swift @@ -111,6 +111,7 @@ struct AppFeatureRunScriptTests { // Pre-populate running state to simulate an already-running script. initialState.repositories.sidebarItems[id: worktree.id]?.runningScripts[id: definition.id] = .init(id: definition.id, tint: definition.resolvedTintColor) + initialState.repositories.applyPostReduceCacheRecomputes() let sent = LockIsolated<[TerminalClient.Command]>([]) let store = TestStore(initialState: initialState) { AppFeature() diff --git a/supacodeTests/AppFeatureTerminalSetupScriptTests.swift b/supacodeTests/AppFeatureTerminalSetupScriptTests.swift index ce0b9a64d..c6b6fbaf9 100644 --- a/supacodeTests/AppFeatureTerminalSetupScriptTests.swift +++ b/supacodeTests/AppFeatureTerminalSetupScriptTests.swift @@ -35,6 +35,7 @@ struct AppFeatureTerminalSetupScriptTests { await store.receive(\.repositories.consumeSetupScript) await store.receive(\.repositories.sidebarItems) { $0.repositories.sidebarItems[id: worktree.id]?.lifecycle = .idle + $0.repositories.applyPostReduceCacheRecomputes([.sidebarStructure, .selectedWorktreeSlice]) } await store.finish() #expect(sent.value == [.createTab(worktree, runSetupScriptIfNew: true, id: nil)]) @@ -107,6 +108,7 @@ struct AppFeatureTerminalSetupScriptTests { await store.receive(\.repositories.consumeSetupScript) await store.receive(\.repositories.sidebarItems) { $0.repositories.sidebarItems[id: worktree.id]?.lifecycle = .idle + $0.repositories.applyPostReduceCacheRecomputes([.sidebarStructure, .selectedWorktreeSlice]) } await store.finish() } @@ -200,6 +202,7 @@ struct AppFeatureTerminalSetupScriptTests { repositoriesState.reconcileSidebarForTesting() if pendingSetupScript { repositoriesState.sidebarItems[id: worktree.id]?.lifecycle = .pending + repositoriesState.applyPostReduceCacheRecomputes() } return repositoriesState } diff --git a/supacodeTests/RepositoriesFeatureCustomizationTests.swift b/supacodeTests/RepositoriesFeatureCustomizationTests.swift index 45976fe0f..fd0c346ef 100644 --- a/supacodeTests/RepositoriesFeatureCustomizationTests.swift +++ b/supacodeTests/RepositoriesFeatureCustomizationTests.swift @@ -95,7 +95,7 @@ struct RepositoriesFeatureCustomizationTests { sidebar.sections[self.repoID, default: .init()].title = "Renamed" sidebar.sections[self.repoID, default: .init()].color = .red } - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } } diff --git a/supacodeTests/RepositoriesFeatureTests.swift b/supacodeTests/RepositoriesFeatureTests.swift index 0d5b92bbc..0b1e114bd 100644 --- a/supacodeTests/RepositoriesFeatureTests.swift +++ b/supacodeTests/RepositoriesFeatureTests.swift @@ -164,6 +164,7 @@ struct RepositoriesFeatureTests { await store.send(.selectWorktree(worktree.id)) { $0.selection = .worktree(worktree.id) $0.sidebarSelectedWorktreeIDs = [worktree.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -184,6 +185,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt2.id) $0.sidebarSelectedWorktreeIDs = [wt2.id] $0.worktreeHistoryBackStack = [wt1.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -263,6 +265,7 @@ struct RepositoriesFeatureTests { await store.send(.selectionChanged([])) { $0.selection = nil $0.sidebarSelectedWorktreeIDs = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -281,6 +284,7 @@ struct RepositoriesFeatureTests { await store.send(.selectionChanged([.archivedWorktrees])) { $0.selection = .archivedWorktrees $0.sidebarSelectedWorktreeIDs = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -296,12 +300,12 @@ struct RepositoriesFeatureTests { await store.send(.repositoryExpansionChanged(repository.id, isExpanded: false)) { $0.$sidebar.withLock { $0.sections[repository.id, default: .init()].collapsed = true } - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } await store.send(.repositoryExpansionChanged(repository.id, isExpanded: true)) { $0.$sidebar.withLock { $0.sections[repository.id, default: .init()].collapsed = false } - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } } @@ -316,7 +320,7 @@ struct RepositoriesFeatureTests { await store.send(.repositoryExpansionChanged(repository.id, isExpanded: false)) { $0.$sidebar.withLock { $0.sections[repository.id, default: .init()].collapsed = true } - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } // Collapsing again should be a no-op. @@ -337,6 +341,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt2.id) $0.sidebarSelectedWorktreeIDs = [wt2.id] $0.worktreeHistoryBackStack = [wt1.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) #expect(store.state.sidebarItems.allSatisfy { !$0.shouldFocusTerminal }) @@ -417,6 +422,7 @@ struct RepositoriesFeatureTests { await store.send(.selectionChanged([.worktree("/tmp/unknown")])) { $0.selection = nil $0.sidebarSelectedWorktreeIDs = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -434,6 +440,7 @@ struct RepositoriesFeatureTests { await store.send(.selectionChanged([.archivedWorktrees, .worktree(worktree.id)])) { $0.selection = .archivedWorktrees $0.sidebarSelectedWorktreeIDs = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -459,11 +466,11 @@ struct RepositoriesFeatureTests { // bit flips on the targeted section. await store.send(.repositoryExpansionChanged(repoB.id, isExpanded: false)) { $0.$sidebar.withLock { $0.sections[repoB.id, default: .init()].collapsed = true } - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } await store.send(.repositoryExpansionChanged(repoA.id, isExpanded: false)) { $0.$sidebar.withLock { $0.sections[repoA.id, default: .init()].collapsed = true } - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } } @@ -1076,7 +1083,7 @@ struct RepositoriesFeatureTests { ) ) { $0.alert = expectedAlert - $0.recomputeSidebarStructureIfChanged() + $0.applyPostReduceCacheRecomputes() } await store.finish() #expect(removed.value == false) @@ -2199,7 +2206,7 @@ struct RepositoriesFeatureTests { state.reconcileSidebarForTesting() state.sidebarItems[id: worktree.id]?.runningScripts[id: definition.id] = .init(id: definition.id, tint: definition.resolvedTintColor) - state.recomputeSidebarStructureIfChanged() + state.applyPostReduceCacheRecomputes() let store = TestStore(initialState: state) { RepositoriesFeature() @@ -2237,7 +2244,7 @@ struct RepositoriesFeatureTests { state.reconcileSidebarForTesting() state.sidebarItems[id: worktree.id]?.runningScripts[id: definition.id] = .init(id: definition.id, tint: definition.resolvedTintColor) - state.recomputeSidebarStructureIfChanged() + state.applyPostReduceCacheRecomputes() let store = TestStore(initialState: state) { RepositoriesFeature() @@ -2268,7 +2275,7 @@ struct RepositoriesFeatureTests { state.reconcileSidebarForTesting() state.sidebarItems[id: worktree.id]?.runningScripts[id: definition.id] = .init(id: definition.id, tint: definition.resolvedTintColor) - state.recomputeSidebarStructureIfChanged() + state.applyPostReduceCacheRecomputes() let store = TestStore(initialState: state) { RepositoriesFeature() @@ -2337,7 +2344,7 @@ struct RepositoriesFeatureTests { .init(id: completing.id, tint: completing.resolvedTintColor) state.sidebarItems[id: worktree.id]?.runningScripts[id: surviving.id] = .init(id: surviving.id, tint: surviving.resolvedTintColor) - state.recomputeSidebarStructureIfChanged() + state.applyPostReduceCacheRecomputes() let store = TestStore(initialState: state) { RepositoriesFeature() @@ -3896,6 +3903,7 @@ struct RepositoriesFeatureTests { } await store.receive(\.sidebarItems) { $0.sidebarItems[id: newWorktree.id]?.lifecycle = .pending + $0.applyPostReduceCacheRecomputes([.sidebarStructure, .selectedWorktreeSlice]) } await store.receive(\.sidebarItems) { $0.sidebarItems[id: newWorktree.id]?.shouldFocusTerminal = true @@ -5226,6 +5234,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt1.id) $0.sidebarSelectedWorktreeIDs = [wt1.id] $0.worktreeHistoryBackStack = [wt2.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5246,6 +5255,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt2.id) $0.sidebarSelectedWorktreeIDs = [wt2.id] $0.worktreeHistoryBackStack = [wt1.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5264,6 +5274,7 @@ struct RepositoriesFeatureTests { await store.receive(\.selectWorktree) { $0.selection = .worktree(wt1.id) $0.sidebarSelectedWorktreeIDs = [wt1.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5286,6 +5297,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt2.id) $0.sidebarSelectedWorktreeIDs = [wt2.id] $0.worktreeHistoryBackStack = [wt1.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5304,6 +5316,7 @@ struct RepositoriesFeatureTests { await store.receive(\.selectWorktree) { $0.selection = .worktree(wt2.id) $0.sidebarSelectedWorktreeIDs = [wt2.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5332,6 +5345,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(feature.id) $0.sidebarSelectedWorktreeIDs = [feature.id] $0.worktreeHistoryBackStack = [main.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5358,6 +5372,7 @@ struct RepositoriesFeatureTests { await store.receive(\.selectWorktree) { $0.selection = .worktree(worktree.id) $0.sidebarSelectedWorktreeIDs = [worktree.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5384,6 +5399,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt3.id) $0.sidebarSelectedWorktreeIDs = [wt3.id] $0.worktreeHistoryBackStack = [wt1.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5410,6 +5426,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt1.id) $0.sidebarSelectedWorktreeIDs = [wt1.id] $0.worktreeHistoryBackStack = [wt3.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5468,6 +5485,7 @@ struct RepositoriesFeatureTests { $0.selection = .worktree(wt1.id) $0.sidebarSelectedWorktreeIDs = [wt1.id] $0.worktreeHistoryBackStack = [wt3.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5491,6 +5509,7 @@ struct RepositoriesFeatureTests { $0.sidebarSelectedWorktreeIDs = [wt2.id] $0.worktreeHistoryBackStack = [wt1.id] $0.worktreeHistoryForwardStack = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5527,6 +5546,7 @@ struct RepositoriesFeatureTests { $0.sidebarSelectedWorktreeIDs = [wt1.id] $0.worktreeHistoryBackStack = [] $0.worktreeHistoryForwardStack = [wt2.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5548,6 +5568,7 @@ struct RepositoriesFeatureTests { $0.sidebarSelectedWorktreeIDs = [wt2.id] $0.worktreeHistoryBackStack = [wt1.id] $0.worktreeHistoryForwardStack = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5598,6 +5619,7 @@ struct RepositoriesFeatureTests { $0.sidebarSelectedWorktreeIDs = [wt1.id] $0.worktreeHistoryBackStack = [] $0.worktreeHistoryForwardStack = [wt3.id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -5615,6 +5637,7 @@ struct RepositoriesFeatureTests { await store.send(.worktreeHistoryBack) { $0.worktreeHistoryBackStack = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } } @@ -5666,6 +5689,7 @@ struct RepositoriesFeatureTests { } await store.receive(\.sidebarItems) { $0.sidebarItems[id: newWorktree.id]?.lifecycle = .pending + $0.applyPostReduceCacheRecomputes([.sidebarStructure, .selectedWorktreeSlice]) } await store.receive(\.sidebarItems) { $0.sidebarItems[id: newWorktree.id]?.shouldFocusTerminal = true @@ -5810,6 +5834,7 @@ struct RepositoriesFeatureTests { await store.send(.selectionChanged([])) { $0.selection = nil $0.sidebarSelectedWorktreeIDs = [] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) #expect(store.state.worktreeHistoryBackStack.isEmpty) @@ -5861,6 +5886,7 @@ struct RepositoriesFeatureTests { $0.sidebarSelectedWorktreeIDs = [target] // Oldest entry is dropped when we exceed the 50-item cap. $0.worktreeHistoryBackStack = (2..<51).map { worktrees[$0].id } + [worktrees[0].id] + $0.applyPostReduceCacheRecomputes(.selectedWorktreeSlice) } await store.receive(\.delegate.selectedWorktreeChanged) } @@ -6018,7 +6044,7 @@ struct RepositoriesFeatureTests { let content = WorktreeDetailView.makeToolbarTitleContent( selectedWorktree: worktree, - selectedRow: folderRow, + selectedRow: SelectedWorktreeSlice(folderRow), repositories: state, hideSubtitleOnMatch: true ) @@ -6039,7 +6065,7 @@ struct RepositoriesFeatureTests { let content = WorktreeDetailView.makeToolbarTitleContent( selectedWorktree: worktree, - selectedRow: mainRow, + selectedRow: SelectedWorktreeSlice(mainRow), repositories: state, hideSubtitleOnMatch: true ) @@ -6063,7 +6089,7 @@ struct RepositoriesFeatureTests { let content = WorktreeDetailView.makeToolbarTitleContent( selectedWorktree: main, - selectedRow: mainRow, + selectedRow: SelectedWorktreeSlice(mainRow), repositories: state, hideSubtitleOnMatch: true ) @@ -6087,7 +6113,7 @@ struct RepositoriesFeatureTests { let content = WorktreeDetailView.makeToolbarTitleContent( selectedWorktree: feature, - selectedRow: featureRow, + selectedRow: SelectedWorktreeSlice(featureRow), repositories: state, hideSubtitleOnMatch: true ) @@ -6110,7 +6136,7 @@ struct RepositoriesFeatureTests { let content = WorktreeDetailView.makeToolbarTitleContent( selectedWorktree: feature, - selectedRow: featureRow, + selectedRow: SelectedWorktreeSlice(featureRow), repositories: state, hideSubtitleOnMatch: false ) @@ -6134,7 +6160,7 @@ struct RepositoriesFeatureTests { let content = WorktreeDetailView.makeToolbarTitleContent( selectedWorktree: worktree, - selectedRow: mainRow, + selectedRow: SelectedWorktreeSlice(mainRow), repositories: state, hideSubtitleOnMatch: true ) diff --git a/supacodeTests/RepositoriesSidebarTestHelpers.swift b/supacodeTests/RepositoriesSidebarTestHelpers.swift index 8a1f3d65f..84e1c79a5 100644 --- a/supacodeTests/RepositoriesSidebarTestHelpers.swift +++ b/supacodeTests/RepositoriesSidebarTestHelpers.swift @@ -3,16 +3,32 @@ import Foundation @testable import supacode +extension AppFeature.State { + /// Mirrors AppFeature's post-reduce hook for TestStore expectations. + /// Equatable diff inside the helper keeps no-op writes from invalidating + /// the menu-bar `WorktreeCommands` snapshot. + @MainActor + mutating func applyPostReduceCacheRecomputes() { + recomputeWorktreeMenuSnapshotIfChanged() + } +} + extension RepositoriesFeature.State { /// Test mirror of the full sidebar pipeline: `syncSidebar` (matching - /// reducer-body handlers that explicitly resync) + the structure recompute - /// the post-reduce hook would run. `$0.reconcileSidebarForTesting()` in a - /// TestStore expectation covers both in one call so tests don't have to - /// remember to mirror each piece separately. + /// reducer-body handlers that explicitly resync) + every cache recompute the + /// post-reduce hook would run. Use this when the action explicitly resyncs. @MainActor mutating func reconcileSidebarForTesting() { RepositoriesFeature.syncSidebar(&self) - recomputeSidebarStructureIfChanged() + applyPostReduceCacheRecomputes() + } + + /// Mirrors the post-reduce hook for TestStore expectations. Pass the same + /// `CacheInvalidations` set the action's `cacheInvalidations` returns so the + /// expected state mutates exactly what the live reducer does, no more. + @MainActor + mutating func applyPostReduceCacheRecomputes(_ invalidations: CacheInvalidations = .all) { + applyCacheRecomputes(invalidations) } /// Convenience init for tests that need a populated row/grouping store from a roster. diff --git a/supacodeTests/SelectedWorktreeSliceCacheTests.swift b/supacodeTests/SelectedWorktreeSliceCacheTests.swift new file mode 100644 index 000000000..b10389373 --- /dev/null +++ b/supacodeTests/SelectedWorktreeSliceCacheTests.swift @@ -0,0 +1,124 @@ +import ComposableArchitecture +import Foundation +import IdentifiedCollections +import SupacodeSettingsShared +import Testing + +@testable import supacode + +/// Pins the load-bearing #289 contract: storms on the focused row that +/// only touch agent / surface / notification fields must NOT mutate the +/// cached `selectedWorktreeSlice` or `toolbarNotificationGroupsCache`. +@MainActor +struct SelectedWorktreeSliceCacheTests { + @Test func agentSnapshotChangeOnFocusedRowDoesNotMutateSlice() async { + let worktree = makeWorktree(id: "/tmp/repo/wt", repoRoot: "/tmp/repo") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var state = RepositoriesFeature.State(reconciledRepositories: [repository]) + state.selection = .worktree(worktree.id) + state.reconcileSidebarForTesting() + let sliceBefore = state.selectedWorktreeSlice + #expect(sliceBefore != nil, "Cache must be populated for the focused row") + + let store = TestStore(initialState: state) { RepositoriesFeature() } + + // Agent storm: only mutates `agents` / `hasAgentActivity`. Slice excludes + // those, so the post-reduce diff must not invalidate the cache. + await store.send( + .sidebarItems(.element(id: worktree.id, action: .agentSnapshotChanged([], hasActivity: false))) + ) + #expect(store.state.selectedWorktreeSlice == sliceBefore) + } + + @Test func terminalProjectionChangeOnFocusedRowDoesNotMutateSlice() async { + let worktree = makeWorktree(id: "/tmp/repo/wt", repoRoot: "/tmp/repo") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var state = RepositoriesFeature.State(reconciledRepositories: [repository]) + state.selection = .worktree(worktree.id) + state.reconcileSidebarForTesting() + let sliceBefore = state.selectedWorktreeSlice + + let store = TestStore(initialState: state) { RepositoriesFeature() } + let surfaceID = UUID() + await store.send( + .sidebarItems( + .element( + id: worktree.id, + action: .terminalProjectionChanged( + WorktreeRowProjection( + surfaceIDs: [surfaceID], + isProgressBusy: false, + hasUnseenNotifications: false, + notifications: [] + ) + ) + ) + ) + ) { + $0.sidebarItems[id: worktree.id]?.surfaceIDs = [surfaceID] + $0.applyPostReduceCacheRecomputes([.sidebarStructure, .toolbarNotificationGroups]) + } + + #expect(store.state.selectedWorktreeSlice == sliceBefore) + } + + @Test func agentSnapshotChangeOnFocusedRowDoesNotMutateNotificationCache() async { + let worktree = makeWorktree(id: "/tmp/repo/wt", repoRoot: "/tmp/repo") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var state = RepositoriesFeature.State(reconciledRepositories: [repository]) + state.selection = .worktree(worktree.id) + state.reconcileSidebarForTesting() + let cacheBefore = state.toolbarNotificationGroupsCache + + let store = TestStore(initialState: state) { RepositoriesFeature() } + await store.send( + .sidebarItems(.element(id: worktree.id, action: .agentSnapshotChanged([], hasActivity: false))) + ) + + #expect(store.state.toolbarNotificationGroupsCache == cacheBefore) + } + + @Test func runningScriptStartedOnFocusedRowMutatesSlice() async { + let worktree = makeWorktree(id: "/tmp/repo/wt", repoRoot: "/tmp/repo") + let repository = makeRepository(id: "/tmp/repo", worktrees: [worktree]) + var state = RepositoriesFeature.State(reconciledRepositories: [repository]) + state.selection = .worktree(worktree.id) + state.reconcileSidebarForTesting() + let scriptID = UUID() + let store = TestStore(initialState: state) { RepositoriesFeature() } + + await store.send( + .sidebarItems( + .element( + id: worktree.id, action: .runningScriptStarted(id: scriptID, tint: .blue) + ) + ) + ) { + $0.sidebarItems[id: worktree.id]?.runningScripts.append(.init(id: scriptID, tint: .blue)) + $0.applyPostReduceCacheRecomputes([.sidebarStructure, .selectedWorktreeSlice]) + } + + // Verify the cache picked up the new running script (sanity that the + // recompute path actually fires for this action). + #expect(store.state.selectedWorktreeSlice?.runningScripts.contains(where: { $0.id == scriptID }) == true) + } + + private func makeWorktree(id: String, repoRoot: String) -> Worktree { + Worktree( + id: id, + name: "wt", + detail: "detail", + workingDirectory: URL(fileURLWithPath: id), + repositoryRootURL: URL(fileURLWithPath: repoRoot) + ) + } + + private func makeRepository(id: String, worktrees: [Worktree]) -> Repository { + Repository( + id: id, + rootURL: URL(fileURLWithPath: id), + name: "repo", + worktrees: IdentifiedArray(uniqueElements: worktrees) + ) + } +} diff --git a/supacodeTests/SidebarItemFeatureTests.swift b/supacodeTests/SidebarItemFeatureTests.swift index effbc924b..8a694ab55 100644 --- a/supacodeTests/SidebarItemFeatureTests.swift +++ b/supacodeTests/SidebarItemFeatureTests.swift @@ -92,7 +92,7 @@ struct SidebarItemFeatureTests { let surface1 = UUID() let surface2 = UUID() let notif = WorktreeTerminalNotification( - surfaceId: surface1, + surfaceID: surface1, title: "Notification", body: "hi", createdAt: Date(timeIntervalSince1970: 0) diff --git a/supacodeTests/TerminalTabFeatureTests.swift b/supacodeTests/TerminalTabFeatureTests.swift new file mode 100644 index 000000000..956b84974 --- /dev/null +++ b/supacodeTests/TerminalTabFeatureTests.swift @@ -0,0 +1,107 @@ +import ComposableArchitecture +import Foundation +import SupacodeSettingsShared +import Testing + +@testable import supacode + +@MainActor +struct TerminalTabFeatureTests { + @Test func projectionChangedShortCircuitsOnEqualPayload() async { + let tabID = TerminalTabID(rawValue: UUID()) + let initial = TerminalTabFeature.State( + id: tabID, + worktreeID: "/tmp/repo", + surfaceIDs: [UUID(uuidString: "00000000-0000-0000-0000-000000000001")!], + activeSurfaceID: UUID(uuidString: "00000000-0000-0000-0000-000000000001")!, + unseenNotificationCount: 0 + ) + let store = TestStore(initialState: initial) { TerminalTabFeature() } + + // Same fields back-in: reducer must mutate nothing. + await store.send( + .projectionChanged( + WorktreeTabProjection( + tabID: tabID, + surfaceIDs: initial.surfaceIDs, + activeSurfaceID: initial.activeSurfaceID, + unseenNotificationCount: 0 + ) + )) + } + + @Test func projectionChangedAppliesEachFieldIndependently() async { + let tabID = TerminalTabID(rawValue: UUID()) + let store = TestStore( + initialState: TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repo") + ) { TerminalTabFeature() } + + let surface = UUID() + await store.send( + .projectionChanged( + WorktreeTabProjection( + tabID: tabID, + surfaceIDs: [surface], + activeSurfaceID: surface, + unseenNotificationCount: 3 + ) + ) + ) { + $0.surfaceIDs = [surface] + $0.activeSurfaceID = surface + $0.unseenNotificationCount = 3 + } + } + + @Test func agentSnapshotChangedShortCircuitsOnEqualArray() async { + let tabID = TerminalTabID(rawValue: UUID()) + let agents = [ + AgentPresenceFeature.AgentInstance(agent: .claude, activity: .busy) + ] + let store = TestStore( + initialState: TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repo", agents: agents) + ) { TerminalTabFeature() } + + await store.send(.agentSnapshotChanged(agents)) + } + + @Test func agentSnapshotChangedReplacesArrayOnDiff() async { + let tabID = TerminalTabID(rawValue: UUID()) + let store = TestStore( + initialState: TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repo") + ) { TerminalTabFeature() } + let agents = [ + AgentPresenceFeature.AgentInstance(agent: .codex, activity: .idle) + ] + + await store.send(.agentSnapshotChanged(agents)) { + $0.agents = agents + } + } + + @Test func progressDisplayChangedShortCircuitsOnEqualDisplay() async { + let tabID = TerminalTabID(rawValue: UUID()) + let display = TerminalTabProgressDisplay(style: .indeterminate) + let store = TestStore( + initialState: TerminalTabFeature.State( + id: tabID, worktreeID: "/tmp/repo", progressDisplay: display + ) + ) { TerminalTabFeature() } + + await store.send(.progressDisplayChanged(display)) + } + + @Test func progressDisplayChangedClearsToNil() async { + let tabID = TerminalTabID(rawValue: UUID()) + let store = TestStore( + initialState: TerminalTabFeature.State( + id: tabID, worktreeID: "/tmp/repo", + progressDisplay: TerminalTabProgressDisplay(style: .determinate(percent: 50)) + ) + ) { TerminalTabFeature() } + + await store.send(.progressDisplayChanged(nil)) { + $0.progressDisplay = nil + } + } +} diff --git a/supacodeTests/TerminalsFeatureTests.swift b/supacodeTests/TerminalsFeatureTests.swift new file mode 100644 index 000000000..08905009f --- /dev/null +++ b/supacodeTests/TerminalsFeatureTests.swift @@ -0,0 +1,167 @@ +import ComposableArchitecture +import Foundation +import Testing + +@testable import supacode + +@MainActor +struct TerminalsFeatureTests { + @Test func tabProjectionChangedInsertsNewTabThenForwards() async { + let tabID = TerminalTabID(rawValue: UUID()) + let surface = UUID() + let store = TestStore(initialState: TerminalsFeature.State()) { TerminalsFeature() } + store.exhaustivity = .off + + await store.send( + .tabProjectionChanged( + worktreeID: "/tmp/repo", + projection: WorktreeTabProjection( + tabID: tabID, + surfaceIDs: [surface], + activeSurfaceID: surface, + unseenNotificationCount: 0 + ) + ) + ) { + $0.terminalTabs.append( + TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repo") + ) + } + await store.receive(\.terminalTabs) + } + + @Test func tabRemovedDropsElementAndRecordsForReplayProtection() async { + let tabID = TerminalTabID(rawValue: UUID()) + var initial = TerminalsFeature.State() + initial.terminalTabs.append( + TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repo") + ) + let store = TestStore(initialState: initial) { TerminalsFeature() } + + await store.send(.tabRemoved(worktreeID: "/tmp/repo", tabID: tabID)) { + $0.terminalTabs.remove(id: tabID) + $0.recentlyRemovedTabIDs = [ + TerminalsFeature.RecentlyRemovedTab(worktreeID: "/tmp/repo", tabID: tabID) + ] + } + } + + @Test func staleTabProjectionAfterRemoveDoesNotReinsertPhantomTab() async { + let tabID = TerminalTabID(rawValue: UUID()) + var initial = TerminalsFeature.State() + initial.terminalTabs.append( + TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repo") + ) + let store = TestStore(initialState: initial) { TerminalsFeature() } + + await store.send(.tabRemoved(worktreeID: "/tmp/repo", tabID: tabID)) { + $0.terminalTabs.remove(id: tabID) + $0.recentlyRemovedTabIDs = [ + TerminalsFeature.RecentlyRemovedTab(worktreeID: "/tmp/repo", tabID: tabID) + ] + } + + // Late projection arrives after the tab was removed in the same worktree: must NOT re-insert. + await store.send( + .tabProjectionChanged( + worktreeID: "/tmp/repo", + projection: WorktreeTabProjection( + tabID: tabID, + surfaceIDs: [], + activeSurfaceID: nil, + unseenNotificationCount: 0 + ) + ) + ) + + #expect(store.state.terminalTabs.isEmpty) + } + + @Test func recentlyRemovedTabIDsAreBoundedByLimit() async { + let initial = TerminalsFeature.State() + let store = TestStore(initialState: initial) { TerminalsFeature() } + + // Remove `limit + 5` distinct tab IDs; only the most recent `limit` survive. + let limit = TerminalsFeature.recentlyRemovedTabLimit + var allIDs: [TerminalTabID] = [] + for _ in 0..<(limit + 5) { + let id = TerminalTabID(rawValue: UUID()) + allIDs.append(id) + await store.send(.tabRemoved(worktreeID: "/tmp/repo", tabID: id)) { + $0.recentlyRemovedTabIDs.append( + TerminalsFeature.RecentlyRemovedTab(worktreeID: "/tmp/repo", tabID: id) + ) + if $0.recentlyRemovedTabIDs.count > limit { + $0.recentlyRemovedTabIDs.removeFirst($0.recentlyRemovedTabIDs.count - limit) + } + } + } + #expect(store.state.recentlyRemovedTabIDs.count == limit) + #expect(store.state.recentlyRemovedTabIDs.first?.tabID == allIDs[5]) + #expect(store.state.recentlyRemovedTabIDs.last?.tabID == allIDs.last) + } + + @Test func worktreeStateTornDownDrainsTabsAndFIFOForThatWorktree() async { + // Two worktrees, two tabs each. Tearing down repoA should leave repoB's + // FIFO + tab features intact. + let tabA1 = TerminalTabID(rawValue: UUID()) + let tabA2 = TerminalTabID(rawValue: UUID()) + let tabB1 = TerminalTabID(rawValue: UUID()) + var initial = TerminalsFeature.State() + initial.terminalTabs.append(TerminalTabFeature.State(id: tabA1, worktreeID: "/tmp/repoA")) + initial.terminalTabs.append(TerminalTabFeature.State(id: tabA2, worktreeID: "/tmp/repoA")) + initial.terminalTabs.append(TerminalTabFeature.State(id: tabB1, worktreeID: "/tmp/repoB")) + initial.recentlyRemovedTabIDs = [ + TerminalsFeature.RecentlyRemovedTab( + worktreeID: "/tmp/repoA", tabID: TerminalTabID(rawValue: UUID())), + TerminalsFeature.RecentlyRemovedTab( + worktreeID: "/tmp/repoB", tabID: TerminalTabID(rawValue: UUID())), + ] + let repoBRecord = initial.recentlyRemovedTabIDs[1] + let store = TestStore(initialState: initial) { TerminalsFeature() } + + await store.send(.worktreeStateTornDown(worktreeID: "/tmp/repoA")) { + $0.recentlyRemovedTabIDs = [repoBRecord] + $0.terminalTabs.remove(id: tabA1) + $0.terminalTabs.remove(id: tabA2) + } + } + + @Test func sameSessionRestoreAfterTeardownReinsertsTabWithReusedUUID() async { + // Simulates the snapshot-restore path: tab removed in worktree A, worktree + // state torn down (FIFO drained for worktreeA), restore replays the same + // persisted UUID. The reinserted projection must not be shadowed. + let tabID = TerminalTabID(rawValue: UUID()) + var initial = TerminalsFeature.State() + initial.terminalTabs.append(TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repoA")) + let store = TestStore(initialState: initial) { TerminalsFeature() } + store.exhaustivity = .off + + await store.send(.tabRemoved(worktreeID: "/tmp/repoA", tabID: tabID)) { + $0.terminalTabs.remove(id: tabID) + $0.recentlyRemovedTabIDs = [ + TerminalsFeature.RecentlyRemovedTab(worktreeID: "/tmp/repoA", tabID: tabID) + ] + } + + await store.send(.worktreeStateTornDown(worktreeID: "/tmp/repoA")) { + $0.recentlyRemovedTabIDs = [] + } + + let surface = UUID() + await store.send( + .tabProjectionChanged( + worktreeID: "/tmp/repoA", + projection: WorktreeTabProjection( + tabID: tabID, + surfaceIDs: [surface], + activeSurfaceID: surface, + unseenNotificationCount: 0 + ) + ) + ) { + $0.terminalTabs.append(TerminalTabFeature.State(id: tabID, worktreeID: "/tmp/repoA")) + } + await store.receive(\.terminalTabs) + } +} diff --git a/supacodeTests/ToolbarNotificationGroupingTests.swift b/supacodeTests/ToolbarNotificationGroupingTests.swift index 7fc1dbbc0..031f9a08c 100644 --- a/supacodeTests/ToolbarNotificationGroupingTests.swift +++ b/supacodeTests/ToolbarNotificationGroupingTests.swift @@ -38,22 +38,27 @@ struct ToolbarNotificationGroupingTests { ) } - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) - manager.state(for: repoAOne).notifications = [ - WorktreeTerminalNotification( - surfaceId: UUID(), title: "A1", body: "done", createdAt: .distantPast, isRead: true - ) - ] - manager.state(for: repoATwo).notifications = [ - WorktreeTerminalNotification(surfaceId: UUID(), title: "A2", body: "done", createdAt: .distantPast) - ] - manager.state(for: repoBOne).notifications = [ - WorktreeTerminalNotification( - surfaceId: UUID(), title: "B1", body: "done", createdAt: .distantPast, isRead: true - ) - ] - - let groups = state.toolbarNotificationGroups(terminalManager: manager) + setRowNotifications( + &state, id: repoAOne.id, + notifications: [ + WorktreeTerminalNotification( + surfaceID: UUID(), title: "A1", body: "done", createdAt: .distantPast, isRead: true + ) + ]) + setRowNotifications( + &state, id: repoATwo.id, + notifications: [ + WorktreeTerminalNotification(surfaceID: UUID(), title: "A2", body: "done", createdAt: .distantPast) + ]) + setRowNotifications( + &state, id: repoBOne.id, + notifications: [ + WorktreeTerminalNotification( + surfaceID: UUID(), title: "B1", body: "done", createdAt: .distantPast, isRead: true + ) + ]) + + let groups = state.computeToolbarNotificationGroups() #expect(groups.map(\.id) == [repoB.id, repoA.id]) #expect(groups[0].worktrees.map(\.id) == [repoBOne.id]) @@ -84,12 +89,13 @@ struct ToolbarNotificationGroupingTests { ) } - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) - manager.state(for: repoAArchived).notifications = [ - WorktreeTerminalNotification(surfaceId: UUID(), title: "Archived", body: "hidden", createdAt: .distantPast) - ] + setRowNotifications( + &state, id: repoAArchived.id, + notifications: [ + WorktreeTerminalNotification(surfaceID: UUID(), title: "Archived", body: "hidden", createdAt: .distantPast) + ]) - let groups = state.toolbarNotificationGroups(terminalManager: manager) + let groups = state.computeToolbarNotificationGroups() #expect(groups.isEmpty) } @@ -104,22 +110,25 @@ struct ToolbarNotificationGroupingTests { var state = RepositoriesFeature.State(reconciledRepositories: [repo]) state.repositoryRoots = [repo.rootURL] - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) - manager.state(for: readOnly).notifications = [ - WorktreeTerminalNotification( - surfaceId: UUID(), title: "Read 1", body: "done", createdAt: .distantPast, isRead: true - ) - ] - manager.state(for: mixed).notifications = [ - WorktreeTerminalNotification( - surfaceId: UUID(), title: "Read 2", body: "done", createdAt: .distantPast, isRead: true - ), - WorktreeTerminalNotification( - surfaceId: UUID(), title: "Unread", body: "new", createdAt: .distantPast, isRead: false - ), - ] - - let groups = state.toolbarNotificationGroups(terminalManager: manager) + setRowNotifications( + &state, id: readOnly.id, + notifications: [ + WorktreeTerminalNotification( + surfaceID: UUID(), title: "Read 1", body: "done", createdAt: .distantPast, isRead: true + ) + ]) + setRowNotifications( + &state, id: mixed.id, + notifications: [ + WorktreeTerminalNotification( + surfaceID: UUID(), title: "Read 2", body: "done", createdAt: .distantPast, isRead: true + ), + WorktreeTerminalNotification( + surfaceID: UUID(), title: "Unread", body: "new", createdAt: .distantPast, isRead: false + ), + ]) + + let groups = state.computeToolbarNotificationGroups() #expect(groups.count == 1) #expect(groups[0].notificationCount == 3) @@ -135,20 +144,31 @@ struct ToolbarNotificationGroupingTests { var state = RepositoriesFeature.State(reconciledRepositories: [repo]) state.repositoryRoots = [repo.rootURL] - let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) - manager.state(for: feature).notifications = [ - WorktreeTerminalNotification( - surfaceId: UUID(), title: "Read", body: "kept", createdAt: .distantPast, isRead: true - ) - ] + setRowNotifications( + &state, id: feature.id, + notifications: [ + WorktreeTerminalNotification( + surfaceID: UUID(), title: "Read", body: "kept", createdAt: .distantPast, isRead: true + ) + ]) - let groups = state.toolbarNotificationGroups(terminalManager: manager) + let groups = state.computeToolbarNotificationGroups() #expect(groups.map(\.id) == [repo.id]) #expect(groups[0].worktrees.map(\.id) == [feature.id]) #expect(groups[0].unseenWorktreeCount == 0) } + private func setRowNotifications( + _ state: inout RepositoriesFeature.State, + id: SidebarItemID, + notifications: [WorktreeTerminalNotification] + ) { + let hasUnseen = notifications.contains(where: { !$0.isRead }) + state.sidebarItems[id: id]?.notifications = IdentifiedArrayOf(uniqueElements: notifications) + state.sidebarItems[id: id]?.hasUnseenNotifications = hasUnseen + } + private func makeWorktree( id: String, name: String, diff --git a/supacodeTests/WorktreeTerminalManagerTests.swift b/supacodeTests/WorktreeTerminalManagerTests.swift index 953912be6..394f1828c 100644 --- a/supacodeTests/WorktreeTerminalManagerTests.swift +++ b/supacodeTests/WorktreeTerminalManagerTests.swift @@ -309,25 +309,25 @@ struct WorktreeTerminalManagerTests { let worktree = makeWorktree() let state = manager.state(for: worktree) - state.notifications = [ + state.setNotificationsForTesting([ WorktreeTerminalNotification( - surfaceId: UUID(), + surfaceID: UUID(), title: "Unread", body: "body", createdAt: .distantPast, isRead: false ) - ] + ]) state.onNotificationIndicatorChanged?() - state.notifications = [ + state.setNotificationsForTesting([ WorktreeTerminalNotification( - surfaceId: UUID(), + surfaceID: UUID(), title: "Read", body: "body", createdAt: .distantPast, isRead: true ) - ] + ]) let stream = manager.eventStream() var iterator = stream.makeAsyncIterator() @@ -384,14 +384,14 @@ struct WorktreeTerminalManagerTests { let worktree = makeWorktree() let state = manager.state(for: worktree) - state.notifications = [ + state.setNotificationsForTesting([ makeNotification(isRead: true), makeNotification(isRead: true), - ] + ]) #expect(manager.hasUnseenNotifications(for: worktree.id) == false) - state.notifications.append(makeNotification(isRead: false)) + state.setNotificationsForTesting(state.notifications + [makeNotification(isRead: false)]) #expect(manager.hasUnseenNotifications(for: worktree.id) == true) } @@ -401,10 +401,10 @@ struct WorktreeTerminalManagerTests { let worktree = makeWorktree() let state = manager.state(for: worktree) - state.notifications = [ + state.setNotificationsForTesting([ makeNotification(isRead: false), makeNotification(isRead: true), - ] + ]) let stream = manager.eventStream() var iterator = stream.makeAsyncIterator() @@ -427,16 +427,16 @@ struct WorktreeTerminalManagerTests { let surfaceA = UUID() let surfaceB = UUID() - state.notifications = [ - makeNotification(surfaceId: surfaceA, isRead: false), - makeNotification(surfaceId: surfaceB, isRead: false), - makeNotification(surfaceId: surfaceB, isRead: true), - ] + state.setNotificationsForTesting([ + makeNotification(surfaceID: surfaceA, isRead: false), + makeNotification(surfaceID: surfaceB, isRead: false), + makeNotification(surfaceID: surfaceB, isRead: true), + ]) state.markNotificationsRead(forSurfaceID: surfaceB) - let aNotifications = state.notifications.filter { $0.surfaceId == surfaceA } - let bNotifications = state.notifications.filter { $0.surfaceId == surfaceB } + let aNotifications = state.notifications.filter { $0.surfaceID == surfaceA } + let bNotifications = state.notifications.filter { $0.surfaceID == surfaceB } #expect(aNotifications.map(\.isRead) == [false]) #expect(bNotifications.map(\.isRead) == [true, true]) @@ -452,10 +452,10 @@ struct WorktreeTerminalManagerTests { let worktree = makeWorktree() let state = manager.state(for: worktree) - state.notifications = [ + state.setNotificationsForTesting([ makeNotification(isRead: false), makeNotification(isRead: false), - ] + ]) state.setNotificationsEnabled(false) @@ -468,10 +468,10 @@ struct WorktreeTerminalManagerTests { let worktree = makeWorktree() let state = manager.state(for: worktree) - state.notifications = [ + state.setNotificationsForTesting([ makeNotification(isRead: false), makeNotification(isRead: true), - ] + ]) state.dismissAllNotifications() @@ -974,8 +974,8 @@ struct WorktreeTerminalManagerTests { let older = Date(timeIntervalSince1970: 1_000) let newer = Date(timeIntervalSince1970: 2_000) - stateA.notifications = [makeNotification(surfaceId: surfaceA.id, isRead: false, createdAt: older)] - stateB.notifications = [makeNotification(surfaceId: surfaceB.id, isRead: false, createdAt: newer)] + stateA.setNotificationsForTesting([makeNotification(surfaceID: surfaceA.id, isRead: false, createdAt: older)]) + stateB.setNotificationsForTesting([makeNotification(surfaceID: surfaceB.id, isRead: false, createdAt: newer)]) let location = manager.latestUnreadNotificationLocation() #expect(location?.worktreeID == worktreeB.id) @@ -995,11 +995,11 @@ struct WorktreeTerminalManagerTests { return } - let alive = makeNotification(surfaceId: surface.id, isRead: false, createdAt: Date(timeIntervalSince1970: 1_000)) - let orphan = makeNotification(surfaceId: UUID(), isRead: false, createdAt: Date(timeIntervalSince1970: 2_000)) + let alive = makeNotification(surfaceID: surface.id, isRead: false, createdAt: Date(timeIntervalSince1970: 1_000)) + let orphan = makeNotification(surfaceID: UUID(), isRead: false, createdAt: Date(timeIntervalSince1970: 2_000)) // The orphan is newer but its surface no longer exists in any tab, so // it must be skipped and the alive notification wins. - state.notifications = [orphan, alive] + state.setNotificationsForTesting([orphan, alive]) let location = manager.latestUnreadNotificationLocation() #expect(location?.surfaceID == surface.id) @@ -1028,13 +1028,13 @@ struct WorktreeTerminalManagerTests { } let orphanSurface = UUID() - stateA.notifications = [ - makeNotification(surfaceId: orphanSurface, isRead: false, createdAt: Date(timeIntervalSince1970: 3)), - makeNotification(surfaceId: surfaceA.id, isRead: false, createdAt: Date(timeIntervalSince1970: 1)), - ] - stateB.notifications = [ - makeNotification(surfaceId: surfaceB.id, isRead: false, createdAt: Date(timeIntervalSince1970: 2)) - ] + stateA.setNotificationsForTesting([ + makeNotification(surfaceID: orphanSurface, isRead: false, createdAt: Date(timeIntervalSince1970: 3)), + makeNotification(surfaceID: surfaceA.id, isRead: false, createdAt: Date(timeIntervalSince1970: 1)), + ]) + stateB.setNotificationsForTesting([ + makeNotification(surfaceID: surfaceB.id, isRead: false, createdAt: Date(timeIntervalSince1970: 2)) + ]) let location = manager.latestUnreadNotificationLocation() #expect(location?.worktreeID == worktreeB.id) @@ -1046,9 +1046,9 @@ struct WorktreeTerminalManagerTests { let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) let worktree = makeWorktree() let state = manager.state(for: worktree) - state.notifications = [ - makeNotification(surfaceId: UUID(), isRead: false, createdAt: .distantPast) - ] + state.setNotificationsForTesting([ + makeNotification(surfaceID: UUID(), isRead: false, createdAt: .distantPast) + ]) #expect(manager.latestUnreadNotificationLocation() == nil) } @@ -1072,12 +1072,14 @@ struct WorktreeTerminalManagerTests { #expect(state.hasUnseenNotification(forTabID: tab) == false) // Notification on the first leaf lights up the tab. - state.notifications = [makeNotification(surfaceId: leaves[0].id, isRead: false, createdAt: .distantPast)] + state.setNotificationsForTesting([makeNotification(surfaceID: leaves[0].id, isRead: false, createdAt: .distantPast)] + ) #expect(state.hasUnseenNotification(forTabID: tab) == true) state.markAllNotificationsRead() // Notification on the second leaf also lights up the tab. - state.notifications = [makeNotification(surfaceId: leaves[1].id, isRead: false, createdAt: .distantPast)] + state.setNotificationsForTesting([makeNotification(surfaceID: leaves[1].id, isRead: false, createdAt: .distantPast)] + ) #expect(state.hasUnseenNotification(forTabID: tab) == true) // Once read, the tab is clean again. @@ -1085,7 +1087,7 @@ struct WorktreeTerminalManagerTests { #expect(state.hasUnseenNotification(forTabID: tab) == false) // A notification tied to a surface outside this tab does NOT light it up. - state.notifications = [makeNotification(surfaceId: UUID(), isRead: false, createdAt: .distantPast)] + state.setNotificationsForTesting([makeNotification(surfaceID: UUID(), isRead: false, createdAt: .distantPast)]) #expect(state.hasUnseenNotification(forTabID: tab) == false) } @@ -1093,9 +1095,9 @@ struct WorktreeTerminalManagerTests { let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) let worktree = makeWorktree() let state = manager.state(for: worktree) - let first = makeNotification(surfaceId: UUID(), isRead: false, createdAt: .distantPast) - let second = makeNotification(surfaceId: UUID(), isRead: false, createdAt: .distantPast) - state.notifications = [first, second] + let first = makeNotification(surfaceID: UUID(), isRead: false, createdAt: .distantPast) + let second = makeNotification(surfaceID: UUID(), isRead: false, createdAt: .distantPast) + state.setNotificationsForTesting([first, second]) manager.markNotificationRead(worktreeID: worktree.id, notificationID: first.id) @@ -1145,12 +1147,12 @@ struct WorktreeTerminalManagerTests { } private func makeNotification( - surfaceId: UUID = UUID(), + surfaceID: UUID = UUID(), isRead: Bool, createdAt: Date = .distantPast ) -> WorktreeTerminalNotification { WorktreeTerminalNotification( - surfaceId: surfaceId, + surfaceID: surfaceID, title: "Title", body: "Body", createdAt: createdAt, @@ -1345,4 +1347,73 @@ struct WorktreeTerminalManagerTests { selectedTabIndex: 0 ) } + + // MARK: - Per-tab input stability (C4). + // + // These pin the projection inputs the per-tab leaf views read so a future + // refactor that broadens the read surface can't silently re-introduce the + // `6590fdaf` cross-tab fan-out regression. They don't measure SwiftUI + // invalidation, they assert the underlying algebra stays per-tab pure. + + @Test func notificationOnTabBLeavesTabAUnseenCountUnchanged() { + let manager = WorktreeTerminalManager(runtime: GhosttyRuntime()) + let worktree = makeWorktree() + let state = manager.state(for: worktree) + + guard + let tabA = state.createTab(), + let tabB = state.createTab(focusing: false), + let surfaceA = state.splitTree(for: tabA).root?.leftmostLeaf(), + let surfaceB = state.splitTree(for: tabB).root?.leftmostLeaf() + else { + Issue.record("Expected two tabs and two surfaces") + return + } + + #expect(state.hasUnseenNotification(forTabID: tabA) == false) + #expect(state.hasUnseenNotification(forTabID: tabB) == false) + + state.setNotificationsForTesting( + state.notifications + [makeNotification(surfaceID: surfaceB.id, isRead: false)] + ) + + #expect(state.hasUnseenNotification(forTabID: tabA) == false) + #expect(state.hasUnseenNotification(forTabID: tabB) == true) + // surfaceIDs(inTab:) does not drift from a notifications mutation. + #expect(state.surfaceIDs(inTab: tabA) == [surfaceA.id]) + #expect(state.surfaceIDs(inTab: tabB) == [surfaceB.id]) + } + + @Test func agentPresenceOnTabBLeavesTabAAgentsUnchanged() { + let (manager, presence) = WorktreeTerminalManager.withPresenceHarness() + let worktree = makeWorktree() + let state = manager.state(for: worktree) + + guard + let tabA = state.createTab(), + let tabB = state.createTab(focusing: false), + let surfaceA = state.splitTree(for: tabA).root?.leftmostLeaf(), + let surfaceB = state.splitTree(for: tabB).root?.leftmostLeaf() + else { + Issue.record("Expected two tabs and two surfaces") + return + } + let tabASurfaces = state.surfaceIDs(inTab: tabA) + let tabBSurfaces = state.surfaceIDs(inTab: tabB) + + func agents(for surfaceIDs: [UUID]) -> [AgentPresenceFeature.AgentInstance] { + presence.state.agents(across: surfaceIDs, badgesEnabled: true) + } + #expect(agents(for: tabASurfaces).isEmpty) + #expect(agents(for: tabBSurfaces).isEmpty) + + presence.send(.hookEventReceived(makeHookEvent(.sessionStart, surfaceID: surfaceB.id, pid: getpid()))) + presence.send(.hookEventReceived(makeHookEvent(.busy, surfaceID: surfaceB.id))) + + #expect(agents(for: tabASurfaces).isEmpty) + #expect(!agents(for: tabBSurfaces).isEmpty) + // Sanity: a sibling mutation also doesn't drift A's surface set. + _ = surfaceA + #expect(state.surfaceIDs(inTab: tabA) == tabASurfaces) + } }