The undo command surface rebuilds — app-owned rows and explicit toolbar targets over FocusedValues
Edit ▸ Undo/Redo become the app's own replaced rows and the board toolbar pair takes explicit targets, both reading the focused session's BoardUndoManager through FocusedValues.undoStack (board windows publish the session's manager, card windows their own) — the nil-target route died with the SwiftUI window latch, 13-native-undo.md ▸ Rules ▸ command surface, re-ruled 2026-08-08. The rows enact the routing predicate themselves: text focus routes ⌘Z to the first responder's own manager, title and enablement included, re-derived at fire time with a beep for the stale window. NativeHistoryProvider turns @Observable so both surfaces re-derive on stack changes; a checkpoint-notification ticker covers plain text managers. .responderAction leaves ToolbarItemSpec with its only user; windowWillReturnUndoManager stays wired for AppKit's own asks. Live-probed on the fixture board (21/21): the row retitles to "Undo Add Lane" and crosses via real ⌘Z key events, ⇧⌘Z redoes via a window-server chord, the toolbar pair validates and fires, search-field and body-editor ⌘Z stay text undo with board stacks untouched, and a card window crosses its own stack with no fall-through. 2698 unit tests green. Claude-Session: https://claude.ai/code/session_014PtZdPwqZuqEDLc6wZMtEy
This commit is contained in:
@@ -600,28 +600,32 @@ struct BoardSessionHistoryTests {
|
||||
|
||||
// MARK: - The command surface
|
||||
|
||||
/// **What the Edit menu's Undo/Redo rows and the toolbar's pair actually do** — driven through the
|
||||
/// platform machinery they ride on rather than described (11-command-nexus.md ▸ Menu commands, the
|
||||
/// M− row; 03-board-ui.md ▸ Toolbar; 13-native-undo.md ▸ Rules).
|
||||
/// **What a window hands AppKit, and what the board toolbar's Undo/Redo pair reaches** — the two
|
||||
/// halves of the command surface that are still platform-shaped, driven through the machinery they
|
||||
/// ride on rather than described (03-board-ui.md ▸ Toolbar; 13-native-undo.md ▸ Rules).
|
||||
///
|
||||
/// ### The app writes none of this, which is exactly why it is tested
|
||||
/// ### What this suite covers since the surface became the app's own
|
||||
///
|
||||
/// There is no custom Undo/Redo menu code anywhere: the rows are the system's own nil-target
|
||||
/// `undo:`/`redo:`, and the toolbar's two items carry the same selectors with the same nil target
|
||||
/// (`BoardToolbar`). Every claim the design makes about them — they enable on a stack with steps,
|
||||
/// they dim under the read-only lock, the *menu* rows retitle themselves to "Undo Move 3 Cards"
|
||||
/// while the *toolbar* labels stay static — is therefore a claim about `NSWindow`'s own validation
|
||||
/// reading the manager this app's window delegate hands back. Nothing here would fail loudly if the
|
||||
/// wiring came undone; it would just quietly stop working, which is what these tests are for.
|
||||
/// **The rows are no longer here.** Edit ▸ Undo/Redo are `CommandGroup(replacing: .undoRedo)` rows
|
||||
/// the app writes and routes itself (13 ▸ Rules ▸ the command-surface bullet, re-ruled 2026-08-08;
|
||||
/// `UndoCommands.swift`, pinned by `UndoCommandsTests.swift`), because the nil-target route they used
|
||||
/// to ride is unreachable on a SwiftUI window: `NSWindow` latches an empty undo manager during
|
||||
/// creation, before `HostedWindowController` installs, so `windowWillReturnUndoManager` is never
|
||||
/// consulted for them. These tests never reproduced that — they attach the delegate before the
|
||||
/// window's first read, which is exactly the ordering a real SwiftUI window denies — and that is the
|
||||
/// diagnosis, not a gap to close: the hook works when it is asked, and the app stopped depending on
|
||||
/// it being asked.
|
||||
///
|
||||
/// ### What a headless run can and cannot reach
|
||||
/// What is pinned below is therefore what remains true and load-bearing:
|
||||
///
|
||||
/// `NSWindow.validateMenuItem(_:)` and `NSWindow.validateUserInterfaceItem(_:)` are the two methods
|
||||
/// AppKit calls once a nil-target lookup has resolved to the window, and both answer fully in a test
|
||||
/// process — which is the half this app owns and the half that can break. The lookup *itself*
|
||||
/// (`NSApp.target(forAction:to:from:)`) needs a **key window**, and a unit-test host has none, so
|
||||
/// "the board window is what the chain resolves to when it is key" is the one link these tests
|
||||
/// cannot close; it is standard responder-chain behaviour with no code of this app's in it.
|
||||
/// - **The delegate hook itself**, which stays wired because it is the right answer wherever *AppKit*
|
||||
/// asks a window's delegate for a manager (`HostedWindowController.windowWillReturnUndoManager`).
|
||||
/// `NSWindow.validateMenuItem(_:)` is the sharpest instrument a headless run has for reading what
|
||||
/// that hook returned — enablement, the lock, the composed title, the two levels — so the menu
|
||||
/// rows still appear here as the *probe*, not as the subject.
|
||||
/// - **The toolbar pair's explicit target** (`BoardToolbar`), which validates and fires against the
|
||||
/// session's `BoardUndoManager` directly. That path is the app's own end to end, and it is the one
|
||||
/// a headless run can close completely.
|
||||
@MainActor
|
||||
@Suite("History ▸ the command surface")
|
||||
struct UndoCommandSurfaceTests {
|
||||
@@ -641,13 +645,17 @@ struct UndoCommandSurfaceTests {
|
||||
return (window, controller)
|
||||
}
|
||||
|
||||
/// A row carrying the platform's own `undo:`/`redo:`, used here as a **probe** rather than as a
|
||||
/// shipped surface: validating one against the window is how a test reads back the manager
|
||||
/// `windowWillReturnUndoManager` returned, title composition and all. The app's own rows carry no
|
||||
/// selector at all (`UndoCommands.swift`).
|
||||
private func menuItem(_ selector: String) -> NSMenuItem {
|
||||
NSMenuItem(title: selector == "undo:" ? "Undo" : "Redo", action: NSSelectorFromString(selector), keyEquivalent: "")
|
||||
}
|
||||
|
||||
// MARK: The menu rows
|
||||
// MARK: What the delegate hands back
|
||||
|
||||
@Test("The Edit menu's rows read the board's stack, and retitle themselves from its step names")
|
||||
@Test("A window's delegate hands back the board's stack, titles composing from its step names")
|
||||
func theMenuRowsReadTheBoardsStack() {
|
||||
let provider = FakeHistoryProvider()
|
||||
let manager = BoardUndoManager(history: provider)
|
||||
@@ -667,9 +675,10 @@ struct UndoCommandSurfaceTests {
|
||||
provider.canUndo = true
|
||||
provider.undoActionName = "Move 3 Cards"
|
||||
|
||||
// 13's "the 06 vocabulary supplies menu titles ('Undo Move 3 Cards'), via NSUndoManager's
|
||||
// dynamic retitling": the app never writes that string — the platform composes it from the
|
||||
// bare phrase the seam vends, and validation is when it lands on the row.
|
||||
// 13's "the vocabulary supplies menu titles ('Undo Move 3 Cards'), via dynamic retitling":
|
||||
// the app never writes that string — the platform composes it from the bare phrase the seam
|
||||
// vends (`BoardUndoManager.undoMenuItemTitle`), and this is where it lands. The app's own
|
||||
// rows read the very same property, one step further out (`UndoCommandRouting.undoTitle`).
|
||||
#expect(window.validateMenuItem(undoRow))
|
||||
#expect(undoRow.title == "Undo Move 3 Cards")
|
||||
#expect(window.validateMenuItem(redoRow) == false)
|
||||
@@ -752,8 +761,10 @@ struct UndoCommandSurfaceTests {
|
||||
#expect(boardWindow.validateMenuItem(boardRow))
|
||||
#expect(boardRow.title == "Undo Move 3 Cards")
|
||||
|
||||
// **No fall-through** (06-history-undo.md ▸ Undo routing): the card window's own stack is
|
||||
// empty, so its row is disabled and ⌘Z beeps — it never reaches the board's step.
|
||||
// **No fall-through** (13-native-undo.md ▸ Undo routing): the card window's own stack is
|
||||
// empty, so its row is disabled and ⌘Z beeps — it never reaches the board's step. The rows
|
||||
// enforce it the same way now, by reading one focused value that is one window's or the
|
||||
// other's (`FocusedValues.undoStack`) and never both.
|
||||
#expect(cardWindow.validateMenuItem(cardRow) == false)
|
||||
#expect(cardRow.title == "Undo")
|
||||
|
||||
@@ -765,92 +776,93 @@ struct UndoCommandSurfaceTests {
|
||||
|
||||
// MARK: The toolbar twins
|
||||
|
||||
@Test("The toolbar pair validates identically to the menu rows — and keeps its static labels")
|
||||
func theToolbarPairMatchesTheMenuRows() throws {
|
||||
/// A board toolbar wired the way `BoardWindowHost` wires one, over `undo`.
|
||||
private func boardToolbar(store: BoardStore, undo: BoardUndoManager?) -> WindowToolbarController {
|
||||
let domain = "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)"
|
||||
return BoardToolbar.controller(
|
||||
store: store,
|
||||
search: BoardSearchPresentation(),
|
||||
zoom: BoardZoomStore(defaults: UserDefaults(suiteName: domain)!),
|
||||
appearance: AppearanceStore(defaults: UserDefaults(suiteName: domain + ".appearance")!, apply: { _ in }),
|
||||
session: DragSession(),
|
||||
undo: undo
|
||||
)
|
||||
}
|
||||
|
||||
/// One real item, built by the real delegate.
|
||||
private func toolbarItem(
|
||||
_ controller: WindowToolbarController,
|
||||
_ identifier: NSToolbarItem.Identifier
|
||||
) throws -> NSToolbarItem {
|
||||
try #require(controller.toolbar(
|
||||
controller.toolbar,
|
||||
itemForItemIdentifier: identifier,
|
||||
willBeInsertedIntoToolbar: true
|
||||
))
|
||||
}
|
||||
|
||||
/// **The pair carries an explicit target now** (13-native-undo.md ▸ Rules ▸ the command-surface
|
||||
/// bullet, re-ruled 2026-08-08) — the toolbar controller, over the session's `BoardUndoManager`,
|
||||
/// where until that ruling both items carried nil targets and `undo:`/`redo:` selectors for the
|
||||
/// responder chain to resolve.
|
||||
///
|
||||
/// This replaces the pin that read the pair's validation *through the window* against the menu
|
||||
/// rows'. The claim it was making — one answer on both surfaces — is unchanged and now stronger:
|
||||
/// they are not two validations that agree, they are one object both of them read
|
||||
/// (`bothSurfacesReadOneManager`).
|
||||
@Test("The toolbar pair targets the session's stack — and keeps its static labels")
|
||||
func theToolbarPairCarriesAnExplicitTarget() throws {
|
||||
let fixture = try makeBoard()
|
||||
defer { fixture.tearDown() }
|
||||
let store = try BoardStore(rootURL: fixture.root)
|
||||
let controller = BoardToolbar.controller(
|
||||
store: store,
|
||||
search: BoardSearchPresentation(),
|
||||
zoom: BoardZoomStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!),
|
||||
appearance: AppearanceStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!, apply: { _ in }),
|
||||
session: DragSession()
|
||||
)
|
||||
let provider = FakeHistoryProvider()
|
||||
let manager = BoardUndoManager(history: provider)
|
||||
let (window, hosted) = hostedWindow(manager)
|
||||
defer { hosted.detach() }
|
||||
let controller = boardToolbar(store: store, undo: manager)
|
||||
|
||||
/// The real items, built by the real delegate — nil target, `undo:`/`redo:` actions.
|
||||
func item(_ identifier: NSToolbarItem.Identifier) throws -> NSToolbarItem {
|
||||
try #require(controller.toolbar(
|
||||
controller.toolbar,
|
||||
itemForItemIdentifier: identifier,
|
||||
willBeInsertedIntoToolbar: true
|
||||
))
|
||||
}
|
||||
let undoItem = try item(.boardUndo)
|
||||
let redoItem = try item(.boardRedo)
|
||||
#expect(undoItem.target == nil, "nil target: the chain resolves it, exactly as the menu row's is")
|
||||
#expect(redoItem.target == nil)
|
||||
let undoItem = try toolbarItem(controller, .boardUndo)
|
||||
let redoItem = try toolbarItem(controller, .boardRedo)
|
||||
#expect(undoItem.target === controller, "the app's own target, not the responder chain's lookup")
|
||||
#expect(redoItem.target === controller)
|
||||
|
||||
// `validateUserInterfaceItem` is what `NSToolbarItem.validate()` asks its resolved target,
|
||||
// and `validateMenuItem` is what a menu row's asks — one predicate, two doors.
|
||||
#expect(window.validateUserInterfaceItem(undoItem) == false)
|
||||
#expect(window.validateUserInterfaceItem(redoItem) == false)
|
||||
undoItem.validate()
|
||||
redoItem.validate()
|
||||
#expect(undoItem.isEnabled == false, "an empty stack dims it")
|
||||
#expect(redoItem.isEnabled == false)
|
||||
|
||||
provider.canUndo = true
|
||||
provider.undoActionName = "Move 3 Cards"
|
||||
provider.canRedo = true
|
||||
provider.redoActionName = "Rename Lane"
|
||||
|
||||
let undoRow = menuItem("undo:")
|
||||
let redoRow = menuItem("redo:")
|
||||
#expect(window.validateUserInterfaceItem(undoItem) == window.validateMenuItem(undoRow))
|
||||
#expect(window.validateUserInterfaceItem(redoItem) == window.validateMenuItem(redoRow))
|
||||
#expect(window.validateUserInterfaceItem(undoItem))
|
||||
#expect(window.validateUserInterfaceItem(redoItem))
|
||||
undoItem.validate()
|
||||
redoItem.validate()
|
||||
#expect(undoItem.isEnabled)
|
||||
#expect(redoItem.isEnabled)
|
||||
|
||||
// 03's one exception to the label rule, proven rather than asserted: validation rewrote the
|
||||
// *menu* row's title and left the toolbar item's label exactly where it was.
|
||||
#expect(undoRow.title == "Undo Move 3 Cards")
|
||||
// 03's one exception to the label rule, proven rather than asserted: the phrase the *menu*
|
||||
// composes ("Undo Move 3 Cards") never reaches a toolbar label, whatever validation does.
|
||||
#expect(manager.undoMenuItemTitle == "Undo Move 3 Cards")
|
||||
#expect(undoItem.label == "Undo")
|
||||
#expect(redoItem.label == "Redo")
|
||||
#expect(undoItem.paletteLabel == "Undo", "the customize palette shows the static label too")
|
||||
}
|
||||
|
||||
@Test("A toolbar item's own validation lands on the board's answer, lock included")
|
||||
func theToolbarItemValidatesThroughTheWindow() throws {
|
||||
func theToolbarItemValidatesThroughTheManager() throws {
|
||||
final class Lock { var isOn = false }
|
||||
let lock = Lock()
|
||||
let fixture = try makeBoard()
|
||||
defer { fixture.tearDown() }
|
||||
let store = try BoardStore(rootURL: fixture.root)
|
||||
let zoomDomain = "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)"
|
||||
defer { UserDefaults.standard.removePersistentDomain(forName: zoomDomain) }
|
||||
let toolbar = BoardToolbar.controller(
|
||||
store: store,
|
||||
search: BoardSearchPresentation(),
|
||||
zoom: BoardZoomStore(defaults: UserDefaults(suiteName: zoomDomain)!),
|
||||
appearance: AppearanceStore(defaults: UserDefaults(suiteName: zoomDomain + ".appearance")!, apply: { _ in }),
|
||||
session: DragSession()
|
||||
)
|
||||
let provider = FakeHistoryProvider()
|
||||
let manager = BoardUndoManager(history: provider, isReadOnly: { lock.isOn })
|
||||
let (window, hosted) = hostedWindow(manager)
|
||||
defer { hosted.detach() }
|
||||
let controller = boardToolbar(store: store, undo: manager)
|
||||
|
||||
let item = try #require(toolbar.toolbar(
|
||||
toolbar.toolbar,
|
||||
itemForItemIdentifier: .boardUndo,
|
||||
willBeInsertedIntoToolbar: true
|
||||
))
|
||||
// The one link a headless run cannot make: `NSToolbarItem.validate()` resolves its target
|
||||
// through the key window, and a test host has none. Standing the window in as the target is
|
||||
// that lookup's *answer* — which is what nil-target means when a board window is key — so
|
||||
// what this asserts is the item's own validation path, end to end from `validate()`.
|
||||
item.target = window
|
||||
let item = try toolbarItem(controller, .boardUndo)
|
||||
// The link a headless run no longer has to fake: validation used to resolve a nil target
|
||||
// through the key window, which a test host does not have, so the window stood in as the
|
||||
// answer. The item is handed its target at construction now, so this is the shipped path
|
||||
// end to end from `validate()`.
|
||||
#expect(item.autovalidates, "AppKit revalidates it on user events; the observation covers the rest")
|
||||
|
||||
item.validate()
|
||||
@@ -870,27 +882,67 @@ struct UndoCommandSurfaceTests {
|
||||
#expect(item.label == "Undo", "no crossing of validation ever moves the label")
|
||||
}
|
||||
|
||||
@Test("The pair's enablement is deliberately not a predicate of the toolbar's own")
|
||||
func theSpecsAbstainFromEnablement() throws {
|
||||
@Test("Clicking the item crosses the board's stack, through the target it was given")
|
||||
func theToolbarItemCrossesTheStack() throws {
|
||||
let fixture = try makeBoard()
|
||||
defer { fixture.tearDown() }
|
||||
let store = try BoardStore(rootURL: fixture.root)
|
||||
let provider = FakeHistoryProvider()
|
||||
provider.canUndo = true
|
||||
provider.canRedo = true
|
||||
let manager = BoardUndoManager(history: provider)
|
||||
let controller = boardToolbar(store: store, undo: manager)
|
||||
|
||||
let undoItem = try toolbarItem(controller, .boardUndo)
|
||||
let redoItem = try toolbarItem(controller, .boardRedo)
|
||||
let target = try #require(undoItem.target as? NSObject)
|
||||
let action = try #require(undoItem.action)
|
||||
|
||||
target.perform(action, with: undoItem)
|
||||
#expect(provider.undoCount == 1, "the click reaches the session's stack, not a responder's")
|
||||
|
||||
let redoTarget = try #require(redoItem.target as? NSObject)
|
||||
redoTarget.perform(try #require(redoItem.action), with: redoItem)
|
||||
#expect(provider.redoCount == 1)
|
||||
}
|
||||
|
||||
/// **One manager, two faces** — what "the toolbar mirrors the menu" means for this pair now that
|
||||
/// neither of them goes through the responder chain (13-native-undo.md ▸ Rules ▸ the
|
||||
/// command-surface bullet, re-ruled 2026-08-08; `BoardToolbar`'s header).
|
||||
///
|
||||
/// This is where the retired `theSpecsAbstainFromEnablement` pin went. Its claim — that the pair
|
||||
/// must never grow a second answer able to disagree with the menu's — is the same claim, made
|
||||
/// the only way it can be now that the items *do* carry a predicate: the predicate and the row
|
||||
/// are reading one object, so they cannot come apart.
|
||||
@Test("The toolbar pair and the Edit menu's rows read one and the same manager")
|
||||
func bothSurfacesReadOneManager() throws {
|
||||
let fixture = try makeBoard()
|
||||
defer { fixture.tearDown() }
|
||||
let store = try BoardStore(rootURL: fixture.root)
|
||||
let provider = FakeHistoryProvider()
|
||||
let manager = BoardUndoManager(history: provider)
|
||||
let specs = BoardToolbar.specs(
|
||||
store: store,
|
||||
search: BoardSearchPresentation(),
|
||||
zoom: BoardZoomStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!),
|
||||
appearance: AppearanceStore(defaults: UserDefaults(suiteName: "dev.rzen.indie.Kanban.history-tests.\(UUID().uuidString)")!, apply: { _ in }),
|
||||
session: DragSession()
|
||||
session: DragSession(),
|
||||
undo: manager
|
||||
)
|
||||
|
||||
// Every other item mirrors its menu row's predicate; these two mirror the *mechanism*. A
|
||||
// spec-level `isEnabled` here would be a second answer able to disagree with the responder
|
||||
// chain's — and it would have to read a stack the toolbar has no route to, since the board's
|
||||
// window is what owns that answer.
|
||||
for identifier in [NSToolbarItem.Identifier.boardUndo, .boardRedo] {
|
||||
let spec = try #require(specs.first { $0.identifier == identifier })
|
||||
#expect(spec.isEnabled, "abstention, not enablement: AppKit's own validation decides")
|
||||
#expect(spec.isOn == nil)
|
||||
// The row's side of it, derived exactly as `UndoMenuRow` derives it: the focused stack, with
|
||||
// nothing text-shaped holding the keyboard.
|
||||
func rowIsEnabled() -> Bool {
|
||||
UndoCommandRouting.canUndo(
|
||||
UndoCommandRouting.routedManager(stack: manager, firstResponder: nil)
|
||||
)
|
||||
}
|
||||
|
||||
for state in [false, true, false] {
|
||||
provider.canUndo = state
|
||||
let spec = try #require(specs.first { $0.identifier == .boardUndo })
|
||||
#expect(spec.isEnabled == state)
|
||||
#expect(spec.isEnabled == rowIsEnabled(), "one object answers both surfaces")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user