diff --git a/Kanban/App/BoardWindowHost.swift b/Kanban/App/BoardWindowHost.swift index 15d797c..338910f 100644 --- a/Kanban/App/BoardWindowHost.swift +++ b/Kanban/App/BoardWindowHost.swift @@ -15,9 +15,23 @@ import os /// it — is `BoardView`'s (03-board-ui.md); this file hands it the store and the window and stays out /// of the way. /// +/// ### Every open passes through a loading state +/// +/// The window appears **immediately** — welcome click, File ▸ Open…, Finder double-click, +/// restoration alike — at its saved frame, its title carrying the registry record's cached name, +/// and its content area holding `BoardLoadingView` until the first snapshot lands +/// (02-architecture.md § Launch and window lifecycle, ruled 2026-07-29). The walk that produces +/// that snapshot runs **off the main actor** (`BoardStoreRegistry.acquireOffMain`), so a board of +/// any size opens as a live window rather than as a beachball, and every restored window walks +/// independently of every other. +/// +/// The window is therefore real, and closeable, before it has a store: ⌘W during the walk cancels +/// it and closes the window. That is why `configureWindow` is in two halves — see +/// `configureLoadingWindow(recordID:)`. +/// /// ### Failure opens welcome, on a row that already exists /// -/// A board that will not load has nothing to show, so its window never appears. But its registry +/// A board that will not load has nothing to show, so its window retires. But its registry /// record is created **before** the load runs (02-architecture.md § Per-board app state, "a first /// open that fails fail-fast still records"), so the failure that joins `AppModel.launchFailures` /// always has a recents row waiting for it — `WelcomeRow.derive` matches the two by path, uniform @@ -59,6 +73,19 @@ struct BoardWindowHost: View { /// window's field. @State private var boardSearch = BoardSearchPresentation() + /// The pre-snapshot surface's grace clock (02 § Launch and window lifecycle). `@State` for + /// `boardInfo`'s reason — one per window, living exactly as long as the window. + @State private var loading = BoardLoadingIndicator() + + /// The open walk, so ⌘W can cancel it by name rather than by waiting for SwiftUI's teardown to + /// get around to it. + @State private var openWalk = BoardOpenWalk() + + /// This board's registry record, from the moment `recordOpen` mints it — which is what the + /// loading window's title reads (`Self.loadingTitle`). `nil` only for the one body evaluation + /// that precedes `start()`. + @State private var recordID: UUID? + @State private var phase: Phase = .opening private enum Phase { @@ -82,16 +109,21 @@ struct BoardWindowHost: View { ) .background(WindowAccessor(controller: windowController)) .navigationTitle(windowTitle) - .task { await start() } + .task { await beginOpening() } .onDisappear { endSessionIfStillOpen() } } @ViewBuilder private var content: some View { switch phase { - case .opening, .failed: - // Nothing to render and nothing worth animating: this window either becomes a board in a - // moment or dismisses itself. + case .opening: + // **The pre-snapshot loading state** (02 § Launch and window lifecycle): empty for the + // grace, a centered spinner after it, never a skeleton. The board replaces it in place + // when `phase` becomes `.open` — a snap, which is what assigning outside `withAnimation` + // means here. + BoardLoadingView(indicator: loading) + case .failed: + // Nothing to render and nothing worth animating: this window is dismissing itself. Color.clear case let .open(store): VStack(spacing: 0) { @@ -154,22 +186,63 @@ struct BoardWindowHost: View { } private var windowTitle: String { - guard case let .open(store) = phase else { return "" } + guard case let .open(store) = phase else { + return Self.loadingTitle( + record: recordID.flatMap { appModel.boardRegistry.record(id: $0) }, + url: ref.url + ) + } return AppModel.displayName(of: store) } + /// What a window that has no snapshot yet is called — **the registry record's cached name**, the + /// same no-scan source the welcome row reads (02 § Launch and window lifecycle: "its chrome + /// carrying the registry record's cached title and icon"; § Per-board app state, "the welcome row + /// reads only the record — it never opens any board's `index.md`"). + /// + /// Read back off the record rather than recomputed, so a board that has opened before shows the + /// title it is known by and a first-ever open shows the folder name — which is that record's + /// provisional display name, not a second rule. The `nil` fallback is the folder name anyway, + /// covering only the body evaluation that precedes `recordOpen`. + /// + /// Static and pure so the rule is checkable without a window (`BoardWindowHostTests`). + static func loadingTitle(record: BoardRecord?, url: URL) -> String { + record?.displayName ?? AppModel.folderDisplayName(of: url) + } + // MARK: - Opening + /// Starts the open as a task of its own, so something can hold it. + /// + /// `.task` cancels on teardown but hands out no handle, and ⌘W during loading needs one *by + /// name* — the ruled cancel is an act of the user's, not a consequence of a window that has + /// already gone away (02 § Launch and window lifecycle). So the walk runs in a task `openWalk` + /// keeps, and the cancellation handler forwards `.task`'s own cancellation into it, leaving both + /// routes — the user's ⌘W and any teardown SwiftUI decides on — ending in the same `cancel()`. + private func beginOpening() async { + let walk = Task { await start() } + openWalk.adopt(walk) + await withTaskCancellationHandler { + await walk.value + } onCancel: { + walk.cancel() + } + } + /// Acquires the board and starts its session, or fails it out to welcome. /// /// The order is load-bearing, and it now has one more step than the load itself does. Security- /// scoped access is claimed **before** anything else, because the record and the load both need - /// it. The registry record comes **before** `acquire` — "the registry record is created before - /// loading" (02 § Per-board app state) — so a fail-fast failure always has a row to land on; - /// `acquire`'s own first act is the tree walk, and a sandboxed read outside the claimed scope is - /// exactly the one that gets refused. `setOpenNow` comes **after** the load succeeds and after - /// the window has demonstrably opened — a flag set on a board that never appeared would hand the - /// next launch a restoration set describing a failure. + /// it. The registry record comes **before** the walk — "the registry record is created before + /// loading" (02 § Per-board app state) — so a fail-fast failure always has a row to land on, and + /// so the loading window has a name to wear; the walk's first act is a directory read, and a + /// sandboxed read outside the claimed scope is exactly the one that gets refused. `setOpenNow` + /// comes **after** the load succeeds and after the window has demonstrably opened — a flag set on + /// a board that never appeared would hand the next launch a restoration set describing a failure. + /// + /// **The walk is the one suspension here**, and everything before it is what makes the window + /// real while it runs: the record, the loading window's chrome, and the grace clock. Everything + /// after it is the snap. private func start() async { guard case .opening = phase else { return } @@ -182,12 +255,31 @@ struct BoardWindowHost: View { // name, both `recordOpen`'s own rule now. This is also this open's one bookmark mint: a // successful load below replaces the name through `syncDisplayState`, which never re-mints. let recordID = appModel.boardRegistry.recordOpen(of: url) + // Published to the view *before* the walk: this is what the title bar reads while loading. + self.recordID = recordID + + // The window is on screen and the user can act on it from here on — placed where they left + // it, and closeable. + configureLoadingWindow(recordID: recordID) + loading.begin() let store: BoardStore do throws(BoardLoadFailure) { - store = try appModel.storeRegistry.acquire(url) + guard let acquired = try await appModel.storeRegistry.acquireOffMain(url) else { + // ⌘W landed while the walk was running, and the walk has now finished into a result + // nobody wants (`acquireOffMain`, discard-on-completion). The window is already + // closing and the registry kept nothing, so the only thing left to balance is this + // open's scoped access. There is no open-now flag to clear: `setOpenNow` is below, + // after the load, so a cancelled open never set one — the very reason it lives there. + Self.logger.debug("board open cancelled during its walk") + loading.end() + access?.stop() + return + } + store = acquired } catch { Self.logger.error("board failed to open: \(error.description, privacy: .public)") + loading.end() access?.stop() phase = .failed appModel.recordLaunchFailure(path: ref.path, message: error.description) @@ -200,6 +292,8 @@ struct BoardWindowHost: View { return } + loading.end() + // The load succeeded — the frontmatter can be trusted now, so it replaces whatever // provisional or stale name the record above was carrying. Through `syncDisplayState`, // deliberately not a second `recordOpen`: this is a display-state refresh, not a second @@ -222,21 +316,36 @@ struct BoardWindowHost: View { dismissWindow(id: WindowID.welcome) } - /// Wires the window: the saved frame on the way in, frame changes on the way back out, the - /// close interception that makes the flush unavoidable, and the title-bar widget. - private func configureWindow(store: BoardStore, recordID: UUID) { - // Filled in here rather than at declaration because the closure captures `openWindow`, an - // environment action; until the board has loaded there is also nothing for Open Card to act - // on, which is exactly what the item's `nil` check reads. - cardOpener.open = openCard - + /// **The half of the wiring a window needs before it has a board** — everything here is about + /// the *window*, and nothing here mentions the store, which is exactly the split + /// 02-architecture.md's loading state forces: this runs before the walk, and + /// `configureWindow(store:recordID:)` runs after it. + /// + /// Three things, and each is a rule from § Launch and window lifecycle: + /// + /// - **The saved frame**, so the window appears "at its saved frame" rather than at the system's + /// cascade and then jumping to the user's place a second later. + /// - **The frame changes**, so a window the user moves *while it loads* is remembered. Not + /// store-dependent and so not worth deferring — the alternative is a slow board's window whose + /// move is silently discarded. + /// - **The close interception**, which is what makes ⌘W during loading mean anything at all. It + /// is replaced wholesale by the flushing version once the board is open (see below); a single + /// closure branching on `phase` would be the same thing spelled as a state read. + /// + /// The title bar keeps AppKit's own title display for now — the string is the record's cached + /// name (`windowTitle`) — and `hideTitle()` follows only once the board-popover widget is there + /// to say the name instead. Hiding it here would leave a loading window with no name anywhere in + /// its chrome, which is precisely what 02 asks the loading state to carry. + private func configureLoadingWindow(recordID: UUID) { windowController.onAttach = { window in guard let saved = appModel.boardRegistry.record(id: recordID)?.windowFrame else { return } window.setFrame(HostedWindowController.placementOnCurrentScreens(for: saved), display: true) } - // The window may already be attached — `viewDidMoveToWindow` fires well before this task's - // load returns — so the placement is applied directly too rather than waiting for a callback - // that has already happened. + // The window may already be attached — `viewDidMoveToWindow` fires before this task's first + // suspension — so the placement is applied directly too rather than waiting for a callback + // that has already happened. The closure stays installed either way: the controller re-fires + // it if SwiftUI swaps the provisional window for the real one (`HostedWindowController + // .detach`). if let window = windowController.window { windowController.onAttach?(window) } @@ -248,12 +357,39 @@ struct BoardWindowHost: View { ) } + windowController.onCloseRequested = { + // **⌘W during the walk** (02 § Launch and window lifecycle: "the walk is cancellable: + // ⌘W during loading cancels it and closes the window"). The window closes *now* — there + // is no store, so there is nothing to flush and nothing to wait for — and the walk's + // tail is wasted work we accept rather than thread a cancellation flag through the + // loader (`BoardStoreRegistry.acquireOffMain`, discard-on-completion). + // + // No open-now flag is cleared here because none was ever set: `setOpenNow` runs after + // the load, so a board that never finished loading is not in the restoration set. The + // ordinary user-close *does* clear it, in `AppModel.closeBoard`, which is the path the + // replacement closure below takes. + openWalk.cancel() + windowController.closeAfterFlush() + } + } + + /// Wires the rest of the window, once there is a board to wire it to: the store's write-through, + /// the close flush, the undo stack, the title-bar widget and the toolbar. + /// + /// Everything here **carries the store or the session**, which is the whole reason it waits for + /// them; the window-level half ran before the walk (`configureLoadingWindow(recordID:)`). + private func configureWindow(store: BoardStore, recordID: UUID) { + // Filled in here rather than at declaration because the closure captures `openWindow`, an + // environment action; until the board has loaded there is also nothing for Open Card to act + // on, which is exactly what the item's `nil` check reads. + cardOpener.open = openCard + // The registry's live write-through (02-architecture.md § Per-board app state) — wired - // the same way `onFrameChanged` just was: a closure that reaches into the registry, - // captured weakly on both sides so neither the store nor this closure's own home keeps - // the other alive past its window. `syncDisplayState` in `start()` already stamped the - // values current as of this open, so nothing is fired here immediately; this only fires - // on the reloads that follow. + // the way `onFrameChanged` was a moment ago in the loading half: a closure that reaches into + // the registry, captured weakly on both sides so neither the store nor this closure's own + // home keeps the other alive past its window. `syncDisplayState` in `start()` already + // stamped the values current as of this open, so nothing is fired here immediately; this + // only fires on the reloads that follow. store.displayStateDelegate = { [weak appModel, weak store] in guard let appModel, let store else { return } appModel.boardRegistry.syncDisplayState( @@ -264,6 +400,10 @@ struct BoardWindowHost: View { ) } + // **Replacing the loading half's cancel-and-close**: from here the window has a session, so + // a close is the flush (02 § Windows, "Close flushes") and the user-close that clears the + // open-now flag. A slot rather than a branch — `onCloseRequested` is one closure, and the + // window that owns it has moved on. windowController.onCloseRequested = { Task { @MainActor in await appModel.closeBoard(ref: ref, cause: .userClose) @@ -306,6 +446,11 @@ struct BoardWindowHost: View { // the same reason. `.navigationTitle(windowTitle)` a few lines up in `body` is untouched — // `window.title` keeps feeding the Window menu, Exposé, VoiceOver and restoration; only the // title bar's own rendering of that string is suppressed. + // + // **After the load, and only after it**, which is why it is not in the loading half above: + // this line and the widget it defers to are one exchange, and a loading window that hid its + // title before the widget existed would carry no name at all — against 02's "its chrome + // carrying the registry record's cached title". windowController.hideTitle() // The board's customizable toolbar (03-board-ui.md ▸ Toolbar) — installed here for the @@ -334,3 +479,33 @@ struct BoardWindowHost: View { } } } + +// MARK: - BoardOpenWalk + +/// The open walk's handle, held for exactly one reason: **⌘W during loading has to be able to cancel +/// it** (02-architecture.md § Launch and window lifecycle). +/// +/// A one-field box rather than the `Task` itself in `@State`, because the thing that cancels it is a +/// closure the window controller holds (`onCloseRequested`) and the thing that fills it is the +/// `.task` that starts the walk — two places that must agree on one task, which is what a reference +/// type is. `@MainActor` like everything else on this path, so the box needs no synchronisation of +/// its own. +/// +/// Cancelling does not stop the walk (see `BoardStoreRegistry.acquireOffMain` for why the walk is +/// deliberately not cooperatively cancellable). It stops the *open*: the task that would have +/// adopted the result never does. +@MainActor +final class BoardOpenWalk { + + private var task: Task? + + init() {} + + func adopt(_ task: Task) { + self.task = task + } + + func cancel() { + task?.cancel() + } +} diff --git a/Kanban/LiveStore/BoardStore.swift b/Kanban/LiveStore/BoardStore.swift index 1c8b9dd..3f54ffa 100644 --- a/Kanban/LiveStore/BoardStore.swift +++ b/Kanban/LiveStore/BoardStore.swift @@ -589,8 +589,12 @@ public final class BoardStore: HealHost { /// 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. + /// The walk runs here, on whatever actor the caller is on — the storeless shape, and the one + /// every test and every synchronous consumer uses. The **window's** open no longer takes it: + /// 02-architecture.md's pre-snapshot loading state (ruled 2026-07-29) gave the caller something + /// to render before the snapshot exists, which retired this init's old rationale ("the caller + /// has nothing to render until it lands"), so `BoardStoreRegistry.acquireOffMain` walks off the + /// main actor and hands the result to `init(rootURL:loaded:)` below. /// /// **It writes nothing, the opened board's defects included.** `defects` is recorded here and /// acted on by whoever wired this store up — `BoardStoreRegistry.acquire` calls @@ -598,8 +602,23 @@ public final class BoardStore: HealHost { /// with a reload behind it rather than a write into a board nothing is watching yet. A store /// built directly (a test, a storeless consumer) heals when it is asked to, and on every reload /// thereafter. - public init(rootURL: URL) throws(BoardLoadFailure) { + public convenience init(rootURL: URL) throws(BoardLoadFailure) { let result = try BoardLoader.load(boardRoot: rootURL) + self.init(rootURL: rootURL, loaded: result) + } + + /// The same board, from a walk that already happened somewhere else. + /// + /// **The designated init, and the only one that is not a load**: `LoadResult` is `Sendable` and + /// `BoardLoader` is stateless statics, so the walk can run anywhere and only this assignment has + /// to be on the main actor — exactly the split `startReload` has used for every reload since the + /// beginning, now available to the *first* load too. It cannot fail, because failing is the + /// walk's job and the walk is over: a caller holding a `LoadResult` holds a board that loaded. + /// + /// `rootURL` is passed rather than read off `result.model` for the reason the property's own doc + /// comment gives — the store's root follows an absorbed rename ahead of the snapshot that will + /// carry it. + public init(rootURL: URL, loaded result: LoadResult) { self.rootURL = rootURL self.snapshot = result.model self.loadWarnings = result.warnings diff --git a/Kanban/LiveStore/BoardStoreRegistry.swift b/Kanban/LiveStore/BoardStoreRegistry.swift index 568ff6a..3eeec00 100644 --- a/Kanban/LiveStore/BoardStoreRegistry.swift +++ b/Kanban/LiveStore/BoardStoreRegistry.swift @@ -106,6 +106,25 @@ public final class BoardStoreRegistry { /// it already has. A `[URL: Entry]` here would be a latent bug with a plausible shape. private var entries: [FileIdentity: Entry] = [:] + /// **The open walk's single flight** — one board, one tree walk, however many windows ask for it + /// at once (02-architecture.md § Launch and window lifecycle: "Restoration is parallel … each + /// walk independent"). + /// + /// Keyed by the same `FileIdentity` the entries are, which is the whole reason it lives here + /// rather than in a caller: the registry already knows that two differently-spelled URLs are one + /// board. Restoration racing a Finder open of the same root, or a board window and its restored + /// card window arriving together, therefore costs **one** walk and produces **one** store — the + /// prior art is the store's own reload coalescing (`requestReload`), applied to the load that + /// happens before a store exists to coalesce on. + /// + /// **Per board, never global**: a slow network board's walk holds nothing but its own key, so + /// every other board opening at the same moment proceeds untouched. + /// + /// The task's failure type is `Never` and its value is a `Result` — `Task`'s typed-throws + /// initializers do not exist, and widening `BoardLoadFailure` to `any Error` on the way through + /// the join would lose exactly the aggregate the decision surface is made of. + private var walksInFlight: [FileIdentity: Task, Never>] = [:] + private static let logger = Logger(subsystem: "dev.rzen.indie.Kanban", category: "store-registry") /// The app owns one instance and passes it down; there is no shared singleton to reach for. @@ -117,7 +136,15 @@ public final class BoardStoreRegistry { // MARK: - Acquire / release - /// The store for `rootURL`, opening the board if this is the first window to ask for it. + /// The store for `rootURL`, opening the board if this is the first window to ask for it — + /// **walking on the caller's own actor**. + /// + /// The synchronous acquire, and still the right one for every caller that cannot render anything + /// before the board exists: a card window joining a board that is already open (the walk never + /// runs there), the storeless surfaces, and the tests. A **board window** takes + /// `acquireOffMain(_:)` instead — 02's pre-snapshot loading state gave that caller something to + /// show while the walk runs, which is the only reason a walk on the main actor was ever + /// tolerable. /// /// **First acquire**: loads the board (fail-fast — the `BoardLoadFailure` is rethrown untouched, /// every collected defect included, because the decision surface is the caller's to host — @@ -138,20 +165,120 @@ public final class BoardStoreRegistry { /// A failed load leaves **nothing behind**: no entry, no watcher, no count. A board that failed /// to open is not open. public func acquire(_ rootURL: URL) throws(BoardLoadFailure) -> BoardStore { - if let identity = FileIdentity(of: rootURL), var entry = entries[identity] { - entry.referenceCount += 1 - entries[identity] = entry - Self.logger.debug("acquire: existing board, \(entry.referenceCount, privacy: .public) references") - return entry.store + if let store = referenceExistingBoard(at: rootURL) { return store } + return try adopt(BoardStore(rootURL: rootURL), rootURL: rootURL) + } + + /// The **board window's** acquire: the tree walk runs off the main actor, and concurrent asks + /// for one board share it (02-architecture.md § Launch and window lifecycle, ruled 2026-07-29). + /// + /// ### What it does, in the order it does it + /// + /// 1. **An already-open board returns immediately**, refcount bumped, no walk — `acquire`'s own + /// first act, unchanged and still synchronous: this is the card-window and second-window path + /// and it must not cost a suspension, let alone a directory walk. + /// 2. **Otherwise one walk**, on a detached task at `.userInitiated`, joined by every other + /// async acquire of the same board that arrives while it runs (`walksInFlight`). + /// 3. **The result lands back here on the main actor** and is turned into the store — through + /// the same `adopt` the synchronous path uses, so the probe, the watcher, the brackets, the + /// bookmark, the entry and the heals are the same wiring by construction rather than by two + /// lists that must be kept in step. + /// + /// ### Cancellation is discard-on-completion + /// + /// ⌘W during the pre-snapshot loading state cancels the *caller's* task ("the walk is + /// cancellable: ⌘W during loading cancels it and closes the window"). The walk itself is one + /// synchronous `BoardLoader.load` call and is deliberately **not** made cooperatively + /// cancellable: the loader would have to check a flag between every directory and every parse, + /// and the reward would be a fraction of a second of a walk nobody is waiting for. So the window + /// closes at once and the walk's tail is wasted work — accepted, and the whole of the cost. + /// + /// What cancellation *does* guarantee is that the discarded walk leaves nothing behind: this + /// returns `nil` before constructing anything, so there is no store, no watcher, no entry and no + /// reference — and the caller's security-scoped access is its own to release. + /// + /// - Returns: the board's store, or `nil` if this acquire was cancelled before its walk landed. + /// `nil` is not a failure: nothing went wrong and nothing was opened. + public func acquireOffMain(_ rootURL: URL) async throws(BoardLoadFailure) -> BoardStore? { + if let store = referenceExistingBoard(at: rootURL) { return store } + + // `nil` for a root that does not exist or whose volume will not answer — there is nothing to + // coalesce on, so such an open walks alone and the loader produces the honest error for it. + let identity = FileIdentity(of: rootURL) + + let walk: Task, Never> + if let identity, let joined = walksInFlight[identity] { + Self.logger.debug("acquire: joining the walk already running for this board") + walk = joined + } else { + walk = Self.walk(rootURL) + if let identity { walksInFlight[identity] = walk } } - let store = try BoardStore(rootURL: rootURL) + let outcome = await walk.value + // Cleared by whichever joiner resumes first; the equality check is what stops a *later* + // walk of the same board from being cleared by a straggler from the previous one. + if let identity, walksInFlight[identity] == walk { walksInFlight[identity] = nil } + + // Discard-on-completion. Before the store is constructed, so a cancelled open leaves the + // registry exactly as it found it. + if Task.isCancelled { + Self.logger.debug("acquire: cancelled before its walk landed — nothing opened") + return nil + } + + // A joiner that resumed first has already built the board; this is the same identity hit as + // step 1, asked again because the world moved while this call was suspended. There is no + // suspension between here and the entry's insertion in `adopt`, so two joiners cannot both + // pass it. + if let store = referenceExistingBoard(at: rootURL) { return store } + + switch outcome { + case let .failure(failure): + throw failure + case let .success(result): + return try adopt(BoardStore(rootURL: rootURL, loaded: result), rootURL: rootURL) + } + } + + /// One tree walk on a task of its own. `Task.detached` rather than `Task { }` for + /// `BoardStore.startReload`'s reason exactly: a task created inside a `@MainActor` method + /// inherits that isolation and would run the walk on the main actor, which is the whole thing + /// this is avoiding. + private static func walk(_ rootURL: URL) -> Task, Never> { + Task.detached(priority: .userInitiated) { + do throws(BoardLoadFailure) { + return .success(try BoardLoader.load(boardRoot: rootURL)) + } catch { + return .failure(error) + } + } + } + + /// The already-open answer, shared by both acquires: the store this board already has, with this + /// caller's reference added. `nil` means "not open here", which is the only case that walks. + private func referenceExistingBoard(at rootURL: URL) -> BoardStore? { + guard let identity = FileIdentity(of: rootURL), var entry = entries[identity] else { return nil } + entry.referenceCount += 1 + entries[identity] = entry + Self.logger.debug("acquire: existing board, \(entry.referenceCount, privacy: .public) references") + return entry.store + } + + /// Everything a freshly loaded board needs before it is anyone's to show — the probe, the + /// watcher, the wiring in both directions, the bookmark, the entry, the heals. + /// + /// **One body for both acquires.** How the store was built (a walk on this actor, or a walk that + /// landed from a detached task) is the only difference between the two paths; everything from + /// here down is identical, and identical by sharing rather than by resemblance. + private func adopt(_ store: BoardStore, rootURL: URL) throws(BoardLoadFailure) -> BoardStore { // **The open-time writability probe** (02-architecture.md § Write-failure surfacing, "An // unwritable board location enters the read-only lock at open"), and this is the seam for // it: every way a board opens — welcome's recents, a Finder open, File ▸ Open, restoration, - // a card window arriving first — funnels through `acquire`, so the probe is wired once here - // instead of at each caller, and no future open path can forget it. + // a card window arriving first — funnels through one of the two acquires and so through + // this method, so the probe is wired once here instead of at each caller, and no future open + // path can forget it. // // **First, immediately after the load.** The lock has to be standing before anything else // in this method can act on the board, and two things below would otherwise write into a diff --git a/Kanban/UI/AccessibilityPhrases.swift b/Kanban/UI/AccessibilityPhrases.swift index 3c3d52f..cddc854 100644 --- a/Kanban/UI/AccessibilityPhrases.swift +++ b/Kanban/UI/AccessibilityPhrases.swift @@ -36,6 +36,13 @@ enum AccessibilityPhrases { return title } + /// What the pre-snapshot loading surface says (02-architecture.md § Launch and window lifecycle; + /// `BoardLoadingView`). The surface draws a bare system spinner and no text — "no skeleton lanes, + /// the motion language animates real data only" — so this is the *only* description a VoiceOver + /// user gets of a board window that is still walking its tree, and a spinner with nothing to say + /// would leave that window silent. + static let boardLoading = "Loading board" + /// "3 cards", "1 card" — the app's **one** plural folding for a card count, borrowed from /// `TrashModel.phrase` rather than restated so a lane's spoken count and the trash column's /// cannot drift apart. diff --git a/Kanban/UI/Board/BoardLoadingView.swift b/Kanban/UI/Board/BoardLoadingView.swift new file mode 100644 index 0000000..6b3f48a --- /dev/null +++ b/Kanban/UI/Board/BoardLoadingView.swift @@ -0,0 +1,105 @@ +import SwiftUI + +// MARK: - BoardLoadingIndicator + +/// **The grace before the spinner** — the whole of the pre-snapshot loading state's timing +/// (02-architecture.md § Launch and window lifecycle, ruled 2026-07-29): +/// +/// > The content area holds a quiet loading surface: a centered system spinner appearing only after +/// > a short grace (~200 ms) so ordinary fast opens never flash it — no skeleton lanes, the motion +/// > language animates real data only. +/// +/// ### Why the grace is an object rather than a `task` modifier +/// +/// Two reasons, and they are the same two `DragSession.holdTimeout` has. It is a **state machine** +/// worth pinning — nothing showing before the grace, the spinner after, nothing showing again once +/// the snapshot lands — and a rule about *when* something appears is only checkable if there is +/// something to ask. And the figure has to be injectable: a suite that had to wait the real 200 ms +/// out, once per case, would be paying wall-clock time to assert a constant. +/// +/// The body the sleep runs is `graceElapsed()`, spelled as a method rather than inlined in the task, +/// so the "after" half can be pinned directly as well as through the clock — again the drag +/// session's shape (`DragSession.expire`). +/// +/// ### It is not a progress report +/// +/// The walk has no progress to report — `BoardLoader.load` is one call that either lands or throws +/// — so this is indeterminate by construction and carries no percentage, no phase, and no cancel +/// affordance of its own. ⌘W is the cancel (`BoardWindowHost`), which is 02's own answer. +@MainActor +@Observable +final class BoardLoadingIndicator { + + /// ~200 ms, 02's figure. Named rather than inlined so the one place that states it is the one + /// place a future ruling would change. + static let standardGrace: Duration = .milliseconds(200) + + /// Whether the spinner is on screen. False until the grace elapses, and false again the moment + /// the surface ends — a board that loaded inside the grace never sets it at all, which is the + /// "ordinary fast opens never flash it" clause. + private(set) var showsSpinner = false + + /// How long the surface stays empty before the spinner appears. A `var` for `holdTimeout`'s + /// reason exactly: the app never writes it, and a test that had to wait the real figure out + /// would be a wall clock standing in for a rule. Not observed — changing it mid-grace is not a + /// thing that happens. + @ObservationIgnored var grace: Duration = BoardLoadingIndicator.standardGrace + + @ObservationIgnored private var graceTask: Task? + + init() {} + + /// Arms the grace. Idempotent: a body that evaluates twice, or a host that configures itself + /// more than once, must not restart the clock a board has already been waiting on. + func begin() { + guard graceTask == nil else { return } + let grace = self.grace + graceTask = Task { @MainActor [weak self] in + try? await Task.sleep(for: grace) + guard !Task.isCancelled, let self else { return } + graceElapsed() + } + } + + /// The grace's own body: the open is taking long enough to be worth explaining. + func graceElapsed() { + showsSpinner = true + } + + /// The surface is over — the snapshot landed, the walk failed, or ⌘W cancelled it. Disarms the + /// grace and clears the spinner, so a host that reuses the indicator starts from rest. + func end() { + graceTask?.cancel() + graceTask = nil + showsSpinner = false + } +} + +// MARK: - BoardLoadingView + +/// The board window's content area **before its first snapshot** — quiet by design. +/// +/// Nothing but a centered system `ProgressView`, and only once the grace has elapsed. No skeleton +/// lanes (02 rules them out explicitly), no board background — the board's own background colour is +/// a fact of the snapshot that has not arrived, so painting a guess here would be a colour that +/// changed at the snap. +/// +/// **The snap is the absence of an animation.** `BoardWindowHost` replaces this view with the board +/// by assigning its phase outside any `withAnimation`, per 02's "the first snapshot replaces the +/// surface in place (a snap — there is no prior arrangement to animate from)" and the Motion +/// vocabulary's reload seam. There is deliberately no transition on this view. +struct BoardLoadingView: View { + + let indicator: BoardLoadingIndicator + + var body: some View { + ZStack { + if indicator.showsSpinner { + ProgressView() + .controlSize(.large) + .accessibilityLabel(AccessibilityPhrases.boardLoading) + } + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + } +} diff --git a/KanbanTests/BoardLoadingTests.swift b/KanbanTests/BoardLoadingTests.swift new file mode 100644 index 0000000..962e3d9 --- /dev/null +++ b/KanbanTests/BoardLoadingTests.swift @@ -0,0 +1,177 @@ +import Foundation +import Testing +@testable import Kanban + +/// **The pre-snapshot loading state** (02-architecture.md § Launch and window lifecycle, ruled +/// 2026-07-29): the board window appears immediately, wearing the registry record's cached name, +/// and its content area stays empty until a short grace has passed — "so ordinary fast opens never +/// flash it". +/// +/// Both halves are rules about *state*, not about rendering, and both are extracted so they can be +/// asked without a window: `BoardLoadingIndicator` is the grace's state machine and +/// `BoardWindowHost.loadingTitle` is the title rule. What SwiftUI does with either — a `ProgressView` +/// in a `ZStack`, a `navigationTitle` — is one line each and is not what could go quietly wrong. +/// +/// **No test here waits the real grace out.** The figure is injectable for exactly that reason +/// (`DragSession.holdTimeout`'s precedent), and the "after" half is also pinned directly through the +/// body the clock runs, so the rule is checkable with no clock at all. + +// MARK: - Helpers + +/// Polls until `condition` holds or the deadline passes — the file's only wait, and it waits for a +/// *fact* (the spinner arrived) rather than for an interval. +@MainActor +private func waitUntil(_ deadline: Duration = .seconds(5), _ condition: () -> Bool) async { + let start = ContinuousClock.now + while ContinuousClock.now - start < deadline { + if condition() { return } + try? await Task.sleep(for: .milliseconds(5)) + } +} + +@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")) + return fixture +} + +/// A registry file in temp — app-side state, never inside a board folder. +@MainActor +private func makeRegistry() throws -> (registry: BoardRegistry, tearDown: () -> Void) { + let folder = FileManager.default.temporaryDirectory + .appendingPathComponent("BoardLoadingTests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: folder, withIntermediateDirectories: true) + let registry = BoardRegistry(storageURL: folder.appendingPathComponent("board-registry.json")) + return (registry, { try? FileManager.default.removeItem(at: folder) }) +} + +// MARK: - Tests + +@MainActor +@Suite("Board loading state") +struct BoardLoadingTests { + + // MARK: The grace + + @Test("Nothing shows before the grace elapses") + func theSurfaceIsEmptyDuringTheGrace() async { + let indicator = BoardLoadingIndicator() + indicator.grace = .seconds(30) + + #expect(!indicator.showsSpinner, "at rest") + indicator.begin() + #expect(!indicator.showsSpinner, "the grace has been armed, not elapsed") + } + + @Test("The spinner appears once the grace elapses") + func theSpinnerArrivesAfterTheGrace() async { + let indicator = BoardLoadingIndicator() + indicator.grace = .milliseconds(20) + + indicator.begin() + await waitUntil { indicator.showsSpinner } + #expect(indicator.showsSpinner) + } + + @Test("The grace's body is the whole of the spinner's arrival") + func graceElapsedIsThePinnableHalf() { + // The clock-free half of the rule above: whatever the duration, *this* is what the sleep + // ends in, so a suite can assert the "after" state without a clock (`DragSession.expire`'s + // precedent). + let indicator = BoardLoadingIndicator() + indicator.graceElapsed() + #expect(indicator.showsSpinner) + } + + @Test("A board that lands inside the grace never flashes the spinner") + func fastOpenNeverFlashes() async { + let indicator = BoardLoadingIndicator() + indicator.grace = .milliseconds(20) + + // The ordinary open: the snapshot arrives before the grace is up. + indicator.begin() + indicator.end() + #expect(!indicator.showsSpinner) + + // And it stays away — the disarmed grace must not fire into a window that has moved on. + try? await Task.sleep(for: .milliseconds(60)) + #expect(!indicator.showsSpinner) + } + + @Test("Ending the surface clears a spinner that had already appeared") + func endClearsTheSpinner() async { + let indicator = BoardLoadingIndicator() + indicator.grace = .milliseconds(20) + + indicator.begin() + await waitUntil { indicator.showsSpinner } + indicator.end() + #expect(!indicator.showsSpinner, "the snapshot replaced the surface in place") + } + + @Test("Arming twice does not restart the clock") + func beginIsIdempotent() async { + let indicator = BoardLoadingIndicator() + indicator.grace = .milliseconds(20) + + indicator.begin() + // A body that evaluates again, or a host that configures itself twice, must not push the + // spinner back by another grace. + indicator.begin() + await waitUntil { indicator.showsSpinner } + #expect(indicator.showsSpinner) + } + + // MARK: The loading window's title + + @Test("A first-ever open wears the record's provisional folder name") + func loadingTitleIsTheFolderNameOnAFirstOpen() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (registry, tearDown) = try makeRegistry() + defer { tearDown() } + + // Exactly what `BoardWindowHost.start()` does before the walk: record, then read the record + // back for the title. The folder name arrives as the record's own provisional display name, + // not as a second rule the window applies for itself. + let recordID = registry.recordOpen(of: fixture.root) + + #expect( + BoardWindowHost.loadingTitle(record: registry.record(id: recordID), url: fixture.root) + == AppModel.folderDisplayName(of: fixture.root) + ) + } + + @Test("A board that has opened before wears its cached title") + func loadingTitleIsTheCachedTitle() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (registry, tearDown) = try makeRegistry() + defer { tearDown() } + + // The previous session's successful load, which is what stamps the cached title. + let first = registry.recordOpen(of: fixture.root) + registry.syncDisplayState(id: first, title: "Roadmap", icon: nil, iconColor: nil) + + // This session's open: the record is found again by file identity, and its cached title is + // what the window is called while it walks — never the folder name it happens to sit in. + let recordID = registry.recordOpen(of: fixture.root) + #expect(recordID == first, "the same board must find the record it already has") + #expect(BoardWindowHost.loadingTitle(record: registry.record(id: recordID), url: fixture.root) == "Roadmap") + } + + @Test("With no record yet the title is still a no-scan name") + func loadingTitleFallsBackToTheFolderName() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + + // The one body evaluation that precedes `recordOpen`. It must not be blank, and it must not + // cost a look inside the board. + #expect( + BoardWindowHost.loadingTitle(record: nil, url: fixture.root) + == AppModel.folderDisplayName(of: fixture.root) + ) + } +} diff --git a/KanbanTests/BoardStoreRegistryTests.swift b/KanbanTests/BoardStoreRegistryTests.swift index 81c8020..0934da5 100644 --- a/KanbanTests/BoardStoreRegistryTests.swift +++ b/KanbanTests/BoardStoreRegistryTests.swift @@ -278,4 +278,123 @@ struct BoardStoreRegistryTests { } #expect(registry.openBoardCount == 0) } + + // MARK: The off-main acquire + // + // The board window's open (02-architecture.md § Launch and window lifecycle, ruled 2026-07-29): + // the walk runs off the main actor so the window can be on screen while it does, concurrent asks + // for one board share it, and a cancelled open leaves nothing behind. These are claims about + // *what is registered*, so they are asserted the way the rest of this file asserts — object + // identity and the entry count, never a duration. + + @Test("An already-open board answers the async acquire without a walk") + func offMainAcquireHitsTheOpenBoard() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let registry = BoardStoreRegistry() + + let first = try registry.acquire(fixture.root) + let second = try await registry.acquireOffMain(fixture.root) + + #expect(second === first, "an open board is the same store however it is asked for") + #expect(registry.openBoardCount == 1) + + // Two references, so the first release must not tear anything down. + registry.release(first) + #expect(registry.liveStore(for: fixture.root) === first) + registry.release(first) + #expect(registry.openBoardCount == 0) + } + + @Test("Concurrent async acquires of one board single-flight into one store") + func offMainAcquiresSingleFlight() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let registry = BoardStoreRegistry() + + // Both calls are in flight before either can finish: each hops to the main actor, and the + // first one to get there suspends on its walk with the second right behind it. + async let first = registry.acquireOffMain(fixture.root) + async let second = registry.acquireOffMain(fixture.root) + let stores = try await (first, second) + + #expect(stores.0 != nil) + #expect(stores.0 === stores.1, "a joined walk must produce one store, not two") + #expect(registry.openBoardCount == 1, "two windows racing one board is still one open board") + + // And both callers really are holding it: the refcount took both, so the first release + // leaves the board standing. + guard let store = stores.0 else { return } + registry.release(store) + #expect(registry.liveStore(for: fixture.root) === store) + registry.release(store) + #expect(registry.openBoardCount == 0) + } + + @Test("Two boards opening at once are two independent walks") + func offMainAcquiresOfDistinctBoardsDoNotShare() async throws { + let first = try makeBoard() + defer { first.tearDown() } + let second = try makeBoard() + defer { second.tearDown() } + let registry = BoardStoreRegistry() + + // Restoration's shape: several windows opening together. The single flight is keyed by file + // identity, so nothing here can queue behind anything else — a slow board holds its own key + // and no other. + async let one = registry.acquireOffMain(first.root) + async let two = registry.acquireOffMain(second.root) + let stores = try await (one, two) + + #expect(stores.0 != nil) + #expect(stores.1 != nil) + #expect(stores.0 !== stores.1) + #expect(registry.openBoardCount == 2) + + if let store = stores.0 { registry.release(store) } + if let store = stores.1 { registry.release(store) } + #expect(registry.openBoardCount == 0) + } + + @Test("A cancelled async acquire builds no store and registers nothing") + func cancelledOffMainAcquireLeavesNothingBehind() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let registry = BoardStoreRegistry() + + // ⌘W during the walk. The walk itself is not cooperatively cancellable — it runs to + // completion and its result is discarded — so what is asserted here is the discard: no + // store, no entry, no watcher, no reference. + let open = Task { try? await registry.acquireOffMain(fixture.root) } + open.cancel() + let store = await open.value + + #expect(store == nil, "a cancelled open answers with nothing rather than a board") + #expect(registry.openBoardCount == 0) + #expect(registry.liveStore(for: fixture.root) == nil) + + // And the board is still openable afterwards: a discarded walk must leave no half-state a + // later open could trip over. + let reopened = try await registry.acquireOffMain(fixture.root) + #expect(reopened != nil) + #expect(registry.openBoardCount == 1) + if let reopened { registry.release(reopened) } + } + + @Test("The async acquire fails fail-fast like the synchronous one") + func offMainAcquireOfABrokenBoardThrows() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + try fixture.item(Ident.lane1, brokenIndex) + let registry = BoardStoreRegistry() + + do throws(BoardLoadFailure) { + _ = try await registry.acquireOffMain(fixture.root) + Issue.record("expected a broken board to fail") + } catch { + #expect(error.primary.path == "\(Ident.lane1)/index.md") + } + // A failed load leaves nothing behind, whichever actor walked it. + #expect(registry.openBoardCount == 0) + } } diff --git a/KanbanTests/BoardStoreTests.swift b/KanbanTests/BoardStoreTests.swift index 5e6b646..04a0a6d 100644 --- a/KanbanTests/BoardStoreTests.swift +++ b/KanbanTests/BoardStoreTests.swift @@ -178,6 +178,31 @@ struct BoardStoreTests { } } + @Test("A store built from a pre-walked result is the store the walking init would have built") + func prewalkedInitMatchesTheWalkingInit() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + + // The two inits the board window's loading state put side by side: the walk on this actor, + // and the same walk run somewhere else and handed over (`BoardStoreRegistry.acquireOffMain` + // runs it on a detached task). If these ever disagreed, an open would show a different board + // depending on which actor walked it. + let walkedHere = try BoardStore(rootURL: fixture.root) + let result = try BoardLoader.load(boardRoot: fixture.root) + let walkedElsewhere = BoardStore(rootURL: fixture.root, loaded: result) + + #expect(walkedElsewhere.rootURL == walkedHere.rootURL) + #expect(walkedElsewhere.snapshot == walkedHere.snapshot) + #expect(walkedElsewhere.loadWarnings == walkedHere.loadWarnings) + #expect(walkedElsewhere.defects.count == walkedHere.defects.count) + // The rest of the opening posture, which is what a second init could quietly get wrong. + #expect(walkedElsewhere.reloadFailure == nil) + #expect(walkedElsewhere.readOnlyLock == nil) + #expect(!walkedElsewhere.isReadOnly) + #expect(walkedElsewhere.selection == .empty) + #expect(walkedElsewhere.reloadGeneration == 0) + } + // MARK: Reloading @Test("A foreign tree change reloads and the snapshot shows the external edit")