From 3a24efcede24db7deabac5d2795d3315f946748c Mon Sep 17 00:00:00 2001 From: onevcat Date: Sat, 13 Jun 2026 14:45:35 +0900 Subject: [PATCH] Address Copilot review on the Open In changes - Make the menu icon cache's MainActor isolation explicit. Every access is during SwiftUI rendering on the main thread, so the cache was already race-free via the module's default actor isolation; the attribute makes that contract compiler-enforced and documents why a lock is the wrong tool here (NSImage isn't Sendable). - Skip the redundant repository-settings write when resetting an already-automatic repo to Automatic, while still re-resolving and reopening so the entry stays consistent with the concrete app rows. --- supacode/Domain/OpenWorktreeAction.swift | 7 ++++++- supacode/Features/App/Reducer/AppFeature.swift | 11 ++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/supacode/Domain/OpenWorktreeAction.swift b/supacode/Domain/OpenWorktreeAction.swift index bac70a38..5ca5d18f 100644 --- a/supacode/Domain/OpenWorktreeAction.swift +++ b/supacode/Domain/OpenWorktreeAction.swift @@ -103,7 +103,12 @@ enum OpenWorktreeAction: CaseIterable, Identifiable { // icons constantly. Only hits are cached so a newly installed app shows up // without invalidation; lookup misses are microseconds. private static let menuIconSize = CGSize(width: 16, height: 16) - private static var menuIconCache: [String: MenuIcon] = [:] + // `@MainActor` makes the cache's isolation explicit and compiler-enforced: + // every `menuIcon` access happens during SwiftUI rendering on the main + // thread, so this shared mutable state is never touched concurrently. (A + // lock would be the wrong tool here — `NSImage` isn't `Sendable`, so the + // cached values shouldn't cross threads in the first place.) + @MainActor private static var menuIconCache: [String: MenuIcon] = [:] var menuIcon: MenuIcon? { switch self { diff --git a/supacode/Features/App/Reducer/AppFeature.swift b/supacode/Features/App/Reducer/AppFeature.swift index 72eb44cb..756b8b0c 100644 --- a/supacode/Features/App/Reducer/AppFeature.swift +++ b/supacode/Features/App/Reducer/AppFeature.swift @@ -554,9 +554,14 @@ struct AppFeature { guard let worktree = state.repositories.selectedTerminalWorktree else { return .none } - let rootURL = worktree.repositoryRootURL - @Shared(.repositorySettings(rootURL)) var repositorySettings - $repositorySettings.withLock { $0.openActionID = OpenWorktreeAction.automaticSettingsID } + // Clearing the pin only matters when the repo isn't already automatic; + // re-resolve and reopen unconditionally so the entry behaves like the + // concrete app rows, where selecting always opens. + if !state.openActionIsAutomatic { + let rootURL = worktree.repositoryRootURL + @Shared(.repositorySettings(rootURL)) var repositorySettings + $repositorySettings.withLock { $0.openActionID = OpenWorktreeAction.automaticSettingsID } + } @Shared(.settingsFile) var settingsFile state.openActionSelection = OpenWorktreeAction.fromSettingsID( OpenWorktreeAction.automaticSettingsID, -- 2.51.2