From 01aa9a0c94b071bbca988e97b1e8721e116f54b0 Mon Sep 17 00:00:00 2001 From: rzen Date: Sun, 9 Aug 2026 17:27:50 -0400 Subject: [PATCH] The titlebar stand-in stops reappearing on a window that attaches after the board has loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- Kanban/App/BoardWindowHost.swift | 18 +++++ KanbanTests/BoardLoadingTests.swift | 118 ++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/Kanban/App/BoardWindowHost.swift b/Kanban/App/BoardWindowHost.swift index 88b3b0d..d9d67ba 100644 --- a/Kanban/App/BoardWindowHost.swift +++ b/Kanban/App/BoardWindowHost.swift @@ -808,6 +808,24 @@ struct BoardWindowHost: View { } self.loadingAccessory = nil + // **`onAttach` itself has to be replaced here, not just the stand-in it already installed** + // (Pipeline card a73bad86, reopened). SwiftUI's dismantle-then-make swap of the provisional + // window for the real one is not guaranteed to land before the load does + // (`HostedWindowController.detach`'s own doc comment) — a fast load and a slow swap means the + // real window's `attach` fires *after* this method has already run. Left alone, + // `configureLoadingWindow`'s closure is still installed at that point, and it builds a fresh + // stand-in and installs it on whatever window attaches next; `HostedWindowController.attach` + // then runs `addTitlebarAccessoryIfPossible()` right after `onAttach`, installing the real + // widget beside it — both in the titlebar, stand-in leading, and nothing left to remove it a + // second time. "Occasionally" was exactly this ordering. The frame placement half survives, + // because a late-swapped window is still a window whose saved frame has not been applied yet + // — only the stand-in-building half goes. + windowController.onAttach = { window in + if let saved = appModel.boardRegistry.record(id: recordID)?.windowFrame { + window.setFrame(HostedWindowController.placementOnCurrentScreens(for: saved), display: true) + } + } + // The window-title widget (03-board-ui.md § Board popover) — **board windows only**, which // is why it is installed here rather than in `WindowAccessor`: welcome, the bootstrap and // card windows share that machinery and have no board to describe. It goes in after the diff --git a/KanbanTests/BoardLoadingTests.swift b/KanbanTests/BoardLoadingTests.swift index 0261e4a..b6e4901 100644 --- a/KanbanTests/BoardLoadingTests.swift +++ b/KanbanTests/BoardLoadingTests.swift @@ -243,3 +243,121 @@ struct BoardLoadingTitlebarStandInTests { #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") + } +}