`configureWindow` removed the loading stand-in from whatever window it happened to be running against, but left `onAttach` installed exactly as `configureLoadingWindow` set it — a closure that builds a fresh stand-in on every attach, deliberately, so it survives the provisional-window swap while the board is still loading. SwiftUI's dismantle-then- make swap has no deadline, though: when it lands after the store has already loaded, the stale closure reinstalls a stand-in on the real window a moment before `attach`'s own `addTitlebarAccessoryIfPossible()` installs the real widget beside it — both in the titlebar, stand-in leading, and nothing left holding a reference to remove it a second time. `onAttach` is now replaced in `configureWindow` too, keeping only the frame- placement half a re-attached window still needs. New suite `BoardLoadingTitlebarSwapRaceTests` drives `HostedWindowController` through the same attach sequence and pins both the fixed and (as a negative control) the unfixed shape. Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy
364 lines
17 KiB
Swift
364 lines
17 KiB
Swift
import AppKit
|
|
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)
|
|
)
|
|
}
|
|
}
|
|
|
|
// MARK: - The loading window's titlebar stand-in
|
|
|
|
/// **The duplicate-name bug's fix** (Pipeline card a73bad86): a board window's system title is kept
|
|
/// permanently hidden by `KanbanApp`'s `.windowToolbarStyle(.unified(showsTitle: false))` — a scene
|
|
/// modifier that cannot wait for the store, so it hides the system title on the loading window too,
|
|
/// before there is a `BoardStore` to build the real widget from. `boardLoadingTitlebarAccessory`
|
|
/// is what fills that gap, and `BoardWindowHost.configureWindow` swaps it for the real widget by
|
|
/// removing it from the window by identity — the same pattern
|
|
/// `HostedWindowController.removeTitlebarAccessory` uses for its own slot, pinned again here because
|
|
/// the swap itself runs inline in a private method with no seam of its own to call directly.
|
|
@MainActor
|
|
@Suite("Board loading state ▸ the titlebar stand-in")
|
|
struct BoardLoadingTitlebarStandInTests {
|
|
|
|
private static func window() -> NSWindow {
|
|
NSWindow(
|
|
contentRect: NSRect(x: 0, y: 0, width: 600, height: 400),
|
|
styleMask: [.titled, .closable, .miniaturizable, .resizable],
|
|
backing: .buffered,
|
|
defer: true
|
|
)
|
|
}
|
|
|
|
@Test("It lays out leading, exactly like the real widget it stands in for")
|
|
func laysOutLeading() {
|
|
let accessory = boardLoadingTitlebarAccessory(title: "Roadmap")
|
|
#expect(accessory.layoutAttribute == .leading)
|
|
// Wide and tall enough that it is never a zero-size, invisible widget — the intrinsic
|
|
// measurement `.intrinsicContentSize` runs settles the exact figure, which is not the part
|
|
// worth pinning; not being clipped to nothing is.
|
|
#expect(accessory.view.frame.width > 0)
|
|
#expect(accessory.view.frame.height > 0)
|
|
}
|
|
|
|
@Test("Removing it by identity leaves any other accessory the window carries untouched")
|
|
func removalByIdentityIsTargeted() {
|
|
let window = Self.window()
|
|
let standIn = boardLoadingTitlebarAccessory(title: "Roadmap")
|
|
let other = boardLoadingTitlebarAccessory(title: "Some Other Board")
|
|
window.addTitlebarAccessoryViewController(standIn)
|
|
window.addTitlebarAccessoryViewController(other)
|
|
#expect(window.titlebarAccessoryViewControllers.count == 2)
|
|
|
|
// `BoardWindowHost.configureWindow`'s own removal: find by identity, remove by index.
|
|
if let index = window.titlebarAccessoryViewControllers.firstIndex(where: { $0 === standIn }) {
|
|
window.removeTitlebarAccessoryViewController(at: index)
|
|
}
|
|
|
|
#expect(window.titlebarAccessoryViewControllers.count == 1)
|
|
#expect(window.titlebarAccessoryViewControllers.first === other, "the untargeted one survives")
|
|
}
|
|
|
|
@Test("A window with no stand-in installed is left alone by the same removal")
|
|
func removalIsANoOpWithoutOne() {
|
|
let window = Self.window()
|
|
let other = boardLoadingTitlebarAccessory(title: "Some Other Board")
|
|
window.addTitlebarAccessoryViewController(other)
|
|
|
|
let standIn: NSTitlebarAccessoryViewController? = nil
|
|
if let standIn, let index = window.titlebarAccessoryViewControllers.firstIndex(where: { $0 === standIn }) {
|
|
window.removeTitlebarAccessoryViewController(at: index)
|
|
}
|
|
|
|
#expect(window.titlebarAccessoryViewControllers.count == 1, "nothing to remove, nothing removed")
|
|
}
|
|
}
|
|
|
|
// MARK: - The reopened bug: a late provisional-to-real swap
|
|
|
|
/// **Pipeline card a73bad86, reopened 2026-08-09**: the removal above is not the whole fix.
|
|
/// `configureWindow` removed the stand-in from *the window it happened to run against*, but left
|
|
/// `windowController.onAttach` exactly as `configureLoadingWindow` had installed it — a closure that
|
|
/// builds a *fresh* stand-in and adds it to whatever window attaches next, deliberately, so it
|
|
/// survives the provisional-window swap while the board is still loading
|
|
/// (`HostedWindowController.detach`'s own doc comment: "install arrives before any window, a
|
|
/// dismantle follows, and only then does the real window attach").
|
|
///
|
|
/// The trouble is that swap has no deadline. When it lands *after* the store has already loaded and
|
|
/// `configureWindow` has already run, the stale closure is still the one `HostedWindowController
|
|
/// .attach` calls — it reinstalls a stand-in on the real window a moment before `attach`'s own
|
|
/// `addTitlebarAccessoryIfPossible()` installs the real widget beside it. Both sit in the titlebar,
|
|
/// stand-in leading — exactly the owner's screenshot — and nothing left with a reference to the
|
|
/// stand-in ever removes it. "Occasionally" was this ordering: whenever the load wins the race
|
|
/// against the swap.
|
|
///
|
|
/// `BoardWindowHost` is a SwiftUI view struct with no seam of its own to call `configureWindow`
|
|
/// directly, so this drives `HostedWindowController` through the same sequence that method does —
|
|
/// `configureLoadingWindow`'s attach, then the fixed `configureWindow`'s replacement of both the
|
|
/// stand-in and `onAttach` itself, then a late attach standing in for the delayed swap — the same
|
|
/// structural level `BoardLoadingTitlebarStandInTests` above tests at.
|
|
@MainActor
|
|
@Suite("Board loading state ▸ the provisional-window swap race")
|
|
struct BoardLoadingTitlebarSwapRaceTests {
|
|
|
|
private static func window() -> NSWindow {
|
|
NSWindow(
|
|
contentRect: NSRect(x: 0, y: 0, width: 600, height: 400),
|
|
styleMask: [.titled, .closable, .miniaturizable, .resizable],
|
|
backing: .buffered,
|
|
defer: true
|
|
)
|
|
}
|
|
|
|
@Test("A late provisional-to-real attach installs only the real widget, never a second stand-in")
|
|
func lateAttachAfterLoadDoesNotReinstallTheStandIn() {
|
|
let controller = HostedWindowController()
|
|
var loadingAccessory: NSTitlebarAccessoryViewController?
|
|
|
|
// `configureLoadingWindow`'s own closure, verbatim: a fresh stand-in per attach, held so a
|
|
// later removal can find it by identity.
|
|
controller.onAttach = { window in
|
|
let accessory = boardLoadingTitlebarAccessory(title: "Roadmap")
|
|
loadingAccessory = accessory
|
|
window.addTitlebarAccessoryViewController(accessory)
|
|
}
|
|
|
|
// The loading window the user sees first — the provisional window's attach.
|
|
let provisional = Self.window()
|
|
controller.attach(to: provisional)
|
|
#expect(provisional.titlebarAccessoryViewControllers.count == 1, "the stand-in, and only the stand-in")
|
|
|
|
// The load finishes before the real-window swap arrives: `configureWindow`'s own sequence,
|
|
// this suite's fix included — removal by identity, then `onAttach` replaced rather than left
|
|
// standing.
|
|
let realWidget = boardLoadingTitlebarAccessory(title: "Roadmap")
|
|
if let loadingAccessory,
|
|
let index = provisional.titlebarAccessoryViewControllers.firstIndex(where: { $0 === loadingAccessory }) {
|
|
provisional.removeTitlebarAccessoryViewController(at: index)
|
|
}
|
|
loadingAccessory = nil
|
|
controller.installTitlebarAccessory(realWidget)
|
|
// The fix under test: `onAttach` keeps only the frame-placement half a re-attached window
|
|
// still needs — no stand-in built or installed for anything that attaches from here on.
|
|
controller.onAttach = { _ in }
|
|
|
|
#expect(provisional.titlebarAccessoryViewControllers.count == 1, "the real widget replaced the stand-in")
|
|
#expect(provisional.titlebarAccessoryViewControllers.first === realWidget)
|
|
|
|
// SwiftUI's dismantle-then-make swap, landing *after* the load — the exact ordering the
|
|
// reopened bug depended on. `detach()` releases the provisional window without discarding the
|
|
// held chrome; the real window's `attach` reinstalls it.
|
|
controller.detach()
|
|
let real = Self.window()
|
|
controller.attach(to: real)
|
|
|
|
#expect(
|
|
real.titlebarAccessoryViewControllers.count == 1,
|
|
"no stand-in reappears on a window that attaches after the board has already loaded"
|
|
)
|
|
#expect(real.titlebarAccessoryViewControllers.first === realWidget, "only the real widget carries over")
|
|
}
|
|
|
|
@Test("Without the fix, the same late attach reinstalls a stand-in beside the real widget")
|
|
func theUnfixedClosureReproducesTheDuplicate() {
|
|
// The regression's own negative space: this pins that the scenario above is not vacuously
|
|
// true by construction — with `configureWindow` never replacing `onAttach` (the bug this
|
|
// card reopened over), the same late swap really does leave two accessories installed.
|
|
let controller = HostedWindowController()
|
|
var loadingAccessory: NSTitlebarAccessoryViewController?
|
|
controller.onAttach = { window in
|
|
let accessory = boardLoadingTitlebarAccessory(title: "Roadmap")
|
|
loadingAccessory = accessory
|
|
window.addTitlebarAccessoryViewController(accessory)
|
|
}
|
|
|
|
let provisional = Self.window()
|
|
controller.attach(to: provisional)
|
|
|
|
let realWidget = boardLoadingTitlebarAccessory(title: "Roadmap")
|
|
if let loadingAccessory,
|
|
let index = provisional.titlebarAccessoryViewControllers.firstIndex(where: { $0 === loadingAccessory }) {
|
|
provisional.removeTitlebarAccessoryViewController(at: index)
|
|
}
|
|
loadingAccessory = nil
|
|
controller.installTitlebarAccessory(realWidget)
|
|
// `onAttach` deliberately left untouched — the pre-fix state.
|
|
|
|
controller.detach()
|
|
let real = Self.window()
|
|
controller.attach(to: real)
|
|
|
|
#expect(real.titlebarAccessoryViewControllers.count == 2, "the bug: a fresh stand-in beside the real widget")
|
|
}
|
|
}
|