diff --git a/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift b/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift new file mode 100644 index 00000000..06674f99 --- /dev/null +++ b/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift @@ -0,0 +1,183 @@ +import ComposableArchitecture +import Foundation +import Sharing +import SwiftUI + +/// Settings → Agents → drill-in editor for one profile (docs-ai 053). Owns +/// every profile-scoped mutation gate (unrestricted confirmation, removal +/// confirmation) and its own alert: the presentation state lives inside the +/// pushed page's state, so an alert can only ever fire while the page that +/// hosts its modifier is mounted. +@Reducer +struct AgentProfileEditorFeature { + @ObservableState + struct State: Equatable { + var profile: AgentProfile + /// Passive filesystem check for the bound profile's home; never invokes a + /// CLI (docs-ai 053: no login-status probing). + var homeInitialized = false + @Presents var alert: AlertState? + + init(profile: AgentProfile) { + self.profile = profile + } + } + + enum Action: BindableAction { + case task + case binding(BindingAction) + case backTapped + case removeTapped + case revealProfileFiles + case homeStatusRefreshed(Bool) + case alert(PresentationAction) + case delegate(Delegate) + } + + enum Alert: Equatable { + case confirmUnrestricted + case removeKeepingFiles + case removeTrashingFiles + } + + @CasePathable + enum Delegate: Equatable { + case profileEdited(AgentProfile) + /// Removal is delegated to the parent: it survives this page's dismissal, + /// so the trash effect cannot be cancelled mid-flight. + case removeProfile(AgentProfile.ID, trashFiles: Bool) + } + + @Dependency(AgentProfileHomeClient.self) var homeClient + @Dependency(\.dismiss) var dismiss + + var body: some Reducer { + BindingReducer() + Reduce { state, action in + switch action { + case .task: + refreshHomeStatus(&state) + return .none + + case .binding: + // `.unrestricted` is never applied silently: the change reverts until + // the user explicitly confirms it (docs-ai 053). + if newlyUnrestricted(state.profile) { + state.profile.executionMode = persistedExecutionMode(for: state.profile.id) + state.alert = Self.unrestrictedAlert() + return .none + } + refreshHomeStatus(&state) + return .send(.delegate(.profileEdited(state.profile))) + + case .backTapped: + return .run { _ in await dismiss() } + + case .removeTapped: + // The confirmation gate keys on the *disk fact*, not the current + // binding intent: a profile that was bound, launched (home created), + // then unbound still owns credentials on disk — deleting it silently + // would orphan them with no UI path back. + guard state.profile.bindsDedicatedHome || homeClient.homeExists(state.profile.id) else { + // Pure presets with no home on disk: removal performs zero file + // operations by construction. + return .send(.delegate(.removeProfile(state.profile.id, trashFiles: false))) + } + state.alert = Self.removalAlert(profile: state.profile) + return .none + + case .revealProfileFiles: + guard state.profile.bindsDedicatedHome else { return .none } + let id = state.profile.id + let client = homeClient + return .run { send in + // Reveal provisions the home when missing; report the fresh status + // so the passive indicator doesn't keep saying "Not initialized". + client.revealHome(id) + await send(.homeStatusRefreshed(client.homeExists(id))) + } + + case .homeStatusRefreshed(let initialized): + state.homeInitialized = initialized + return .none + + case .alert(.presented(.confirmUnrestricted)): + state.profile.executionMode = .unrestricted + return .send(.delegate(.profileEdited(state.profile))) + + case .alert(.presented(.removeKeepingFiles)): + return .send(.delegate(.removeProfile(state.profile.id, trashFiles: false))) + + case .alert(.presented(.removeTrashingFiles)): + return .send(.delegate(.removeProfile(state.profile.id, trashFiles: true))) + + case .alert: + return .none + + case .delegate: + return .none + } + } + .ifLet(\.$alert, action: \.alert) + } + + /// "Newly" is judged against the persisted settings, exactly like the + /// pre-split reducer did: confirming once persists `.unrestricted`, so + /// re-toggling after an intermediate `.standard` asks again. + private func newlyUnrestricted(_ profile: AgentProfile) -> Bool { + guard profile.executionMode == .unrestricted else { return false } + return persistedExecutionMode(for: profile.id) != .unrestricted + } + + private func persistedExecutionMode(for id: AgentProfile.ID) -> AgentExecutionMode { + @Shared(.userGlobalSettings) var persisted + return persisted.agentProfiles.first { $0.id == id }?.executionMode ?? .standard + } + + private func refreshHomeStatus(_ state: inout State) { + guard state.profile.bindsDedicatedHome else { + state.homeInitialized = false + return + } + state.homeInitialized = homeClient.homeExists(state.profile.id) + } + + static func unrestrictedAlert() -> AlertState { + AlertState { + TextState("Allow Unrestricted Execution?") + } actions: { + ButtonState(role: .destructive, action: .confirmUnrestricted) { + TextState("Allow Unrestricted") + } + ButtonState(role: .cancel) { + TextState("Cancel") + } + } message: { + TextState( + "The agent will run without permission prompts or sandboxing. " + + "It can execute any command and modify any file your user can." + ) + } + } + + static func removalAlert(profile: AgentProfile) -> AlertState { + AlertState { + TextState("Remove “\(profile.name)”?") + } actions: { + ButtonState(action: .removeKeepingFiles) { + TextState("Remove Profile") + } + ButtonState(role: .destructive, action: .removeTrashingFiles) { + TextState("Remove and Trash Files") + } + ButtonState(role: .cancel) { + TextState("Cancel") + } + } message: { + TextState( + "This profile has its own home with login credentials and files. " + + "“Remove Profile” keeps them on disk; “Remove and Trash Files” moves the folder to the Trash." + ) + } + } +} diff --git a/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift b/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift index 9c65c673..e814cdf2 100644 --- a/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift +++ b/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift @@ -3,23 +3,17 @@ import Foundation import Sharing import SwiftUI -/// Settings → Agents: the global agent profile collection (docs-ai 053). -/// List order is the recommendation fallback order; edits persist to -/// `UserGlobalSettings` the same way global custom commands do. +/// Settings → Agents: the global agent profile list (docs-ai 053). List order +/// is the recommendation fallback order; edits persist to +/// `UserGlobalSettings` the same way global custom commands do. Editing one +/// profile is a drill-in `AgentProfileEditorFeature` presented tree-style, so +/// editor-scoped presentation state lives with the pushed page. @Reducer struct AgentProfilesFeature { @ObservableState struct State: Equatable { var settings: UserGlobalSettings = .default - var selectedProfileID: AgentProfile.ID? - /// Passive filesystem check for the selected bound profile's home; never - /// invokes a CLI (docs-ai 053: no login-status probing). - var selectedHomeInitialized = false - @Presents var alert: AlertState? - - var selectedProfile: AgentProfile? { - selectedProfileID.flatMap { id in settings.agentProfiles.first { $0.id == id } } - } + @Presents var editor: AgentProfileEditorFeature.State? } enum Action: BindableAction { @@ -27,21 +21,12 @@ struct AgentProfilesFeature { case settingsLoaded(UserGlobalSettings) case binding(BindingAction) case addProfile(AgentProfileRuntime) - case removeSelectedTapped case moveProfiles(IndexSet, Int) - case selectProfile(AgentProfile.ID?) - case revealProfileFiles - case homeStatusRefreshed(AgentProfile.ID, Bool) - case alert(PresentationAction) + case profileTapped(AgentProfile.ID) + case editor(PresentationAction) case delegate(Delegate) } - enum Alert: Equatable { - case confirmUnrestricted(AgentProfile.ID) - case removeKeepingFiles(AgentProfile.ID) - case removeTrashingFiles(AgentProfile.ID) - } - @CasePathable enum Delegate: Equatable { case settingsChanged(UserGlobalSettings) @@ -62,26 +47,11 @@ struct AgentProfilesFeature { case .settingsLoaded(let settings): state.settings = settings.normalized() - if state.selectedProfileID == nil { - state.selectedProfileID = state.settings.agentProfiles.first?.id - } - refreshHomeStatus(&state) return .none case .binding: - // `.unrestricted` is never applied silently: the change reverts until - // the user explicitly confirms it (docs-ai 053). - @Shared(.userGlobalSettings) var persisted - if let pendingID = newlyUnrestrictedProfileID(persisted: persisted, edited: state.settings) { - revertExecutionMode(&state, profileID: pendingID, persisted: persisted) - state.alert = Self.unrestrictedAlert(profileID: pendingID) - return .none - } - // State deliberately stays as typed — normalizing here would fight - // the Name field (trim trailing spaces mid-word) and drop the profile - // outright the moment the field is cleared. Blank names are handled - // at the persistence boundary instead. - refreshHomeStatus(&state) + // Only list-level bindings arrive here (the enabled checkboxes); + // profile edits flow through the editor's delegate. return persist(state.settings) case .addProfile(let runtime): @@ -92,62 +62,32 @@ struct AgentProfilesFeature { ) state.settings.agentProfiles.append(profile) state.settings = state.settings.normalized() - state.selectedProfileID = profile.id - refreshHomeStatus(&state) + state.editor = AgentProfileEditorFeature.State(profile: profile) return persist(state.settings) - case .removeSelectedTapped: - guard let profile = state.selectedProfile else { return .none } - // The confirmation gate keys on the *disk fact*, not the current - // binding intent: a profile that was bound, launched (home created), - // then unbound still owns credentials on disk — deleting it silently - // would orphan them with no UI path back. - guard profile.bindsDedicatedHome || homeClient.homeExists(profile.id) else { - // Pure presets with no home on disk: removal performs zero file - // operations by construction. - removeProfile(&state, id: profile.id) - return persist(state.settings) - } - state.alert = Self.removalAlert(profile: profile) - return .none - case .moveProfiles(let source, let destination): state.settings.agentProfiles.move(fromOffsets: source, toOffset: destination) return persist(state.settings) - case .selectProfile(let id): - state.selectedProfileID = id - refreshHomeStatus(&state) - return .none - - case .revealProfileFiles: - guard let profile = state.selectedProfile, profile.bindsDedicatedHome else { return .none } - let id = profile.id - let client = homeClient - return .run { send in - // Reveal provisions the home when missing; report the fresh status - // so the passive indicator doesn't keep saying "Not initialized". - client.revealHome(id) - await send(.homeStatusRefreshed(id, client.homeExists(id))) + case .profileTapped(let id): + guard let profile = state.settings.agentProfiles.first(where: { $0.id == id }) else { + return .none } - - case .homeStatusRefreshed(let profileID, let initialized): - guard state.selectedProfileID == profileID else { return .none } - state.selectedHomeInitialized = initialized + state.editor = AgentProfileEditorFeature.State(profile: profile) return .none - case .alert(.presented(.confirmUnrestricted(let profileID))): - guard let index = state.settings.agentProfiles.firstIndex(where: { $0.id == profileID }) + case .editor(.presented(.delegate(.profileEdited(let profile)))): + guard let index = state.settings.agentProfiles.firstIndex(where: { $0.id == profile.id }) else { return .none } - state.settings.agentProfiles[index].executionMode = .unrestricted + state.settings.agentProfiles[index] = profile return persist(state.settings) - case .alert(.presented(.removeKeepingFiles(let profileID))): - removeProfile(&state, id: profileID) - return persist(state.settings) - - case .alert(.presented(.removeTrashingFiles(let profileID))): - removeProfile(&state, id: profileID) + case .editor(.presented(.delegate(.removeProfile(let profileID, let trashFiles)))): + state.settings.agentProfiles.removeAll { $0.id == profileID } + // Dismissing pops the drill-in editor back to the list; the trash + // effect runs here so the dismissal cannot cancel it. + state.editor = nil + guard trashFiles else { return persist(state.settings) } let client = homeClient return .merge( persist(state.settings), @@ -160,23 +100,27 @@ struct AgentProfilesFeature { } ) - case .alert: + case .editor: return .none case .delegate: return .none } } - .ifLet(\.$alert, action: \.alert) + .ifLet(\.$editor, action: \.editor) { + AgentProfileEditorFeature() + } } private func persist(_ settings: UserGlobalSettings) -> Effect { - .run { send in - @Shared(.userGlobalSettings) var storedSettings - let sanitized = Self.sanitizedForPersistence(settings, persisted: storedSettings) - $storedSettings.withLock { $0 = sanitized } - await send(.delegate(.settingsChanged(sanitized))) - } + // The write happens synchronously in the reducer: this pane's state is + // nil'd on every Settings sidebar switch, and `ifLet` cancels in-flight + // child effects — a persist living inside `.run` could be cancelled and + // silently drop the edit (or resurrect a removed profile). + @Shared(.userGlobalSettings) var storedSettings + let sanitized = Self.sanitizedForPersistence(settings, persisted: storedSettings) + $storedSettings.withLock { $0 = sanitized } + return .send(.delegate(.settingsChanged(sanitized))) } /// A blank name must never reach disk: decode-time normalization drops @@ -200,80 +144,4 @@ struct AgentProfilesFeature { } return settings.normalized() } - - private func removeProfile(_ state: inout State, id: AgentProfile.ID) { - state.settings.agentProfiles.removeAll { $0.id == id } - if state.selectedProfileID == id { - state.selectedProfileID = state.settings.agentProfiles.first?.id - } - refreshHomeStatus(&state) - } - - private func refreshHomeStatus(_ state: inout State) { - guard let profile = state.selectedProfile, profile.bindsDedicatedHome else { - state.selectedHomeInitialized = false - return - } - state.selectedHomeInitialized = homeClient.homeExists(profile.id) - } - - private func newlyUnrestrictedProfileID( - persisted: UserGlobalSettings, - edited: UserGlobalSettings - ) -> AgentProfile.ID? { - edited.agentProfiles.first { profile in - profile.executionMode == .unrestricted - && persisted.agentProfiles.first { $0.id == profile.id }?.executionMode != .unrestricted - }?.id - } - - private func revertExecutionMode( - _ state: inout State, - profileID: AgentProfile.ID, - persisted: UserGlobalSettings - ) { - guard let index = state.settings.agentProfiles.firstIndex(where: { $0.id == profileID }) - else { return } - state.settings.agentProfiles[index].executionMode = - persisted.agentProfiles.first { $0.id == profileID }?.executionMode ?? .standard - } - - static func unrestrictedAlert(profileID: AgentProfile.ID) -> AlertState { - AlertState { - TextState("Allow Unrestricted Execution?") - } actions: { - ButtonState(role: .destructive, action: .confirmUnrestricted(profileID)) { - TextState("Allow Unrestricted") - } - ButtonState(role: .cancel) { - TextState("Cancel") - } - } message: { - TextState( - "The agent will run without permission prompts or sandboxing. " - + "It can execute any command and modify any file your user can." - ) - } - } - - static func removalAlert(profile: AgentProfile) -> AlertState { - AlertState { - TextState("Remove “\(profile.name)”?") - } actions: { - ButtonState(action: .removeKeepingFiles(profile.id)) { - TextState("Remove Profile") - } - ButtonState(role: .destructive, action: .removeTrashingFiles(profile.id)) { - TextState("Remove and Trash Files") - } - ButtonState(role: .cancel) { - TextState("Cancel") - } - } message: { - TextState( - "This profile has its own home with login credentials and files. " - + "“Remove Profile” keeps them on disk; “Remove and Trash Files” moves the folder to the Trash." - ) - } - } } diff --git a/supacode/Features/Settings/Reducer/SettingsFeature.swift b/supacode/Features/Settings/Reducer/SettingsFeature.swift index 41656ea6..84cffa8c 100644 --- a/supacode/Features/Settings/Reducer/SettingsFeature.swift +++ b/supacode/Features/Settings/Reducer/SettingsFeature.swift @@ -282,6 +282,8 @@ struct SettingsFeature { state.showNotificationDotOnDock = normalizedSettings.showNotificationDotOnDock state.externalDiffToolID = normalizedSettings.externalDiffToolID state.externalDiffCustomCommand = normalizedSettings.externalDiffCustomCommand + state.canvasDefaultLayout = normalizedSettings.canvasDefaultLayout + state.detectRepositoryIconsAutomatically = normalizedSettings.detectRepositoryIconsAutomatically state.syncGlobalDefaults(from: normalizedSettings) return .send(.delegate(.settingsChanged(normalizedSettings))) @@ -345,6 +347,11 @@ struct SettingsFeature { } case .uninstallCLIButtonTapped: + // Uninstall is only reachable from the Settings UI, so its result + // must always alert — without this, a palette-triggered install + // (showAlert: false) leaves the flag stuck and a failed uninstall + // would report nothing at all. + state.cliInstallShowAlert = true let installPath = cliDefaultInstallPath return .run { [cliInstallClient] send in do { @@ -428,12 +435,7 @@ struct SettingsFeature { state.selection = selection ?? .general return .none - case .alert(.dismiss): - state.alert = nil - return .none - case .alert(.presented(.openSystemNotificationSettings)): - state.alert = nil return .run { _ in await systemNotificationClient.openSettings() } @@ -463,6 +465,10 @@ struct SettingsFeature { .ifLet(\.agentProfiles, action: \.agentProfiles) { AgentProfilesFeature() } + // Without this, alert state is only cleared by the view's dismiss + // writeback: state set while the Settings window is closed (or closed + // while an alert is up) would wedge as permanently "presented". + .ifLet(\.$alert, action: \.alert) } private func persist( diff --git a/supacode/Features/Settings/Views/AgentProfileEditorView.swift b/supacode/Features/Settings/Views/AgentProfileEditorView.swift new file mode 100644 index 00000000..f0f3c25c --- /dev/null +++ b/supacode/Features/Settings/Views/AgentProfileEditorView.swift @@ -0,0 +1,205 @@ +import ComposableArchitecture +import SwiftUI + +/// Settings → Agents → drill-in editor page for one profile. Presented by +/// `AgentProfilesSettingsView` via `navigationDestination`; owns the alert +/// presentation because its feature owns the alert state. +struct AgentProfileEditorView: View { + @Bindable var store: StoreOf + + var body: some View { + VStack(alignment: .leading, spacing: 0) { + header + Form { + profileSection + advancedSection + removalSection + } + .formStyle(.grouped) + } + .task { store.send(.task) } + .alert($store.scope(state: \.alert, action: \.alert)) + } + + /// Page header: back capsule plus the profile identity. The window title + /// stays a constant "Agents", so the drill-in page names itself here. + private var header: some View { + HStack(spacing: 12) { + Button { + store.send(.backTapped) + } label: { + Label("Back", systemImage: "chevron.left") + .labelStyle(.iconOnly) + } + .buttonStyle(.glass) + .keyboardShortcut("[", modifiers: .command) + .help("Back to Agent Profiles (⌘[)") + + VStack(alignment: .leading, spacing: 2) { + Text(store.profile.name) + .font(.title3.weight(.semibold)) + Text(AgentRuntimeAdapterRegistry.displayName(for: store.profile.runtime.agent)) + .font(.caption) + .foregroundStyle(.secondary) + } + Spacer() + } + .padding(.leading, 4) + } + + private var profileSection: some View { + Section("Profile") { + TextField("Name", text: $store.profile.name) + Picker("Agent", selection: $store.profile.runtime) { + ForEach(AgentProfileRuntime.allCases) { runtime in + Text(AgentRuntimeAdapterRegistry.displayName(for: runtime.agent)).tag(runtime) + } + } + optionalTextRow( + title: "Model", + prompt: "Runtime default", + text: $store.profile.model + ) + effortRow + Picker("Execution Mode", selection: $store.profile.executionMode) { + Text("Standard").tag(AgentExecutionMode.standard) + Text("Unrestricted").tag(AgentExecutionMode.unrestricted) + } + switch store.profile.effectiveExecutionMode { + case .standard: + EmptyView() + case .unrestricted: + Text( + store.profile.executionMode == .unrestricted + ? "Runs without permission prompts or sandboxing." + : "Extra arguments enable unrestricted execution — no permission prompts or sandboxing." + ) + .font(.caption) + .foregroundStyle(.red) + case .followsExtraArguments: + Text("Effective execution mode follows your extra arguments.") + .font(.caption) + .foregroundStyle(.secondary) + } + Picker("Open In", selection: $store.profile.placement) { + Text("New Tab").tag(AgentProfilePlacement.tab) + Text("New Split").tag(AgentProfilePlacement.split) + } + if store.profile.placement == .split { + Picker("Split Direction", selection: $store.profile.splitDirection) { + ForEach(UserCustomSplitDirection.allCases) { direction in + Text(direction.title).tag(direction) + } + } + } + } + } + + private var advancedSection: some View { + Section("Advanced") { + optionalTextRow( + title: "Extra Arguments", + prompt: "--flag value", + text: Binding( + get: { store.profile.extraArguments.isEmpty ? nil : store.profile.extraArguments }, + set: { $store.profile.extraArguments.wrappedValue = $0 ?? "" } + ) + ) + Toggle("Use Dedicated Home", isOn: $store.profile.bindsDedicatedHome) + .help("Keep a separate login, usage, and configuration for this profile") + if store.profile.bindsDedicatedHome { + Text( + "This profile gets its own runtime home: separate login and usage, " + + "but also separate skills, global instructions, and session history. " + + "The first launch signs in through the agent itself." + ) + .font(.caption) + .foregroundStyle(.secondary) + LabeledContent( + "Profile Home", + value: store.homeInitialized ? "Initialized" : "Not initialized yet" + ) + Button("Reveal Profile Files") { + store.send(.revealProfileFiles) + } + .help("Open the profile's home folder in Finder") + } + launchPreview + } + } + + private var removalSection: some View { + Section { + Button(role: .destructive) { + store.send(.removeTapped) + } label: { + Text("Remove Profile…") + } + .help("Remove this profile") + } + } + + private var effortRow: some View { + HStack { + optionalTextRow( + title: "Reasoning Effort", + prompt: "Runtime default", + text: $store.profile.reasoningEffort + ) + Menu { + ForEach(effortSuggestions, id: \.self) { suggestion in + Button(suggestion) { $store.profile.reasoningEffort.wrappedValue = suggestion } + } + Button("Runtime Default") { $store.profile.reasoningEffort.wrappedValue = nil } + } label: { + Image(systemName: "chevron.up.chevron.down") + .accessibilityLabel("Effort suggestions") + } + .menuStyle(.borderlessButton) + .fixedSize() + .help("Pick a known effort level, or type any value") + } + } + + private var launchPreview: some View { + VStack(alignment: .leading, spacing: 4) { + Text("Launch Preview") + Text(previewText) + .font(.callout.monospaced()) + .foregroundStyle(.secondary) + .textSelection(.enabled) + .lineLimit(nil) + } + } + + private func optionalTextRow( + title: String, + prompt: String, + text: Binding + ) -> some View { + TextField( + title, + text: Binding( + get: { text.wrappedValue ?? "" }, + set: { value in + let trimmed = value.trimmingCharacters(in: .whitespaces) + text.wrappedValue = trimmed.isEmpty ? nil : value + } + ), + prompt: Text(prompt) + ) + } + + private var previewText: String { + let plan = try? AgentProfileLaunchPlanner.plan( + for: store.profile, + homeBaseDirectory: SupacodePaths.agentProfileHomesDirectory + ) + return plan?.previewText ?? "Unavailable" + } + + private var effortSuggestions: [String] { + AgentRuntimeAdapterRegistry.adapter(for: store.profile.runtime.agent)?.reasoningEffortSuggestions + ?? [] + } +} diff --git a/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift b/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift index 7b119bd0..7a3ecab3 100644 --- a/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift +++ b/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift @@ -1,23 +1,31 @@ import ComposableArchitecture import SwiftUI -/// Settings → Agents: profile list plus the editor for the selected profile. +/// Settings → Agents: the profile list, with a drill-in editor page per +/// profile (System Settings style). The drill-in is a state-driven content +/// swap, not a nested `NavigationStack`: a stack inside the split view's +/// detail column drops the first programmatic push, blocks sidebar-driven +/// detail switches while pushed, and reconfigures the titlebar layout. /// List order is the recommendation fallback order. struct AgentProfilesSettingsView: View { @Bindable var store: StoreOf var body: some View { - Form { - profileListSection - if let profile = store.selectedProfile, let binding = profileBinding(profile.id) { - editorSection(profile: profile, binding: binding) - advancedSection(profile: profile, binding: binding) + Group { + if let editorStore = store.scope(state: \.editor, action: \.editor.presented) { + AgentProfileEditorView(store: editorStore) + .transition(.push(from: .trailing)) + } else { + Form { + profileListSection + } + .formStyle(.grouped) + .transition(.push(from: .leading)) } } - .formStyle(.grouped) + .animation(.default, value: store.editor == nil) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) .task { store.send(.task) } - .alert($store.scope(state: \.alert, action: \.alert)) } private var profileListSection: some View { @@ -41,14 +49,6 @@ struct AgentProfilesSettingsView: View { } .fixedSize() .help("Add a new agent profile") - - Button { - store.send(.removeSelectedTapped) - } label: { - Label("Remove", systemImage: "minus") - } - .disabled(store.selectedProfile == nil) - .help("Remove the selected profile") Spacer() } } header: { @@ -72,7 +72,7 @@ struct AgentProfilesSettingsView: View { .help("Show this profile in the Agents menu") } Button { - store.send(.selectProfile(profile.id)) + store.send(.profileTapped(profile.id)) } label: { HStack(spacing: 8) { Text(profile.name) @@ -85,16 +85,16 @@ struct AgentProfilesSettingsView: View { Spacer() Text(AgentRuntimeAdapterRegistry.displayName(for: profile.runtime.agent)) .foregroundStyle(.secondary) + Image(systemName: "chevron.right") + .font(.caption.weight(.semibold)) + .foregroundStyle(.tertiary) + .accessibilityHidden(true) } .contentShape(Rectangle()) } .buttonStyle(.plain) .help("Edit this profile") } - .listRowBackground( - store.selectedProfileID == profile.id - ? Color.accentColor.opacity(0.12) : Color.clear - ) .contextMenu { Button("Move Up") { move(profile.id, by: -1) } .disabled(index(of: profile.id) == 0) @@ -103,150 +103,6 @@ struct AgentProfilesSettingsView: View { } } - private func editorSection(profile: AgentProfile, binding: Binding) -> some View { - Section("Profile") { - TextField("Name", text: binding.name) - Picker("Agent", selection: binding.runtime) { - ForEach(AgentProfileRuntime.allCases) { runtime in - Text(AgentRuntimeAdapterRegistry.displayName(for: runtime.agent)).tag(runtime) - } - } - optionalTextRow( - title: "Model", - prompt: "Runtime default", - text: binding.model - ) - effortRow(profile: profile, binding: binding) - Picker("Execution Mode", selection: binding.executionMode) { - Text("Standard").tag(AgentExecutionMode.standard) - Text("Unrestricted").tag(AgentExecutionMode.unrestricted) - } - switch profile.effectiveExecutionMode { - case .standard: - EmptyView() - case .unrestricted: - Text( - profile.executionMode == .unrestricted - ? "Runs without permission prompts or sandboxing." - : "Extra arguments enable unrestricted execution — no permission prompts or sandboxing." - ) - .font(.caption) - .foregroundStyle(.red) - case .followsExtraArguments: - Text("Effective execution mode follows your extra arguments.") - .font(.caption) - .foregroundStyle(.secondary) - } - Picker("Open In", selection: binding.placement) { - Text("New Tab").tag(AgentProfilePlacement.tab) - Text("New Split").tag(AgentProfilePlacement.split) - } - if profile.placement == .split { - Picker("Split Direction", selection: binding.splitDirection) { - ForEach(UserCustomSplitDirection.allCases) { direction in - Text(direction.title).tag(direction) - } - } - } - } - } - - private func advancedSection(profile: AgentProfile, binding: Binding) -> some View { - Section("Advanced") { - optionalTextRow( - title: "Extra Arguments", - prompt: "--flag value", - text: Binding( - get: { profile.extraArguments.isEmpty ? nil : profile.extraArguments }, - set: { binding.wrappedValue.extraArguments = $0 ?? "" } - ) - ) - Toggle("Use Dedicated Home", isOn: binding.bindsDedicatedHome) - .help("Keep a separate login, usage, and configuration for this profile") - if profile.bindsDedicatedHome { - Text( - "This profile gets its own runtime home: separate login and usage, " - + "but also separate skills, global instructions, and session history. " - + "The first launch signs in through the agent itself." - ) - .font(.caption) - .foregroundStyle(.secondary) - LabeledContent( - "Profile Home", - value: store.selectedHomeInitialized ? "Initialized" : "Not initialized yet" - ) - Button("Reveal Profile Files") { - store.send(.revealProfileFiles) - } - .help("Open the profile's home folder in Finder") - } - launchPreview(profile: profile) - } - } - - private func effortRow(profile: AgentProfile, binding: Binding) -> some View { - HStack { - optionalTextRow( - title: "Reasoning Effort", - prompt: "Runtime default", - text: binding.reasoningEffort - ) - Menu { - ForEach(effortSuggestions(for: profile.runtime), id: \.self) { suggestion in - Button(suggestion) { binding.wrappedValue.reasoningEffort = suggestion } - } - Button("Runtime Default") { binding.wrappedValue.reasoningEffort = nil } - } label: { - Image(systemName: "chevron.up.chevron.down") - .accessibilityLabel("Effort suggestions") - } - .menuStyle(.borderlessButton) - .fixedSize() - .help("Pick a known effort level, or type any value") - } - } - - private func launchPreview(profile: AgentProfile) -> some View { - VStack(alignment: .leading, spacing: 4) { - Text("Launch Preview") - Text(previewText(for: profile)) - .font(.callout.monospaced()) - .foregroundStyle(.secondary) - .textSelection(.enabled) - .lineLimit(nil) - } - } - - private func optionalTextRow( - title: String, - prompt: String, - text: Binding - ) -> some View { - TextField( - title, - text: Binding( - get: { text.wrappedValue ?? "" }, - set: { value in - let trimmed = value.trimmingCharacters(in: .whitespaces) - text.wrappedValue = trimmed.isEmpty ? nil : value - } - ), - prompt: Text(prompt) - ) - } - - private func previewText(for profile: AgentProfile) -> String { - let plan = try? AgentProfileLaunchPlanner.plan( - for: profile, - homeBaseDirectory: SupacodePaths.agentProfileHomesDirectory - ) - return plan?.previewText ?? "Unavailable" - } - - private func effortSuggestions(for runtime: AgentProfileRuntime) -> [String] { - AgentRuntimeAdapterRegistry.adapter(for: runtime.agent)?.reasoningEffortSuggestions ?? [] - } - private func index(of id: AgentProfile.ID) -> Int? { store.settings.agentProfiles.firstIndex { $0.id == id } } diff --git a/supacode/Features/Settings/Views/SettingsView.swift b/supacode/Features/Settings/Views/SettingsView.swift index 5d951de9..8e71d612 100644 --- a/supacode/Features/Settings/Views/SettingsView.swift +++ b/supacode/Features/Settings/Views/SettingsView.swift @@ -128,6 +128,10 @@ struct SettingsView: View { state: \.agentProfiles, action: \.agentProfiles ) { + // The title stays constant across the drill-in: the editor page + // identifies itself in its own header, and swapping title + // modifiers with the pages destabilizes the hand-configured + // window toolbar. AgentProfilesSettingsView(store: agentProfilesStore) .navigationTitle("Agents") .navigationSubtitle("Agent profiles and launch presets") diff --git a/supacodeTests/AgentProfileEditorFeatureTests.swift b/supacodeTests/AgentProfileEditorFeatureTests.swift new file mode 100644 index 00000000..d0332d15 --- /dev/null +++ b/supacodeTests/AgentProfileEditorFeatureTests.swift @@ -0,0 +1,117 @@ +import ComposableArchitecture +import DependenciesTestSupport +import Foundation +import Sharing +import Testing + +@testable import supacode + +@MainActor +struct AgentProfileEditorFeatureTests { + @Test(.dependencies) func editsDelegateProfileEdited() async { + let profile = AgentProfile(name: "Codex", runtime: .codex) + let store = TestStore(initialState: AgentProfileEditorFeature.State(profile: profile)) { + AgentProfileEditorFeature() + } + + var edited = profile + edited.name = "Codex · Deep" + await store.send(.binding(.set(\.profile, edited))) { + $0.profile.name = "Codex · Deep" + } + await store.receive(\.delegate.profileEdited) + } + + @Test(.dependencies) func unrestrictedRequiresExplicitConfirmation() async { + let profile = AgentProfile(name: "Codex", runtime: .codex) + let storage = SettingsTestStorage() + let (store, persisted) = withDependencies { + $0.settingsFileStorage = storage.storage + } operation: { + @Shared(.userGlobalSettings) var settings + $settings.withLock { $0.agentProfiles = [profile] } + let store = TestStore(initialState: AgentProfileEditorFeature.State(profile: profile)) { + AgentProfileEditorFeature() + } + return (store, $settings) + } + + var edited = profile + edited.executionMode = .unrestricted + + // The binding write reverts and asks first. + await store.send(.binding(.set(\.profile, edited))) { + $0.alert = AgentProfileEditorFeature.unrestrictedAlert() + } + + await store.send(.alert(.presented(.confirmUnrestricted))) { + $0.alert = nil + $0.profile.executionMode = .unrestricted + } + await store.receive(\.delegate.profileEdited) + #expect(persisted.wrappedValue.agentProfiles.first?.executionMode == .standard) + } + + @Test(.dependencies) func removingBoundProfileConfirmsAndDelegatesTrash() async { + var bound = AgentProfile(name: "Codex · Work", runtime: .codex) + bound.bindsDedicatedHome = true + let store = TestStore(initialState: AgentProfileEditorFeature.State(profile: bound)) { + AgentProfileEditorFeature() + } + + await store.send(.removeTapped) { + $0.alert = AgentProfileEditorFeature.removalAlert(profile: bound) + } + await store.send(.alert(.presented(.removeTrashingFiles))) { + $0.alert = nil + } + await store.receive(\.delegate.removeProfile) + } + + @Test(.dependencies) func unboundProfileWithHomeOnDiskStillConfirmsRemoval() async { + // bind → launch (home created) → unbind → remove: the gate keys on the + // disk fact, so the credentials never get orphaned silently. + let unbound = AgentProfile(name: "Codex · Was Bound", runtime: .codex) + let store = TestStore(initialState: AgentProfileEditorFeature.State(profile: unbound)) { + AgentProfileEditorFeature() + } withDependencies: { + $0[AgentProfileHomeClient.self].homeExists = { _ in true } + } + + await store.send(.removeTapped) { + $0.alert = AgentProfileEditorFeature.removalAlert(profile: unbound) + } + await store.send(.alert(.presented(.removeKeepingFiles))) { + $0.alert = nil + } + await store.receive(\.delegate.removeProfile) + } + + @Test(.dependencies) func removingPurePresetSkipsConfirmation() async { + let preset = AgentProfile(name: "Claude", runtime: .claude) + let store = TestStore(initialState: AgentProfileEditorFeature.State(profile: preset)) { + AgentProfileEditorFeature() + } + + // No confirmation and no file operations for a preset with no home. + await store.send(.removeTapped) + await store.receive(\.delegate.removeProfile) + } + + @Test(.dependencies) func revealRefreshesThePassiveHomeStatus() async { + var bound = AgentProfile(name: "Codex · Work", runtime: .codex) + bound.bindsDedicatedHome = true + let homeOnDisk = LockIsolated(false) + let store = TestStore(initialState: AgentProfileEditorFeature.State(profile: bound)) { + AgentProfileEditorFeature() + } withDependencies: { + $0[AgentProfileHomeClient.self].homeExists = { _ in homeOnDisk.value } + $0[AgentProfileHomeClient.self].revealHome = { _ in homeOnDisk.setValue(true) } + } + + await store.send(.revealProfileFiles) + await store.receive(\.homeStatusRefreshed) { + $0.homeInitialized = true + } + } +} diff --git a/supacodeTests/AgentProfilesFeatureTests.swift b/supacodeTests/AgentProfilesFeatureTests.swift index 97a76d5c..3b7224ee 100644 --- a/supacodeTests/AgentProfilesFeatureTests.swift +++ b/supacodeTests/AgentProfilesFeatureTests.swift @@ -8,7 +8,7 @@ import Testing @MainActor struct AgentProfilesFeatureTests { - @Test(.dependencies) func taskLoadsSettingsAndSelectsFirstProfile() async { + @Test(.dependencies) func taskLoadsSettingsWithoutPushingAnEditor() async { let profile = AgentProfile(name: "Codex", runtime: .codex) let storage = SettingsTestStorage() let store = withDependencies { @@ -21,14 +21,15 @@ struct AgentProfilesFeatureTests { } } + // The root page is the list; a non-nil editor would push the drill-in. await store.send(.task) await store.receive(\.settingsLoaded) { $0.settings.agentProfiles = [profile] - $0.selectedProfileID = profile.id } + #expect(store.state.editor == nil) } - @Test(.dependencies) func addProfileAppendsSelectsAndPersists() async { + @Test(.dependencies) func addProfileAppendsOpensEditorAndPersists() async { let storage = SettingsTestStorage() let store = withDependencies { $0.settingsFileStorage = storage.storage @@ -47,45 +48,32 @@ struct AgentProfilesFeatureTests { ) await store.send(.addProfile(.claude)) { $0.settings.agentProfiles = [expected] - $0.selectedProfileID = expected.id + $0.editor = AgentProfileEditorFeature.State(profile: expected) } await store.receive(\.delegate.settingsChanged) } - @Test(.dependencies) func unrestrictedRequiresExplicitConfirmation() async { + @Test(.dependencies) func profileTappedPushesItsEditor() async { let profile = AgentProfile(name: "Codex", runtime: .codex) let storage = SettingsTestStorage() - let (store, persisted) = withDependencies { + let store = withDependencies { $0.settingsFileStorage = storage.storage } operation: { @Shared(.userGlobalSettings) var settings $settings.withLock { $0.agentProfiles = [profile] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.selectedProfileID = profile.id - let store = TestStore(initialState: initial) { + return TestStore(initialState: initial) { AgentProfilesFeature() } - return (store, $settings) } - var edited = store.state.settings - edited.agentProfiles[0].executionMode = .unrestricted - - // The binding write reverts and asks first. - await store.send(.binding(.set(\.settings, edited))) { - $0.alert = AgentProfilesFeature.unrestrictedAlert(profileID: profile.id) - } - - await store.send(.alert(.presented(.confirmUnrestricted(profile.id)))) { - $0.alert = nil - $0.settings.agentProfiles[0].executionMode = .unrestricted + await store.send(.profileTapped(profile.id)) { + $0.editor = AgentProfileEditorFeature.State(profile: profile) } - await store.receive(\.delegate.settingsChanged) - #expect(persisted.wrappedValue.agentProfiles.first?.executionMode == .unrestricted) } - @Test(.dependencies) func blankNameNeverPersistsOrDeletesTheProfile() async { + @Test(.dependencies) func profileEditedDelegateUpdatesTheListAndSanitizesPersistence() async { let profile = AgentProfile(name: "Codex", runtime: .codex) let storage = SettingsTestStorage() let (store, persisted) = withDependencies { @@ -95,18 +83,18 @@ struct AgentProfilesFeatureTests { $settings.withLock { $0.agentProfiles = [profile] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.selectedProfileID = profile.id + initial.editor = AgentProfileEditorFeature.State(profile: profile) let store = TestStore(initialState: initial) { AgentProfilesFeature() } return (store, $settings) } - // Clearing the Name field keeps the profile editable in state and keeps - // the last persisted name on disk — never a silent deletion. - var edited = store.state.settings - edited.agentProfiles[0].name = " " - await store.send(.binding(.set(\.settings, edited))) { + // A blank in-progress name reaches the list state but never the disk — + // the persisted profile keeps its last non-blank name. + var edited = profile + edited.name = " " + await store.send(.editor(.presented(.delegate(.profileEdited(edited))))) { $0.settings.agentProfiles[0].name = " " } await store.receive(\.delegate.settingsChanged) @@ -114,15 +102,15 @@ struct AgentProfilesFeatureTests { #expect(persisted.wrappedValue.agentProfiles.first?.name == "Codex") // Typing the new name persists it normally. - edited.agentProfiles[0].name = "Codex · Deep" - await store.send(.binding(.set(\.settings, edited))) { + edited.name = "Codex · Deep" + await store.send(.editor(.presented(.delegate(.profileEdited(edited))))) { $0.settings.agentProfiles[0].name = "Codex · Deep" } await store.receive(\.delegate.settingsChanged) #expect(persisted.wrappedValue.agentProfiles.first?.name == "Codex · Deep") } - @Test(.dependencies) func removingBoundProfileConfirmsAndCanTrashHome() async { + @Test(.dependencies) func editorRemovalDelegatePopsAndTrashesTheHome() async { var bound = AgentProfile(name: "Codex · Work", runtime: .codex) bound.bindsDedicatedHome = true let storage = SettingsTestStorage() @@ -134,7 +122,7 @@ struct AgentProfilesFeatureTests { $settings.withLock { $0.agentProfiles = [bound] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.selectedProfileID = bound.id + initial.editor = AgentProfileEditorFeature.State(profile: bound) return TestStore(initialState: initial) { AgentProfilesFeature() } withDependencies: { @@ -144,81 +132,70 @@ struct AgentProfilesFeatureTests { } } - await store.send(.removeSelectedTapped) { - $0.alert = AgentProfilesFeature.removalAlert(profile: bound) - } - await store.send(.alert(.presented(.removeTrashingFiles(bound.id)))) { - $0.alert = nil + // Removal pops the editor back to the list and trashes in the parent, so + // the dismissal cannot cancel the file operation. + await store.send(.editor(.presented(.delegate(.removeProfile(bound.id, trashFiles: true))))) { $0.settings.agentProfiles = [] - $0.selectedProfileID = nil + $0.editor = nil } await store.receive(\.delegate.settingsChanged) await store.finish() #expect(trashed.value == [bound.id]) } - @Test(.dependencies) func unboundProfileWithHomeOnDiskStillConfirmsRemoval() async { - // bind → launch (home created) → unbind → remove: the gate keys on the - // disk fact, so the credentials never get orphaned silently. - let unbound = AgentProfile(name: "Codex · Was Bound", runtime: .codex) + @Test(.dependencies) func editorRemovalDelegateCanKeepFilesOnDisk() async { + let preset = AgentProfile(name: "Claude", runtime: .claude) let storage = SettingsTestStorage() let trashed = LockIsolated<[AgentProfile.ID]>([]) let store = withDependencies { $0.settingsFileStorage = storage.storage } operation: { @Shared(.userGlobalSettings) var settings - $settings.withLock { $0.agentProfiles = [unbound] } + $settings.withLock { $0.agentProfiles = [preset] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.selectedProfileID = unbound.id + initial.editor = AgentProfileEditorFeature.State(profile: preset) return TestStore(initialState: initial) { AgentProfilesFeature() } withDependencies: { - $0[AgentProfileHomeClient.self].homeExists = { _ in true } $0[AgentProfileHomeClient.self].trashHome = { id in trashed.withValue { $0.append(id) } } } } - await store.send(.removeSelectedTapped) { - $0.alert = AgentProfilesFeature.removalAlert(profile: unbound) - } - await store.send(.alert(.presented(.removeTrashingFiles(unbound.id)))) { - $0.alert = nil + await store.send(.editor(.presented(.delegate(.removeProfile(preset.id, trashFiles: false))))) { $0.settings.agentProfiles = [] - $0.selectedProfileID = nil + $0.editor = nil } await store.receive(\.delegate.settingsChanged) await store.finish() - #expect(trashed.value == [unbound.id]) + #expect(trashed.value.isEmpty) } - @Test(.dependencies) func revealRefreshesThePassiveHomeStatus() async { - var bound = AgentProfile(name: "Codex · Work", runtime: .codex) - bound.bindsDedicatedHome = true + @Test(.dependencies) func togglingEnabledPersistsFromTheList() async { + let profile = AgentProfile(name: "Codex", runtime: .codex) let storage = SettingsTestStorage() - let homeOnDisk = LockIsolated(false) - let store = withDependencies { + let (store, persisted) = withDependencies { $0.settingsFileStorage = storage.storage } operation: { @Shared(.userGlobalSettings) var settings - $settings.withLock { $0.agentProfiles = [bound] } + $settings.withLock { $0.agentProfiles = [profile] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.selectedProfileID = bound.id - return TestStore(initialState: initial) { + let store = TestStore(initialState: initial) { AgentProfilesFeature() - } withDependencies: { - $0[AgentProfileHomeClient.self].homeExists = { _ in homeOnDisk.value } - $0[AgentProfileHomeClient.self].revealHome = { _ in homeOnDisk.setValue(true) } } + return (store, $settings) } - await store.send(.revealProfileFiles) - await store.receive(\.homeStatusRefreshed) { - $0.selectedHomeInitialized = true + var edited = store.state.settings + edited.agentProfiles[0].isEnabled = false + await store.send(.binding(.set(\.settings, edited))) { + $0.settings.agentProfiles[0].isEnabled = false } + await store.receive(\.delegate.settingsChanged) + #expect(persisted.wrappedValue.agentProfiles.first?.isEnabled == false) } @Test func launchConfigRootOnlyAppliesToTheLaunchedRuntime() { @@ -235,29 +212,6 @@ struct AgentProfilesFeatureTests { #expect(identity.configRoot(forDetected: .claude) == nil) } - @Test(.dependencies) func removingPurePresetSkipsConfirmationAndFileOperations() async { - let preset = AgentProfile(name: "Claude", runtime: .claude) - let storage = SettingsTestStorage() - let store = withDependencies { - $0.settingsFileStorage = storage.storage - } operation: { - @Shared(.userGlobalSettings) var settings - $settings.withLock { $0.agentProfiles = [preset] } - var initial = AgentProfilesFeature.State() - initial.settings = settings - initial.selectedProfileID = preset.id - return TestStore(initialState: initial) { - AgentProfilesFeature() - } - } - - await store.send(.removeSelectedTapped) { - $0.settings.agentProfiles = [] - $0.selectedProfileID = nil - } - await store.receive(\.delegate.settingsChanged) - } - @Test(.dependencies) func moveReordersFallbackPriority() async { let first = AgentProfile(name: "First", runtime: .codex) let second = AgentProfile(name: "Second", runtime: .claude) diff --git a/supacodeTests/SettingsFeatureTests.swift b/supacodeTests/SettingsFeatureTests.swift index 99ce21e3..34e7d6fa 100644 --- a/supacodeTests/SettingsFeatureTests.swift +++ b/supacodeTests/SettingsFeatureTests.swift @@ -564,4 +564,67 @@ struct SettingsFeatureTests { await store.send(.clearTerminalLayoutSnapshotButtonTapped) await store.receive(\.delegate.terminalLayoutSnapshotCleared) } + + @Test(.dependencies) func alertButtonActionClearsThePresentationInTheReducer() async { + let store = TestStore(initialState: SettingsFeature.State()) { + SettingsFeature() + } + + await store.send(.showNotificationPermissionAlert(errorMessage: nil)) { + $0.alert = AlertState { + TextState("Prowl cannot send system notifications") + } actions: { + ButtonState(action: .openSystemNotificationSettings) { + TextState("Open System Settings") + } + ButtonState(role: .cancel, action: .dismiss) { + TextState("Cancel") + } + } message: { + TextState( + "Notification permission is turned off. Open System Settings to allow Prowl to send notifications." + ) + } + } + + // The presentation reducer must clear the ephemeral alert itself — if the + // clearing only happened through the view's dismiss writeback, alert + // state set while the Settings window is closed would wedge as + // permanently "presented". + await store.send(.alert(.presented(.dismiss))) { + $0.alert = nil + } + } + + @Test(.dependencies) func uninstallAlwaysAlertsEvenAfterSilentInstall() async { + let store = TestStore(initialState: SettingsFeature.State()) { + SettingsFeature() + } withDependencies: { + $0.cliInstallClient.install = { _ in } + $0.cliInstallClient.uninstall = { _ in } + $0.cliInstallClient.installationStatus = { _ in .notInstalled } + } + + // A palette-triggered install suppresses its own alert… + await store.send(.installCLIButtonTapped(showAlert: false)) { + $0.cliInstallShowAlert = false + } + await store.receive(\.cliInstallCompleted) + await store.receive(\.delegate.cliInstallCompleted) + + // …but a later uninstall from Settings must still report its result. + await store.send(.uninstallCLIButtonTapped) { + $0.cliInstallShowAlert = true + } + await store.receive(\.cliInstallCompleted) { + $0.alert = AlertState { + TextState("Command Line Tool Uninstalled") + } actions: { + ButtonState(action: .dismiss) { TextState("OK") } + } message: { + TextState("The prowl command line tool has been removed.") + } + } + await store.receive(\.delegate.cliInstallCompleted) + } } -- 2.51.2 From b5359b7b3868286aeaddaeed79eba96b36b90433 Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 31 Jul 2026 00:19:45 +0900 Subject: [PATCH 2/4] Refactor Settings navigation to native SwiftUI --- .../000-plan.md | 100 +++++++++++++++++ .../001-action.md | 70 ++++++++++++ docs-ai/README.md | 1 + docs/components/agent-profiles.md | 8 ++ docs/components/settings.md | 15 ++- supacode/App/SettingsWindowOpener.swift | 58 ++++++++++ supacode/App/supacodeApp.swift | 28 ++--- .../SettingsWindow/SettingsWindowClient.swift | 2 +- supacode/Commands/WindowCommands.swift | 51 +++++++-- .../Reducer/AppFeature+CommandPalette.swift | 7 +- .../AppFeature+SettingsNavigation.swift | 14 +++ .../Features/App/Reducer/AppFeature.swift | 14 +-- .../BusinessLogic/DebugWindowManager.swift | 9 +- .../Views/SidebarFooterView.swift | 9 +- .../BusinessLogic/SettingsWindowManager.swift | 104 ------------------ .../Reducer/AgentProfileEditorFeature.swift | 5 - .../Reducer/AgentProfilesFeature.swift | 34 +++--- .../Views/AgentProfileEditorView.swift | 45 ++------ .../Views/AgentProfilesSettingsView.swift | 68 +++++------- .../Settings/Views/SettingsDetailView.swift | 3 - .../Settings/Views/SettingsView.swift | 38 +------ supacodeTests/AgentProfilesFeatureTests.swift | 45 +++++--- .../AppFeatureSettingsSelectionTests.swift | 18 +++ supacodeTests/SettingsWindowOpenerTests.swift | 36 ++++++ .../WindowCloseShortcutPolicyTests.swift | 35 ++---- supacodeTests/WindowSurfacingTests.swift | 4 +- 26 files changed, 476 insertions(+), 345 deletions(-) create mode 100644 docs-ai/054-native-settings-navigation/000-plan.md create mode 100644 docs-ai/054-native-settings-navigation/001-action.md create mode 100644 supacode/App/SettingsWindowOpener.swift create mode 100644 supacode/Features/App/Reducer/AppFeature+SettingsNavigation.swift delete mode 100644 supacode/Features/Settings/BusinessLogic/SettingsWindowManager.swift create mode 100644 supacodeTests/SettingsWindowOpenerTests.swift diff --git a/docs-ai/054-native-settings-navigation/000-plan.md b/docs-ai/054-native-settings-navigation/000-plan.md new file mode 100644 index 00000000..637e7379 --- /dev/null +++ b/docs-ai/054-native-settings-navigation/000-plan.md @@ -0,0 +1,100 @@ +# 054 — Native Settings Navigation: Plan + +| | | +| --- | --- | +| **Status** | Implemented (see [001-action.md](001-action.md)) | +| **Anchor date** | 2026-07-30 | +| **Primary PRs** | — (filled on merge) | +| **Related** | [036 window-management-hardening](../036-window-management-hardening/000-plan.md), [053 agent-profiles](../053-agent-profiles/000-plan.md) | + +## Background + +Prowl's Settings window was created and retained by +`SettingsWindowManager`. It owned an `NSWindow`, titlebar appearance, +activation, closing, and a local Cmd-W monitor. The Agents editor was a +conditional detail-content swap with custom transitions and an in-page Back +button. + +That architecture made a core macOS surface look bespoke. The initial SwiftUI +`Settings`-scene experiment also retained a preference-style titlebar that did +not compose correctly with the split view: its traffic lights, sidebar toggle, +and drill-in Back control were not arranged like a regular macOS Settings +window. The target is instead a SwiftUI-managed singleton window with native +split-view and navigation chrome. + +## Goals + +- Use a SwiftUI-owned singleton `Window` scene for Settings; no Settings-owned + `NSWindow`, event monitor, custom titlebar, or manual Back button. +- Keep top-level sections in `NavigationSplitView`, including the system + sidebar toggle and the expected traffic-light/titlebar layout. +- Push Agent Profile editing with a real `NavigationStack`, giving macOS its + standard compact Back control and title placement. +- Keep navigation state in TCA, including a deterministic reset when the user + changes a sidebar section while an editor is pushed. +- Preserve every opening path: Cmd-comma/app menu, sidebar footer, Command + Palette, repository settings, and Manage Agent Profiles. + +### Non-goals + +- Redesigning individual Settings forms, profile fields, or persistence. +- Changing the main-window reopening or terminal-window management paths. +- Building custom replicas of the macOS titlebar, sidebar toggle, or Back + button. + +## Design + +### Singleton SwiftUI window + +`SupacodeApp` declares `Window("Settings", id: WindowID.settings)`, which is +SwiftUI's singleton auxiliary-window scene. `openWindow(id:)` presents or +brings that same window forward; it replaces the manager's retained +`NSWindow`. + +`SettingsWindowOpener` is a narrow bridge for reducer-driven entry points that +do not have a `View` environment. It stores only SwiftUI's +`OpenWindowAction`; `SettingsWindowClient` remains the TCA dependency boundary. +The reducer sends `SettingsFeature.Action.setSelection` before requesting the +window, so direct entries retain their intended section. Local view entries use +the environment's `openWindow` action directly. + +The standard window toolbar remains `.automatic`. This deliberately leaves +traffic lights, the sidebar toggle, navigation title, and Back placement to +macOS instead of arranging any of them manually. A focused-scene close action +lets the shared Close Window command use SwiftUI's `dismiss` for Settings, so +Cmd-W continues to work even when the main-window terminal close actions are +currently registered. + +### TCA-backed profile drill-in + +`SettingsView` owns the top-level `NavigationSplitView` selection. Selecting a +non-Agent section clears `SettingsFeature.agentProfiles`, which destroys any +active drill-in state rather than leaving a stale profile editor visible. + +`AgentProfilesFeature.State.path` is `StackState`. +`AgentProfilesSettingsView` binds it to `NavigationStack(path:)` and uses +`NavigationLink(state:)` for profile rows. `StackAction` writes system Back +pops directly into the reducer-owned route. Add Profile appends a destination; +editor delegates update or remove the matching profile in the parent state. + +## Alternatives and decisions + +| Alternative | Decision | +| --- | --- | +| Retain `SettingsWindowManager` and improve its titlebar | Rejected. It keeps the lifecycle and chrome outside SwiftUI. | +| Use SwiftUI's special `Settings` scene | Rejected after a visual prototype. Its preference-scene chrome did not yield the desired split-window placement for traffic lights, sidebar toggle, and nested Back navigation. | +| Direct `navigationDestination` in the split detail column | Rejected. It replaces the detail root rather than creating the desired push/back hierarchy. | +| Conditional editor-content swap and custom Back | Rejected. It imitates navigation but cannot integrate with native window navigation controls. | +| Wrap the entire split view in one stack | Rejected. Top-level sidebar selection must remain available while the detail column drills in. | + +## Acceptance gates + +- Root Settings window visually uses the standard macOS window chrome. +- Agents → profile uses the system-provided Back control beside the navigation + title; Back returns to the profile list. +- The sidebar remains visible and selectable during an editor drill-in; choosing + another section shows that section's root content. +- Cmd-comma opens/re-surfaces the one Settings window, and Cmd-W closes it. +- Targeted TCA tests cover route push/pop, mutations through the path, and + sidebar-reset behavior; the app builds and manual debug-window verification + captures the root, Agents, editor, sidebar toggle, and close/reopen paths. diff --git a/docs-ai/054-native-settings-navigation/001-action.md b/docs-ai/054-native-settings-navigation/001-action.md new file mode 100644 index 00000000..34fa34a1 --- /dev/null +++ b/docs-ai/054-native-settings-navigation/001-action.md @@ -0,0 +1,70 @@ +# 054 — Native Settings Navigation: Action Log + +| | | +| --- | --- | +| **Status** | Implemented | +| **Anchor date** | 2026-07-30 | +| **Primary PRs** | — (filled on merge) | + +## Outcome + +Settings is now a SwiftUI-owned singleton `Window` scene rather than a retained +AppKit `NSWindow`. Its standard macOS toolbar chrome is composed by SwiftUI: +the traffic lights sit in the sidebar field, the system sidebar toggle remains +available, and a drilled-in Agent Profile editor receives the compact native +Back control beside its title. + +The Agents page now has genuine navigation instead of a conditional content +swap. The profile list is the root of a `NavigationStack`; selecting a profile +pushes its `AgentProfileEditorFeature`, and the system Back control pops the +TCA-owned `StackState` path back to the list. + +## Implementation map + +- `supacode/App/supacodeApp.swift` — declares + `Window("Settings", id: WindowID.settings)` with default system toolbar + styling and the shared app store/environment values. +- `supacode/App/SettingsWindowOpener.swift` + + `supacode/Clients/SettingsWindow/SettingsWindowClient.swift` — bridge + reducer-driven entry points to `openWindow(id:)`; they never retain or create + an `NSWindow`. +- `supacode/Commands/WindowCommands.swift` and + `SidebarFooterView.swift` — route Cmd-comma/menu and the sidebar gear through + the SwiftUI window action. A focused-scene close action preserves Cmd-W for + Settings when terminal close commands are active in the main scene. +- `supacode/Features/Settings/Views/SettingsView.swift` — uses the normal + `NavigationSplitView` sidebar toggle and leaves all toolbar placement to + SwiftUI/macOS. +- `AgentProfilesFeature` / `AgentProfilesSettingsView` — replace + `@Presents editor` and a custom page swap with TCA `StackState`, + `NavigationStack(path:)`, and `NavigationLink(state:)`. +- `AgentProfileEditorView` — relies on its navigation title and the system Back + behavior; no in-page Back control or custom transition remains. + +## Verification + +- Targeted tests passed: `SettingsWindowOpenerTests`, `AgentProfilesFeatureTests`, + `AgentProfileEditorFeatureTests`, `AppFeatureSettingsSelectionTests`, the + Command Palette Settings-open case, `WindowCloseShortcutPolicyTests`, and + `WindowSurfacingTests`. +- `make check` completed cleanly and `make build-app` completed without errors + or warnings; the full `make test` suite also passed. +- In an isolated Debug Prowl instance, verified visually and through + Accessibility: + - Cmd-comma opens the singleton Settings window with native traffic lights, + sidebar toggle, and titlebar placement. + - Agents → Codex Default pushes the editor and shows the system Back button + beside `Codex Default`; activating it returns to the Agents list. + - Selecting General while the editor is visible returns to General rather + than retaining stale Agent detail state. + - The sidebar toggle hides and restores the sidebar. + - Cmd-W closes Settings from both its root and a drilled-in editor; Cmd-comma + subsequently reopens it. + +## Deviation from the initial experiment + +The planned special SwiftUI `Settings` scene was intentionally not shipped. +Its preference-scene chrome still produced an inset sidebar and misplaced +navigation controls in the nested split/stack combination. A standard +singleton `Window` scene is also public SwiftUI, while matching the target +macOS Settings-style layout in the actual debug app. diff --git a/docs-ai/README.md b/docs-ai/README.md index a5559526..32310e88 100644 --- a/docs-ai/README.md +++ b/docs-ai/README.md @@ -105,3 +105,4 @@ agent-facing manual for that). | 051 | [repository-icon-detection](051-repository-icon-detection/000-plan.md) | 2026-07-25 | High-confidence local repository icon detection on add; deferred, reviewable Foundation Model recommendations | | 052 | [sidebar-context-menus](052-sidebar-context-menus/000-plan.md) | 2026-07-27 | Sidebar context menu overhaul: worktree terminal actions, header/workspace path actions, PR click-through | | 053 | [agent-profiles](053-agent-profiles/000-plan.md) | 2026-07-29 | Agent Profile V1: preset-first Claude Code/Codex launch profiles, opt-in account isolation, path routing, and future handoff seam | +| 054 | [native-settings-navigation](054-native-settings-navigation/000-plan.md) | 2026-07-30 | SwiftUI-owned singleton Settings window, native sidebar/detail navigation, and Agent Profile drill-ins | diff --git a/docs/components/agent-profiles.md b/docs/components/agent-profiles.md index 76056d65..32e2fd23 100644 --- a/docs/components/agent-profiles.md +++ b/docs/components/agent-profiles.md @@ -38,6 +38,14 @@ types into an existing shell. The new pane records its profile identity at creation: the Active Agents rows and the capsule show the profile's display name (frozen at launch — later renames don't relabel live panes). +## Managing profiles + +Open **Settings → Agents** to see the ordered profile list. Click a profile to +push its editor; the native Back control returns to the list while the Settings +sidebar remains available. Adding a profile opens the same editor immediately. +Changing another Settings sidebar section leaves the editor and opens that +section's root. + **Recommended** resolves in three tiers: the repo's **Default Agent Profile** (Repo Settings) → the last profile explicitly launched in this repo → the first enabled profile in the Settings list order. Each tier only matches an diff --git a/docs/components/settings.md b/docs/components/settings.md index 6997621c..aaddf133 100644 --- a/docs/components/settings.md +++ b/docs/components/settings.md @@ -10,11 +10,20 @@ ## Opening `⌘,` (`open_settings`), the app menu, or Command Palette → "Open Settings". The -window is a sidebar of tabs plus a detail pane. +window is a native macOS split window: the sidebar selects top-level sections, +and its toolbar's sidebar control can hide or show that sidebar. Re-opening +Settings brings the existing Settings window forward rather than creating +another one. `⌘W` closes it. -## Tabs +Most sections are roots in the detail pane. A section can drill in without +losing the sidebar; for example, **Agents** → a profile pushes its editor and +macOS shows the standard Back control beside the editor title. Back returns to +the Agents list, while selecting another sidebar item leaves the drill-in and +opens that section's root. -| Tab | Controls | +## Sections + +| Section | Controls | |-----|----------| | **General** | Appearance (system/light/dark), default app for opening worktrees, diff tool, confirm-before-quit, default view mode, window chrome tint, automatic repository icon detection, toolbar buttons (Run / Open-in-editor), dim unfocused splits, Active Agents panel auto-show & terminal titles. | | **Notifications** | In-app alerts, notification sound picker (Never / system sounds / Prowl Classic), macOS system notifications, move-notified-to-top, command-finished notification + threshold, Dock badge & bounce. → [notifications](notifications.md) | diff --git a/supacode/App/SettingsWindowOpener.swift b/supacode/App/SettingsWindowOpener.swift new file mode 100644 index 00000000..5a770520 --- /dev/null +++ b/supacode/App/SettingsWindowOpener.swift @@ -0,0 +1,58 @@ +import SwiftUI + +/// Bridges reducer-driven Settings requests to SwiftUI's `openWindow` action. +/// +/// Settings is a singleton `Window(_:id:)` scene. Keeping its presentation in +/// SwiftUI lets the scene own the native window lifecycle while reducer and +/// command-palette entry points can still surface it without reaching for +/// AppKit window management. +@MainActor +final class SettingsWindowOpener { + static let shared = SettingsWindowOpener() + + private var opener: (() -> Void)? + var hasRegisteredOpener: Bool { + opener != nil + } + + init() {} + + func register(_ opener: @escaping () -> Void) { + self.opener = opener + } + + @discardableResult + func openSettingsWindow() -> Bool { + guard let opener else { return false } + opener() + return true + } +} + +/// Registers SwiftUI's window-opening action from a live view host. The +/// registration is refreshed whenever the main window appears. +private struct SettingsWindowOpenerRegistrar: ViewModifier { + @Environment(\.openWindow) private var openWindow + + func body(content: Content) -> some View { + content.onAppear { + SettingsWindowOpener.shared.register(openWindow: openWindow) + } + } +} + +extension View { + func registersSettingsWindowOpener() -> some View { + modifier(SettingsWindowOpenerRegistrar()) + } +} + +extension SettingsWindowOpener { + @discardableResult + func register(openWindow: OpenWindowAction) -> Bool { + register { + openWindow(id: WindowID.settings) + } + return hasRegisteredOpener + } +} diff --git a/supacode/App/supacodeApp.swift b/supacode/App/supacodeApp.swift index ef35481e..814acb46 100644 --- a/supacode/App/supacodeApp.swift +++ b/supacode/App/supacodeApp.swift @@ -267,11 +267,6 @@ struct SupacodeApp: App { appDelegate.appStore = appStore appDelegate.terminalManager = terminalManager appDelegate.cliSocketServer = cliServer - SettingsWindowManager.shared.configure( - store: appStore, - ghosttyShortcuts: shortcuts, - commandKeyObserver: keyObserver - ) #if DEBUG DebugWindowManager.shared.configure(store: appStore) #endif @@ -1111,6 +1106,7 @@ struct SupacodeApp: App { } } .registersMainWindowOpener() + .registersSettingsWindowOpener() .onAppear { WindowLifecycleDiagnostics.logWithWindows("mainWindow content onAppear") WindowLifecycleDiagnostics.noteMainWindowAppeared() @@ -1140,8 +1136,7 @@ struct SupacodeApp: App { store: store, terminalManager: terminalManager, ghosttyShortcuts: ghosttyShortcuts, - resolvedKeybindings: store.resolvedKeybindings, - settingsWindowManager: SettingsWindowManager.shared + resolvedKeybindings: store.resolvedKeybindings ) } CommandGroup(after: .textEditing) { @@ -1161,16 +1156,6 @@ struct SupacodeApp: App { store: store.scope(state: \.updates, action: \.updates), resolvedKeybindings: store.resolvedKeybindings ) - CommandGroup(replacing: .appSettings) { - Button("Settings...") { - SettingsWindowManager.shared.show() - } - .modifier( - KeyboardShortcutModifier( - shortcut: store.resolvedKeybindings.keyboardShortcut(for: AppShortcuts.CommandID.openSettings) - ) - ) - } CommandGroup(after: .appSettings) { Button("Install Command Line Tool") { store.send(.settings(.installCLIButtonTapped(showAlert: false))) @@ -1215,6 +1200,15 @@ struct SupacodeApp: App { .help(helpText(title: "Quit Prowl", commandID: AppShortcuts.CommandID.quitApplication)) } } + Window("Settings", id: WindowID.settings) { + SettingsView(store: store) + .environment(ghosttyShortcuts) + .environment(commandKeyObserver) + .environment(\.resolvedKeybindings, store.resolvedKeybindings) + .preferredColorScheme(store.settings.appearanceMode.colorScheme) + } + .defaultSize(width: 900, height: 600) + .windowToolbarStyle(.automatic) } private func syncGhosttyManagedShortcuts(with resolvedKeybindings: ResolvedKeybindingMap) { diff --git a/supacode/Clients/SettingsWindow/SettingsWindowClient.swift b/supacode/Clients/SettingsWindow/SettingsWindowClient.swift index 9e1b3d5d..55e23c6e 100644 --- a/supacode/Clients/SettingsWindow/SettingsWindowClient.swift +++ b/supacode/Clients/SettingsWindow/SettingsWindowClient.swift @@ -6,7 +6,7 @@ struct SettingsWindowClient { extension SettingsWindowClient: DependencyKey { static let liveValue = SettingsWindowClient { - SettingsWindowManager.shared.show() + _ = SettingsWindowOpener.shared.openSettingsWindow() } static let testValue = SettingsWindowClient {} diff --git a/supacode/Commands/WindowCommands.swift b/supacode/Commands/WindowCommands.swift index 9b95845d..4ce61a83 100644 --- a/supacode/Commands/WindowCommands.swift +++ b/supacode/Commands/WindowCommands.swift @@ -6,9 +6,8 @@ struct WindowCommands: Commands { let terminalManager: WorktreeTerminalManager let ghosttyShortcuts: GhosttyShortcutManager let resolvedKeybindings: ResolvedKeybindingMap - @Bindable var settingsWindowManager: SettingsWindowManager - @Dependency(SettingsWindowClient.self) private var settingsWindowClient @Environment(\.openWindow) private var openWindow + @FocusedValue(\.closeSettingsWindowAction) private var closeSettingsWindowAction @FocusedValue(\.closeTabAction) private var closeTabAction @FocusedValue(\.closeSurfaceAction) private var closeSurfaceAction @FocusedValue(\.selectPreviousTerminalTabAction) private var selectPreviousTerminalTabAction @@ -22,6 +21,7 @@ struct WindowCommands: Commands { var body: some Commands { let mainWindowOpenerRegistered = MainWindowOpener.shared.register(openWindow: openWindow) + let settingsOpenerRegistered = SettingsWindowOpener.shared.register(openWindow: openWindow) let closeSurfaceHotkey = ghosttyShortcuts.keyboardShortcut(for: "close_surface") let closeTabHotkey = ghosttyShortcuts.keyboardShortcut(for: "close_tab") let shelfHasOpenBooks = @@ -30,12 +30,17 @@ struct WindowCommands: Commands { closeSurfaceShortcut: closeSurfaceHotkey, closeTabShortcut: closeTabHotkey, hasTerminalCloseTarget: closeTabAction != nil || closeSurfaceAction != nil, - shelfHasOpenBooks: shelfHasOpenBooks + shelfHasOpenBooks: shelfHasOpenBooks, + settingsWindowIsFocused: closeSettingsWindowAction != nil ) CommandGroup(replacing: .saveItem) { Button("Close Window", systemImage: "xmark") { - NSApplication.shared.keyWindow?.performClose(nil) + if let closeSettingsWindowAction { + closeSettingsWindowAction() + } else { + NSApplication.shared.keyWindow?.performClose(nil) + } } .modifier( KeyboardShortcutModifier( @@ -48,8 +53,6 @@ struct WindowCommands: Commands { repositories: store.repositories, terminalManager: terminalManager ) - let isSettingsOpen = settingsWindowManager.isOpen - CommandGroup(replacing: .windowArrangement) { Button("Select Previous Tab") { selectPreviousTerminalTabAction?() @@ -143,12 +146,18 @@ struct WindowCommands: Commands { } .help("Show main window") - if isSettingsOpen { - Button("Settings") { - settingsWindowClient.show() - } - .help("Show Settings window") + } + + CommandGroup(replacing: .appSettings) { + Button("Settings...") { + guard settingsOpenerRegistered else { return } + _ = SettingsWindowOpener.shared.openSettingsWindow() } + .modifier( + KeyboardShortcutModifier( + shortcut: resolvedKeybindings.keyboardShortcut(for: AppShortcuts.CommandID.openSettings) + ) + ) } } } @@ -158,8 +167,15 @@ enum WindowCloseShortcutPolicy { closeSurfaceShortcut: KeyboardShortcut?, closeTabShortcut: KeyboardShortcut?, hasTerminalCloseTarget: Bool, - shelfHasOpenBooks: Bool = false + shelfHasOpenBooks: Bool = false, + settingsWindowIsFocused: Bool = false ) -> KeyboardShortcut? { + // Cmd-W must always close the native singleton Settings window. Its + // focused scene action is independent of a terminal's retained close + // targets in the main window. + if settingsWindowIsFocused { + return KeyboardShortcut("w") + } // `shelfHasOpenBooks` keeps Cmd+W with the terminal layer through the brief // gap between closing a book's last tab and Shelf advancing to the next // book. Without it, `hasTerminalCloseTarget` momentarily flips false during @@ -193,6 +209,17 @@ private struct SelectPreviousTerminalTabActionKey: FocusedValueKey { typealias Value = FocusedAction } +private struct CloseSettingsWindowActionKey: FocusedValueKey { + typealias Value = FocusedAction +} + +extension FocusedValues { + var closeSettingsWindowAction: FocusedAction? { + get { self[CloseSettingsWindowActionKey.self] } + set { self[CloseSettingsWindowActionKey.self] = newValue } + } +} + extension FocusedValues { var selectPreviousTerminalTabAction: FocusedAction? { get { self[SelectPreviousTerminalTabActionKey.self] } diff --git a/supacode/Features/App/Reducer/AppFeature+CommandPalette.swift b/supacode/Features/App/Reducer/AppFeature+CommandPalette.swift index 85dba674..82ba9c4b 100644 --- a/supacode/Features/App/Reducer/AppFeature+CommandPalette.swift +++ b/supacode/Features/App/Reducer/AppFeature+CommandPalette.swift @@ -76,12 +76,7 @@ extension AppFeature { return .send(.updates(.checkForUpdates)) case .openSettings: - return .merge( - .send(.settings(.setSelection(.general))), - .run { _ in - await settingsWindowClient.show() - } - ) + return openSettingsEffect(selecting: .general) case .newWorktree: return .send(.repositories(.worktreeCreation(.createRandomWorktree))) diff --git a/supacode/Features/App/Reducer/AppFeature+SettingsNavigation.swift b/supacode/Features/App/Reducer/AppFeature+SettingsNavigation.swift new file mode 100644 index 00000000..c8c9297c --- /dev/null +++ b/supacode/Features/App/Reducer/AppFeature+SettingsNavigation.swift @@ -0,0 +1,14 @@ +import ComposableArchitecture + +extension AppFeature { + /// Resolves the destination before asking SwiftUI to surface its Settings + /// window, so programmatic entry points never briefly show a stale section. + func openSettingsEffect(selecting selection: SettingsSection) -> Effect { + .concatenate( + .send(.settings(.setSelection(selection))), + .run { _ in + await settingsWindowClient.show() + } + ) + } +} diff --git a/supacode/Features/App/Reducer/AppFeature.swift b/supacode/Features/App/Reducer/AppFeature.swift index 4ffb7f5d..e9ef4d68 100644 --- a/supacode/Features/App/Reducer/AppFeature.swift +++ b/supacode/Features/App/Reducer/AppFeature.swift @@ -362,12 +362,7 @@ struct AppFeature { return .none } let selection = SettingsSection.repository(repositoryID) - return .merge( - .send(.settings(.setSelection(selection))), - .run { _ in - await settingsWindowClient.show() - } - ) + return openSettingsEffect(selecting: selection) case .repositories(.delegate(.showDiff(let worktreeID))): guard let worktree = state.repositories.worktree(for: worktreeID) else { @@ -720,12 +715,7 @@ struct AppFeature { return launchAgentProfile(profileID, state: &state) case .openAgentProfilesSettings: - return .merge( - .send(.settings(.setSelection(.agents))), - .run { _ in - await settingsWindowClient.show() - } - ) + return openSettingsEffect(selecting: .agents) case .runCustomCommand(let commandID): guard let worktree = actionTargetWorktree(repositories: state.repositories) else { diff --git a/supacode/Features/Debug/BusinessLogic/DebugWindowManager.swift b/supacode/Features/Debug/BusinessLogic/DebugWindowManager.swift index 1a90942f..e46ae7d7 100644 --- a/supacode/Features/Debug/BusinessLogic/DebugWindowManager.swift +++ b/supacode/Features/Debug/BusinessLogic/DebugWindowManager.swift @@ -4,11 +4,10 @@ import SwiftUI #if DEBUG - /// Manages the singleton Debug Window. Lifecycle mirrors - /// `SettingsWindowManager`: cache the `NSWindow`, deminiaturise + - /// front it on subsequent shows, never release on close so opening - /// is cheap. Configured once during app bootstrap with the root - /// store so the window can mirror the user's appearance setting. + /// Manages the singleton Debug Window. This debug-only auxiliary surface + /// caches its `NSWindow`, deminiaturises and fronts it on subsequent shows, + /// and is configured once during app bootstrap with the root store so it can + /// mirror the user's appearance setting. @MainActor final class DebugWindowManager { static let shared = DebugWindowManager() diff --git a/supacode/Features/Repositories/Views/SidebarFooterView.swift b/supacode/Features/Repositories/Views/SidebarFooterView.swift index 72c50b42..4bf8ca11 100644 --- a/supacode/Features/Repositories/Views/SidebarFooterView.swift +++ b/supacode/Features/Repositories/Views/SidebarFooterView.swift @@ -4,6 +4,7 @@ import SwiftUI struct SidebarFooterView: View { let store: StoreOf @Environment(\.surfaceBottomChromeBackgroundOpacity) private var surfaceBottomChromeBackgroundOpacity + @Environment(\.openWindow) private var openWindow @Environment(\.openURL) private var openURL @Environment(\.resolvedKeybindings) private var resolvedKeybindings @Environment(AskAgentHelpPresenter.self) private var askAgentHelp @@ -84,10 +85,12 @@ struct SidebarFooterView: View { commandID: AppShortcuts.CommandID.archivedWorktrees, in: resolvedKeybindings )) - Button("Settings", systemImage: "gearshape") { - SettingsWindowManager.shared.show() + Button { + openWindow(id: WindowID.settings) + } label: { + Image(systemName: "gearshape") + .accessibilityLabel("Settings") } - .labelStyle(.iconOnly) .help( AppShortcuts.helpText( title: "Settings", diff --git a/supacode/Features/Settings/BusinessLogic/SettingsWindowManager.swift b/supacode/Features/Settings/BusinessLogic/SettingsWindowManager.swift deleted file mode 100644 index 95b2b16b..00000000 --- a/supacode/Features/Settings/BusinessLogic/SettingsWindowManager.swift +++ /dev/null @@ -1,104 +0,0 @@ -import AppKit -import ComposableArchitecture -import SwiftUI - -@MainActor -@Observable -final class SettingsWindowManager { - @ObservationIgnored static let shared = SettingsWindowManager() - - private(set) var isOpen: Bool = false - - @ObservationIgnored private var settingsWindow: NSWindow? - @ObservationIgnored private var store: StoreOf? - @ObservationIgnored private var ghosttyShortcuts: GhosttyShortcutManager? - @ObservationIgnored private var commandKeyObserver: CommandKeyObserver? - @ObservationIgnored private var localEventMonitor: Any? - @ObservationIgnored private var willCloseObserver: NSObjectProtocol? - - private init() {} - - func configure( - store: StoreOf, - ghosttyShortcuts: GhosttyShortcutManager, - commandKeyObserver: CommandKeyObserver - ) { - self.store = store - self.ghosttyShortcuts = ghosttyShortcuts - self.commandKeyObserver = commandKeyObserver - } - - func show() { - if let existingWindow = settingsWindow { - if existingWindow.isMiniaturized { - existingWindow.deminiaturize(nil) - } - existingWindow.makeKeyAndOrderFront(nil) - isOpen = true - return - } - - guard let store, let ghosttyShortcuts, let commandKeyObserver else { - return - } - let settingsView = SettingsView(store: store) - .environment(ghosttyShortcuts) - .environment(commandKeyObserver) - let hostingController = NSHostingController(rootView: settingsView) - - let window = NSWindow(contentViewController: hostingController) - window.title = "Settings" - window.titleVisibility = .hidden - window.identifier = NSUserInterfaceItemIdentifier(WindowID.settings) - window.styleMask = [.titled, .closable, .miniaturizable, .resizable, .fullSizeContentView] - window.tabbingMode = .disallowed - window.collectionBehavior = [.moveToActiveSpace] - window.titlebarAppearsTransparent = true - window.toolbarStyle = .unified - window.toolbar = NSToolbar(identifier: "SettingsToolbar") - if #unavailable(macOS 15.0) { - window.toolbar?.showsBaselineSeparator = false - } - window.isReleasedWhenClosed = false - window.isExcludedFromWindowsMenu = true - window.setContentSize(NSSize(width: 800, height: 600)) - window.minSize = NSSize(width: 800, height: 500) - - willCloseObserver = NotificationCenter.default.addObserver( - forName: NSWindow.willCloseNotification, - object: window, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { - self?.isOpen = false - } - } - - window.center() - window.makeKeyAndOrderFront(nil) - - settingsWindow = window - localEventMonitor = NSEvent.addLocalMonitorForEvents(matching: .keyDown) { [weak window] event in - guard let window, event.window === window else { return event } - if SettingsWindowKeyboardShortcutPolicy.isCloseWindowShortcut( - modifierFlags: event.modifierFlags, - charactersIgnoringModifiers: event.charactersIgnoringModifiers - ) { - window.performClose(nil) - return nil - } - return event - } - isOpen = true - } -} - -enum SettingsWindowKeyboardShortcutPolicy { - static func isCloseWindowShortcut( - modifierFlags: NSEvent.ModifierFlags, - charactersIgnoringModifiers: String? - ) -> Bool { - modifierFlags.intersection(.deviceIndependentFlagsMask) == .command - && charactersIgnoringModifiers == "w" - } -} diff --git a/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift b/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift index 06674f99..265c2e85 100644 --- a/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift +++ b/supacode/Features/Settings/Reducer/AgentProfileEditorFeature.swift @@ -26,7 +26,6 @@ struct AgentProfileEditorFeature { enum Action: BindableAction { case task case binding(BindingAction) - case backTapped case removeTapped case revealProfileFiles case homeStatusRefreshed(Bool) @@ -49,7 +48,6 @@ struct AgentProfileEditorFeature { } @Dependency(AgentProfileHomeClient.self) var homeClient - @Dependency(\.dismiss) var dismiss var body: some Reducer { BindingReducer() @@ -70,9 +68,6 @@ struct AgentProfileEditorFeature { refreshHomeStatus(&state) return .send(.delegate(.profileEdited(state.profile))) - case .backTapped: - return .run { _ in await dismiss() } - case .removeTapped: // The confirmation gate keys on the *disk fact*, not the current // binding intent: a profile that was bound, launched (home created), diff --git a/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift b/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift index e814cdf2..8ced6380 100644 --- a/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift +++ b/supacode/Features/Settings/Reducer/AgentProfilesFeature.swift @@ -6,14 +6,14 @@ import SwiftUI /// Settings → Agents: the global agent profile list (docs-ai 053). List order /// is the recommendation fallback order; edits persist to /// `UserGlobalSettings` the same way global custom commands do. Editing one -/// profile is a drill-in `AgentProfileEditorFeature` presented tree-style, so -/// editor-scoped presentation state lives with the pushed page. +/// profile is a native drill-in `AgentProfileEditorFeature` represented by a +/// stack path. @Reducer struct AgentProfilesFeature { @ObservableState struct State: Equatable { var settings: UserGlobalSettings = .default - @Presents var editor: AgentProfileEditorFeature.State? + var path = StackState() } enum Action: BindableAction { @@ -22,8 +22,7 @@ struct AgentProfilesFeature { case binding(BindingAction) case addProfile(AgentProfileRuntime) case moveProfiles(IndexSet, Int) - case profileTapped(AgentProfile.ID) - case editor(PresentationAction) + case path(StackActionOf) case delegate(Delegate) } @@ -62,31 +61,24 @@ struct AgentProfilesFeature { ) state.settings.agentProfiles.append(profile) state.settings = state.settings.normalized() - state.editor = AgentProfileEditorFeature.State(profile: profile) + state.path.append(AgentProfileEditorFeature.State(profile: profile)) return persist(state.settings) case .moveProfiles(let source, let destination): state.settings.agentProfiles.move(fromOffsets: source, toOffset: destination) return persist(state.settings) - case .profileTapped(let id): - guard let profile = state.settings.agentProfiles.first(where: { $0.id == id }) else { - return .none - } - state.editor = AgentProfileEditorFeature.State(profile: profile) - return .none - - case .editor(.presented(.delegate(.profileEdited(let profile)))): + case .path(.element(id: _, action: .delegate(.profileEdited(let profile)))): guard let index = state.settings.agentProfiles.firstIndex(where: { $0.id == profile.id }) else { return .none } state.settings.agentProfiles[index] = profile return persist(state.settings) - case .editor(.presented(.delegate(.removeProfile(let profileID, let trashFiles)))): + case .path(.element(id: _, action: .delegate(.removeProfile(let profileID, let trashFiles)))): state.settings.agentProfiles.removeAll { $0.id == profileID } - // Dismissing pops the drill-in editor back to the list; the trash - // effect runs here so the dismissal cannot cancel it. - state.editor = nil + // Dismissing returns the split view to the Agents root before the + // trash effect runs, so it cannot be cancelled with the destination. + state.path.removeAll() guard trashFiles else { return persist(state.settings) } let client = homeClient return .merge( @@ -100,21 +92,21 @@ struct AgentProfilesFeature { } ) - case .editor: + case .path: return .none case .delegate: return .none } } - .ifLet(\.$editor, action: \.editor) { + .forEach(\.path, action: \.path) { AgentProfileEditorFeature() } } private func persist(_ settings: UserGlobalSettings) -> Effect { // The write happens synchronously in the reducer: this pane's state is - // nil'd on every Settings sidebar switch, and `ifLet` cancels in-flight + // removed on every Settings sidebar switch, and `forEach` cancels in-flight // child effects — a persist living inside `.run` could be cancelled and // silently drop the edit (or resurrect a removed profile). @Shared(.userGlobalSettings) var storedSettings diff --git a/supacode/Features/Settings/Views/AgentProfileEditorView.swift b/supacode/Features/Settings/Views/AgentProfileEditorView.swift index f0f3c25c..47b3f03d 100644 --- a/supacode/Features/Settings/Views/AgentProfileEditorView.swift +++ b/supacode/Features/Settings/Views/AgentProfileEditorView.swift @@ -1,52 +1,23 @@ import ComposableArchitecture import SwiftUI -/// Settings → Agents → drill-in editor page for one profile. Presented by -/// `AgentProfilesSettingsView` via `navigationDestination`; owns the alert -/// presentation because its feature owns the alert state. +/// Settings → Agents → native drill-in editor for one profile. The feature +/// owns the alert presentation because its state lives with this destination. struct AgentProfileEditorView: View { @Bindable var store: StoreOf var body: some View { - VStack(alignment: .leading, spacing: 0) { - header - Form { - profileSection - advancedSection - removalSection - } - .formStyle(.grouped) + Form { + profileSection + advancedSection + removalSection } + .formStyle(.grouped) + .navigationTitle(store.profile.name) .task { store.send(.task) } .alert($store.scope(state: \.alert, action: \.alert)) } - /// Page header: back capsule plus the profile identity. The window title - /// stays a constant "Agents", so the drill-in page names itself here. - private var header: some View { - HStack(spacing: 12) { - Button { - store.send(.backTapped) - } label: { - Label("Back", systemImage: "chevron.left") - .labelStyle(.iconOnly) - } - .buttonStyle(.glass) - .keyboardShortcut("[", modifiers: .command) - .help("Back to Agent Profiles (⌘[)") - - VStack(alignment: .leading, spacing: 2) { - Text(store.profile.name) - .font(.title3.weight(.semibold)) - Text(AgentRuntimeAdapterRegistry.displayName(for: store.profile.runtime.agent)) - .font(.caption) - .foregroundStyle(.secondary) - } - Spacer() - } - .padding(.leading, 4) - } - private var profileSection: some View { Section("Profile") { TextField("Name", text: $store.profile.name) diff --git a/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift b/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift index 7a3ecab3..a77c9ca5 100644 --- a/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift +++ b/supacode/Features/Settings/Views/AgentProfilesSettingsView.swift @@ -1,31 +1,25 @@ import ComposableArchitecture import SwiftUI -/// Settings → Agents: the profile list, with a drill-in editor page per -/// profile (System Settings style). The drill-in is a state-driven content -/// swap, not a nested `NavigationStack`: a stack inside the split view's -/// detail column drops the first programmatic push, blocks sidebar-driven -/// detail switches while pushed, and reconfigures the titlebar layout. +/// Settings → Agents: the profile list, with a native drill-in editor page per +/// profile. `NavigationStack` is driven by TCA's `StackState`, so the system +/// Back control writes its pop directly to the reducer-owned route. /// List order is the recommendation fallback order. struct AgentProfilesSettingsView: View { @Bindable var store: StoreOf var body: some View { - Group { - if let editorStore = store.scope(state: \.editor, action: \.editor.presented) { - AgentProfileEditorView(store: editorStore) - .transition(.push(from: .trailing)) - } else { - Form { - profileListSection - } - .formStyle(.grouped) - .transition(.push(from: .leading)) + NavigationStack(path: $store.scope(state: \.path, action: \.path)) { + Form { + profileListSection } + .formStyle(.grouped) + .navigationTitle("Agents") + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) + .task { store.send(.task) } + } destination: { editorStore in + AgentProfileEditorView(store: editorStore) } - .animation(.default, value: store.editor == nil) - .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) - .task { store.send(.task) } } private var profileListSection: some View { @@ -71,28 +65,9 @@ struct AgentProfilesSettingsView: View { .toggleStyle(.checkbox) .help("Show this profile in the Agents menu") } - Button { - store.send(.profileTapped(profile.id)) - } label: { - HStack(spacing: 8) { - Text(profile.name) - if profile.bindsDedicatedHome { - Image(systemName: "person.crop.circle.badge.checkmark") - .foregroundStyle(.secondary) - .accessibilityLabel("Dedicated account") - .help("Uses a dedicated home with its own account") - } - Spacer() - Text(AgentRuntimeAdapterRegistry.displayName(for: profile.runtime.agent)) - .foregroundStyle(.secondary) - Image(systemName: "chevron.right") - .font(.caption.weight(.semibold)) - .foregroundStyle(.tertiary) - .accessibilityHidden(true) - } - .contentShape(Rectangle()) + NavigationLink(state: AgentProfileEditorFeature.State(profile: profile)) { + profileLabel(profile) } - .buttonStyle(.plain) .help("Edit this profile") } .contextMenu { @@ -103,6 +78,21 @@ struct AgentProfilesSettingsView: View { } } + private func profileLabel(_ profile: AgentProfile) -> some View { + HStack(spacing: 8) { + Text(profile.name) + if profile.bindsDedicatedHome { + Image(systemName: "person.crop.circle.badge.checkmark") + .foregroundStyle(.secondary) + .accessibilityLabel("Dedicated account") + .help("Uses a dedicated home with its own account") + } + Spacer() + Text(AgentRuntimeAdapterRegistry.displayName(for: profile.runtime.agent)) + .foregroundStyle(.secondary) + } + } + private func index(of id: AgentProfile.ID) -> Int? { store.settings.agentProfiles.firstIndex { $0.id == id } } diff --git a/supacode/Features/Settings/Views/SettingsDetailView.swift b/supacode/Features/Settings/Views/SettingsDetailView.swift index dc3b0b42..0a235865 100644 --- a/supacode/Features/Settings/Views/SettingsDetailView.swift +++ b/supacode/Features/Settings/Views/SettingsDetailView.swift @@ -9,9 +9,6 @@ struct SettingsDetailView: View { var body: some View { content - .scenePadding(.top) - .scenePadding(.horizontal) - .scenePadding(.bottom) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) } } diff --git a/supacode/Features/Settings/Views/SettingsView.swift b/supacode/Features/Settings/Views/SettingsView.swift index 8e71d612..bac51630 100644 --- a/supacode/Features/Settings/Views/SettingsView.swift +++ b/supacode/Features/Settings/Views/SettingsView.swift @@ -1,20 +1,11 @@ import ComposableArchitecture import SwiftUI -extension View { - @ViewBuilder - fileprivate func removingSidebarToggle() -> some View { - if #available(macOS 14.0, *) { - toolbar(removing: .sidebarToggle) - } else { - self - } - } -} - struct SettingsView: View { @Bindable var store: StoreOf @Bindable var settingsStore: StoreOf + @Environment(\.dismiss) private var dismiss + @State private var columnVisibility: NavigationSplitViewVisibility = .all init(store: StoreOf) { self.store = store @@ -27,7 +18,7 @@ struct SettingsView: View { let customTitles = store.repositories.repositoryCustomTitles let selection = settingsStore.selection ?? .general - NavigationSplitView(columnVisibility: .constant(.all)) { + NavigationSplitView(columnVisibility: $columnVisibility) { VStack(spacing: 0) { List(selection: $settingsStore.selection.sending(\.setSelection)) { Label("General", systemImage: "gearshape") @@ -62,7 +53,6 @@ struct SettingsView: View { .listStyle(.sidebar) .frame(minWidth: 220, maxHeight: .infinity) .navigationSplitViewColumnWidth(220) - .removingSidebarToggle() } } detail: { switch selection { @@ -70,43 +60,36 @@ struct SettingsView: View { SettingsDetailView { AppearanceSettingsView(store: settingsStore) .navigationTitle("General") - .navigationSubtitle("Appearance and preferences") } case .notifications: SettingsDetailView { NotificationsSettingsView(store: settingsStore) .navigationTitle("Notifications") - .navigationSubtitle("In-app alerts and delivery") } case .shortcuts: SettingsDetailView { ShortcutsSettingsView(store: settingsStore) .navigationTitle("Shortcuts") - .navigationSubtitle("Global keybindings") } case .worktree: SettingsDetailView { WorktreeSettingsView(store: settingsStore) .navigationTitle("Worktree") - .navigationSubtitle("Archive behavior") } case .updates: SettingsDetailView { UpdatesSettingsView(settingsStore: settingsStore, updatesStore: updatesStore) .navigationTitle("Updates") - .navigationSubtitle("Update preferences") } case .advanced: SettingsDetailView { AdvancedSettingsView(store: settingsStore) .navigationTitle("Advanced") - .navigationSubtitle("Analytics and diagnostics") } case .github: SettingsDetailView { GithubSettingsView(store: settingsStore) .navigationTitle("GitHub") - .navigationSubtitle("GitHub CLI integration") } case .customCommands: SettingsDetailView { @@ -116,7 +99,6 @@ struct SettingsView: View { ) { GlobalCustomCommandsView(store: globalCustomCommandsStore) .navigationTitle("Global Commands") - .navigationSubtitle("Global terminal actions and toolbar buttons") } else { ProgressView() .frame(maxWidth: .infinity, maxHeight: .infinity) @@ -128,13 +110,7 @@ struct SettingsView: View { state: \.agentProfiles, action: \.agentProfiles ) { - // The title stays constant across the drill-in: the editor page - // identifies itself in its own header, and swapping title - // modifiers with the pages destabilizes the hand-configured - // window toolbar. AgentProfilesSettingsView(store: agentProfilesStore) - .navigationTitle("Agents") - .navigationSubtitle("Agent profiles and launch presets") } else { ProgressView() .frame(maxWidth: .infinity, maxHeight: .infinity) @@ -150,14 +126,12 @@ struct SettingsView: View { RepositorySettingsView(store: repositorySettingsStore) .id(repository.id) .navigationTitle(customTitles[repository.id] ?? repository.name) - .navigationSubtitle(repository.rootURL.path(percentEncoded: false)) } else { // Settled placeholder while the scoped store is briefly nil (e.g. mid // repository switch), instead of `IfLetStore` flashing an empty pane. ProgressView() .frame(maxWidth: .infinity, maxHeight: .infinity) .navigationTitle(customTitles[repository.id] ?? repository.name) - .navigationSubtitle(repository.rootURL.path(percentEncoded: false)) } } } else { @@ -173,10 +147,8 @@ struct SettingsView: View { .navigationSplitViewStyle(.balanced) .alert($settingsStore.scope(state: \.alert, action: \.alert)) .frame(minWidth: 800, minHeight: 500) - .background { - WindowAppearanceSetter(colorScheme: settingsStore.appearanceMode.colorScheme) - WindowLevelSetter(level: .normal) + .focusedSceneAction(\.closeSettingsWindowAction, enabled: true) { + dismiss() } - .ignoresSafeArea(.container, edges: .top) } } diff --git a/supacodeTests/AgentProfilesFeatureTests.swift b/supacodeTests/AgentProfilesFeatureTests.swift index 3b7224ee..ca67d63c 100644 --- a/supacodeTests/AgentProfilesFeatureTests.swift +++ b/supacodeTests/AgentProfilesFeatureTests.swift @@ -21,12 +21,12 @@ struct AgentProfilesFeatureTests { } } - // The root page is the list; a non-nil editor would push the drill-in. + // The root page is the list; a non-empty path would push the drill-in. await store.send(.task) await store.receive(\.settingsLoaded) { $0.settings.agentProfiles = [profile] } - #expect(store.state.editor == nil) + #expect(store.state.path.isEmpty) } @Test(.dependencies) func addProfileAppendsOpensEditorAndPersists() async { @@ -48,12 +48,12 @@ struct AgentProfilesFeatureTests { ) await store.send(.addProfile(.claude)) { $0.settings.agentProfiles = [expected] - $0.editor = AgentProfileEditorFeature.State(profile: expected) + $0.path.append(AgentProfileEditorFeature.State(profile: expected)) } await store.receive(\.delegate.settingsChanged) } - @Test(.dependencies) func profileTappedPushesItsEditor() async { + @Test(.dependencies) func systemBackPopsTheEditorPath() async { let profile = AgentProfile(name: "Codex", runtime: .codex) let storage = SettingsTestStorage() let store = withDependencies { @@ -68,8 +68,13 @@ struct AgentProfilesFeatureTests { } } - await store.send(.profileTapped(profile.id)) { - $0.editor = AgentProfileEditorFeature.State(profile: profile) + await store.send( + .path(.push(id: 0, state: AgentProfileEditorFeature.State(profile: profile))) + ) { + $0.path.append(AgentProfileEditorFeature.State(profile: profile)) + } + await store.send(.path(.popFrom(id: 0))) { + $0.path.removeAll() } } @@ -83,7 +88,7 @@ struct AgentProfilesFeatureTests { $settings.withLock { $0.agentProfiles = [profile] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.editor = AgentProfileEditorFeature.State(profile: profile) + initial.path.append(AgentProfileEditorFeature.State(profile: profile)) let store = TestStore(initialState: initial) { AgentProfilesFeature() } @@ -94,7 +99,9 @@ struct AgentProfilesFeatureTests { // the persisted profile keeps its last non-blank name. var edited = profile edited.name = " " - await store.send(.editor(.presented(.delegate(.profileEdited(edited))))) { + await store.send( + .path(.element(id: 0, action: .delegate(.profileEdited(edited)))) + ) { $0.settings.agentProfiles[0].name = " " } await store.receive(\.delegate.settingsChanged) @@ -103,7 +110,9 @@ struct AgentProfilesFeatureTests { // Typing the new name persists it normally. edited.name = "Codex · Deep" - await store.send(.editor(.presented(.delegate(.profileEdited(edited))))) { + await store.send( + .path(.element(id: 0, action: .delegate(.profileEdited(edited)))) + ) { $0.settings.agentProfiles[0].name = "Codex · Deep" } await store.receive(\.delegate.settingsChanged) @@ -122,7 +131,7 @@ struct AgentProfilesFeatureTests { $settings.withLock { $0.agentProfiles = [bound] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.editor = AgentProfileEditorFeature.State(profile: bound) + initial.path.append(AgentProfileEditorFeature.State(profile: bound)) return TestStore(initialState: initial) { AgentProfilesFeature() } withDependencies: { @@ -132,11 +141,13 @@ struct AgentProfilesFeatureTests { } } - // Removal pops the editor back to the list and trashes in the parent, so + // Removal pops the editor path back to the list and trashes in the parent, so // the dismissal cannot cancel the file operation. - await store.send(.editor(.presented(.delegate(.removeProfile(bound.id, trashFiles: true))))) { + await store.send( + .path(.element(id: 0, action: .delegate(.removeProfile(bound.id, trashFiles: true)))) + ) { $0.settings.agentProfiles = [] - $0.editor = nil + $0.path.removeAll() } await store.receive(\.delegate.settingsChanged) await store.finish() @@ -154,7 +165,7 @@ struct AgentProfilesFeatureTests { $settings.withLock { $0.agentProfiles = [preset] } var initial = AgentProfilesFeature.State() initial.settings = settings - initial.editor = AgentProfileEditorFeature.State(profile: preset) + initial.path.append(AgentProfileEditorFeature.State(profile: preset)) return TestStore(initialState: initial) { AgentProfilesFeature() } withDependencies: { @@ -164,9 +175,11 @@ struct AgentProfilesFeatureTests { } } - await store.send(.editor(.presented(.delegate(.removeProfile(preset.id, trashFiles: false))))) { + await store.send( + .path(.element(id: 0, action: .delegate(.removeProfile(preset.id, trashFiles: false)))) + ) { $0.settings.agentProfiles = [] - $0.editor = nil + $0.path.removeAll() } await store.receive(\.delegate.settingsChanged) await store.finish() diff --git a/supacodeTests/AppFeatureSettingsSelectionTests.swift b/supacodeTests/AppFeatureSettingsSelectionTests.swift index b0f73903..98ed021b 100644 --- a/supacodeTests/AppFeatureSettingsSelectionTests.swift +++ b/supacodeTests/AppFeatureSettingsSelectionTests.swift @@ -162,4 +162,22 @@ struct AppFeatureSettingsSelectionTests { $0.settings.repositorySettings = nil } } + + @Test func selectingAnotherSectionClearsAgentProfileEditorState() async { + let profile = AgentProfile(name: "Codex", runtime: .codex) + var state = AppFeature.State(settings: SettingsFeature.State()) + state.settings.selection = .agents + var agentProfiles = AgentProfilesFeature.State() + agentProfiles.settings = UserGlobalSettings(customCommands: [], agentProfiles: [profile]) + agentProfiles.path.append(AgentProfileEditorFeature.State(profile: profile)) + state.settings.agentProfiles = agentProfiles + let store = TestStore(initialState: state) { + AppFeature() + } + + await store.send(.settings(.setSelection(.general))) { + $0.settings.selection = .general + $0.settings.agentProfiles = nil + } + } } diff --git a/supacodeTests/SettingsWindowOpenerTests.swift b/supacodeTests/SettingsWindowOpenerTests.swift new file mode 100644 index 00000000..e432e876 --- /dev/null +++ b/supacodeTests/SettingsWindowOpenerTests.swift @@ -0,0 +1,36 @@ +import Testing + +@testable import supacode + +@MainActor +struct SettingsWindowOpenerTests { + @Test func returnsFalseWhenNoOpenerRegistered() { + let opener = SettingsWindowOpener() + + #expect(opener.hasRegisteredOpener == false) + #expect(opener.openSettingsWindow() == false) + } + + @Test func invokesRegisteredOpenerAndReturnsTrue() { + let opener = SettingsWindowOpener() + var callCount = 0 + opener.register { callCount += 1 } + + #expect(opener.hasRegisteredOpener == true) + #expect(opener.openSettingsWindow() == true) + #expect(callCount == 1) + } + + @Test func reregisteringReplacesPreviousOpener() { + let opener = SettingsWindowOpener() + var firstCount = 0 + var secondCount = 0 + opener.register { firstCount += 1 } + opener.register { secondCount += 1 } + + _ = opener.openSettingsWindow() + + #expect(firstCount == 0) + #expect(secondCount == 1) + } +} diff --git a/supacodeTests/WindowCloseShortcutPolicyTests.swift b/supacodeTests/WindowCloseShortcutPolicyTests.swift index 9a289f71..372610f5 100644 --- a/supacodeTests/WindowCloseShortcutPolicyTests.swift +++ b/supacodeTests/WindowCloseShortcutPolicyTests.swift @@ -1,4 +1,3 @@ -import AppKit import SwiftUI import Testing @@ -86,33 +85,17 @@ struct WindowCloseShortcutPolicyTests { #expect(shortcut?.key == "w") #expect(shortcut?.modifiers == .command) } -} - -struct SettingsWindowShortcutPolicyTests { - @Test func commandWClosesSettingsWindow() { - #expect( - SettingsWindowKeyboardShortcutPolicy.isCloseWindowShortcut( - modifierFlags: .command, - charactersIgnoringModifiers: "w" - ) - ) - } - @Test func modifiedCommandWDoesNotCloseSettingsWindow() { - #expect( - !SettingsWindowKeyboardShortcutPolicy.isCloseWindowShortcut( - modifierFlags: [.command, .shift], - charactersIgnoringModifiers: "w" - ) + @Test func closeWindowClaimsCommandWForFocusedSettingsWindow() { + let shortcut = WindowCloseShortcutPolicy.closeWindowShortcut( + closeSurfaceShortcut: KeyboardShortcut("w"), + closeTabShortcut: KeyboardShortcut("w"), + hasTerminalCloseTarget: true, + shelfHasOpenBooks: true, + settingsWindowIsFocused: true ) - } - @Test func commandOtherKeyDoesNotCloseSettingsWindow() { - #expect( - !SettingsWindowKeyboardShortcutPolicy.isCloseWindowShortcut( - modifierFlags: .command, - charactersIgnoringModifiers: "q" - ) - ) + #expect(shortcut?.key == "w") + #expect(shortcut?.modifiers == .command) } } diff --git a/supacodeTests/WindowSurfacingTests.swift b/supacodeTests/WindowSurfacingTests.swift index b66f061f..0b2b8cef 100644 --- a/supacodeTests/WindowSurfacingTests.swift +++ b/supacodeTests/WindowSurfacingTests.swift @@ -6,7 +6,7 @@ struct WindowSurfacingTests { @Test func mainWindowCandidateIgnoresUntaggedHelperWindows() { let snapshots = [ MainWindowSurface.Snapshot(identifier: nil, isVisible: true), - MainWindowSurface.Snapshot(identifier: WindowID.settings, isVisible: true), + MainWindowSurface.Snapshot(identifier: "auxiliary", isVisible: true), ] #expect(MainWindowSurface.mainWindowIndex(in: snapshots) == nil) @@ -42,7 +42,7 @@ struct WindowSurfacingTests { @Test func windowCountsSeparateMainAndVisibleWindows() { let snapshots = [ MainWindowSurface.Snapshot(identifier: nil, isVisible: true), - MainWindowSurface.Snapshot(identifier: WindowID.settings, isVisible: true), + MainWindowSurface.Snapshot(identifier: "auxiliary", isVisible: true), MainWindowSurface.Snapshot(identifier: WindowID.main, isVisible: false), MainWindowSurface.Snapshot(identifier: WindowID.main, isVisible: true), ] -- 2.51.2 From f6b07aee5f3e8f60f7aaa906f684e7d56b86471b Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 31 Jul 2026 00:58:04 +0900 Subject: [PATCH 3/4] Polish Settings window layout --- .../000-plan.md | 23 ++++--- .../001-action.md | 19 +++--- docs/components/settings.md | 6 +- supacode/App/supacodeApp.swift | 2 +- .../SettingsSidebarToggleHider.swift | 36 ++++++++++ .../Settings/Views/SettingsView.swift | 67 +++++++++---------- .../Views/ShortcutsSettingsView.swift | 3 +- .../Settings/Views/UpdatesSettingsView.swift | 38 +++++------ 8 files changed, 116 insertions(+), 78 deletions(-) create mode 100644 supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift diff --git a/docs-ai/054-native-settings-navigation/000-plan.md b/docs-ai/054-native-settings-navigation/000-plan.md index 637e7379..d77fa249 100644 --- a/docs-ai/054-native-settings-navigation/000-plan.md +++ b/docs-ai/054-native-settings-navigation/000-plan.md @@ -26,8 +26,8 @@ split-view and navigation chrome. - Use a SwiftUI-owned singleton `Window` scene for Settings; no Settings-owned `NSWindow`, event monitor, custom titlebar, or manual Back button. -- Keep top-level sections in `NavigationSplitView`, including the system - sidebar toggle and the expected traffic-light/titlebar layout. +- Keep top-level sections in `NavigationSplitView`, with a persistent sidebar + and the expected traffic-light/titlebar layout. - Push Agent Profile editing with a real `NavigationStack`, giving macOS its standard compact Back control and title placement. - Keep navigation state in TCA, including a deterministic reset when the user @@ -39,8 +39,7 @@ split-view and navigation chrome. - Redesigning individual Settings forms, profile fields, or persistence. - Changing the main-window reopening or terminal-window management paths. -- Building custom replicas of the macOS titlebar, sidebar toggle, or Back - button. +- Building custom replicas of the macOS titlebar or Back button. ## Design @@ -59,11 +58,14 @@ window, so direct entries retain their intended section. Local view entries use the environment's `openWindow` action directly. The standard window toolbar remains `.automatic`. This deliberately leaves -traffic lights, the sidebar toggle, navigation title, and Back placement to -macOS instead of arranging any of them manually. A focused-scene close action -lets the shared Close Window command use SwiftUI's `dismiss` for Settings, so -Cmd-W continues to work even when the main-window terminal close actions are -currently registered. +traffic lights, navigation title, and Back placement to macOS instead of +arranging any of them manually. The sidebar column uses SwiftUI's documented +`.toolbar(removing: .sidebarToggle)` and a narrow AppKit fallback removes that +same system item only when macOS 26 leaves it present in a standalone `Window` +scene. The navigation and layout remain SwiftUI-owned. A focused-scene close +action lets the shared Close Window command use SwiftUI's `dismiss` for +Settings, so Cmd-W continues to work even when the main-window terminal close +actions are currently registered. ### TCA-backed profile drill-in @@ -97,4 +99,5 @@ editor delegates update or remove the matching profile in the parent state. - Cmd-comma opens/re-surfaces the one Settings window, and Cmd-W closes it. - Targeted TCA tests cover route push/pop, mutations through the path, and sidebar-reset behavior; the app builds and manual debug-window verification - captures the root, Agents, editor, sidebar toggle, and close/reopen paths. + captures the root, Agents, editor, persistent sidebar, and close/reopen + paths. diff --git a/docs-ai/054-native-settings-navigation/001-action.md b/docs-ai/054-native-settings-navigation/001-action.md index 34fa34a1..7251844f 100644 --- a/docs-ai/054-native-settings-navigation/001-action.md +++ b/docs-ai/054-native-settings-navigation/001-action.md @@ -10,9 +10,9 @@ Settings is now a SwiftUI-owned singleton `Window` scene rather than a retained AppKit `NSWindow`. Its standard macOS toolbar chrome is composed by SwiftUI: -the traffic lights sit in the sidebar field, the system sidebar toggle remains -available, and a drilled-in Agent Profile editor receives the compact native -Back control beside its title. +the traffic lights sit in the sidebar field, the sidebar remains persistently +visible, and a drilled-in Agent Profile editor receives the compact native Back +control beside its title. The Agents page now has genuine navigation instead of a conditional content swap. The profile list is the root of a `NavigationStack`; selecting a profile @@ -32,9 +32,13 @@ TCA-owned `StackState` path back to the list. `SidebarFooterView.swift` — route Cmd-comma/menu and the sidebar gear through the SwiftUI window action. A focused-scene close action preserves Cmd-W for Settings when terminal close commands are active in the main scene. -- `supacode/Features/Settings/Views/SettingsView.swift` — uses the normal - `NavigationSplitView` sidebar toggle and leaves all toolbar placement to - SwiftUI/macOS. +- `supacode/Features/Settings/Views/SettingsView.swift` — uses + `NavigationSplitView` with a persistent column binding and SwiftUI's default + sidebar-toggle removal. +- `SettingsSidebarToggleHider` — a no-op-on-fixed-systems AppKit fallback for + the macOS 26 standalone-`Window` bug where the system sidebar item remains + after SwiftUI requests its removal. It removes no navigation or layout + responsibility from SwiftUI. - `AgentProfilesFeature` / `AgentProfilesSettingsView` — replace `@Presents editor` and a custom page swap with TCA `StackState`, `NavigationStack(path:)`, and `NavigationLink(state:)`. @@ -52,12 +56,11 @@ TCA-owned `StackState` path back to the list. - In an isolated Debug Prowl instance, verified visually and through Accessibility: - Cmd-comma opens the singleton Settings window with native traffic lights, - sidebar toggle, and titlebar placement. + persistent sidebar, and titlebar placement. - Agents → Codex Default pushes the editor and shows the system Back button beside `Codex Default`; activating it returns to the Agents list. - Selecting General while the editor is visible returns to General rather than retaining stale Agent detail state. - - The sidebar toggle hides and restores the sidebar. - Cmd-W closes Settings from both its root and a drilled-in editor; Cmd-comma subsequently reopens it. diff --git a/docs/components/settings.md b/docs/components/settings.md index aaddf133..afa35b2a 100644 --- a/docs/components/settings.md +++ b/docs/components/settings.md @@ -11,9 +11,9 @@ `⌘,` (`open_settings`), the app menu, or Command Palette → "Open Settings". The window is a native macOS split window: the sidebar selects top-level sections, -and its toolbar's sidebar control can hide or show that sidebar. Re-opening -Settings brings the existing Settings window forward rather than creating -another one. `⌘W` closes it. +and remains visible while Settings is open. Re-opening Settings brings the +existing Settings window forward rather than creating another one. `⌘W` closes +it. Most sections are roots in the detail pane. A section can drill in without losing the sidebar; for example, **Agents** → a profile pushes its editor and diff --git a/supacode/App/supacodeApp.swift b/supacode/App/supacodeApp.swift index 814acb46..b038d372 100644 --- a/supacode/App/supacodeApp.swift +++ b/supacode/App/supacodeApp.swift @@ -1207,7 +1207,7 @@ struct SupacodeApp: App { .environment(\.resolvedKeybindings, store.resolvedKeybindings) .preferredColorScheme(store.settings.appearanceMode.colorScheme) } - .defaultSize(width: 900, height: 600) + .defaultSize(width: 840, height: 680) .windowToolbarStyle(.automatic) } diff --git a/supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift b/supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift new file mode 100644 index 00000000..61d02c5d --- /dev/null +++ b/supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift @@ -0,0 +1,36 @@ +import AppKit +import SwiftUI + +/// Falls back to removing the system sidebar item when SwiftUI leaves it in a +/// standalone Settings window despite `.toolbar(removing: .sidebarToggle)`. +struct SettingsSidebarToggleHider: NSViewRepresentable { + func makeNSView(context: Context) -> SettingsSidebarToggleHiderView { + SettingsSidebarToggleHiderView() + } + + func updateNSView(_ nsView: SettingsSidebarToggleHiderView, context: Context) { + nsView.removeSidebarToggleIfNeeded() + } +} + +final class SettingsSidebarToggleHiderView: NSView { + private static let sidebarToggleIdentifier = NSToolbarItem.Identifier( + "com.apple.SwiftUI.navigationSplitView.toggleSidebar" + ) + + override func viewDidMoveToWindow() { + super.viewDidMoveToWindow() + DispatchQueue.main.async { [weak self] in + self?.removeSidebarToggleIfNeeded() + } + } + + func removeSidebarToggleIfNeeded() { + guard let toolbar = window?.toolbar, + let index = toolbar.items.firstIndex(where: { $0.itemIdentifier == Self.sidebarToggleIdentifier }) + else { + return + } + toolbar.removeItem(at: index) + } +} diff --git a/supacode/Features/Settings/Views/SettingsView.swift b/supacode/Features/Settings/Views/SettingsView.swift index bac51630..e588547d 100644 --- a/supacode/Features/Settings/Views/SettingsView.swift +++ b/supacode/Features/Settings/Views/SettingsView.swift @@ -5,7 +5,6 @@ struct SettingsView: View { @Bindable var store: StoreOf @Bindable var settingsStore: StoreOf @Environment(\.dismiss) private var dismiss - @State private var columnVisibility: NavigationSplitViewVisibility = .all init(store: StoreOf) { self.store = store @@ -18,42 +17,41 @@ struct SettingsView: View { let customTitles = store.repositories.repositoryCustomTitles let selection = settingsStore.selection ?? .general - NavigationSplitView(columnVisibility: $columnVisibility) { - VStack(spacing: 0) { - List(selection: $settingsStore.selection.sending(\.setSelection)) { - Label("General", systemImage: "gearshape") - .tag(SettingsSection.general) - Label("Notifications", systemImage: "bell") - .tag(SettingsSection.notifications) - Label("Shortcuts", systemImage: "keyboard") - .tag(SettingsSection.shortcuts) - Label("Worktree", systemImage: "archivebox") - .tag(SettingsSection.worktree) - Label("Updates", systemImage: "arrow.down.circle") - .tag(SettingsSection.updates) - Label("Advanced", systemImage: "gearshape.2") - .tag(SettingsSection.advanced) - Label("GitHub", systemImage: "arrow.triangle.branch") - .tag(SettingsSection.github) - Label("Commands", systemImage: "globe") - .tag(SettingsSection.customCommands) - Label("Agents", systemImage: "sparkles") - .tag(SettingsSection.agents) + NavigationSplitView(columnVisibility: .constant(.all)) { + List(selection: $settingsStore.selection.sending(\.setSelection)) { + Label("General", systemImage: "gearshape") + .tag(SettingsSection.general) + Label("Notifications", systemImage: "bell") + .tag(SettingsSection.notifications) + Label("Shortcuts", systemImage: "keyboard") + .tag(SettingsSection.shortcuts) + Label("Worktree", systemImage: "archivebox") + .tag(SettingsSection.worktree) + Label("Updates", systemImage: "arrow.down.circle") + .tag(SettingsSection.updates) + Label("Advanced", systemImage: "gearshape.2") + .tag(SettingsSection.advanced) + Label("GitHub", systemImage: "arrow.triangle.branch") + .tag(SettingsSection.github) + Label("Commands", systemImage: "globe") + .tag(SettingsSection.customCommands) + Label("Agents", systemImage: "sparkles") + .tag(SettingsSection.agents) - Section("Repositories") { - ForEach(repositories) { repository in - RepoDisplayName( - fallbackName: repository.name, - customTitle: customTitles[repository.id] - ) - .tag(SettingsSection.repository(repository.id)) - } + Section("Repositories") { + ForEach(repositories) { repository in + RepoDisplayName( + fallbackName: repository.name, + customTitle: customTitles[repository.id] + ) + .tag(SettingsSection.repository(repository.id)) } } - .listStyle(.sidebar) - .frame(minWidth: 220, maxHeight: .infinity) - .navigationSplitViewColumnWidth(220) } + .listStyle(.sidebar) + .frame(minWidth: 220, maxHeight: .infinity) + .navigationSplitViewColumnWidth(220) + .toolbar(removing: .sidebarToggle) } detail: { switch selection { case .general: @@ -145,8 +143,9 @@ struct SettingsView: View { } } .navigationSplitViewStyle(.balanced) + .background(SettingsSidebarToggleHider()) .alert($settingsStore.scope(state: \.alert, action: \.alert)) - .frame(minWidth: 800, minHeight: 500) + .frame(minWidth: 800, minHeight: 600) .focusedSceneAction(\.closeSettingsWindowAction, enabled: true) { dismiss() } diff --git a/supacode/Features/Settings/Views/ShortcutsSettingsView.swift b/supacode/Features/Settings/Views/ShortcutsSettingsView.swift index e092e4ad..38f6e840 100644 --- a/supacode/Features/Settings/Views/ShortcutsSettingsView.swift +++ b/supacode/Features/Settings/Views/ShortcutsSettingsView.swift @@ -56,6 +56,7 @@ struct ShortcutsSettingsView: View { } .disabled(store.keybindingUserOverrides.overrides.isEmpty) } + .padding(.horizontal, 16) HStack(spacing: 12) { Text("Command") @@ -75,7 +76,7 @@ struct ShortcutsSettingsView: View { } .font(.caption.weight(.semibold)) .foregroundStyle(.secondary) - .padding(.horizontal, 12) + .padding(.horizontal, 16) List { ForEach(visibleGroups) { group in diff --git a/supacode/Features/Settings/Views/UpdatesSettingsView.swift b/supacode/Features/Settings/Views/UpdatesSettingsView.swift index dd6cc2a1..e5e29d73 100644 --- a/supacode/Features/Settings/Views/UpdatesSettingsView.swift +++ b/supacode/Features/Settings/Views/UpdatesSettingsView.swift @@ -6,35 +6,31 @@ struct UpdatesSettingsView: View { let updatesStore: StoreOf var body: some View { - VStack(alignment: .leading, spacing: 0) { - Form { - Section { - Toggle( - "Check for updates automatically", - isOn: $settingsStore.updatesAutomaticallyCheckForUpdates - ) - } header: { - Text("Automatic Updates") - } footer: { - Text( - "When a new version is available, a small badge appears next to the notifications bell. " - + "Click it to review, install, and choose future background downloads." - ) - .font(.callout) - .foregroundStyle(.secondary) - } + Form { + Section { + Toggle( + "Check for updates automatically", + isOn: $settingsStore.updatesAutomaticallyCheckForUpdates + ) + } header: { + Text("Automatic Updates") + } footer: { + Text( + "When a new version is available, a small badge appears next to the notifications bell. " + + "Click it to review, install, and choose future background downloads." + ) + .font(.callout) + .foregroundStyle(.secondary) } - .formStyle(.grouped) - HStack { + Section { Button("Check for Updates Now") { updatesStore.send(.checkForUpdates) } .help("Check for updates now") - Spacer() } - .padding(.top) } + .formStyle(.grouped) .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading) } } -- 2.51.2 From 2ed9c21bc4c3d9ad306299252b839eb8647786bd Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 31 Jul 2026 01:13:46 +0900 Subject: [PATCH 4/4] Restore native Settings sidebar chrome --- .../000-plan.md | 25 +++++++------ .../001-action.md | 21 ++++++----- docs/components/settings.md | 6 ++-- .../SettingsSidebarToggleHider.swift | 36 ------------------- .../Settings/Views/SettingsView.swift | 5 ++- 5 files changed, 27 insertions(+), 66 deletions(-) delete mode 100644 supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift diff --git a/docs-ai/054-native-settings-navigation/000-plan.md b/docs-ai/054-native-settings-navigation/000-plan.md index d77fa249..dedf2cb3 100644 --- a/docs-ai/054-native-settings-navigation/000-plan.md +++ b/docs-ai/054-native-settings-navigation/000-plan.md @@ -26,8 +26,8 @@ split-view and navigation chrome. - Use a SwiftUI-owned singleton `Window` scene for Settings; no Settings-owned `NSWindow`, event monitor, custom titlebar, or manual Back button. -- Keep top-level sections in `NavigationSplitView`, with a persistent sidebar - and the expected traffic-light/titlebar layout. +- Keep top-level sections in `NavigationSplitView`, including the system + sidebar toggle and the expected traffic-light/titlebar layout. - Push Agent Profile editing with a real `NavigationStack`, giving macOS its standard compact Back control and title placement. - Keep navigation state in TCA, including a deterministic reset when the user @@ -39,7 +39,8 @@ split-view and navigation chrome. - Redesigning individual Settings forms, profile fields, or persistence. - Changing the main-window reopening or terminal-window management paths. -- Building custom replicas of the macOS titlebar or Back button. +- Building custom replicas of the macOS titlebar, sidebar toggle, or Back + button. ## Design @@ -58,14 +59,13 @@ window, so direct entries retain their intended section. Local view entries use the environment's `openWindow` action directly. The standard window toolbar remains `.automatic`. This deliberately leaves -traffic lights, navigation title, and Back placement to macOS instead of -arranging any of them manually. The sidebar column uses SwiftUI's documented -`.toolbar(removing: .sidebarToggle)` and a narrow AppKit fallback removes that -same system item only when macOS 26 leaves it present in a standalone `Window` -scene. The navigation and layout remain SwiftUI-owned. A focused-scene close -action lets the shared Close Window command use SwiftUI's `dismiss` for -Settings, so Cmd-W continues to work even when the main-window terminal close -actions are currently registered. +traffic lights, the sidebar toggle, navigation title, and Back placement to +macOS instead of arranging any of them manually. On macOS 26, removing the +last sidebar toolbar item also changes titlebar ownership and moves the traffic +lights out of the sidebar well, so Settings intentionally keeps the standard +system toggle. A focused-scene close action lets the shared Close Window command +use SwiftUI's `dismiss` for Settings, so Cmd-W continues to work even when the +main-window terminal close actions are currently registered. ### TCA-backed profile drill-in @@ -99,5 +99,4 @@ editor delegates update or remove the matching profile in the parent state. - Cmd-comma opens/re-surfaces the one Settings window, and Cmd-W closes it. - Targeted TCA tests cover route push/pop, mutations through the path, and sidebar-reset behavior; the app builds and manual debug-window verification - captures the root, Agents, editor, persistent sidebar, and close/reopen - paths. + captures the root, Agents, editor, sidebar toggle, and close/reopen paths. diff --git a/docs-ai/054-native-settings-navigation/001-action.md b/docs-ai/054-native-settings-navigation/001-action.md index 7251844f..8b88d269 100644 --- a/docs-ai/054-native-settings-navigation/001-action.md +++ b/docs-ai/054-native-settings-navigation/001-action.md @@ -10,9 +10,9 @@ Settings is now a SwiftUI-owned singleton `Window` scene rather than a retained AppKit `NSWindow`. Its standard macOS toolbar chrome is composed by SwiftUI: -the traffic lights sit in the sidebar field, the sidebar remains persistently -visible, and a drilled-in Agent Profile editor receives the compact native Back -control beside its title. +the traffic lights sit in the sidebar field, the system sidebar toggle remains +available, and a drilled-in Agent Profile editor receives the compact native +Back control beside its title. The Agents page now has genuine navigation instead of a conditional content swap. The profile list is the root of a `NavigationStack`; selecting a profile @@ -32,13 +32,11 @@ TCA-owned `StackState` path back to the list. `SidebarFooterView.swift` — route Cmd-comma/menu and the sidebar gear through the SwiftUI window action. A focused-scene close action preserves Cmd-W for Settings when terminal close commands are active in the main scene. -- `supacode/Features/Settings/Views/SettingsView.swift` — uses - `NavigationSplitView` with a persistent column binding and SwiftUI's default - sidebar-toggle removal. -- `SettingsSidebarToggleHider` — a no-op-on-fixed-systems AppKit fallback for - the macOS 26 standalone-`Window` bug where the system sidebar item remains - after SwiftUI requests its removal. It removes no navigation or layout - responsibility from SwiftUI. +- `supacode/Features/Settings/Views/SettingsView.swift` — uses the normal + `NavigationSplitView` sidebar toggle and leaves all toolbar placement to + SwiftUI/macOS. Removing the toggle was tested but changes the macOS 26 + titlebar/sidebar relationship, leaving the traffic lights outside the + sidebar well, so that workaround was not retained. - `AgentProfilesFeature` / `AgentProfilesSettingsView` — replace `@Presents editor` and a custom page swap with TCA `StackState`, `NavigationStack(path:)`, and `NavigationLink(state:)`. @@ -56,11 +54,12 @@ TCA-owned `StackState` path back to the list. - In an isolated Debug Prowl instance, verified visually and through Accessibility: - Cmd-comma opens the singleton Settings window with native traffic lights, - persistent sidebar, and titlebar placement. + sidebar toggle, and titlebar placement. - Agents → Codex Default pushes the editor and shows the system Back button beside `Codex Default`; activating it returns to the Agents list. - Selecting General while the editor is visible returns to General rather than retaining stale Agent detail state. + - The sidebar toggle hides and restores the sidebar. - Cmd-W closes Settings from both its root and a drilled-in editor; Cmd-comma subsequently reopens it. diff --git a/docs/components/settings.md b/docs/components/settings.md index afa35b2a..aaddf133 100644 --- a/docs/components/settings.md +++ b/docs/components/settings.md @@ -11,9 +11,9 @@ `⌘,` (`open_settings`), the app menu, or Command Palette → "Open Settings". The window is a native macOS split window: the sidebar selects top-level sections, -and remains visible while Settings is open. Re-opening Settings brings the -existing Settings window forward rather than creating another one. `⌘W` closes -it. +and its toolbar's sidebar control can hide or show that sidebar. Re-opening +Settings brings the existing Settings window forward rather than creating +another one. `⌘W` closes it. Most sections are roots in the detail pane. A section can drill in without losing the sidebar; for example, **Agents** → a profile pushes its editor and diff --git a/supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift b/supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift deleted file mode 100644 index 61d02c5d..00000000 --- a/supacode/Features/Settings/BusinessLogic/SettingsSidebarToggleHider.swift +++ /dev/null @@ -1,36 +0,0 @@ -import AppKit -import SwiftUI - -/// Falls back to removing the system sidebar item when SwiftUI leaves it in a -/// standalone Settings window despite `.toolbar(removing: .sidebarToggle)`. -struct SettingsSidebarToggleHider: NSViewRepresentable { - func makeNSView(context: Context) -> SettingsSidebarToggleHiderView { - SettingsSidebarToggleHiderView() - } - - func updateNSView(_ nsView: SettingsSidebarToggleHiderView, context: Context) { - nsView.removeSidebarToggleIfNeeded() - } -} - -final class SettingsSidebarToggleHiderView: NSView { - private static let sidebarToggleIdentifier = NSToolbarItem.Identifier( - "com.apple.SwiftUI.navigationSplitView.toggleSidebar" - ) - - override func viewDidMoveToWindow() { - super.viewDidMoveToWindow() - DispatchQueue.main.async { [weak self] in - self?.removeSidebarToggleIfNeeded() - } - } - - func removeSidebarToggleIfNeeded() { - guard let toolbar = window?.toolbar, - let index = toolbar.items.firstIndex(where: { $0.itemIdentifier == Self.sidebarToggleIdentifier }) - else { - return - } - toolbar.removeItem(at: index) - } -} diff --git a/supacode/Features/Settings/Views/SettingsView.swift b/supacode/Features/Settings/Views/SettingsView.swift index e588547d..21e50ece 100644 --- a/supacode/Features/Settings/Views/SettingsView.swift +++ b/supacode/Features/Settings/Views/SettingsView.swift @@ -5,6 +5,7 @@ struct SettingsView: View { @Bindable var store: StoreOf @Bindable var settingsStore: StoreOf @Environment(\.dismiss) private var dismiss + @State private var columnVisibility: NavigationSplitViewVisibility = .all init(store: StoreOf) { self.store = store @@ -17,7 +18,7 @@ struct SettingsView: View { let customTitles = store.repositories.repositoryCustomTitles let selection = settingsStore.selection ?? .general - NavigationSplitView(columnVisibility: .constant(.all)) { + NavigationSplitView(columnVisibility: $columnVisibility) { List(selection: $settingsStore.selection.sending(\.setSelection)) { Label("General", systemImage: "gearshape") .tag(SettingsSection.general) @@ -51,7 +52,6 @@ struct SettingsView: View { .listStyle(.sidebar) .frame(minWidth: 220, maxHeight: .infinity) .navigationSplitViewColumnWidth(220) - .toolbar(removing: .sidebarToggle) } detail: { switch selection { case .general: @@ -143,7 +143,6 @@ struct SettingsView: View { } } .navigationSplitViewStyle(.balanced) - .background(SettingsSidebarToggleHider()) .alert($settingsStore.scope(state: \.alert, action: \.alert)) .frame(minWidth: 800, minHeight: 600) .focusedSceneAction(\.closeSettingsWindowAction, enabled: true) {