From 353ae5bfe2b95b86245c7bda5ff14365af04df6a Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 5 Jul 2026 20:37:33 -0400 Subject: [PATCH 1/6] Persist Foreman repo state as one record per repo and add favorites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapse ForemanConfiguration's parallel enabledRepoIDs/repoOptions into a single RepoConfiguration record per RepoID (isEnabled, isFavorite, options), dropping the old on-disk format (Foreman has no users yet). Add a persisted, worker-free Repo.isFavorite, and RepoSection — the pure, tested enabled/ disabled sectioning rule that floats favorites to the top of each section. Co-authored-by: Cursor --- Foreman/Foreman/Sources/ForemanSession.swift | 6 ++ Foreman/Foreman/Sources/PreviewSupport.swift | 33 +++++-- .../ForemanCore/Sources/ForemanServices.swift | 33 ++++--- Foreman/ForemanCore/Sources/Repo.swift | 20 ++++- Foreman/ForemanCore/Sources/RepoSection.swift | 54 ++++++++++++ .../Sources/WorkerConfigStore.swift | 85 ++++++++++++------- .../Tests/ForemanCoreTestSupport.swift | 4 +- .../Tests/ForemanServicesTests.swift | 74 ++++++++++++---- .../ForemanCore/Tests/RepoSectionTests.swift | 75 ++++++++++++++++ Foreman/ForemanCore/Tests/RepoTests.swift | 25 ++++++ .../Tests/WorkerConfigStoreTests.swift | 23 +++-- 11 files changed, 349 insertions(+), 83 deletions(-) create mode 100644 Foreman/ForemanCore/Sources/RepoSection.swift create mode 100644 Foreman/ForemanCore/Tests/RepoSectionTests.swift diff --git a/Foreman/Foreman/Sources/ForemanSession.swift b/Foreman/Foreman/Sources/ForemanSession.swift index aaebcd85..d60be20c 100644 --- a/Foreman/Foreman/Sources/ForemanSession.swift +++ b/Foreman/Foreman/Sources/ForemanSession.swift @@ -17,6 +17,12 @@ final class ForemanSession { services.repos } + /// The discovered repos grouped for the sidebar: enabled on top, disabled + /// below, favorites floated to the top of each section. + var repoSections: [RepoSection] { + services.repoSections + } + /// Global settings; `SettingsView` writes through these observable /// properties (persistence and rescans happen in Core). var settings: AppSettings { diff --git a/Foreman/Foreman/Sources/PreviewSupport.swift b/Foreman/Foreman/Sources/PreviewSupport.swift index 3fa975d5..7a04c4a3 100644 --- a/Foreman/Foreman/Sources/PreviewSupport.swift +++ b/Foreman/Foreman/Sources/PreviewSupport.swift @@ -12,12 +12,21 @@ session(repoNames: []) } - /// A session with a few discovered repos, all stopped. + /// A session with a few discovered repos across both sidebar sections: + /// two enabled (one of them favorited) and a favorited-but-disabled one. static func populatedSession() -> ForemanSession { - session(repoNames: ["Broadway", "Site", "Stuff"]) + session( + repoNames: ["Broadway", "Site", "Stuff"], + enabled: ["Broadway", "Stuff"], + favorites: ["Broadway", "Site"], + ) } - private static func session(repoNames: [String]) -> ForemanSession { + private static func session( + repoNames: [String], + enabled: Set = [], + favorites: Set = [], + ) -> ForemanSession { let base = FileManager.default.temporaryDirectory .appendingPathComponent("ForemanPreview-\(UUID().uuidString)") let scanDirectory = base.appendingPathComponent("Development") @@ -34,18 +43,30 @@ withIntermediateDirectories: true, ) } + var repos: [RepoID: RepoConfiguration] = [:] + for name in enabled.union(favorites) { + let id = RepoID(rootURL: scanDirectory + .appendingPathComponent(name, isDirectory: true)) + repos[id] = RepoConfiguration( + isEnabled: enabled.contains(name), + isFavorite: favorites.contains(name), + options: .standard, + ) + } let store = WorkerConfigStore(directory: base.appendingPathComponent("config")) try store.save(ForemanConfiguration( scanDirectory: scanDirectory, agentExecutable: nil, - enabledRepoIDs: [], - repoOptions: [:], + repos: repos, )) let session = ForemanSession(services: ForemanServices( configStore: store, logDirectory: base.appendingPathComponent("logs"), )) - session.start() + // Populate the tree from the saved config (seeding + // isEnabled/isFavorite) via a plain rescan — not start(), whose + // launch restore would spawn real workers. Previews never spawn. + session.rescan() return session } catch { fatalError("Preview fixture setup failed: \(error)") diff --git a/Foreman/ForemanCore/Sources/ForemanServices.swift b/Foreman/ForemanCore/Sources/ForemanServices.swift index b3233541..d6d51054 100644 --- a/Foreman/ForemanCore/Sources/ForemanServices.swift +++ b/Foreman/ForemanCore/Sources/ForemanServices.swift @@ -46,6 +46,12 @@ public final class ForemanServices { discovery.repos } + /// The discovered repos grouped for the sidebar: enabled on top, disabled + /// below, favorites floated to the top of each section. See ``RepoSection``. + public var repoSections: [RepoSection] { + RepoSection.sections(from: discovery.repos) + } + public var isAnyWorkerLive: Bool { discovery.isAnyWorkerLive } @@ -165,10 +171,12 @@ public final class ForemanServices { /// repo whose worker exits after the root is gone must degrade to a /// no-op, not crash on a dangling reference. private func makeRepo(_ scanned: ScannedRepo) -> Repo { - Repo( + let record = configuration.configuration(for: scanned.id) + return Repo( scanned: scanned, - isEnabled: configuration.enabledRepoIDs.contains(scanned.id), - options: configuration.options(for: scanned.id), + isEnabled: record.isEnabled, + isFavorite: record.isFavorite, + options: record.options, worker: Worker( name: scanned.name, workerDirectory: scanned.rootURL, @@ -184,17 +192,16 @@ public final class ForemanServices { } private func repoDidChange(_ repo: Repo) { - if repo.isEnabled { - configuration.enabledRepoIDs.insert(repo.id) - } else { - configuration.enabledRepoIDs.remove(repo.id) - } - if repo.options == .standard { - // Standard options read identically to an absent entry; dropping - // the entry keeps the file from accumulating no-op records. - configuration.repoOptions.removeValue(forKey: repo.id) + var record = configuration.configuration(for: repo.id) + record.isEnabled = repo.isEnabled + record.isFavorite = repo.isFavorite + record.options = repo.options + if record == .standard { + // A fully default record reads identically to an absent entry; + // dropping it keeps the file from accumulating no-op records. + configuration.repos.removeValue(forKey: repo.id) } else { - configuration.repoOptions[repo.id] = repo.options + configuration.repos[repo.id] = record } persist() } diff --git a/Foreman/ForemanCore/Sources/Repo.swift b/Foreman/ForemanCore/Sources/Repo.swift index cb88e2ea..02beef9c 100644 --- a/Foreman/ForemanCore/Sources/Repo.swift +++ b/Foreman/ForemanCore/Sources/Repo.swift @@ -44,6 +44,17 @@ public final class Repo: Identifiable { } } + /// Whether the user pinned this repo to the top of its sidebar section. + /// Pure presentation metadata — unlike `isEnabled` it has no worker side + /// effect, it only notifies the persistence funnel; reassigning the + /// current value is a no-op. + public var isFavorite: Bool { + didSet { + guard oldValue != isFavorite else { return } + onPersistentChange(self) + } + } + private static let logger = ForemanLog.channel(.repo) private let resolveExecutable: @MainActor () throws -> URL @@ -53,16 +64,20 @@ public final class Repo: Identifiable { /// - scanned: The on-disk identity from ``RepoDiscovery``. /// - isEnabled: The saved desired state. Assigning in `init` does not /// start the worker — launch restore calls ``startIfEnabled()``. + /// - isFavorite: The saved favorite flag (pins the repo to the top of + /// its sidebar section). /// - options: The saved worker options. /// - worker: The repo's worker (the owning tree wires its /// `onStateChange`). /// - resolveExecutable: Locates the `cursor-agent` binary at start /// time; a throw lands in the worker's `.failed` state. - /// - onPersistentChange: Invoked after every `isEnabled`/`options` - /// change so the owning tree can write through and save. + /// - onPersistentChange: Invoked after every + /// `isEnabled`/`isFavorite`/`options` change so the owning tree can + /// write through and save. public init( scanned: ScannedRepo, isEnabled: Bool, + isFavorite: Bool, options: WorkerOptions, worker: Worker, resolveExecutable: @escaping @MainActor () throws -> URL, @@ -73,6 +88,7 @@ public final class Repo: Identifiable { rootURL = scanned.rootURL self.worker = worker self.isEnabled = isEnabled + self.isFavorite = isFavorite self.options = options self.resolveExecutable = resolveExecutable self.onPersistentChange = onPersistentChange diff --git a/Foreman/ForemanCore/Sources/RepoSection.swift b/Foreman/ForemanCore/Sources/RepoSection.swift new file mode 100644 index 00000000..1ac10bb5 --- /dev/null +++ b/Foreman/ForemanCore/Sources/RepoSection.swift @@ -0,0 +1,54 @@ +import Foundation + +/// A grouping of ``Repo``s for the sidebar: enabled repos on top, disabled +/// below, with favorites floated to the top of each group. +/// +/// This is the pure ordering rule behind the "smarter" repo list. It lives in +/// Core (not the view) so the sectioning and sort are unit-tested; the app +/// target only renders a `Section` per element. +/// +/// The struct itself is not actor-isolated (so its `Identifiable` conformance +/// stays nonisolated); only ``sections(from:)`` is `@MainActor`, since reading +/// each ``Repo``'s enabled/favorite state is main-actor work. +public struct RepoSection: Identifiable { + /// Which repos a section holds. The `Kind` carries no display text — + /// section titles are the view's concern. + public enum Kind: Hashable, Sendable { + /// Repos whose worker is enabled (the persisted desired state). + case enabled + /// Repos whose worker is disabled. + case disabled + } + + public let kind: Kind + /// The section's repos, favorites first (see ``sections(from:)``). + public let repos: [Repo] + + public var id: Kind { + kind + } + + public init(kind: Kind, repos: [Repo]) { + self.kind = kind + self.repos = repos + } + + /// Splits `repos` into the enabled and disabled sections, floating + /// favorites to the top of each. `repos` is expected already name-sorted + /// (``RepoDiscovery`` sorts its listing), and the partition is stable, so + /// non-favorites keep their alphabetical order and favorites lead each + /// section in that same order. Empty sections are omitted, so an all-enabled + /// list renders a single section. + @MainActor + public static func sections(from repos: [Repo]) -> [RepoSection] { + func favoritesFirst(_ repos: [Repo]) -> [Repo] { + repos.filter(\.isFavorite) + repos.filter { !$0.isFavorite } + } + + return [ + RepoSection(kind: .enabled, repos: favoritesFirst(repos.filter(\.isEnabled))), + RepoSection(kind: .disabled, repos: favoritesFirst(repos.filter { !$0.isEnabled })), + ] + .filter { !$0.repos.isEmpty } + } +} diff --git a/Foreman/ForemanCore/Sources/WorkerConfigStore.swift b/Foreman/ForemanCore/Sources/WorkerConfigStore.swift index ed44ddfc..7c51efbe 100644 --- a/Foreman/ForemanCore/Sources/WorkerConfigStore.swift +++ b/Foreman/ForemanCore/Sources/WorkerConfigStore.swift @@ -1,7 +1,36 @@ import Foundation -/// Everything Foreman persists: which repos have their worker enabled, each -/// repo's ``WorkerOptions``, and the global settings. +/// Everything Foreman persists for a single repository: whether its worker +/// should run, whether the user favorited it, and its ``WorkerOptions``. +/// +/// One record per repo — rather than parallel maps keyed by ``RepoID`` — so +/// the enabled flag, the favorite flag, and the options can't drift apart. +public struct RepoConfiguration: Codable, Equatable, Sendable { + /// Whether this repo's worker should be running. Enabled workers are + /// restarted on app launch. + public var isEnabled: Bool + /// Whether the user pinned this repo to the top of its sidebar section. + public var isFavorite: Bool + /// This repo's worker options. + public var options: WorkerOptions + + public init(isEnabled: Bool, isFavorite: Bool, options: WorkerOptions) { + self.isEnabled = isEnabled + self.isFavorite = isFavorite + self.options = options + } + + /// The record an uncustomized repo reads as: disabled, unfavorited, + /// standard options. A repo whose state equals this needs no persisted + /// entry — absence reads identically. + public static let standard = RepoConfiguration( + isEnabled: false, + isFavorite: false, + options: .standard, + ) +} + +/// Everything Foreman persists: the per-repo records and the global settings. public struct ForemanConfiguration: Codable, Equatable, Sendable { /// Directory scanned for git repositories; `nil` means the default /// (`~/Development`, applied by ``AppSettings/resolvedScanDirectory`` — @@ -10,58 +39,50 @@ public struct ForemanConfiguration: Codable, Equatable, Sendable { /// Explicit `cursor-agent` executable; `nil` means auto-locate via /// ``CursorAgentLocator``. public var agentExecutable: URL? - /// Repos whose worker should be running. Enabled workers are restarted on - /// app launch. - public var enabledRepoIDs: Set - /// Per-repo worker options; repos absent from the map use - /// ``WorkerOptions/standard``. - public var repoOptions: [RepoID: WorkerOptions] + /// Per-repo persisted state, keyed by ``RepoID``. Repos absent from the + /// map read as ``RepoConfiguration/standard`` (absence is expected, not an + /// error). + public var repos: [RepoID: RepoConfiguration] public init( scanDirectory: URL?, agentExecutable: URL?, - enabledRepoIDs: Set, - repoOptions: [RepoID: WorkerOptions], + repos: [RepoID: RepoConfiguration], ) { self.scanDirectory = scanDirectory self.agentExecutable = agentExecutable - self.enabledRepoIDs = enabledRepoIDs - self.repoOptions = repoOptions + self.repos = repos } /// The configuration a fresh install starts from. public static let initial = ForemanConfiguration( scanDirectory: nil, agentExecutable: nil, - enabledRepoIDs: [], - repoOptions: [:], + repos: [:], ) - /// Options for `repo`, falling back to ``WorkerOptions/standard`` for - /// repos that were never customized (absence is expected, not an error). - public func options(for repo: RepoID) -> WorkerOptions { - repoOptions[repo] ?? .standard + /// The persisted record for `repo`, falling back to + /// ``RepoConfiguration/standard`` for repos that were never customized + /// (absence is expected, not an error). + public func configuration(for repo: RepoID) -> RepoConfiguration { + repos[repo] ?? .standard } - /// Drops `enabledRepoIDs` / `repoOptions` entries for repos that live - /// under `scanDirectory` but are no longer in `discovered` — entries for - /// deleted or renamed repos would otherwise accumulate forever. Entries - /// *outside* `scanDirectory` are kept: they belong to another scan - /// directory's history and re-apply when the user switches back. - /// Returns whether anything was removed. + /// Drops `repos` entries for repositories that live under `scanDirectory` + /// but are no longer in `discovered` — entries for deleted or renamed + /// repos would otherwise accumulate forever. Entries *outside* + /// `scanDirectory` are kept: they belong to another scan directory's + /// history and re-apply when the user switches back. Returns whether + /// anything was removed. public mutating func prune(discovered: Set, under scanDirectory: URL) -> Bool { let prefix = scanDirectory.standardizedFileURL.path + "/" - func isStale(_ id: RepoID) -> Bool { + let stale = repos.keys.filter { id in id.rawValue.hasPrefix(prefix) && !discovered.contains(id) } + guard !stale.isEmpty else { return false } - let staleEnabled = enabledRepoIDs.filter(isStale) - let staleOptions = repoOptions.keys.filter(isStale) - guard !staleEnabled.isEmpty || !staleOptions.isEmpty else { return false } - - enabledRepoIDs.subtract(staleEnabled) - for key in staleOptions { - repoOptions.removeValue(forKey: key) + for key in stale { + repos.removeValue(forKey: key) } return true } diff --git a/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift b/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift index 6896be88..21236e94 100644 --- a/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift +++ b/Foreman/ForemanCore/Tests/ForemanCoreTestSupport.swift @@ -19,12 +19,14 @@ final class SleepAssertionRecorder: SleepAssertionBackend { } /// Builds a live `Repo` over a fixed executable for tree-level tests: -/// disabled, standard options, no-op persistence and state-change hooks. +/// disabled, unfavorited, standard options, no-op persistence and +/// state-change hooks. @MainActor func makeStubRepo(scanned: ScannedRepo, logDirectory: URL, executable: URL) -> Repo { Repo( scanned: scanned, isEnabled: false, + isFavorite: false, options: .standard, worker: Worker( name: scanned.name, diff --git a/Foreman/ForemanCore/Tests/ForemanServicesTests.swift b/Foreman/ForemanCore/Tests/ForemanServicesTests.swift index 837cd2f9..1016caab 100644 --- a/Foreman/ForemanCore/Tests/ForemanServicesTests.swift +++ b/Foreman/ForemanCore/Tests/ForemanServicesTests.swift @@ -39,8 +39,7 @@ struct ForemanServicesTests { var configuration = ForemanConfiguration( scanDirectory: scanDirectory, agentExecutable: executable, - enabledRepoIDs: [], - repoOptions: [:], + repos: [:], ) configure(&configuration, scanDirectory) try store.save(configuration) @@ -75,10 +74,10 @@ struct ForemanServicesTests { "Idle", "Restored", ]) { configuration, scanDirectory in - configuration.enabledRepoIDs = [ + configuration.repos[ RepoID(rootURL: scanDirectory .appendingPathComponent("Restored", isDirectory: true)), - ] + ] = RepoConfiguration(isEnabled: true, isFavorite: false, options: .standard) } fixture.services.start() @@ -134,10 +133,11 @@ struct ForemanServicesTests { try await waitUntil("worker reaches running") { repo.worker.state.isLive } - #expect(try fixture.store.load().enabledRepoIDs == [repo.id]) + #expect(try fixture.store.load().repos[repo.id]?.isEnabled == true) repo.isEnabled = false - #expect(try fixture.store.load().enabledRepoIDs == []) + // Back to a fully default record: the entry is dropped entirely. + #expect(try fixture.store.load().repos[repo.id] == nil) try await waitUntil("worker stops") { repo.worker.state == .stopped } @@ -151,10 +151,42 @@ struct ForemanServicesTests { var renamed = WorkerOptions.standard renamed.displayName = "Renamed" repo.options = renamed - #expect(try fixture.store.load().repoOptions[repo.id] == renamed) + #expect(try fixture.store.load().repos[repo.id]?.options == renamed) repo.options = .standard - #expect(try fixture.store.load().repoOptions[repo.id] == nil) + #expect(try fixture.store.load().repos[repo.id] == nil) + } + + @Test func favoritingWritesThroughAndClearingDropsTheEntry() throws { + let fixture = try makeFixture(repoNames: ["Thing"]) + fixture.services.start() + let repo = try #require(fixture.services.repos.first) + + repo.isFavorite = true + #expect(try fixture.store.load().repos[repo.id]?.isFavorite == true) + + // Favoriting alone doesn't enable the worker. + #expect(try fixture.store.load().repos[repo.id]?.isEnabled == false) + #expect(repo.worker.state == .stopped) + + // Unfavoriting a repo with no other customization drops the entry. + repo.isFavorite = false + #expect(try fixture.store.load().repos[repo.id] == nil) + } + + @Test func savedFavoriteIsRestoredOnLaunch() throws { + let fixture = try makeFixture(repoNames: ["Pinned"]) { configuration, scanDirectory in + configuration.repos[ + RepoID(rootURL: scanDirectory + .appendingPathComponent("Pinned", isDirectory: true)), + ] = RepoConfiguration(isEnabled: false, isFavorite: true, options: .standard) + } + + fixture.services.start() + + let pinned = try #require(fixture.services.repos.first) + #expect(pinned.isFavorite) + #expect(!pinned.isEnabled) } @Test func settingsChangesPersistAndAScanDirectoryChangeRescans() throws { @@ -188,21 +220,29 @@ struct ForemanServicesTests { let gone = RepoID(rootURL: scanDirectory .appendingPathComponent("Gone", isDirectory: true)) stale = gone - configuration.enabledRepoIDs = [foreign, gone] - configuration.repoOptions[gone] = WorkerOptions( - displayName: "Gone", - assignment: .shared, - labels: [], - idleReleaseTimeoutSeconds: 0, - verbose: false, + configuration.repos[foreign] = RepoConfiguration( + isEnabled: true, + isFavorite: false, + options: .standard, + ) + configuration.repos[gone] = RepoConfiguration( + isEnabled: true, + isFavorite: false, + options: WorkerOptions( + displayName: "Gone", + assignment: .shared, + labels: [], + idleReleaseTimeoutSeconds: 0, + verbose: false, + ), ) } fixture.services.start() let saved = try fixture.store.load() - #expect(saved.enabledRepoIDs == [foreign]) - #expect(try saved.repoOptions[#require(stale)] == nil) + #expect(saved.repos[foreign]?.isEnabled == true) + #expect(try saved.repos[#require(stale)] == nil) } @Test func failedRescanKeepsReposAndReportsTheIssue() throws { diff --git a/Foreman/ForemanCore/Tests/RepoSectionTests.swift b/Foreman/ForemanCore/Tests/RepoSectionTests.swift new file mode 100644 index 00000000..b78e2418 --- /dev/null +++ b/Foreman/ForemanCore/Tests/RepoSectionTests.swift @@ -0,0 +1,75 @@ +@_spi(Testing) import ForemanCore +import Foundation +import Testing + +@MainActor +struct RepoSectionTests { + /// Builds a repo with the given desired/favorite state *without* spawning a + /// worker: `init` assigns `isEnabled` directly (only `startIfEnabled()` or + /// the toggle's `didSet` would start one), so these stay pure. + private func makeRepo(_ name: String, enabled: Bool, favorite: Bool) -> Repo { + let root = URL(fileURLWithPath: "/tmp/RepoSectionTests/\(name)", isDirectory: true) + return Repo( + scanned: ScannedRepo(name: name, rootURL: root), + isEnabled: enabled, + isFavorite: favorite, + options: .standard, + worker: Worker( + name: name, + workerDirectory: root, + logDirectory: URL(fileURLWithPath: "/tmp/RepoSectionTests/logs", isDirectory: true), + onStateChange: {}, + ), + resolveExecutable: { URL(fileURLWithPath: "/usr/bin/true") }, + onPersistentChange: { _ in }, + ) + } + + private func names(_ sections: [RepoSection], _ kind: RepoSection.Kind) -> [String] { + sections.first { $0.kind == kind }?.repos.map(\.name) ?? [] + } + + @Test func partitionsByEnabledWithTheEnabledSectionFirst() { + let sections = RepoSection.sections(from: [ + makeRepo("Alpha", enabled: true, favorite: false), + makeRepo("Beta", enabled: false, favorite: false), + ]) + + #expect(sections.map(\.kind) == [.enabled, .disabled]) + #expect(names(sections, .enabled) == ["Alpha"]) + #expect(names(sections, .disabled) == ["Beta"]) + } + + @Test func favoritesFloatToTheTopOfEachSectionKeepingOrderOtherwise() { + // Input is name-sorted, as RepoDiscovery delivers it. + let sections = RepoSection.sections(from: [ + makeRepo("Ada", enabled: true, favorite: false), + makeRepo("Zoe", enabled: true, favorite: true), + makeRepo("Bob", enabled: false, favorite: false), + makeRepo("Xor", enabled: false, favorite: true), + ]) + + // The favorite leads each section; the rest keep alphabetical order. + #expect(names(sections, .enabled) == ["Zoe", "Ada"]) + #expect(names(sections, .disabled) == ["Xor", "Bob"]) + } + + @Test func multipleFavoritesKeepTheirRelativeOrder() { + let sections = RepoSection.sections(from: [ + makeRepo("Ann", enabled: true, favorite: true), + makeRepo("Ben", enabled: true, favorite: false), + makeRepo("Cat", enabled: true, favorite: true), + ]) + + #expect(names(sections, .enabled) == ["Ann", "Cat", "Ben"]) + } + + @Test func emptySectionsAreOmitted() { + let allEnabled = RepoSection.sections(from: [ + makeRepo("Only", enabled: true, favorite: false), + ]) + #expect(allEnabled.map(\.kind) == [.enabled]) + + #expect(RepoSection.sections(from: []).isEmpty) + } +} diff --git a/Foreman/ForemanCore/Tests/RepoTests.swift b/Foreman/ForemanCore/Tests/RepoTests.swift index f57f101c..949ce09c 100644 --- a/Foreman/ForemanCore/Tests/RepoTests.swift +++ b/Foreman/ForemanCore/Tests/RepoTests.swift @@ -18,6 +18,7 @@ struct RepoTests { return Repo( scanned: scanned, isEnabled: false, + isFavorite: false, options: .standard, worker: Worker( name: scanned.name, @@ -124,6 +125,30 @@ struct RepoTests { #expect(fixture.persistedChanges == 1) } + // MARK: - Favorite + + @Test func favoritingNotifiesPersistenceWithoutTouchingTheWorker() throws { + let fixture = try makeFixture() + + fixture.repo.isFavorite = true + #expect(fixture.repo.isFavorite) + #expect(fixture.persistedChanges == 1) + // Favorite is pure metadata: it must not start a worker. + #expect(fixture.repo.worker.state == .stopped) + + fixture.repo.isFavorite = false + #expect(fixture.persistedChanges == 2) + #expect(fixture.repo.worker.state == .stopped) + } + + @Test func reassigningTheSameFavoriteValueIsANoOp() throws { + let fixture = try makeFixture() + + fixture.repo.isFavorite = false + + #expect(fixture.persistedChanges == 0) + } + // MARK: - Launch restore @Test func startIfEnabledStartsOnlyWhenEnabled() async throws { diff --git a/Foreman/ForemanCore/Tests/WorkerConfigStoreTests.swift b/Foreman/ForemanCore/Tests/WorkerConfigStoreTests.swift index 2a0736c5..b23522fd 100644 --- a/Foreman/ForemanCore/Tests/WorkerConfigStoreTests.swift +++ b/Foreman/ForemanCore/Tests/WorkerConfigStoreTests.swift @@ -17,8 +17,7 @@ struct WorkerConfigStoreTests { let configuration = ForemanConfiguration( scanDirectory: URL(fileURLWithPath: "/Users/dev/Code"), agentExecutable: URL(fileURLWithPath: "/usr/local/bin/cursor-agent"), - enabledRepoIDs: [repo], - repoOptions: [repo: options], + repos: [repo: RepoConfiguration(isEnabled: true, isFavorite: true, options: options)], ) try store.save(configuration) @@ -45,10 +44,10 @@ struct WorkerConfigStoreTests { } } - @Test func optionsForAnUncustomizedRepoAreStandard() { + @Test func configurationForAnUncustomizedRepoIsStandard() { let configuration = ForemanConfiguration.initial - #expect(configuration.options(for: RepoID(rawValue: "/nowhere")) == .standard) + #expect(configuration.configuration(for: RepoID(rawValue: "/nowhere")) == .standard) } @Test func pruneDropsVanishedReposOnlyUnderTheScanDirectory() { @@ -60,20 +59,17 @@ struct WorkerConfigStoreTests { let sibling = RepoID(rawValue: "/Users/dev/CodeArchive/Old") // A different scan directory's history — must survive too. let elsewhere = RepoID(rawValue: "/Users/dev/Other/Repo") - var options = WorkerOptions.standard - options.verbose = true + let record = RepoConfiguration(isEnabled: true, isFavorite: false, options: .standard) var configuration = ForemanConfiguration( scanDirectory: scanDirectory, agentExecutable: nil, - enabledRepoIDs: [kept, vanished, sibling, elsewhere], - repoOptions: [kept: options, vanished: options, sibling: options], + repos: [kept: record, vanished: record, sibling: record, elsewhere: record], ) let changed = configuration.prune(discovered: [kept], under: scanDirectory) #expect(changed) - #expect(configuration.enabledRepoIDs == [kept, sibling, elsewhere]) - #expect(configuration.repoOptions == [kept: options, sibling: options]) + #expect(configuration.repos == [kept: record, sibling: record, elsewhere: record]) } @Test func pruneWithNothingStaleReportsNoChange() { @@ -82,8 +78,11 @@ struct WorkerConfigStoreTests { var configuration = ForemanConfiguration( scanDirectory: scanDirectory, agentExecutable: nil, - enabledRepoIDs: [repo], - repoOptions: [:], + repos: [repo: RepoConfiguration( + isEnabled: true, + isFavorite: false, + options: .standard, + )], ) let before = configuration From e46488f3bfb74e769c9b2f5deeca802900c900ed Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 5 Jul 2026 20:37:39 -0400 Subject: [PATCH 2/6] Section the Foreman sidebar and add favorite affordances Render session.repoSections as Enabled/Disabled sections in the sidebar, show a star on favorited rows with a right-click Add/Remove Favorite context menu, and add a star toggle to the worker detail view's toolbar. Co-authored-by: Cursor --- Foreman/Foreman/Sources/MainWindowView.swift | 19 ++++++++++++++++--- .../Foreman/Sources/WorkerDetailView.swift | 15 +++++++++++++++ Foreman/Foreman/Sources/WorkerRowView.swift | 15 +++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/Foreman/Foreman/Sources/MainWindowView.swift b/Foreman/Foreman/Sources/MainWindowView.swift index 148f3d05..dc627ebf 100644 --- a/Foreman/Foreman/Sources/MainWindowView.swift +++ b/Foreman/Foreman/Sources/MainWindowView.swift @@ -39,9 +39,15 @@ struct MainWindowView: View { private var sidebar: some View { List(selection: $selection) { - ForEach(session.repos) { repo in - WorkerRowView(repo: repo) - .tag(repo.id) + // Enabled repos on top, disabled below, favorites floated to the + // top of each section (ordering computed in ForemanCore). + ForEach(session.repoSections) { section in + Section(title(for: section.kind)) { + ForEach(section.repos) { repo in + WorkerRowView(repo: repo) + .tag(repo.id) + } + } } } .overlay { @@ -56,6 +62,13 @@ struct MainWindowView: View { .navigationTitle("Foreman") } + private func title(for kind: RepoSection.Kind) -> String { + switch kind { + case .enabled: "Enabled" + case .disabled: "Disabled" + } + } + @ViewBuilder private var issueBanner: some View { if let issue = session.issueMessage { diff --git a/Foreman/Foreman/Sources/WorkerDetailView.swift b/Foreman/Foreman/Sources/WorkerDetailView.swift index 260944b1..dbc2fd08 100644 --- a/Foreman/Foreman/Sources/WorkerDetailView.swift +++ b/Foreman/Foreman/Sources/WorkerDetailView.swift @@ -63,6 +63,21 @@ struct WorkerDetailView: View { .formStyle(.grouped) .navigationTitle(repo.name) .navigationSubtitle(repo.rootURL.path) + .toolbar { + ToolbarItem { + Button { + repo.isFavorite.toggle() + } label: { + Label( + repo.isFavorite ? "Remove from Favorites" : "Add to Favorites", + systemImage: repo.isFavorite ? "star.fill" : "star", + ) + } + .help(repo.isFavorite + ? "Remove this repo from favorites." + : "Pin this repo to the top of its section.") + } + } } /// The transient actions the enable toggle can't express, shown only in diff --git a/Foreman/Foreman/Sources/WorkerRowView.swift b/Foreman/Foreman/Sources/WorkerRowView.swift index c5bb68d1..2df24341 100644 --- a/Foreman/Foreman/Sources/WorkerRowView.swift +++ b/Foreman/Foreman/Sources/WorkerRowView.swift @@ -19,6 +19,13 @@ struct WorkerRowView: View { } } + if repo.isFavorite { + Image(systemName: "star.fill") + .font(.caption2) + .foregroundStyle(.yellow) + .help("Favorite") + } + Spacer() Toggle("Worker enabled", isOn: $repo.isEnabled) @@ -27,6 +34,14 @@ struct WorkerRowView: View { .controlSize(.small) } .padding(.vertical, 2) + .contextMenu { + Button( + repo.isFavorite ? "Remove from Favorites" : "Add to Favorites", + systemImage: repo.isFavorite ? "star.slash" : "star", + ) { + repo.isFavorite.toggle() + } + } } } From f233826c497d40f8fc0cd53746c54d85fe59f466 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 5 Jul 2026 20:37:44 -0400 Subject: [PATCH 3/6] Update Foreman docs for per-repo config, favorites, and sidebar sections Co-authored-by: Cursor --- Foreman/Foreman/AGENTS.md | 26 +++++++++++++-------- Foreman/ForemanCore/AGENTS.md | 43 +++++++++++++++++++++++------------ Foreman/ForemanCore/README.md | 22 ++++++++++++++---- 3 files changed, 63 insertions(+), 28 deletions(-) diff --git a/Foreman/Foreman/AGENTS.md b/Foreman/Foreman/AGENTS.md index 1e2eafcb..164da377 100644 --- a/Foreman/Foreman/AGENTS.md +++ b/Foreman/Foreman/AGENTS.md @@ -27,9 +27,10 @@ formatting, global conventions) and ForemanCore's [`Project.swift`](../../Project.swift) (don't switch it to `.extendingDefault`, which injects `NSMainStoryboardFile` on macOS). - [`ForemanSession`](Sources/ForemanSession.swift) – a deliberately thin - facade: owns the `ForemanServices` root, forwards the app-level intents - (`start`, `rescan`, `stopAllWorkers`), and re-exposes `repos`, `settings`, - `issueMessage`, and the liveness flags. There is no mirroring layer — + facade: owns the `ForemanServices` root, forwards the app-level intents + (`start`, `rescan`, `stopAllWorkers`), and re-exposes `repos`, + `repoSections`, `settings`, `issueMessage`, and the liveness flags. There is + no mirroring layer — views bind the Core `Repo` objects directly (`$repo.isEnabled`, `repo.worker.state`), and toggle/options/settings semantics (declarative toggle, persistence funnel, retry/restart intents) live in ForemanCore; @@ -55,15 +56,21 @@ formatting, global conventions) and ForemanCore's sequence on `isAnyWorkerLive`. Don't migrate back to `MenuBarExtra` without re-verifying all of the above. - [`MainWindowView`](Sources/MainWindowView.swift) – the window content: a - `NavigationSplitView` with repo rows in the sidebar and the selected - worker's detail on the right, plus the toolbar (sleep badge, Rescan, a - `SettingsLink`, Quit) and the issue banner. The detail carries + `NavigationSplitView` whose sidebar renders `session.repoSections` — an + **Enabled** section on top and a **Disabled** section below, favorites + floated to the top of each (the ordering itself is computed in + `ForemanCore`'s `RepoSection`; the view only maps `Kind` → a section title) — + and the selected worker's detail on the right, plus the toolbar (sleep + badge, Rescan, a `SettingsLink`, Quit) and the issue banner. The detail + carries `.id(repo.id)` so per-repo `@State` (options draft, log tail) resets on selection change. `onAppear` covers the first-open scan; later opens rescan via the window delegate (no file watching). - [`WorkerRowView`](Sources/WorkerRowView.swift) – sidebar row: status dot, - name (+ a red "failed" hint), toggle. Actions and details live in the - detail pane. + name (+ a red "failed" hint), a star glyph when favorited, and the enable + toggle. A right-click context menu toggles `repo.isFavorite` (the other + favorite affordance is the detail toolbar's star button). Actions and + details live in the detail pane. - [`WorkerDetailView`](Sources/WorkerDetailView.swift) – one worker's status/pid/uptime (uptime renders live via `Text(_, style: .relative)`), the exact command the next start will spawn, the inline options editor, @@ -71,7 +78,8 @@ formatting, global conventions) and ForemanCore's controls: **Retry** for an enabled repo in `.failed`, **Restart** while `.running` — both call the `Repo` intents (no view-local process logic) and never touch the persisted toggle. Start/Stop as user actions stay the - enable toggle. + enable toggle. Its `.toolbar` also contributes a star button (merged into + the window toolbar for the selected repo) that toggles `repo.isFavorite`. - [`WorkerLogView`](Sources/WorkerLogView.swift) – tails the worker's log file via `LogTailReader`, polling once a second while visible. One `Tail` enum so "no file yet", content, and a read failure can't coexist; polls diff --git a/Foreman/ForemanCore/AGENTS.md b/Foreman/ForemanCore/AGENTS.md index 844afa2f..a1270b74 100644 --- a/Foreman/ForemanCore/AGENTS.md +++ b/Foreman/ForemanCore/AGENTS.md @@ -41,8 +41,10 @@ build system, formatting, and global conventions. Read that first. observable `issueMessage`; per-repo failures live on each repo's worker. - [`Repo`](Sources/Repo.swift) – one repository in the tree: identity (`RepoID` = canonical absolute path, `name`, `rootURL`), the persisted - intent (`isEnabled`, `options` — both `didSet`-guarded and funneled to the - root), and its `Worker`. **The toggle is declarative**: `isEnabled` records + intent (`isEnabled`, `isFavorite`, `options` — all `didSet`-guarded and + funneled to the root), and its `Worker`. `isFavorite` is pure sidebar- + ordering metadata with **no** worker side effect (only persistence), unlike + `isEnabled`. **The toggle is declarative**: `isEnabled` records desired state, `worker.state` reports the actual one; a locate failure lands as `.failed` via `recordStartFailure` with the toggle still on, and switching a failed repo off acknowledges the failure (`Worker.stop()` @@ -88,14 +90,27 @@ build system, formatting, and global conventions. Read that first. struct, not tuples. `arguments(workerDirectory:)` is the **only** place that spells CLI flags: worker-level flags before the `start` subcommand, `--verbose` after it. -- [`ForemanConfiguration` / `WorkerConfigStore`](Sources/WorkerConfigStore.swift) - – the persisted state (scan directory, explicit agent executable, - enabled-repo set, per-repo options) and its JSON store. `load()` - distinguishes *missing* (first launch → `.initial`) from *corrupt* (throws); - don't collapse the two. `prune(discovered:under:)` drops entries for repos - gone from the scan directory but must keep entries *outside* it — they're - another scan directory's history (the prefix check appends "/" so a - sibling like `~/CodeArchive` doesn't match a `~/Code` scan directory). +- [`RepoConfiguration` / `ForemanConfiguration` / `WorkerConfigStore`](Sources/WorkerConfigStore.swift) + – the persisted state and its JSON store. `RepoConfiguration` is the + **single per-repo record** (`isEnabled`, `isFavorite`, `options`) — one + struct rather than parallel maps keyed by `RepoID`, so the flags and options + can't drift apart; `ForemanConfiguration` holds `scanDirectory`, + `agentExecutable`, and `repos: [RepoID: RepoConfiguration]`. A repo whose + record equals `RepoConfiguration.standard` (disabled, unfavorited, standard + options) has no entry — absence reads identically, so the persistence funnel + drops it. `load()` distinguishes *missing* (first launch → `.initial`) from + *corrupt* (throws); don't collapse the two. `prune(discovered:under:)` drops + `repos` entries for repos gone from the scan directory but must keep entries + *outside* it — they're another scan directory's history (the prefix check + appends "/" so a sibling like `~/CodeArchive` doesn't match a `~/Code` scan + directory). +- [`RepoSection`](Sources/RepoSection.swift) – the pure sidebar ordering rule: + `sections(from:)` splits the discovered repos into an `.enabled` section on + top and a `.disabled` section below, floating favorites to the top of each + (stable, so the name-sorted input order is otherwise preserved) and omitting + an empty section. Lives in Core (not the view) so the ordering is + unit-tested; the `Kind` carries no display text — section titles are the + view's concern. - [`LogTailReader`](Sources/LogTailReader.swift) – `tail(of:maxBytes:)` reads the end of a worker log file for display. `nil` means *no file yet* (a never-started worker — legitimate, not an error); other I/O failures @@ -126,10 +141,10 @@ build system, formatting, and global conventions. Read that first. there; vanished repos are stopped and drained, not dropped mid-exit, and a reappearing directory resurrects its draining `Repo` so a single id can never have two live processes. -- **Absence vs failure.** A repo with no customized options reads as - `WorkerOptions.standard` (expected absence); an unreadable or undecodable - config file throws (real failure). Keep that split — no `try?` or silent - resets. +- **Absence vs failure.** A repo with no persisted entry reads as + `RepoConfiguration.standard` (disabled, unfavorited, `WorkerOptions.standard` + — expected absence); an unreadable or undecodable config file throws (real + failure). Keep that split — no `try?` or silent resets. - **CLI defaults are deferred to, not duplicated.** An empty pool name or zero idle timeout omits the flag so the CLI's own default applies; don't bake the CLI's default values into rendered argv. diff --git a/Foreman/ForemanCore/README.md b/Foreman/ForemanCore/README.md index 088f6c79..98d312bc 100644 --- a/Foreman/ForemanCore/README.md +++ b/Foreman/ForemanCore/README.md @@ -56,8 +56,10 @@ let argv = repo.options.arguments(workerDirectory: repo.rootURL) scan, failed save) on the observable `issueMessage`. - **`Repo` / `RepoID`** — one discovered repository as an `@Observable` object: identity (`name`, `rootURL`, typed `RepoID` = canonical absolute - path), the persisted intent (`isEnabled`, `options` — mutations start/stop - the worker and persist automatically), and its `Worker`. Transient intents + path), the persisted intent (`isEnabled`, `isFavorite`, `options` — + mutations persist automatically; `isEnabled` also starts/stops the worker, + while `isFavorite` is pure sidebar-ordering metadata), and its `Worker`. + Transient intents that don't change the persisted desired state: `retry()` (fresh attempt for an enabled repo in `.failed`) and `restart()` (respawn a running worker with the current options — the apply path for options edited while @@ -81,6 +83,11 @@ let argv = repo.options.arguments(workerDirectory: repo.rootURL) directory (a subdirectory with a `.git` entry — directory or file, so worktrees count) as `ScannedRepo` values. Hidden directories are skipped; nesting is not searched. Throws when the scan directory can't be listed. +- **`RepoSection`** — the pure sidebar ordering rule. `sections(from:)` groups + the discovered repos into an `.enabled` section on top and a `.disabled` one + below, floating favorites to the top of each (stable — otherwise the + name-sorted order is preserved) and omitting an empty section. `ForemanServices` + exposes it as `repoSections`. - **`AppSettings`** — observable global settings: `scanDirectory` (default `~/Development` via `resolvedScanDirectory`) and `agentExecutable` (`nil` = auto-locate). Assignments persist and — for the scan directory — @@ -92,9 +99,14 @@ let argv = repo.options.arguments(workerDirectory: repo.rootURL) (`--idle-release-timeout`), and `verbose` (`start --verbose`). `arguments(workerDirectory:)` renders the full argv; `.standard` is the CLI-default configuration. -- **`ForemanConfiguration`** — everything Foreman persists: the scan directory - (default `~/Development`), an explicit `cursor-agent` executable (or `nil` - for auto-locate), the enabled-repo set, and the per-repo options map. +- **`RepoConfiguration` / `ForemanConfiguration`** — everything Foreman + persists. `RepoConfiguration` is the single per-repo record (`isEnabled`, + `isFavorite`, `options`) — one struct instead of parallel maps keyed by + `RepoID`, so the flags and options can't drift apart. `ForemanConfiguration` + holds the scan directory (default `~/Development`), an explicit + `cursor-agent` executable (or `nil` for auto-locate), and + `repos: [RepoID: RepoConfiguration]`; a repo left at + `RepoConfiguration.standard` has no entry (absence reads identically). `prune(discovered:under:)` drops entries for repos that vanished from the scan directory while keeping entries outside it (another directory's history, re-applied when the user switches back). From e46c7fd68cabfecb0fd14cfbaff1be438ef6cb7f Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 5 Jul 2026 20:42:37 -0400 Subject: [PATCH 4/6] Animate sidebar row moves when repos change section or favorite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attach .animation(.snappy, value:) to the sidebar List keyed on the flattened row order, so a repo tweens as it moves between the Enabled/Disabled sections or floats within a section on favorite — regardless of what triggered it. Co-authored-by: Cursor --- Foreman/Foreman/Sources/MainWindowView.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Foreman/Foreman/Sources/MainWindowView.swift b/Foreman/Foreman/Sources/MainWindowView.swift index dc627ebf..e6c840c8 100644 --- a/Foreman/Foreman/Sources/MainWindowView.swift +++ b/Foreman/Foreman/Sources/MainWindowView.swift @@ -50,6 +50,10 @@ struct MainWindowView: View { } } } + // Tween row moves whenever the order changes — whatever the trigger + // (enable toggle moving a repo across sections, favorite floating it + // within one, or a rescan adding/removing repos). + .animation(.snappy, value: rowOrder) .overlay { if session.repos.isEmpty { ContentUnavailableView { @@ -69,6 +73,13 @@ struct MainWindowView: View { } } + /// The flattened row-identity order across sections. Animating on this + /// lets the sidebar tween a row moving to another section or reordering + /// within one, without coupling the animation to any single trigger. + private var rowOrder: [RepoID] { + session.repoSections.flatMap { $0.repos.map(\.id) } + } + @ViewBuilder private var issueBanner: some View { if let issue = session.issueMessage { From e731173898e6638fbb2c37f4bd7b36df66bc1875 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 5 Jul 2026 20:51:25 -0400 Subject: [PATCH 5/6] Glide repo rows across the sidebar group boundary Replace the sidebar's SwiftUI Sections with a single flat ForEach of headers and rows sharing one identity space, so a repo moving between the Enabled and Disabled groups animates as one glide rather than a cross-container remove/insert. Group headers render inline and are selection-disabled; the .animation is keyed on the flattened row identities. Co-authored-by: Cursor --- Foreman/Foreman/AGENTS.md | 21 ++++-- Foreman/Foreman/Sources/MainWindowView.swift | 67 +++++++++++++++----- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/Foreman/Foreman/AGENTS.md b/Foreman/Foreman/AGENTS.md index 164da377..c7433a0d 100644 --- a/Foreman/Foreman/AGENTS.md +++ b/Foreman/Foreman/AGENTS.md @@ -56,13 +56,20 @@ formatting, global conventions) and ForemanCore's sequence on `isAnyWorkerLive`. Don't migrate back to `MenuBarExtra` without re-verifying all of the above. - [`MainWindowView`](Sources/MainWindowView.swift) – the window content: a - `NavigationSplitView` whose sidebar renders `session.repoSections` — an - **Enabled** section on top and a **Disabled** section below, favorites - floated to the top of each (the ordering itself is computed in - `ForemanCore`'s `RepoSection`; the view only maps `Kind` → a section title) — - and the selected worker's detail on the right, plus the toolbar (sleep - badge, Rescan, a `SettingsLink`, Quit) and the issue banner. The detail - carries + `NavigationSplitView` whose sidebar renders `session.repoSections` as an + **Enabled** group on top and a **Disabled** group below, favorites floated + to the top of each (the ordering itself is computed in `ForemanCore`'s + `RepoSection`; the view only maps `Kind` → a group title). It flattens the + sections into a **single `ForEach` of `SidebarRow`s** (group headers + + repo rows sharing one identity space) rather than SwiftUI `Section`s **on + purpose**: a lone `ForEach` lets a repo *glide* across the group boundary + when it toggles enabled, where separate `Section` containers would animate a + cross-container remove/insert instead. `.animation(.snappy, value:)` keyed on + the flattened row identities tweens every reorder (toggle, favorite, rescan); + headers are `.selectionDisabled()`. Don't reintroduce `Section` without + re-checking the cross-group move animation. The right pane shows the selected + worker's detail, plus the toolbar (sleep badge, Rescan, a `SettingsLink`, + Quit) and the issue banner. The detail carries `.id(repo.id)` so per-repo `@State` (options draft, log tail) resets on selection change. `onAppear` covers the first-open scan; later opens rescan via the window delegate (no file watching). diff --git a/Foreman/Foreman/Sources/MainWindowView.swift b/Foreman/Foreman/Sources/MainWindowView.swift index e6c840c8..5af4556f 100644 --- a/Foreman/Foreman/Sources/MainWindowView.swift +++ b/Foreman/Foreman/Sources/MainWindowView.swift @@ -39,21 +39,30 @@ struct MainWindowView: View { private var sidebar: some View { List(selection: $selection) { - // Enabled repos on top, disabled below, favorites floated to the - // top of each section (ordering computed in ForemanCore). - ForEach(session.repoSections) { section in - Section(title(for: section.kind)) { - ForEach(section.repos) { repo in + // One flat ForEach — group headers and repo rows share a single + // identity space — so a repo moving between the Enabled and + // Disabled groups animates as one glide instead of a cross-Section + // remove/insert. Ordering is computed in ForemanCore; the view + // only flattens it into headers + rows and renders them. + ForEach(sidebarRows) { row in + switch row { + case let .header(kind): + Text(title(for: kind)) + .font(.caption) + .fontWeight(.semibold) + .foregroundStyle(.secondary) + .padding(.top, 6) + .selectionDisabled() + case let .repo(id, repo): WorkerRowView(repo: repo) - .tag(repo.id) - } + .tag(id) } } } - // Tween row moves whenever the order changes — whatever the trigger - // (enable toggle moving a repo across sections, favorite floating it - // within one, or a rescan adding/removing repos). - .animation(.snappy, value: rowOrder) + // Tween the diff whenever the flattened order changes — whatever the + // trigger (enable toggle moving a repo across groups, favorite floating + // it within one, or a rescan adding/removing repos). + .animation(.snappy, value: sidebarRows.map(\.id)) .overlay { if session.repos.isEmpty { ContentUnavailableView { @@ -66,6 +75,16 @@ struct MainWindowView: View { .navigationTitle("Foreman") } + /// The sidebar's rows as one flat, stably-identified sequence: each + /// non-empty section contributes a group header followed by its repos. + /// A single identity space is what lets a repo glide across the group + /// boundary rather than fade out of one section and into another. + private var sidebarRows: [SidebarRow] { + session.repoSections.flatMap { section in + [SidebarRow.header(section.kind)] + section.repos.map { .repo(id: $0.id, repo: $0) } + } + } + private func title(for kind: RepoSection.Kind) -> String { switch kind { case .enabled: "Enabled" @@ -73,11 +92,27 @@ struct MainWindowView: View { } } - /// The flattened row-identity order across sections. Animating on this - /// lets the sidebar tween a row moving to another section or reordering - /// within one, without coupling the animation to any single trigger. - private var rowOrder: [RepoID] { - session.repoSections.flatMap { $0.repos.map(\.id) } + /// One rendered sidebar entry — a group header or a repo — carried in a + /// single `ForEach` so cross-group moves animate as a glide. Identity spans + /// both kinds: a header keys off its section kind, a repo off its `RepoID`. + /// + /// The repo's `RepoID` is captured alongside the `Repo` so the nonisolated + /// `Identifiable.id` getter needn't touch the `@MainActor`-isolated `Repo`. + private enum SidebarRow: Identifiable { + case header(RepoSection.Kind) + case repo(id: RepoID, repo: Repo) + + enum ID: Hashable { + case header(RepoSection.Kind) + case repo(RepoID) + } + + var id: ID { + switch self { + case let .header(kind): .header(kind) + case let .repo(id, _): .repo(id) + } + } } @ViewBuilder From 0c49d245c885336b55d977adff051f85cb620412 Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Sun, 5 Jul 2026 21:02:45 -0400 Subject: [PATCH 6/6] Address review: doc fix, simpler repoDidChange, favorite-retention test - Correct the RepoSection doc (the app renders group headers in a flat list, not SwiftUI Sections). - Build the persisted record directly from the repo in repoDidChange instead of reading-then-overwriting the existing entry. - Cover that disabling a favorited repo keeps its entry with isFavorite set. Co-authored-by: Cursor --- .../ForemanCore/Sources/ForemanServices.swift | 10 ++++---- Foreman/ForemanCore/Sources/RepoSection.swift | 3 ++- .../Tests/ForemanServicesTests.swift | 23 +++++++++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/Foreman/ForemanCore/Sources/ForemanServices.swift b/Foreman/ForemanCore/Sources/ForemanServices.swift index d6d51054..75e2685c 100644 --- a/Foreman/ForemanCore/Sources/ForemanServices.swift +++ b/Foreman/ForemanCore/Sources/ForemanServices.swift @@ -192,10 +192,12 @@ public final class ForemanServices { } private func repoDidChange(_ repo: Repo) { - var record = configuration.configuration(for: repo.id) - record.isEnabled = repo.isEnabled - record.isFavorite = repo.isFavorite - record.options = repo.options + // The repo is the source of truth for its whole persisted record. + let record = RepoConfiguration( + isEnabled: repo.isEnabled, + isFavorite: repo.isFavorite, + options: repo.options, + ) if record == .standard { // A fully default record reads identically to an absent entry; // dropping it keeps the file from accumulating no-op records. diff --git a/Foreman/ForemanCore/Sources/RepoSection.swift b/Foreman/ForemanCore/Sources/RepoSection.swift index 1ac10bb5..5d233a10 100644 --- a/Foreman/ForemanCore/Sources/RepoSection.swift +++ b/Foreman/ForemanCore/Sources/RepoSection.swift @@ -5,7 +5,8 @@ import Foundation /// /// This is the pure ordering rule behind the "smarter" repo list. It lives in /// Core (not the view) so the sectioning and sort are unit-tested; the app -/// target only renders a `Section` per element. +/// target renders each section as a group header followed by its rows (a +/// single flat list, so rows glide across the group boundary). /// /// The struct itself is not actor-isolated (so its `Identifiable` conformance /// stays nonisolated); only ``sections(from:)`` is `@MainActor`, since reading diff --git a/Foreman/ForemanCore/Tests/ForemanServicesTests.swift b/Foreman/ForemanCore/Tests/ForemanServicesTests.swift index 1016caab..ae4e2139 100644 --- a/Foreman/ForemanCore/Tests/ForemanServicesTests.swift +++ b/Foreman/ForemanCore/Tests/ForemanServicesTests.swift @@ -174,6 +174,29 @@ struct ForemanServicesTests { #expect(try fixture.store.load().repos[repo.id] == nil) } + @Test func disablingAFavoritedRepoKeepsItsEntry() async throws { + let fixture = try makeFixture(repoNames: ["Thing"]) + fixture.services.start() + let repo = try #require(fixture.services.repos.first) + + repo.isFavorite = true + repo.isEnabled = true + try await waitUntil("worker reaches running") { + repo.worker.state.isLive + } + + // Disabling clears the enabled flag but the favorite is separate state, + // so the record survives (not dropped as a no-op). + repo.isEnabled = false + let saved = try #require(try fixture.store.load().repos[repo.id]) + #expect(!saved.isEnabled) + #expect(saved.isFavorite) + + try await waitUntil("worker stops") { + repo.worker.state == .stopped + } + } + @Test func savedFavoriteIsRestoredOnLaunch() throws { let fixture = try makeFixture(repoNames: ["Pinned"]) { configuration, scanDirectory in configuration.repos[