From 66516d38d17e223a83ba50dedcde7330678c7a18 Mon Sep 17 00:00:00 2001 From: rzen Date: Sun, 26 Jul 2026 19:14:15 -0400 Subject: [PATCH] =?UTF-8?q?Build=20BoardStore=20=E2=80=94=20one-way=20flow?= =?UTF-8?q?=20and=20resilient=20reloads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-board @Observable MainActor hub (Kanban/LiveStore/): watcher signals drive off-main tree walks with a generation guard, single-flight coalescing (strongest pending origin, watcher's merge rule), and the resilience contract — a failed reload never replaces a good snapshot, per-file breakage never locks editing, and a wholesale operation (performWholesale) arms a reload-must-succeed-or-lock floor so a failed post-bracket reload flips the board read-only until a good reload heals it. Selection is a pure UUID-set value re-resolved on every swap; liveness flips eject. performWrite brackets the watcher so Writer round-trips come back app-mediated. 14 store tests; full suite 279 tests in 54 suites green. Three findings filed on the Redesign board. Claude-Session: https://claude.ai/code/session_018BjQRYBR6jQja3jCRi5S3A --- Kanban/LiveStore/BoardStore.swift | 519 +++++++++++++++++++++++++++ Kanban/LiveStore/FolderWatcher.swift | 9 +- KanbanTests/BoardStoreTests.swift | 487 +++++++++++++++++++++++++ 3 files changed, 1013 insertions(+), 2 deletions(-) create mode 100644 Kanban/LiveStore/BoardStore.swift create mode 100644 KanbanTests/BoardStoreTests.swift diff --git a/Kanban/LiveStore/BoardStore.swift b/Kanban/LiveStore/BoardStore.swift new file mode 100644 index 0000000..7b72e4d --- /dev/null +++ b/Kanban/LiveStore/BoardStore.swift @@ -0,0 +1,519 @@ +import Foundation +import Observation +import os + +// MARK: - Vocabulary + +/// Why a board is refusing writes — the read-only lock's cause, and the whole of its vocabulary +/// today. +/// +/// 02-architecture.md names three ways a board enters the lock and only the first is built here. +/// `vanishedRoot` (§ Write-failure surfacing — a root that is gone, after bookmark re-resolution +/// found nothing) and `unwritableLocation` (§ — a read-only volume or a permission-denied folder, +/// probed at open) arrive with the cards that implement them. The banner turns a case of this into +/// user-facing phrasing (§ The banner surface); the store only owns the truth of it. +public enum ReadOnlyLockReason: Sendable, Equatable { + /// A reload that followed a bracketed wholesale operation failed, so the last-good snapshot on + /// screen may describe a tree that no longer exists — after a branch switch, a different branch + /// entirely. Writes derived from it would land nonsense, so every write is refused until a + /// reload succeeds (02-architecture.md § Live-reload resilience, "A failed reload after a + /// bracketed operation locks the board read-only"). + case bracketedReloadFailed +} + +/// The refusal `BoardStore.performWrite` throws when the board is locked read-only. +/// +/// **Deliberately temporary, and deliberately not a `BoardWriteError`.** The lock is a *store* +/// condition, not a filesystem outcome: nothing was attempted, no path failed, and folding it into +/// `BoardWriteError.io(message:)` would misrepresent a policy refusal as an I/O error in the one +/// place — the banner — where the distinction is the whole point. Widening `BoardWriteError` to +/// carry a refusal case is the banner card's job (02-architecture.md § Write-failure surfacing: +/// "The operation is a closed enum, not a string"), and it will unify this vocabulary with the +/// Writer's. Until then this thin error keeps `performWrite`'s honesty at the cost of an untyped +/// `throws` on its signature. +public enum BoardStoreWriteRefusal: Error, Sendable, Equatable, CustomStringConvertible { + case readOnlyLocked(ReadOnlyLockReason) + + public var description: String { + switch self { + case let .readOnlyLocked(reason): + "the board is read-only (\(reason))" + } + } +} + +/// Which side of the live/tombstoned boundary something sits on. +/// +/// A selection is **homogeneous by liveness** (04-interactions.md § The trash): it never mixes live +/// and tombstoned items, so the side is a property of the selection as a whole rather than of each +/// member — which is exactly what makes re-resolution across a reload a matching rule rather than a +/// partition. +public enum Liveness: Sendable, Equatable { + case live + case trashed + + /// The side an item's own tombstone flag puts it on. `Lane.isDeleted`/`Card.isDeleted` are + /// presence-of-the-key, not validity, so a malformed `deleted:` still reads as trashed — see + /// their doc comments in `BoardModel.swift`. + init(isDeleted: Bool) { + self = isDeleted ? .trashed : .live + } +} + +// MARK: - Selection + +/// The board's selection: a set of UUIDs over the snapshot, plus the liveness side it lives on. +/// +/// **UUIDs, never indices or copies of items** (02-architecture.md § Live-reload resilience, +/// "Selection survives reloads by UUID"): a reload swaps the whole snapshot as a value, and +/// anything holding positions or item copies would be silently wrong the moment an agent files a +/// card. Re-resolution against the new snapshot is `resolved(against:)`, and it is a *pure +/// function* on purpose — the transient-state container (02-architecture.md § Changes from Kanban) +/// will absorb this type and apply the same rule to drag membership and the pending cut, so the +/// rule must be reusable rather than buried in the store's reload path. +public struct Selection: Sendable, Equatable { + public var ids: Set + public var liveness: Liveness + + public init(ids: Set = [], liveness: Liveness = .live) { + self.ids = ids + self.liveness = liveness + } + + /// Nothing selected, on the live side — the state a board opens in and the state + /// `BoardStore.clearSelection()` returns to. + public static let empty = Selection() + + public var isEmpty: Bool { ids.isEmpty } + + /// This selection re-grounded on `snapshot`: the members that are still there, **on the same + /// liveness side**, and nothing else. + /// + /// Two rules, both settled in 02-architecture.md § Live-reload resilience: + /// + /// - **Vanished members leave silently.** No substitute is invented, no successor is picked — + /// an empty result is a legitimate outcome. (App-mediated deletion is deliberately different: + /// ⌫ selects the successor sibling, because that is an act rather than a surprise — + /// 04-interactions.md ▸ The map. That belongs to the delete command, not here.) + /// - **A liveness flip is a vanish.** A foreign edit that tombstones a selected live card — or + /// restores a selected tombstoned one — ejects it, keeping 04-interactions.md's + /// homogeneous-by-liveness invariant true across reloads so menu validation never sees a + /// mixed selection. + /// + /// The match is on the item's *own* tombstone flag. A live card sitting under a tombstoned lane + /// therefore survives re-resolution even though the board does not render it — the loader keeps + /// it in the snapshot, and "still present, same side" is the rule as written. + public func resolved(against snapshot: BoardModel) -> Selection { + guard !ids.isEmpty else { return self } + + var survivors: Set = [] + survivors.reserveCapacity(ids.count) + for lane in snapshot.lanes { + if ids.contains(lane.id), Liveness(isDeleted: lane.isDeleted) == liveness { + survivors.insert(lane.id) + } + for card in lane.cards where ids.contains(card.id) { + if Liveness(isDeleted: card.isDeleted) == liveness { + survivors.insert(card.id) + } + } + } + return Selection(ids: survivors, liveness: liveness) + } +} + +// MARK: - BoardStore + +/// The per-board hub: one live snapshot, one reload pipeline, and the read-side conditions the +/// board window renders (02-architecture.md § Layering ▸ Components). +/// +/// **It enforces the one-way flow — files → watcher → loader → store → views — by never mutating +/// its snapshot from the write path.** A user action runs the Writer, the Writer touches disk, the +/// watcher notices, and the change arrives here as a reload like any external edit. The app trusts +/// its own writes no more than anyone else's; that is what makes external editors and agents +/// first-class, and it is why there is no `snapshot` setter anywhere below `apply(_:generation:)`. +/// +/// ### What this type actually owns +/// +/// 1. **The reload pipeline.** At most one tree walk in flight, off the main actor; signals arriving +/// during one coalesce into a single follow-up; only the newest result applies. +/// 2. **The failure rules.** A failed reload never replaces a good snapshot; an ordinary failure +/// raises the banner condition and leaves editing alone; a failure after a bracketed wholesale +/// operation locks the board read-only; the next success clears both. +/// 3. **Selection across reloads.** Re-resolved by UUID and liveness on every applied snapshot. +/// +/// ### What it deliberately does not own +/// +/// The `FolderWatcher` itself — the registry owns one watcher and one store per board and wires +/// them together (`watcherBrackets`, `handleWatcherEvent(_:)`), so this type can be built and tested +/// without a filesystem stream. The transient-state container is also still to come: selection lives +/// here for now, and the **new-card placeholder** will live beside it — a pseudo-card with no disk +/// presence and no UUID, overlaid on the snapshot rather than merged into it (02-architecture.md § +/// Layering, the one named exception to the one-way flow). Nothing here precludes that: `snapshot` +/// is a pure value swap with no identity assumptions, so an overlay can simply be rendered on top of +/// whatever the latest reload produced. +@MainActor +@Observable +public final class BoardStore { + + // MARK: Read-side state + + /// The last good tree walk. Replaced wholesale by a successful reload and **never** by the write + /// path — see the type's doc comment for why. A failed reload leaves it exactly as it was. + public private(set) var snapshot: BoardModel + + /// Tolerated anomalies from the load that produced `snapshot` (stray folders, an indexless + /// UUID-shaped folder, a board-level `deleted:`). Replaced with the snapshot, so they always + /// describe the tree currently on screen. + public private(set) var loadWarnings: [LoadWarning] + + /// The standing read-side condition: the error from the last reload that failed, `nil` when the + /// board is healthy. `BoardLoadError` already carries fail-fast's specifics — the offending path + /// and what is wrong with it — which is the whole of what the banner needs to render + /// (02-architecture.md § Live-reload resilience). This is a *condition*, not a one-shot: it + /// stands until a reload succeeds, and it heals without ceremony when one does. + public private(set) var reloadFailure: BoardLoadError? + + /// Non-`nil` while the board refuses writes. Cleared by the next successful reload, per "the + /// next successful reload clears both the banner and the lock". + public private(set) var readOnlyLock: ReadOnlyLockReason? + + public var isReadOnly: Bool { readOnlyLock != nil } + + /// The board's selection, re-resolved against every snapshot this store applies. + public private(set) var selection: Selection + + /// The board root this store was opened on. Constant for now: absorbing a rename by + /// re-resolving the security-scoped bookmark (02-architecture.md § Write-failure surfacing) is + /// the registry's job and a later card's, and it will arrive together with `.rootChanged` + /// handling. + public let rootURL: URL + + // MARK: Wiring + + /// The watcher's bracket calls, injected rather than owned: the registry holds the watcher and + /// the store together, and a store that reached into a watcher it did not own could not be + /// tested without one. `nil` means "no watcher attached" — every operation below still behaves, + /// it simply has nothing to suspend, which is exactly the shape unit tests want. + @ObservationIgnored + public var watcherBrackets: (begin: @MainActor () -> Void, end: @MainActor () -> Void)? + + // MARK: Reload machinery + + /// Monotonic id of the most recently *started* reload — and therefore also the number of tree + /// walks this store has run since it opened, which is what makes the coalescing rule assertable + /// from a test rather than merely plausible. + /// + /// Two rules are expressed as comparisons against it: **only the newest result applies** (a + /// result whose generation is no longer the current one is dropped), and **the wholesale + /// expectation binds to a load started after it was armed** (`wholesaleReloadFloor`). + @ObservationIgnored + private(set) var reloadGeneration = 0 + + /// Whether a tree walk is running. At most one ever is: a second concurrent walk would buy + /// nothing (both would produce the same snapshot) and would make "the newest result wins" a race + /// rather than a rule. + @ObservationIgnored + private var reloadInFlight = false + + /// A reload owed but not started, because one was already running when the signal arrived — a + /// **flag, not a queue**: any number of signals during one walk coalesce into exactly one + /// follow-up, because the follow-up is a full tree walk that covers all of them. The origin is + /// merged by the watcher's own precedence rule (`reconciling > appMediated > foreign`) so a + /// foreign event folding into a pending reconciling one never downgrades it. + @ObservationIgnored + private var pendingReload: WatchOrigin? + + /// The generation from which a bracketed wholesale operation's expectation applies, or `nil` + /// when no operation is outstanding. + /// + /// **Why a floor and not a boolean.** The rule is "the reload that ends this wholesale operation + /// must succeed, or the board locks" — and a boolean cannot tell that reload apart from one that + /// was *already in flight* when the operation ended, which observed a tree from before the + /// operation touched it and therefore proves nothing about the result. Arming stores + /// `reloadGeneration + 1`: the next walk to be *started*. An in-flight walk's generation is + /// below the floor and passes through without consuming it; the first walk started at or after + /// it consumes it — succeeding clears everything as usual, failing engages the lock. + /// + /// Ordinary (non-wholesale) reload failures never set the lock, because ordinary breakage is + /// per-file: the snapshot still describes the tree and editing around the broken file is safe. + @ObservationIgnored + private var wholesaleReloadFloor: Int? + + /// Consumers suspended in `awaitQuiescence()`, resumed together the moment nothing is running + /// and nothing is owed. + @ObservationIgnored + private var quiescenceWaiters: [CheckedContinuation] = [] + + /// Awaited off the main actor **after** a tree walk finishes and **before** its result is + /// applied — the one seam this type keeps, `nil` in production. + /// + /// It exists because two of the contracts above are ordering claims about work that runs + /// concurrently with the main actor ("only the newest result applies"; "the wholesale + /// expectation never binds to a walk already in flight"), and a test that cannot pin a finished + /// walk open can only approximate them with sleeps — which would make the suite slow, flaky, and + /// silent about the very race it exists to rule out. + @ObservationIgnored + var loadBarrier: (@Sendable () async -> Void)? + + private static let logger = Logger(subsystem: "dev.rzen.indie.Kanban", category: "store") + + // MARK: - Init + + /// Opens a board: one synchronous tree walk, and **no fallback if it fails**. + /// + /// Fail-fast is the *initial-load* contract (01-storage-format.md § Malformed input): there is + /// no last-good snapshot to keep on screen yet, so a broken board throws its `BoardLoadError` + /// instead of constructing a store that would have nothing to show. Every rule below — the + /// banner, the lock, "a failed reload never replaces a good snapshot" — exists only *because* + /// this one succeeded. + /// + /// The walk is synchronous because the caller has nothing to render until it lands; the + /// asynchronous, off-main pipeline starts with the first reload. + public init(rootURL: URL) throws(BoardLoadError) { + let result = try BoardLoader.load(boardRoot: rootURL) + self.rootURL = rootURL + self.snapshot = result.model + self.loadWarnings = result.warnings + self.reloadFailure = nil + self.readOnlyLock = nil + self.selection = .empty + } + + // MARK: - Inbound signals + + /// The single inbound signal — everything the outside world tells this store arrives here. + /// + /// Deliberately one door: the watcher, a test, and (later) the registry's wake/activation + /// reconciliation all speak the same two-case vocabulary, so there is exactly one place where a + /// filesystem change becomes a reload. + public func handleWatcherEvent(_ event: WatcherEvent) { + switch event { + case let .treeChanged(origin): + requestReload(origin) + + case .rootChanged: + // Stubbed on purpose, and the stub is the whole of the honest answer today: the settled + // response is to re-resolve the board's security-scoped bookmark and either + // `reattach(to:)` the watcher at the new location — a rename absorbed with no banner and + // no lock — or enter the vanished-root read-only lock (02-architecture.md § + // Write-failure surfacing). Both halves need the registry's bookmark, which this store + // deliberately does not own yet, and a guess here would be a *wrong* guess: treating a + // rename as a vanish would lock a board that is merely somewhere else. The card that + // brings the bookmark replaces this case; until then a root change simply leaves the + // last-good snapshot on screen, which is the same thing every failure path does. + Self.logger.debug("rootChanged ignored — bookmark re-resolution is not built yet") + } + } + + /// Starts a reload, or banks one if a walk is already running. + private func requestReload(_ origin: WatchOrigin) { + guard !reloadInFlight else { + pendingReload = WatchOrigin.merged(pendingReload, origin) + return + } + startReload(origin) + } + + // MARK: - The reload pipeline + + /// Runs one tree walk **off the main actor** and applies its result on it. + /// + /// Off-main because a board of any size is a directory walk plus a YAML parse per item, and the + /// whole point of the value-type snapshot is that this work can happen anywhere: `BoardLoader` + /// is stateless statics and `LoadResult` is `Sendable`, so the only thing that has to be on the + /// main actor is the assignment at the end. `Task.detached` rather than `Task { }`: a task + /// created inside a `@MainActor` method inherits that isolation and would run the walk on the + /// main actor — the exact thing this is avoiding. + /// + /// `origin` is not branched on: every reload is a full tree walk, so no origin is less safe than + /// another. It is carried because the *policy* around a reload differs later — a `.reconciling` + /// sweep re-probes writability (02-architecture.md § Write-failure surfacing) — and because it is + /// the one thing that makes a reload's provenance legible in the log. + private func startReload(_ origin: WatchOrigin) { + reloadGeneration += 1 + let generation = reloadGeneration + let root = rootURL + let barrier = loadBarrier + reloadInFlight = true + Self.logger.debug("reload \(generation, privacy: .public) started (\(origin.rawValue, privacy: .public))") + + Task.detached(priority: .userInitiated) { [weak self] in + // `do throws(BoardLoadError)`: without the annotation the `catch` widens to `any Error` + // and the loader's typed error is lost on the way into `Result`. + let outcome: Result + do throws(BoardLoadError) { + outcome = .success(try BoardLoader.load(boardRoot: root)) + } catch { + outcome = .failure(error) + } + await barrier?() + await self?.apply(outcome, generation: generation) + } + } + + /// Lands one walk's result and starts whatever it uncovered. + private func apply(_ outcome: Result, generation: Int) { + reloadInFlight = false + + // The stale-apply guard. Serialization means this should not trigger today, but "only the + // newest result applies" is the rule the wholesale floor and every future overlapping-load + // change are written against, so it is enforced rather than assumed. + if generation == reloadGeneration { + land(outcome, generation: generation) + } + + startPendingReload() + resumeQuiescenceWaitersIfQuiet() + } + + private func land(_ outcome: Result, generation: Int) { + // Consumed here, before the branch, because *both* outcomes end the expectation: a wholesale + // operation gets exactly one reload to prove itself, and a second failure after it is + // ordinary per-file breakage again. + let endsWholesaleOperation: Bool + if let floor = wholesaleReloadFloor, generation >= floor { + wholesaleReloadFloor = nil + endsWholesaleOperation = true + } else { + endsWholesaleOperation = false + } + + switch outcome { + case let .success(result): + snapshot = result.model + loadWarnings = result.warnings + // One success clears both conditions — transient breakage self-heals and a locked board + // unlocks without the user doing anything but fixing the file. + reloadFailure = nil + readOnlyLock = nil + selection = selection.resolved(against: result.model) + + case let .failure(error): + // `snapshot`, `loadWarnings` and `selection` are untouched: a failed reload never + // replaces a good snapshot, and a selection over a snapshot that did not change has + // nothing to re-resolve against. + reloadFailure = error + if endsWholesaleOperation { + readOnlyLock = .bracketedReloadFailed + } + Self.logger.error("reload \(generation, privacy: .public) failed: \(error.description, privacy: .public)") + } + } + + private func startPendingReload() { + guard !reloadInFlight, let origin = pendingReload else { return } + pendingReload = nil + startReload(origin) + } + + // MARK: - Write gate + + /// Runs a synchronous Writer operation inside the watcher bracket, so the churn it produces + /// rounds back as one app-mediated reload rather than a scatter of foreign ones. + /// + /// The store does **not** touch its snapshot here, before or after. `operation` puts bytes on + /// disk; the watcher notices; the reload applies. That indirection is the one-way flow, and it is + /// why this method's only jobs are the gate and the bracket. + /// + /// - Throws: `BoardStoreWriteRefusal.readOnlyLocked` if the board is locked read-only — checked + /// *before* the bracket opens, so a refusal costs no suspended watcher and no owed reload. + /// Otherwise rethrows whatever `operation` threw, which is a `BoardWriteError`. The untyped + /// `throws` is the price of those being two different error types today; see + /// `BoardStoreWriteRefusal` for why they are, and why they will not stay that way. + /// + /// One ergonomic wart, recorded so it is not rediscovered: when `operation` returns a value, + /// Swift cannot infer `T` and the closure's thrown type at the same time — the thrown type + /// widens to `any Error` and the call fails to compile. Such a call site spells the closure out + /// (`{ () throws(BoardWriteError) -> ItemID in … }`). `Void`-returning operations, which are + /// most of them, infer cleanly. + @discardableResult + public func performWrite(_ operation: () throws(BoardWriteError) -> T) throws -> T { + if let readOnlyLock { + throw BoardStoreWriteRefusal.readOnlyLocked(readOnlyLock) + } + watcherBrackets?.begin() + // `defer`, not a trailing call: a Writer operation that fails partway has still touched disk, + // and an unbalanced bracket would leave the watcher suspended for the rest of the session. + defer { watcherBrackets?.end() } + return try operation() + } + + /// Runs an operation that rewrites the tree **wholesale** — pull-rebase, branch switch, undo + /// restore (06-history-undo.md, 07-sync-collab.md) — under the bracket, and arms the rule that + /// its closing reload must succeed. + /// + /// Two things distinguish this from `performWrite`: + /// + /// - The bracket is load-bearing rather than tidy: a reload landing mid-operation would render a + /// half-checked-out tree. + /// - Failure of the closing reload **locks the board** (`ReadOnlyLockReason.bracketedReloadFailed`). + /// After a wholesale change the last-good snapshot may describe a different branch entirely, so + /// writes derived from it would land nonsense — unlike ordinary per-file breakage, where the + /// snapshot still describes the tree. + /// + /// The expectation is armed on **every** exit path, a thrown error included: an operation that + /// died partway is precisely the case where the tree's state is unknown and the next reload had + /// better be the authority on it. + /// + /// - Throws: `BoardStoreWriteRefusal.readOnlyLocked` if the board is already locked — a locked + /// board refuses to *start* wholesale work, not just ordinary writes. Otherwise rethrows + /// `operation`'s error. (Spelled `throws` rather than `rethrows` because of that refusal: a + /// `rethrows` function may only throw errors its closure threw.) + public func performWholesale(_ operation: () throws -> Void) throws { + if let readOnlyLock { + throw BoardStoreWriteRefusal.readOnlyLocked(readOnlyLock) + } + watcherBrackets?.begin() + defer { + // Ordered: arm first, then close the bracket. `endBracket()` is what schedules the + // post-bracket reload, and with a `nil` watcher a consumer may signal by hand the instant + // this returns — either way the floor has to be in place before any walk can start. + wholesaleReloadFloor = reloadGeneration + 1 + watcherBrackets?.end() + } + try operation() + } + + // MARK: - Selection + + /// Replaces the selection. + /// + /// Minimal on purpose — the transient-state container will absorb this along with drag state and + /// the pending cut, and give the selection its real grammar (extend, range, successor-on-delete). + /// Deliberately *not* filtered against the snapshot: a caller selects what it is rendering, and + /// `Selection.resolved(against:)` on the next reload is what keeps the set honest over time. + public func select(_ ids: Set, liveness: Liveness) { + selection = Selection(ids: ids, liveness: liveness) + } + + /// Selects nothing — Escape's last step outward (04-interactions.md ▸ Grammar). + public func clearSelection() { + selection = .empty + } + + // MARK: - Quiescence + + /// Suspends until no reload is running and none is owed. + /// + /// The store is not a request/response object — a signal in does not produce a result out — so + /// this is how a consumer (and every test below) says "let the pipeline settle" without polling. + /// Returns immediately when the store is already quiet. + public func awaitQuiescence() async { + guard !isQuiescent else { return } + await withCheckedContinuation { continuation in + quiescenceWaiters.append(continuation) + } + } + + private var isQuiescent: Bool { !reloadInFlight && pendingReload == nil } + + private func resumeQuiescenceWaitersIfQuiet() { + guard isQuiescent, !quiescenceWaiters.isEmpty else { return } + let waiters = quiescenceWaiters + quiescenceWaiters.removeAll() + for waiter in waiters { + waiter.resume() + } + } +} diff --git a/Kanban/LiveStore/FolderWatcher.swift b/Kanban/LiveStore/FolderWatcher.swift index 053c05c..a41864f 100644 --- a/Kanban/LiveStore/FolderWatcher.swift +++ b/Kanban/LiveStore/FolderWatcher.swift @@ -28,7 +28,7 @@ public enum WatchOrigin: String, Sendable, Equatable { /// reload has to cover" — a reconciling reload assumes nothing about the tree, an /// app-mediated one is the tail of an operation the app ran, a foreign one is an observed /// external edit. - fileprivate var precedence: Int { + var precedence: Int { switch self { case .foreign: 0 case .appMediated: 1 @@ -41,7 +41,12 @@ public enum WatchOrigin: String, Sendable, Equatable { /// This is the *never downgrade* rule — a foreign event landing on a pending app-mediated or /// reconciling delivery does not weaken it. See `FolderWatcher.schedule(_:)` for why the /// resulting blur is accepted rather than engineered away. - fileprivate static func merged(_ existing: WatchOrigin?, _ incoming: WatchOrigin) -> WatchOrigin { + /// + /// Internal rather than `fileprivate`: `BoardStore` coalesces signals that arrive while a reload + /// is already running and owes the same never-downgrade guarantee on its side of the handoff. + /// Two coalescing points, one rule — the watcher's debounce and the store's pending-reload flag + /// must never disagree about which origin a merged span carries. + static func merged(_ existing: WatchOrigin?, _ incoming: WatchOrigin) -> WatchOrigin { guard let existing else { return incoming } return existing.precedence >= incoming.precedence ? existing : incoming } diff --git a/KanbanTests/BoardStoreTests.swift b/KanbanTests/BoardStoreTests.swift new file mode 100644 index 0000000..9ffa7f4 --- /dev/null +++ b/KanbanTests/BoardStoreTests.swift @@ -0,0 +1,487 @@ +import Foundation +import Testing +@testable import Kanban + +/// `BoardStore`'s correctness is almost entirely about *what survives what*: a failed reload must +/// not eat a good snapshot, a selection must not survive a liveness flip, a wholesale operation's +/// expectation must not be consumed by a walk that predates it. So these tests drive the store +/// through its one inbound door — `handleWatcherEvent(_:)` — against real boards in real temp +/// directories, and never through a `FolderWatcher` (which has its own suite, and whose FSEvents +/// timing would turn every assertion here into a race). +/// +/// **No sleeps stand in for ordering.** `awaitQuiescence()` is how a test waits for the pipeline, +/// and the one test that needs a load pinned *open* mid-flight uses `BoardStore.loadBarrier` to pin +/// it rather than guessing at a duration. The single polling helper (`waitUntil`) waits for an +/// actor to report that a load has arrived at that barrier — a fact, not an elapsed time. + +// MARK: - Fixtures + +/// `WriterFixture`, `Ident` and `Item` live in `WriterTestSupport.swift`; a board store needs +/// exactly what the writer suites needed — hand-written `index.md` files in a temp tree — so this +/// file borrows them rather than growing a third copy. + +/// Frontmatter that opens and closes correctly but does not parse: an unclosed flow sequence. The +/// shape a non-atomic external write leaves behind when a reload catches it mid-flight, which is the +/// case 02-architecture.md § Live-reload resilience is written about. +private let brokenIndex = "---\nschema: 1\norder: 1024\nlabels: [a, b\n---\nbody\n" + +/// A card whose `deleted:` key is present — the tombstone an agent or a hand-edit adds to a file the +/// user currently has selected. +private func tombstoned(order: String, title: String) -> String { + """ + --- + schema: 1 + title: \(title) + order: \(order) + deleted: 2026-03-03T09:00:00Z + --- + \(title) body. + + """ +} + +/// The board every test starts from: two lanes, two cards in the first, and one stray folder so +/// `loadWarnings` has something real to carry. +@MainActor +private func makeBoard() throws -> WriterFixture { + let fixture = try WriterFixture() + try fixture.item("", Item.board) + try fixture.item(Ident.lane1, Item.rich(order: "1024", title: "Todo")) + try fixture.item("\(Ident.lane1)/\(Ident.card1)", Item.rich(order: "1024", title: "First")) + try fixture.item("\(Ident.lane1)/\(Ident.card2)", Item.rich(order: "2048", title: "Second")) + try fixture.item(Ident.lane2, Item.rich(order: "2048", title: "Doing")) + try fixture.item("notes", "not a board item at all\n") + return fixture +} + +private func lane(_ id: String, in snapshot: BoardModel) -> Lane? { + snapshot.lanes.first { $0.id.rawValue == id } +} + +private func cardTitles(inLane id: String, of snapshot: BoardModel) -> [String] { + (lane(id, in: snapshot)?.cards ?? []).compactMap(\.title.value) +} + +/// Relative paths as `BoardLoadError` reports them — root-relative, `/`-joined. +private func indexPath(_ components: String...) -> String { + (components + ["index.md"]).joined(separator: "/") +} + +// MARK: - Assertion helpers + +/// The refusal-side twin of `WriterTestSupport.writeFailure`: runs `operation` expecting the store +/// to turn it away, and hands back the refusal for inspection. +@MainActor +@discardableResult +private func refusal(_ operation: () throws -> Void) -> BoardStoreWriteRefusal? { + do { + try operation() + Issue.record("expected the store to refuse the write, but it ran") + return nil + } catch let error as BoardStoreWriteRefusal { + return error + } catch { + Issue.record("expected a BoardStoreWriteRefusal, got \(error)") + return nil + } +} + +/// Counts the bracket calls a store makes, standing in for the watcher the registry will wire up. +@MainActor +private final class BracketLog { + private(set) var begins = 0 + private(set) var ends = 0 + + /// Wired into a store the way the registry will wire its watcher. + func attach(to store: BoardStore) { + store.watcherBrackets = (begin: { self.begins += 1 }, end: { self.ends += 1 }) + } +} + +/// Holds a finished tree walk open until a test says otherwise, and reports when one has arrived. +/// +/// This is what makes the "an in-flight walk never consumes the wholesale expectation" test a proof +/// rather than a coin flip: without it, whether the walk in question had already applied would +/// depend on how fast the disk was. +private actor LoadGate { + private(set) var arrivals = 0 + private var isOpen = false + private var waiters: [CheckedContinuation] = [] + + func arrive() async { + arrivals += 1 + guard !isOpen else { return } + await withCheckedContinuation { waiters.append($0) } + } + + func open() { + isOpen = true + let pending = waiters + waiters.removeAll() + for waiter in pending { + waiter.resume() + } + } +} + +/// Polls until `condition` holds or the deadline passes. Used only to notice that a load has reached +/// the gate — the ordering itself is enforced by the gate, never by the interval. +private func waitUntil(_ deadline: Duration = .seconds(5), _ condition: @Sendable () async -> Bool) async { + let start = ContinuousClock.now + while ContinuousClock.now - start < deadline { + if await condition() { return } + try? await Task.sleep(for: .milliseconds(5)) + } +} + +// MARK: - Tests + +@MainActor +@Suite("BoardStore") +struct BoardStoreTests { + + // MARK: Opening + + @Test("A valid board loads its snapshot and its warnings at init") + func initialLoadSucceeds() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + + let store = try BoardStore(rootURL: fixture.root) + + #expect(store.rootURL == fixture.root) + #expect(store.snapshot.lanes.count == 2) + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["First", "Second"]) + #expect(store.loadWarnings.contains(.nonUUIDFolderIgnored(path: "notes"))) + #expect(store.reloadFailure == nil) + #expect(store.readOnlyLock == nil) + #expect(!store.isReadOnly) + #expect(store.selection == .empty) + } + + @Test("A broken board throws at init rather than constructing a store — fail-fast") + func initialLoadFailsFast() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + try fixture.item(Ident.lane1, brokenIndex) + + // There is nothing to fall back on before the first load, so every later rule — the banner, + // the lock, "a failed reload never replaces a good snapshot" — has no meaning here. + do throws(BoardLoadError) { + _ = try BoardStore(rootURL: fixture.root) + Issue.record("expected the initial load to fail") + } catch { + #expect(error.path == indexPath(Ident.lane1)) + if case .unparseableYAML = error.reason {} else { + Issue.record("expected unparseable YAML, got \(error.reason)") + } + } + } + + // MARK: Reloading + + @Test("A foreign tree change reloads and the snapshot shows the external edit") + func foreignChangeReloads() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + + // Exactly what an agent or an editor does: a card folder appears, with no Writer and no + // bracket anywhere near it. + try fixture.item("\(Ident.lane1)/\(Ident.card3)", Item.rich(order: "3072", title: "Third")) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["First", "Second", "Third"]) + #expect(store.reloadFailure == nil) + } + + @Test("A failed reload keeps the last good snapshot, and the next success heals it") + func failedReloadKeepsTheSnapshot() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let lastGood = store.snapshot + + try fixture.item("\(Ident.lane1)/\(Ident.card1)", brokenIndex) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(store.snapshot == lastGood, "a failed reload never replaces a good snapshot") + #expect(store.reloadFailure?.path == indexPath(Ident.lane1, Ident.card1)) + #expect(store.loadWarnings.contains(.nonUUIDFolderIgnored(path: "notes")), "warnings describe the snapshot on screen, so they stay with it") + + // Transient breakage self-heals: the watcher kept watching and the fix arrives as an + // ordinary reload. + try fixture.item("\(Ident.lane1)/\(Ident.card1)", Item.rich(order: "1024", title: "Fixed")) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(store.reloadFailure == nil) + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["Fixed", "Second"]) + } + + @Test("An ordinary failed reload does not lock the board — editing continues around the breakage") + func ordinaryFailureDoesNotLock() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + + try fixture.item("\(Ident.lane1)/\(Ident.card1)", brokenIndex) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + #expect(store.reloadFailure != nil) + #expect(store.readOnlyLock == nil) + #expect(!store.isReadOnly) + + // Per-file breakage: the snapshot still describes the tree, so a real write through the + // Writer is safe and must go through. + // The explicit closure signature is `performWrite`'s one ergonomic wart: with a non-`Void` + // result, `T` and the closure's thrown type cannot both be inferred, and the thrown type + // widens to `any Error`. See `performWrite`'s doc comment. + let created = try store.performWrite { () throws(BoardWriteError) -> ItemID in + try BoardWriter.createCard(inLane: fixture.url(Ident.lane2), title: "Filed anyway") + } + #expect(fixture.exists("\(Ident.lane2)/\(created.rawValue)/index.md")) + } + + // MARK: Wholesale operations + + @Test("A wholesale operation whose reload succeeds leaves no lock and no leaked expectation") + func wholesaleSuccessConsumesTheExpectation() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let brackets = BracketLog() + brackets.attach(to: store) + + try store.performWholesale { + try fixture.item(Ident.lane3, Item.rich(order: "3072", title: "Done")) + } + #expect(brackets.begins == 1) + #expect(brackets.ends == 1) + + store.handleWatcherEvent(.treeChanged(.appMediated)) + await store.awaitQuiescence() + + #expect(store.readOnlyLock == nil) + #expect(store.reloadFailure == nil) + #expect(store.snapshot.lanes.count == 3) + + // The expectation was consumed by that reload rather than left armed: an ordinary failure + // afterwards is per-file breakage again, and must not lock. + try fixture.item("\(Ident.lane1)/\(Ident.card1)", brokenIndex) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(store.reloadFailure != nil) + #expect(store.readOnlyLock == nil, "the wholesale expectation must not outlive the reload that consumed it") + } + + @Test("A failed reload after a wholesale operation locks the board, refuses writes, and clears on the next success") + func wholesaleFailureLocksAndHeals() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let brackets = BracketLog() + brackets.attach(to: store) + let lastGood = store.snapshot + + // The shape of a branch switch that lands a tree this app cannot read: the snapshot on + // screen now describes something that is not there any more. + try store.performWholesale { + try fixture.item("\(Ident.lane1)/\(Ident.card1)", brokenIndex) + } + store.handleWatcherEvent(.treeChanged(.appMediated)) + await store.awaitQuiescence() + + #expect(store.readOnlyLock == .bracketedReloadFailed) + #expect(store.isReadOnly) + #expect(store.reloadFailure?.path == indexPath(Ident.lane1, Ident.card1)) + #expect(store.snapshot == lastGood) + + // Writes are refused, and refused *before* the bracket opens — a refusal must not leave the + // watcher suspended. + var ran = false + let refused = refusal { try store.performWrite { ran = true } } + #expect(refused == .readOnlyLocked(.bracketedReloadFailed)) + #expect(!ran) + #expect(brackets.begins == 1, "the refusal opened no bracket") + #expect(brackets.ends == 1) + + // A locked board refuses to *start* wholesale work too. + refusal { try store.performWholesale { ran = true } } + #expect(!ran) + + // Reading stays live throughout — keeping the last-good snapshot is the point of the lock. + store.select([ItemID(rawValue: Ident.card2)], liveness: .live) + #expect(store.selection.ids.count == 1) + + try fixture.item("\(Ident.lane1)/\(Ident.card1)", Item.rich(order: "1024", title: "Repaired")) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(store.readOnlyLock == nil, "the next successful reload clears both the banner and the lock") + #expect(store.reloadFailure == nil) + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["Repaired", "Second"]) + + try store.performWrite { ran = true } + #expect(ran) + } + + @Test("The wholesale expectation binds to the walk started after it, never to one already in flight") + func wholesaleExpectationSurvivesAnInFlightLoad() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + + let gate = LoadGate() + store.loadBarrier = { await gate.arrive() } + + // Walk 1 reads the tree while it is still valid, then parks before applying. + store.handleWatcherEvent(.treeChanged(.foreign)) + await waitUntil { await gate.arrivals >= 1 } + + // Broken only now — walk 1 is past its read, walk 2 is not. + try fixture.item("\(Ident.lane1)/\(Ident.card1)", brokenIndex) + + // Armed while walk 1 is demonstrably still in flight. A boolean flag would be consumed by + // walk 1's *successful* application and would leave walk 2's failure unlocked. + try store.performWholesale {} + store.handleWatcherEvent(.treeChanged(.appMediated)) + + await gate.open() + await store.awaitQuiescence() + + #expect(store.reloadGeneration == 2, "two walks: the one in flight, then the one the operation owed") + #expect(store.reloadFailure?.path == indexPath(Ident.lane1, Ident.card1)) + #expect(store.readOnlyLock == .bracketedReloadFailed) + } + + // MARK: Selection across reloads + + @Test("A selected item that vanished from the tree leaves the selection; the survivor stays") + func selectionDropsVanishedMembers() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + store.select([ItemID(rawValue: Ident.card1), ItemID(rawValue: Ident.card2)], liveness: .live) + + try FileManager.default.removeItem(at: fixture.url("\(Ident.lane1)/\(Ident.card1)")) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(store.selection.ids == [ItemID(rawValue: Ident.card2)]) + #expect(store.selection.liveness == .live) + } + + @Test("A liveness flip is a vanish: a selected card tombstoned externally leaves the selection") + func selectionEjectsALivenessFlip() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + store.select([ItemID(rawValue: Ident.card1), ItemID(rawValue: Ident.card2)], liveness: .live) + + try fixture.item("\(Ident.lane1)/\(Ident.card1)", tombstoned(order: "1024", title: "First")) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + // Still in the snapshot — the trash renders it — but no longer on the selection's side of + // the boundary, so the homogeneous-by-liveness invariant survives a foreign edit. + let flipped = lane(Ident.lane1, in: store.snapshot)?.cards.first { $0.id.rawValue == Ident.card1 } + #expect(flipped?.isDeleted == true) + #expect(store.selection.ids == [ItemID(rawValue: Ident.card2)]) + } + + @Test("A selection can resolve to nothing, and nothing is invented to replace it") + func selectionCanResolveToNothing() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + store.select([ItemID(rawValue: Ident.card1), ItemID(rawValue: Ident.card2)], liveness: .live) + + try FileManager.default.removeItem(at: fixture.url("\(Ident.lane1)/\(Ident.card1)")) + try FileManager.default.removeItem(at: fixture.url("\(Ident.lane1)/\(Ident.card2)")) + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + + #expect(store.selection.ids.isEmpty) + #expect(store.selection.liveness == .live, "the side survives even when the membership does not") + + store.select([ItemID(rawValue: Ident.lane1)], liveness: .live) + store.clearSelection() + #expect(store.selection == .empty) + } + + // MARK: Coalescing + + @Test("A burst of signals during one walk coalesces into exactly one follow-up") + func signalBurstCoalescesIntoOneFollowUp() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + + try fixture.item("\(Ident.lane1)/\(Ident.card3)", Item.rich(order: "3072", title: "Third")) + // No `await` between them, so the main actor never yields and no result can land in the + // middle: the first signal starts a walk, the other four fold into one pending reload. + for _ in 0..<5 { + store.handleWatcherEvent(.treeChanged(.foreign)) + } + await store.awaitQuiescence() + + #expect(store.reloadGeneration == 2, "five signals, one walk plus one follow-up") + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["First", "Second", "Third"]) + #expect(store.reloadFailure == nil) + } + + // MARK: Brackets + + @Test("performWrite brackets exactly once, whether the operation returns or throws") + func performWriteBracketsExactlyOnce() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let brackets = BracketLog() + brackets.attach(to: store) + + var ran = false + try store.performWrite { ran = true } + #expect(ran) + #expect(brackets.begins == 1) + #expect(brackets.ends == 1) + + // A Writer operation that fails partway has still touched disk, so the bracket has to close + // on the throwing path too — an unbalanced one would suspend the watcher for the session. + let boom = BoardWriteError(operation: "probe", path: "/nowhere", reason: .io(message: "disk full")) + do { + try store.performWrite { () throws(BoardWriteError) -> Void in throw boom } + Issue.record("expected the operation's own error to propagate") + } catch let error as BoardWriteError { + #expect(error == boom, "the store rethrows the Writer's error untouched") + } catch { + Issue.record("expected a BoardWriteError, got \(error)") + } + #expect(brackets.begins == 2) + #expect(brackets.ends == 2) + } + + // MARK: Root changes + + @Test("A root change is accepted and leaves the last good snapshot alone") + func rootChangeIsAStubToday() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + let lastGood = store.snapshot + + // Documented no-op until the registry's bookmark arrives: re-resolve-or-lock needs an + // identity this store does not own yet, and guessing would lock a board that merely moved. + store.handleWatcherEvent(.rootChanged) + await store.awaitQuiescence() + + #expect(store.snapshot == lastGood) + #expect(store.reloadFailure == nil) + #expect(store.readOnlyLock == nil) + #expect(store.reloadGeneration == 0, "a root change schedules no reload today") + } +}