diff --git a/Kanban/App/AppModel.swift b/Kanban/App/AppModel.swift index a112d05..bed07b3 100644 --- a/Kanban/App/AppModel.swift +++ b/Kanban/App/AppModel.swift @@ -557,7 +557,14 @@ public final class AppModel { store: store, recordID: recordID, history: history, - undoManager: BoardUndoManager(history: history), + // The lock's enablement half (13-native-undo.md ▸ Rules): Undo and Redo disable with the + // other mutating commands while the board refuses writes, and the stack survives to + // resume when it clears. Weak, so the adapter is never the reason a closed board's store + // stays alive; a store that has gone answers "writable", which is moot — its stack went + // with it. + undoManager: BoardUndoManager(history: history, isReadOnly: { [weak store] in + store?.isReadOnly ?? false + }), cardRefs: [], access: access ) diff --git a/Kanban/History/BoardStoreHistory.swift b/Kanban/History/BoardStoreHistory.swift index 6657b6f..39db7d8 100644 --- a/Kanban/History/BoardStoreHistory.swift +++ b/Kanban/History/BoardStoreHistory.swift @@ -23,9 +23,19 @@ import os /// Nothing a step closes over is read from the snapshot at crossing time: every closure below /// captures ids, folder URLs, orders, titles, style values and body bytes as **values**, computed /// from the pre-write snapshot the store was holding when the gesture ran. That is what makes a step -/// meaningful minutes later, after any number of reloads, and it is what the next milestone's -/// field-level staleness predicate compares against — each step already carries both sides of its -/// write (13 ▸ Rules ▸ staleness validation). +/// meaningful minutes later, after any number of reloads, and it is what the **field-level staleness +/// predicate** compares against — each step carries both sides of its write (13 ▸ Rules ▸ staleness +/// validation). +/// +/// ### Every step declares what its write left behind +/// +/// Beside the two closures, each registration below hands the funnel two `[HistoryExpectation]` +/// lists: what the board must look like for the undo to be safe (the state the *forward* write left), +/// and what it must look like for the redo to be safe (the state the *undo* leaves). One entry per +/// item the gesture touched, naming that item's folder, whether it should be there and live, and the +/// fields the write actually set — "extend the registration to carry whatever the predicate needs, no +/// more". A field this gesture never wrote is never listed, which is what makes a foreign edit +/// elsewhere — another card, another field of the same card — leave the step alone. @MainActor extension BoardStore { @@ -39,11 +49,20 @@ extension BoardStore { /// weak one this method installs: a stack outliving its session must not be the reason a board /// stays in memory. /// - /// **A step that could not write answers `.skipped`.** `HistoryStepOutcome` has two cases and the - /// honest reading of a failed inverse is the second one — nothing landed, so nothing goes on the - /// opposite stack — and `performWrite` has already posted the banner that says why. The read-only - /// lock, the other way a write is refused here, cannot reach this path in practice: a lock - /// disables Undo and Redo with the rest of the mutating commands (13 ▸ Rules ▸ locks). + /// `subject` is what the **skip banner** quotes: the item's title where the step names exactly + /// one titled item, and otherwise `nil`, which falls back to the step's own 06 phrase. That is + /// two readings of one sentence rather than two sentences — 13's own example is an item ("Undo + /// skipped — 'Fix login' changed outside Lanework") and a batch has no single item to name, so it + /// says what it is instead ("Undo skipped — 'Move 3 Cards' changed outside Lanework"). An + /// untitled item takes the phrase too: "Untitled" is a rendering, never a value (03-board-ui.md + /// § Card face). + /// + /// **A failed inverse answers `.failed`, a stale one `.skipped`** — the distinction the stack + /// depends on (`HistoryStepOutcome`): a stale step is dropped and ⌘Z falls through, a failed one + /// stays put with `performWrite`'s error banner already standing. The read-only lock reaches this + /// path only from a direct call — the adapter disables Undo and Redo under it + /// (`BoardUndoManager`, 13 ▸ Rules ▸ locks) — and is a refusal rather than a staleness, so it too + /// leaves the stack where it is: the lock clears, the stack resumes. /// /// The seam is spelled with an **untyped** `throws`, deliberately and at a cost worth recording: /// every inverse below is made of `BoardWriter` calls and throws nothing but `BoardWriteError`, @@ -54,25 +73,48 @@ extension BoardStore { /// than swallows the case that cannot happen. func registerStep( _ name: String, + subject: String? = nil, + undoExpects: [HistoryExpectation], + redoExpects: [HistoryExpectation], undo: @escaping @MainActor (BoardStore) throws -> Void, redo: @escaping @MainActor (BoardStore) throws -> Void ) { guard let history else { return } + let named = subject ?? name history.register(HistoryStep( name: name, - undo: { [weak self] in BoardStore.cross(self, undo) }, - redo: { [weak self] in BoardStore.cross(self, redo) } + undo: { [weak self] direction in + BoardStore.cross(self, direction, named, undoExpects, undo) + }, + redo: { [weak self] direction in + BoardStore.cross(self, direction, named, redoExpects, redo) + } )) } - /// Runs one side of a step as an ordinary bracketed write. + /// Validates one side of a step against disk, then runs it as an ordinary bracketed write. + /// + /// The order is the whole rule: **nothing is written until every target still holds what this + /// step left there** (13 ▸ Rules ▸ staleness validation, "Never apply a stale inverse on top of + /// someone else's newer write"). A stale target posts the info-tone row that explains the skip and + /// answers `.skipped`, which pops the step and falls the crossing through to the next one down. private static func cross( _ store: BoardStore?, + _ direction: HistoryDirection, + _ subject: String, + _ expectations: [HistoryExpectation], _ write: @MainActor (BoardStore) throws -> Void ) -> HistoryStepOutcome { // The board went away under its own stack — a session torn down between the registration and - // the ⌘Z. Nothing to write to, so nothing ran. - guard let store else { return .skipped } + // the ⌘Z. There is nothing to write to and nothing to say it on, and a step whose board is + // gone is not one to pop: the stack is about to be cleared with the session anyway. + guard let store else { return .failed } + + guard HistoryStaleness.isCurrent(expectations, under: store.rootURL) else { + store.banners.postSkippedStep(direction, subject: subject) + return .skipped + } + var foreign: (any Error)? let landed: Void? = try? store.performWrite { () throws(BoardWriteError) -> Void in do { @@ -87,9 +129,11 @@ extension BoardStore { } if let foreign { historyLogger.error("an inverse threw something that is not a write error: \(String(describing: foreign), privacy: .public)") - return .skipped + return .failed } - return landed == nil ? .skipped : .applied + // `nil` is either the Writer's `BoardWriteError` (bannered on the way past) or the read-only + // lock's refusal (the lock row is already standing). Both are failures, never staleness. + return landed == nil ? .failed : .applied } private static let historyLogger = Logger(subsystem: "dev.rzen.indie.Kanban", category: "history") @@ -124,10 +168,22 @@ extension BoardStore { /// hand, a foreign edit — goes with the folder and does not come back on redo. That follows from /// 13's own two rulings (remove the folder; attachment operations register nothing in v1) rather /// than from anything decided here. - func registerCreation(_ items: [CreatedItem], kind: HistoryPhrase.Kind) { + /// + /// **Its staleness predicate is existence and liveness, and deliberately nothing else** (13: + /// "existence/liveness for create/delete/restore steps"). A create's after-value *is* the item's + /// being there, so the undo validates that the folders are still there and still live and the + /// redo that they are still gone. The same honest edge follows: a foreign *edit* inside a created + /// card does not stop ⌘Z from removing it, because the create never wrote that field — while a + /// foreign *delete* does, since the trash row the user is looking at is not this step's to purge. + func registerCreation(_ items: [CreatedItem], kind: HistoryPhrase.Kind, subject: String? = nil) { guard !items.isEmpty else { return } let operation: WriteOperation = kind == .lane ? .createLane : .createCard - registerStep(HistoryPhrase.name(.add, kind: kind, count: items.count)) { _ in + registerStep( + HistoryPhrase.name(.add, kind: kind, count: items.count), + subject: subject, + undoExpects: items.map { .live($0.folder) }, + redoExpects: items.map { .absent($0.folder) } + ) { _ in // Reversed, so a lane and a card created by one gesture unwind child-first — the same // order they were made in, read backwards. for item in items.reversed() { @@ -170,6 +226,48 @@ extension BoardStore { } } + /// The style fields a gesture actually set — **one entry per dimension it did not `.keep`**, so a + /// step that only chose a colour never validates the symbol beside it. + /// + /// `.remove` is spelled as `nil`, which is the after-value the None well leaves: the key is gone, + /// and the item renders the level's default (03-board-ui.md § Styling ▸ Controls). + static func styledFields(background: StyleChange, icon: StyleChange) -> [ExpectedField] { + field(background, as: ExpectedField.background) + field(icon, as: ExpectedField.icon) + } + + /// The same dimensions, holding the values the *inverse* restores — what the redo half validates + /// against once ⌘Z has run. + /// + /// The prior is read as `FieldValue.value`, which is `nil` for a missing key **and** for a + /// malformed one, exactly matching `restore(_:to:in:)`: a malformed prior reads as a removal, so + /// the state the undo leaves is an absent key either way. + static func restoredStyleFields( + background: StyleChange, + priorBackground: FieldValue, + icon: StyleChange, + priorIcon: FieldValue + ) -> [ExpectedField] { + var fields: [ExpectedField] = [] + if background != .keep { + fields.append(.background(priorBackground.value)) + } + if icon != .keep { + fields.append(.icon(priorIcon.value)) + } + return fields + } + + private static func field( + _ change: StyleChange, + as make: (String?) -> ExpectedField + ) -> [ExpectedField] { + switch change { + case .keep: [] + case let .set(value): [make(value)] + case .remove: [make(nil)] + } + } + /// Puts a lane's `width` back — the integer it held, or no key at all. /// /// Note the asymmetry with the forward write, which removes the key at one unit (the diff --git a/Kanban/History/BoardUndoManager.swift b/Kanban/History/BoardUndoManager.swift index 48ea3cd..a12a74f 100644 --- a/Kanban/History/BoardUndoManager.swift +++ b/Kanban/History/BoardUndoManager.swift @@ -32,25 +32,54 @@ import AppKit /// clears a window's undo manager on paths this app does not control, and a card window closing must /// not empty the board's stack (13-native-undo.md ▸ Rules — the stack belongs to the *session*, and /// its one clearing point is that session's teardown). +/// +/// ### The read-only lock disables Undo and Redo here +/// +/// "Every read-only lock (vanished root, failed reload after wholesale ops, unwritable location) +/// disables Undo/Redo with the other mutating commands; **the stack itself survives the lock and +/// resumes when it clears**" (13 ▸ Rules). This is the right place for it and the only one: every +/// surface that offers ⌘Z — the Edit menu's nil-target row, the toolbar pair, a card window's +/// responder chain — validates through this object, so answering `false` here disables all of them +/// at once, exactly as the lock's other victims disable through menu validation (02-architecture.md +/// § "The lock's scope"). Putting it in the *provider* would have been the same answer in the wrong +/// place: the stack is not the thing that is locked, the board is, and a Pro session binding the git +/// provider must inherit the rule without reimplementing it. public final class BoardUndoManager: UndoManager { /// The substrate this manager is a face for. Strong: the session owns both, and the manager is /// only ever reachable while the session that made it is alive. private let history: any HistoryProviding - public init(history: any HistoryProviding) { + /// Whether the board is refusing writes — `BoardStore.isReadOnly`, read through a closure rather + /// than by holding the store. The adapter is deliberately store-free (it is a face for a *seam*, + /// and Pro binds a different substrate behind the same one), and a closure is what lets the + /// composition root wire the board's own truth in without this file learning what a `BoardStore` + /// is. The default answers "writable", which is what a manager built without a board — a test of + /// the adapter's own grammar — should have. + private let isReadOnly: @MainActor () -> Bool + + public init(history: any HistoryProviding, isReadOnly: @escaping @MainActor () -> Bool = { false }) { self.history = history + self.isReadOnly = isReadOnly super.init() } // MARK: Enablement - public override var canUndo: Bool { history.canUndo } + /// **False under the lock, whatever the stack holds.** The steps are still there — this is an + /// enablement answer, not a clearing — so the first ⌘Z after the lock clears crosses the step it + /// would have crossed before it landed. + public override var canUndo: Bool { !isReadOnly() && history.canUndo } - public override var canRedo: Bool { history.canRedo } + public override var canRedo: Bool { !isReadOnly() && history.canRedo } // MARK: Crossing + /// Not gated on the lock, deliberately: `undo:` reaches a manager only through a menu item or + /// toolbar button that has already validated against `canUndo`, and a crossing that somehow + /// started anyway is refused one layer down by `performWrite` — which leaves the step on the + /// stack (`HistoryStepOutcome.failed`), the same place this enablement rule keeps it. A second + /// guard here would be a second answer to one question. public override func undo() { history.undo() } public override func redo() { history.redo() } diff --git a/Kanban/History/HistoryProviding.swift b/Kanban/History/HistoryProviding.swift index bdfc616..11bfa2e 100644 --- a/Kanban/History/HistoryProviding.swift +++ b/Kanban/History/HistoryProviding.swift @@ -1,15 +1,43 @@ import Foundation +// MARK: - HistoryDirection + +/// Which command the user pressed — ⌘Z or ⇧⌘Z. +/// +/// **Not which half of a step is running.** A step that has already been undone goes onto the redo +/// stack *reversed* (`HistoryStep.reversed`), so the closure ⇧⌘Z crosses is the one registered as +/// `redo` and the closure a second ⌘Z crosses is the one registered as `undo` — the halves swap, and +/// a step cannot tell which it is by looking at itself. What it has to be told is the **command**, +/// because the one sentence a step ever says out loud names it: "Undo skipped — 'Fix login' changed +/// outside Lanework" (13-native-undo.md ▸ Rules), with "Redo skipped" as its mirror. +public enum HistoryDirection: Sendable, Equatable { + case undo + case redo + + /// The stack a step lands on once it has been crossed this way. + public var opposite: HistoryDirection { + self == .undo ? .redo : .undo + } +} + // MARK: - HistoryStepOutcome /// What happened when a step was asked to walk its write back (or forward again). /// -/// The second case is 13-native-undo.md ▸ Rules ▸ **staleness validation**, in the vocabulary the -/// provider needs it in: "target folder gone, or the field no longer holding the step's after-value -/// → the step is **skipped, not applied**: popped from the stack ... and ⌘Z falls through to the -/// next step". The provider reads this answer and nothing else — the *predicate* (field-level, -/// settled) and the info-tone banner that explains a skip both belong to the step, which is the only -/// side that knows what it wrote and which board to say it on. +/// The three cases are three different fates for the *stack*, which is the only thing the provider +/// asks about: +/// +/// - `.applied` — the write landed; the step's mirror image joins the opposite stack. +/// - `.skipped` — **stale** (13-native-undo.md ▸ Rules ▸ staleness validation): "target folder gone, +/// or the field no longer holding the step's after-value → the step is **skipped, not applied**: +/// popped from the stack ... and ⌘Z falls through to the next step". The step is dropped and the +/// crossing continues. +/// - `.failed` — the inverse was attempted and could not be written. The step **stays**, and the +/// crossing stops (see the case's own note). +/// +/// The provider reads this answer and nothing else — the *predicate* (field-level, settled) and the +/// info-tone banner that explains a skip both belong to the step, which is the only side that knows +/// what it wrote and which board to say it on. public enum HistoryStepOutcome: Equatable, Sendable { /// The step ran. Its mirror image joins the opposite stack. @@ -19,6 +47,25 @@ public enum HistoryStepOutcome: Equatable, Sendable { /// its inverse would clobber somebody else's newer edit. Nothing ran, the step is dropped, and /// the crossing continues with the next one down. case skipped + + /// The step tried and could not: the Writer refused the inverse (a disk error, a permission + /// problem, a board that has gone read-only under the stack). Nothing landed. + /// + /// **The step stays put and the crossing stops** — the one thing that distinguishes this from + /// `.skipped`, and the reason the case exists. 13 is silent here, so the posture is the honest + /// reading of its two rules: a *stale* step is one the board has moved past, so dropping it + /// loses nothing; a *failed* one is a step the user still means to cross, refused by a condition + /// that is usually momentary (a full disk, an unplugged volume), so popping it would spend their + /// only route back on a transient error. The failure has already banners itself as an ordinary + /// write failure (`BoardStore.performWrite` posts before it rethrows), which is exactly the + /// vocabulary 02-architecture.md § Write-failure surfacing gives it — and which is why a failure + /// must *not* also raise the info-tone skip row: two rows for one event would say the step is + /// both gone and retryable. + /// + /// Stopping rather than falling through follows from the same reading: fall-through exists to + /// walk past steps the board no longer has a use for, and a disk that just refused one write is + /// not a reason to attempt N more. + case failed } // MARK: - HistoryStep @@ -47,11 +94,15 @@ public enum HistoryStepOutcome: Equatable, Sendable { /// never part of it**: the platform composes and localizes that (`BoardUndoManager`), and a step /// that spelled it would read "Undo Undo Move Card" in the Edit menu. /// -/// ### Both closures are `@MainActor` +/// ### Both closures are `@MainActor`, and both are handed the direction /// -/// Everything they touch — the store, the snapshot, the banners — is, and a step exists to be run -/// from a menu command. Marking them says so at the seam instead of leaving each provider to -/// rediscover it. +/// Everything they touch — the store, the snapshot, the banners — is `@MainActor`, and a step exists +/// to be run from a menu command. Marking them says so at the seam instead of leaving each provider +/// to rediscover it. +/// +/// The `HistoryDirection` argument is **which command the user pressed**, not which half is running: +/// `reversed` swaps the two closures, so the half a ⌘Z crosses is the `redo` one as often as not, +/// and the skip sentence has to name the keystroke rather than the half (see `HistoryDirection`). public struct HistoryStep { /// The menu phrase, unprefixed — see the type's note. @@ -59,16 +110,16 @@ public struct HistoryStep { /// Walks the board back across this step. Registered at the Writer boundary as the *inverse* of /// the write that just landed. - public let undo: @MainActor () -> HistoryStepOutcome + public let undo: @MainActor (HistoryDirection) -> HistoryStepOutcome /// Walks it forward again — the original write, replayed. Reached only after `undo` applied, /// because that is the only way a step reaches the redo stack. - public let redo: @MainActor () -> HistoryStepOutcome + public let redo: @MainActor (HistoryDirection) -> HistoryStepOutcome public init( name: String, - undo: @escaping @MainActor () -> HistoryStepOutcome, - redo: @escaping @MainActor () -> HistoryStepOutcome + undo: @escaping @MainActor (HistoryDirection) -> HistoryStepOutcome, + redo: @escaping @MainActor (HistoryDirection) -> HistoryStepOutcome ) { self.name = name self.undo = undo @@ -131,7 +182,8 @@ public protocol HistoryProviding: AnyObject { var redoActionName: String? { get } /// Crosses one step backwards. A stale step is skipped rather than applied, and the crossing - /// falls through to the next one (13 ▸ Rules ▸ staleness validation); an empty stack does + /// falls through to the next one (13 ▸ Rules ▸ staleness validation); a step whose write *failed* + /// stays where it is and stops the crossing (`HistoryStepOutcome.failed`); an empty stack does /// nothing. func undo() diff --git a/Kanban/History/HistoryStaleness.swift b/Kanban/History/HistoryStaleness.swift new file mode 100644 index 0000000..226251a --- /dev/null +++ b/Kanban/History/HistoryStaleness.swift @@ -0,0 +1,242 @@ +import Foundation + +// MARK: - ExpectedField + +/// One field a step's write left holding a known value — the unit 13-native-undo.md ▸ Rules' +/// **field-level** predicate compares. +/// +/// "Each step registers both sides of its write anyway (the before-value is the inverse; the +/// after-value is what its write set), so validation compares the targeted field's current value +/// against the expected after-value" (settled, ruled 2026-07-27). These cases are therefore exactly +/// the fields the app's own inverses write and no others: a step declares what *it* set, never what +/// it merely read, so a foreign edit to a field this gesture never touched cannot skip anything. +/// +/// **`nil` means the key is absent**, which is a real after-value throughout this app rather than a +/// missing one: an emptied rename removes `title`, a width of one unit removes `width`, and the None +/// well removes `background` (the remove-at-default family). A **malformed** value on disk matches +/// neither an absence nor a value — it is not something this app writes, so finding one is finding +/// somebody else's edit. +public enum ExpectedField: Sendable, Equatable { + + /// `title` — renames at every level. + case title(String?) + + /// `order` — every reorder, every move's landing rank, and the rank half of a drag-restore. + case order(Double) + + /// `width` — the lane resize. `nil` is the one-unit default, whose key the write removes. + case width(Int?) + + /// `background` — the styling gesture's colour dimension. + case background(String?) + + /// `icon` — the styling gesture's symbol dimension. + case icon(String?) + + /// The body span, **byte for byte** — the Edit session's step, and the one inverse in the app + /// whose fidelity is not field-level (13: "body steps compare bytes"). + case body(String) +} + +// MARK: - HistoryExpectation + +/// What one folder must currently hold for a step to be safe to cross — the state that step's write +/// left it in. +/// +/// Two halves, both of them 13's: **existence and liveness** ("target folder gone ... → the step is +/// skipped"; "existence/liveness for create/delete/restore steps"), and the **field-level** +/// comparison above. A step carries one of these per item it touched, so a multi-card move validates +/// three targets and a single rename validates one — which is the whole of "a foreign change to an +/// unrelated item must not skip anything": an item no step named is an item no expectation mentions. +/// +/// **The folder's *path* is the parent check.** A move's step expects the card at its destination +/// path; a card that a foreign writer moved elsewhere leaves nothing at that path, so the ordinary +/// existence half already answers "moved away" without a parent field of its own. +public struct HistoryExpectation: Sendable, Equatable { + + /// Where the item this step wrote to should be — the destination for a move, the item's own + /// folder for everything else, and the board root for the board's own rename and styling. + public let folder: URL + + /// Whether the item should be there, and if so on which side of the tombstone. + public let presence: Presence + + /// The fields the step's write set, with the values it set them to. Empty for a step whose + /// whole subject *is* existence — a create, a delete, a Put Back. + public let fields: [ExpectedField] + + /// Where an item stands, as the trash's own three-way reading of it. + public enum Presence: Sendable, Equatable { + /// There, and rendered: no `deleted:` on the item **or on any ancestor**. Liveness is + /// effective, the same ancestor walk `BoardStore.liveItem` and the card windows' fate rule + /// apply — a card under a tombstoned lane renders nowhere, so it is as gone as a deleted one. + case live + + /// There, and tombstoned — a trash row, or a card hidden under a tombstoned lane. + case tombstoned + + /// Not there at all: the folder is gone. What an undone create leaves, and what a redone one + /// expects to find before putting it back. + case absent + } + + public init(folder: URL, presence: Presence, fields: [ExpectedField]) { + self.folder = folder + self.presence = presence + self.fields = fields + } + + /// The item is live and its fields say what the step set them to. + public static func live(_ folder: URL, _ fields: ExpectedField...) -> HistoryExpectation { + HistoryExpectation(folder: folder, presence: .live, fields: fields) + } + + /// The same, for a caller whose field list is computed — the styling gesture's, which varies per + /// dimension. A label rather than a second variadic, so `.live(folder)` stays unambiguous. + public static func live(_ folder: URL, fields: [ExpectedField]) -> HistoryExpectation { + HistoryExpectation(folder: folder, presence: .live, fields: fields) + } + + /// The item is tombstoned and its fields say what the step set them to. + public static func tombstoned(_ folder: URL, _ fields: ExpectedField...) -> HistoryExpectation { + HistoryExpectation(folder: folder, presence: .tombstoned, fields: fields) + } + + /// Nothing is at this path. + public static func absent(_ folder: URL) -> HistoryExpectation { + HistoryExpectation(folder: folder, presence: .absent, fields: []) + } + + /// The variant for a step whose target's liveness is not known until the gesture runs — the Edit + /// session's, which is registered against a card that may have been tombstoned out from under + /// the buffer (05-card-window.md ▸ Deletion & lifecycle). + public static func item( + _ folder: URL, + tombstoned: Bool, + _ fields: ExpectedField... + ) -> HistoryExpectation { + HistoryExpectation(folder: folder, presence: tombstoned ? .tombstoned : .live, fields: fields) + } +} + +// MARK: - HistoryStaleness + +/// The staleness predicate: does the board still look the way this step's write left it? +/// (13-native-undo.md ▸ Rules ▸ staleness validation.) +/// +/// ### It reads disk, not the snapshot +/// +/// 13 says "re-checks its target against the current snapshot at ⌘Z time", and *current* is the +/// load-bearing word: the store's `snapshot` is by construction one reload behind every write the +/// app makes (the one-way flow means a write is only visible once the watcher round-trips it), so +/// validating against it would make a second ⌘Z pressed inside the debounce window compare against a +/// board that still shows the first one's *pre*-state — every rapid undo run would false-skip. Disk +/// is what "current" can honestly mean at the instant a step is crossed, and it is also what an +/// inverse is about to write to. +/// +/// ### Lazily, never eagerly +/// +/// Nothing here is called by the watcher, the ledger, or any background sweep: "invalidation is lazy +/// (settled — ruled 2026-07-27): staleness is discovered at ⌘Z time, never by background pruning ... +/// The stack always looks full". This type has exactly one caller, `BoardStore.cross`, one line +/// before the inverse would have been written. +public enum HistoryStaleness { + + /// Whether every target a step named still holds what that step left there. + /// + /// `root` is the board's current root, which the liveness walk stops at — a lane's parent. + public static func isCurrent(_ expectations: [HistoryExpectation], under root: URL) -> Bool { + expectations.allSatisfy { isCurrent($0, under: root) } + } + + /// One target's answer. + /// + /// A file that cannot be read or parsed fails every expectation but `.absent`: an `index.md` + /// somebody has just broken is not one holding this step's after-value, and the honest reading of + /// "the field no longer holds it" covers a field that can no longer be read at all. + public static func isCurrent(_ expectation: HistoryExpectation, under root: URL) -> Bool { + guard expectation.presence != .absent else { + return !FileManager.default.fileExists(atPath: expectation.folder.path) + } + guard let document = index(at: expectation.folder) else { return false } + + let tombstoned = isEffectivelyTombstoned(expectation.folder, document: document, under: root) + guard tombstoned == (expectation.presence == .tombstoned) else { return false } + + return expectation.fields.allSatisfy { matches($0, in: document) } + } + + // MARK: Fields + + static func matches(_ field: ExpectedField, in document: FrontmatterDocument) -> Bool { + switch field { + case let .title(expected): equal(document.title, expected) + case let .order(expected): document.order.value == expected + case let .width(expected): equal(document.width, expected) + case let .background(expected): equal(document.background, expected) + case let .icon(expected): equal(document.icon, expected) + case let .body(expected): document.body == expected + } + } + + /// A present value matches a present expectation by value; an absence matches an absence. + /// + /// **A malformed field matches nothing**, deliberately: `FieldValue.malformed` is a shape this + /// app never writes (`background: [a, b]` is somebody's hand edit), so a step that expected its + /// own removed key must not read one as "gone" and clobber it. + private static func equal(_ actual: FieldValue, _ expected: Value?) -> Bool { + switch (actual, expected) { + case (.missing, nil): true + case let (.valid(value), .some(expected)): value == expected + default: false + } + } + + // MARK: Liveness + + /// Whether the item at `folder` renders — **presence of `deleted:`, not its validity** + /// (`Lane.isDeleted`'s rule), walked up through the ancestors the way every other liveness + /// question in this app is. + /// + /// The board root is never tombstoned however its own frontmatter reads: a board-level `deleted:` + /// is a tolerated load *warning* (01-storage-format.md § Deletion), not a state that hides the + /// board from itself. + private static func isEffectivelyTombstoned( + _ folder: URL, + document: FrontmatterDocument, + under root: URL + ) -> Bool { + guard !isRoot(folder, root) else { return false } + guard document.deleted.isMissing else { return true } + + // Lane and card are the only levels below the root, so this walks at most twice; the bound + // is there so a folder that is not under this root at all (a step registered before a + // mid-session root change) ends rather than climbing to `/`. + var parent = folder.deletingLastPathComponent() + for _ in 0 ..< 4 { + guard !isRoot(parent, root) else { return false } + guard let ancestor = index(at: parent) else { return false } + if !ancestor.deleted.isMissing { return true } + parent = parent.deletingLastPathComponent() + } + return false + } + + private static func isRoot(_ folder: URL, _ root: URL) -> Bool { + folder.standardizedFileURL.path == root.standardizedFileURL.path + } + + // MARK: Reading + + /// The item's `index.md` as the app reads it, or `nil` when there is no readable, parseable one + /// there — a folder that is gone, an indexless folder, bytes that are not UTF-8, frontmatter + /// somebody has just broken. + private static func index(at folder: URL) -> FrontmatterDocument? { + let indexURL = folder.appendingPathComponent(BoardLoader.indexFileName) + guard let data = try? Data(contentsOf: indexURL), + let text = String(validating: data, as: UTF8.self), + let document = try? FrontmatterDocument.parse(text) + else { return nil } + return document + } +} diff --git a/Kanban/History/NativeHistoryProvider.swift b/Kanban/History/NativeHistoryProvider.swift index 6a7f27a..ad41432 100644 --- a/Kanban/History/NativeHistoryProvider.swift +++ b/Kanban/History/NativeHistoryProvider.swift @@ -2,126 +2,117 @@ import Foundation // MARK: - NativeHistoryProvider -/// The base edition's undo substrate: one `NSUndoManager`-backed stack per board session -/// (13-native-undo.md). +/// The base edition's undo substrate: one stack per board session (13-native-undo.md). /// -/// ### Why `NSUndoManager` at all, when the steps are ours +/// ### Two arrays, and why not `NSUndoManager` /// -/// Because the *command surface* is the platform's. Edit ▸ Undo and Edit ▸ Redo are the system's own -/// nil-target `undo:`/`redo:` rows (11-command-nexus.md), the toolbar pair carries the same actions +/// This provider was an `NSUndoManager` for exactly one milestone, on the argument that the *command +/// surface* is the platform's — Edit ▸ Undo and Edit ▸ Redo are the system's own nil-target +/// `undo:`/`redo:` rows (11-command-nexus.md), the toolbar pair carries the same actions /// (`BoardToolbar`), and both light up, disable and **retitle** from whatever `UndoManager` the -/// focused window hands back — "Undo Move 3 Cards" is `NSUndoManager`'s dynamic retitling, which 13 -/// names as the mechanism and 12 records as the one both editions share. Reimplementing the stack -/// over two arrays would mean reimplementing that, badly. +/// focused window hands back. All of that is still true, and none of it lives here: the retitling is +/// `BoardUndoManager.undoMenuItemTitle` composing `undoMenuTitle(forUndoActionName:)` over the bare +/// phrase this seam vends as a `String?`. The adapter is the `UndoManager`; the substrate never +/// needed to be one. /// -/// It stays *behind* `HistoryProviding` regardless: nothing outside this file learns that base's -/// stack is an `NSUndoManager`, which is what lets pro-m1 bind git to the same seam -/// (12 ▸ The provider seam). +/// What forced the change is the staleness milestone's third outcome. `HistoryStepOutcome.failed` +/// means **the step stays put** — a disk error is retryable, so ⌘Z must still be able to reach the +/// step it just could not write. `NSUndoManager` pops a group before running it and offers no way to +/// put it back: a registration made while undoing lands on the *redo* stack by its own documented +/// rule, and one made after the crossing returns clears the redo stack outright. Either way a failed +/// undo would have quietly destroyed something. Two arrays express all three outcomes exactly, and +/// the grammar they have to implement is four lines long. /// /// ### One `register` call is exactly one step /// -/// `groupsByEvent` is turned **off** and every registration is wrapped in its own group. The default -/// (on) closes a group at the end of the run-loop turn, which would silently fold two gestures that -/// happened to land in one event into a single ⌘Z — the opposite of 13's "one gesture, one undo -/// step", and the coalescing decision belongs to the Writer call site (a multi-card move registers -/// *one* step with a plural title), never to the run loop's timing. +/// Nothing here groups, coalesces, or waits for the end of a run-loop turn — 13's "one gesture, one +/// undo step" is a property of the Writer call sites (a multi-card move registers *one* step with a +/// plural title), and the substrate's job is to not have opinions about it. This is what +/// `NSUndoManager`'s `groupsByEvent = false` was buying, as an absence rather than a setting. /// -/// ### Undo flips to redo by re-registering +/// ### Undo flips to redo by reversing /// -/// A step that applies pushes its own mirror image back onto the manager from inside the undo — and -/// `NSUndoManager` routes a registration made while it is undoing onto the **redo** stack, and vice -/// versa. That single rule gives the whole classic dance (undo → redo → undo …) with no second stack -/// of our own, and it is why a *skipped* step leaves nothing behind: it registers nothing, so the -/// empty group is discarded and the step is simply gone (13: "popped from the stack ... and ⌘Z falls -/// through to the next step"). +/// A step that applies is pushed onto the opposite stack **reversed** — its two halves swapped +/// (`HistoryStep.reversed`) — which gives the whole classic dance (undo → redo → undo …) with one +/// rule. Both stacks therefore hold steps oriented so that *crossing them means calling `undo`*, and +/// a skipped step leaves nothing behind at all: it is popped and never re-pushed, which is 13's +/// "popped from the stack ... and ⌘Z falls through to the next step". @MainActor public final class NativeHistoryProvider: HistoryProviding { - /// The stack. `private` and never vended: see the type's note. - private let manager = UndoManager() + /// The two stacks, top last. Both hold steps oriented for crossing — see the type's note. + private var undoSteps: [HistoryStep] = [] + private var redoSteps: [HistoryStep] = [] - /// Set by the step just crossed when it declined to apply, read by the crossing loop below. - /// A flag rather than a return value because the manager, not this object, calls the step. - private var lastCrossingSkipped = false - - public init() { - // See "One `register` call is exactly one step", above. - manager.groupsByEvent = false - } + public init() {} // MARK: - HistoryProviding - public var canUndo: Bool { manager.canUndo } + public var canUndo: Bool { !undoSteps.isEmpty } - public var canRedo: Bool { manager.canRedo } + public var canRedo: Bool { !redoSteps.isEmpty } - /// `NSUndoManager` answers `""` — not `nil` — for a stack with nothing on it *and* for a step - /// registered without a name, so the emptiness check is the honest one. - public var undoActionName: String? { - guard canUndo, !manager.undoActionName.isEmpty else { return nil } - return manager.undoActionName - } + /// The phrase the menu title is composed from, or `nil` when there is nothing to cross — and + /// also `nil` for a step registered without a name, which is the emptiness the adapter's `""` + /// contract is written against. + public var undoActionName: String? { name(of: undoSteps.last) } - public var redoActionName: String? { - guard canRedo, !manager.redoActionName.isEmpty else { return nil } - return manager.redoActionName - } + public var redoActionName: String? { name(of: redoSteps.last) } + /// Records one undoable step and clears the redo stack — the classic rule, and the one every + /// substrate shares. public func register(_ step: HistoryStep) { - push(step) + undoSteps.append(step) + redoSteps.removeAll() } - public func undo() { - cross(manager.undo, while: { self.manager.canUndo }) - } + public func undo() { cross(.undo) } - public func redo() { - cross(manager.redo, while: { self.manager.canRedo }) - } + public func redo() { cross(.redo) } public func clear() { - manager.removeAllActions() + undoSteps.removeAll() + redoSteps.removeAll() } - // MARK: - The stack + // MARK: - The crossing - /// Registers `step` as one group of its own, named for the menu. - /// - /// The target is `self` and the payload rides in the closure, which is `NSUndoManager`'s - /// block-based form: the manager references its target **unowned** (its own documented rule), so - /// the provider owning the manager that references the provider is not a cycle. - private func push(_ step: HistoryStep) { - manager.beginUndoGrouping() - manager.registerUndo(withTarget: self) { provider in - provider.apply(step) - } - // Inside the group, deliberately: the name belongs to the group being closed, and on the - // way back it is what retitles Redo. - manager.setActionName(step.name) - manager.endUndoGrouping() - } - - /// Runs one step's action and records what happened. - private func apply(_ step: HistoryStep) { - switch step.undo() { - case .applied: - // Lands on the opposite stack — see "Undo flips to redo by re-registering". - push(step.reversed) - case .skipped: - lastCrossingSkipped = true - } - } - - /// Crosses one step, and keeps going while the steps it crosses decline to apply — 13's + /// Crosses one step, and keeps going while the steps it crosses decline as **stale** — 13's /// fall-through: "the step is skipped, not applied ... and ⌘Z falls through to the next step". /// - /// The `while` guard is the manager's own emptiness, so a stack of nothing but stale steps - /// empties itself and stops rather than spinning. - private func cross(_ step: () -> Void, while more: () -> Bool) { - repeat { - guard more() else { return } - lastCrossingSkipped = false - step() - } while lastCrossingSkipped + /// The loop's own exit is an empty stack, so a stack of nothing but stale steps empties itself + /// and stops rather than spinning. The other two outcomes each end the crossing after one step: + /// an applied step is the ⌘Z the user asked for, and a failed one leaves the stack exactly as it + /// found it (`HistoryStepOutcome.failed`). + private func cross(_ direction: HistoryDirection) { + while let step = pop(direction) { + switch step.undo(direction) { + case .applied: + push(step.reversed, onto: direction.opposite) + return + case .skipped: + continue + case .failed: + push(step, onto: direction) + return + } + } + } + + private func pop(_ direction: HistoryDirection) -> HistoryStep? { + direction == .undo ? undoSteps.popLast() : redoSteps.popLast() + } + + private func push(_ step: HistoryStep, onto direction: HistoryDirection) { + if direction == .undo { + undoSteps.append(step) + } else { + redoSteps.append(step) + } + } + + private func name(of step: HistoryStep?) -> String? { + guard let name = step?.name, !name.isEmpty else { return nil } + return name } } diff --git a/Kanban/LiveStore/BannerCenter.swift b/Kanban/LiveStore/BannerCenter.swift index 9b42c93..4214423 100644 --- a/Kanban/LiveStore/BannerCenter.swift +++ b/Kanban/LiveStore/BannerCenter.swift @@ -320,6 +320,25 @@ public final class BannerCenter { signposts.insert(InfoSignpost(message: message), at: 0) } + /// **The skipped undo step** (13-native-undo.md ▸ Rules ▸ staleness validation): an inverse found + /// its target holding somebody else's newer value, so it was popped rather than applied and ⌘Z + /// fell through to the next step. This is the row that says so. + /// + /// **A signpost, and no new class** — the vocabulary's answer rather than a compromise. 13 asks + /// for an "info-tone banner", and 02-architecture.md § The banner surface gives the info tone + /// exactly two halves: the pinned in-progress row with its spinner, and the passive signpost. + /// Nothing is in flight here, so the passive half is the whole of the choice. It also reads + /// right: unlike a `loss` row (warning tone, "content that didn't arrive though nothing failed"), + /// **nothing was lost and nothing failed** — the file holds exactly what its most recent writer + /// meant it to, the stack moved on to a step that did apply, and the user's ⌘Z did something. A + /// row that ranks last and may collapse behind "+N more" is the honest weight for that: calm by + /// design, nothing gated on seeing it instantly. And it is emphatically not a `oneShot`, which + /// carries a `BoardWriteError` — a *failed* inverse posts one of those instead, and the two rows + /// must stay distinguishable (`HistoryStepOutcome`). + public func postSkippedStep(_ direction: HistoryDirection, subject: String) { + postSignpost(Self.skippedStepMessage(direction, subject: subject)) + } + /// One item a degraded paste could not bring its attachments with — what /// `degradedPasteMessage(for:)` names. /// @@ -796,6 +815,30 @@ public final class BannerCenter { return "Moved '\(name)' into attachments — \(subject)" } + /// The skipped-step line — 13-native-undo.md ▸ Rules' own example sentence, "Undo skipped — 'Fix + /// login' changed outside Lanework", with ⇧⌘Z's mirror ("Redo skipped — …"). + /// + /// **The verb is the command the user pressed**, not the half of the step that declined: a step + /// already undone sits on the redo stack reversed, so the closure ⇧⌘Z crosses is the one + /// registered as `redo`, and a sentence naming the half would tell the user they pressed the + /// other key (`HistoryDirection`). + /// + /// **`subject` is quoted whatever it names** — the item's title for a step with one target + /// ("'Fix login'"), the step's own 06 phrase for a batch or an untitled item ("'Move 3 Cards'"). + /// One sentence shape for both readings, chosen where the step is registered + /// (`BoardStore.registerStep`) because that is the only place that knows how many items it named. + /// + /// **"changed outside Lanework"** is the design's own wording and stays literal: it is the whole + /// explanation the row owes — the app did not decline out of caution, somebody else wrote to that + /// item, and the reason the step is gone is that applying it would have thrown their edit away. + public nonisolated static func skippedStepMessage(_ direction: HistoryDirection, subject: String) -> String { + let verb = switch direction { + case .undo: "Undo" + case .redo: "Redo" + } + return "\(verb) skipped — '\(subject)' changed outside Lanework" + } + /// The suspended-history line. It names the *consequence* the user cares about — undo and the /// flush-before-overwrite guarantee are degraded — rather than the git mechanics, and carries /// the diagnosis as its tail. diff --git a/Kanban/LiveStore/BoardStore.swift b/Kanban/LiveStore/BoardStore.swift index 6dd8ab8..5cc2516 100644 --- a/Kanban/LiveStore/BoardStore.swift +++ b/Kanban/LiveStore/BoardStore.swift @@ -879,11 +879,11 @@ public final class BoardStore { /// hand-written `width: 1` is legal and preserved until the app itself next edits width — the /// unchanged-units guard below skips it, so only a real change reaches the remove. private func writeLaneWidths(_ changes: [(id: ItemID, units: Int)]) { - let writes: [(folder: URL, units: Int, prior: FieldValue)] = changes.compactMap { change in + let writes: [(folder: URL, units: Int, prior: FieldValue, title: String?)] = changes.compactMap { change in guard let lane = snapshot.lanes.first(where: { $0.id == change.id }), LaneLayoutMath.displayUnits(of: lane) != change.units else { return nil } - return (rootURL.appendingPathComponent(change.id.rawValue), change.units, lane.width) + return (rootURL.appendingPathComponent(change.id.rawValue), change.units, lane.width, lane.title.value) } guard !writes.isEmpty else { return } @@ -900,7 +900,18 @@ public final class BoardStore { // resize → prior width (13-native-undo.md ▸ Rules). One step whatever the batch's size — the // menu items step every selected lane in one gesture, and one gesture is one step. - registerStep(HistoryPhrase.name(.resize, kind: .lane, count: writes.count)) { _ in + // + // The validated field is `width`, read as the app reads it: a lane landing on one unit has + // **no key at all** (the remove-at-default rule above), which is a real after-value and the + // one `nil` here means. The redo's expectation is the prior as the inverse restores it — + // `prior.value`, which is `nil` for a missing *and* for a malformed prior, exactly matching + // `restoreWidth`'s own reading. + registerStep( + HistoryPhrase.name(.resize, kind: .lane, count: writes.count), + subject: writes.count == 1 ? writes[0].title : nil, + undoExpects: writes.map { .live($0.folder, .width($0.units == 1 ? nil : $0.units)) }, + redoExpects: writes.map { .live($0.folder, .width($0.prior.value)) } + ) { _ in for write in writes { try BoardWriter.updateIndex(inItemFolder: write.folder, operation: .resize(title: nil)) { document in Self.restoreWidth(write.prior, in: &document) @@ -1029,6 +1040,7 @@ public final class BoardStore { /// gesture" — and the reload shows the true state, which is the honest one. public func applyStyle(to target: StyleTarget, background: StyleChange = .keep, icon: StyleChange = .keep) { let edits: [( + id: ItemID?, folder: URL, background: StyleChange, icon: StyleChange, @@ -1040,6 +1052,7 @@ public final class BoardStore { let icon = Self.effective(icon, against: subject.icon) guard background != .keep || icon != .keep else { return nil } return ( + id: subject.id, folder: subject.folder, background: background, icon: icon, @@ -1069,7 +1082,28 @@ public final class BoardStore { case .lane: .lane case .card: .card } - registerStep(HistoryPhrase.name(.restyle, kind: kind, count: edits.count)) { _ in + // **Per dimension, not per item**: a gesture that set only `background` validates only + // `background`, so a foreign `icon:` edit on the very same card leaves the step alone. That is + // the field-level predicate read at its narrowest, and it is free — `effective(_:against:)` + // has already narrowed each dimension to what this write actually changed. + let subject = edits.count == 1 + ? edits[0].id.flatMap { Self.liveItem($0, in: snapshot)?.title } + : nil + registerStep( + HistoryPhrase.name(.restyle, kind: kind, count: edits.count), + subject: subject, + undoExpects: edits.map { + .live($0.folder, fields: Self.styledFields(background: $0.background, icon: $0.icon)) + }, + redoExpects: edits.map { + .live($0.folder, fields: Self.restoredStyleFields( + background: $0.background, + priorBackground: $0.priorBackground, + icon: $0.icon, + priorIcon: $0.priorIcon + )) + } + ) { _ in for edit in edits { try BoardWriter.updateIndex(inItemFolder: edit.folder, operation: .style(title: nil)) { document in Self.restore(edit.priorBackground, to: FrontmatterKeys.background, in: &document) @@ -1216,7 +1250,7 @@ public final class BoardStore { // may have written is inside the captured bytes, so a redo puts the card back where the // gesture put it, not merely at the bottom of the lane. if let item = createdItem(at: laneFolder.appendingPathComponent(created.rawValue, isDirectory: true), kind: .card) { - registerCreation([item], kind: .card) + registerCreation([item], kind: .card, subject: title) } return created } @@ -1284,8 +1318,17 @@ public final class BoardStore { // rename → restore title (13-native-undo.md ▸ Rules). The prior title is the *typed* value, // `nil` for an untitled item — so undoing a rename that gave an untitled card a name takes // the `title` key away again rather than writing `title: ""`. + // + // The validated field is `title` and nothing else: an agent that restyles this very card + // between the rename and the ⌘Z has not touched what this step wrote, so the undo applies — + // "a foreign change to an unrelated item must not skip anything", read one level finer. let priorTitle = target.title - registerStep(HistoryPhrase.name(.rename, kind: target.cardID == nil ? .lane : .card)) { _ in + registerStep( + HistoryPhrase.name(.rename, kind: target.cardID == nil ? .lane : .card), + subject: newTitle ?? priorTitle, + undoExpects: [.live(folder, .title(newTitle))], + redoExpects: [.live(folder, .title(priorTitle))] + ) { _ in try Self.setTitle(priorTitle, at: folder) } redo: { _ in try Self.setTitle(newTitle, at: folder) @@ -1437,13 +1480,31 @@ public final class BoardStore { /// Liveness is `writeCardBody`'s deliberately blind walk (`cardBodyTarget`), so a session that /// ended because its card was tombstoned still registers — the keystrokes survived into the /// tombstoned folder, and their undo has to be able to reach the same place. + /// + /// ### Its staleness predicate is the bytes, and the side of the trash the card was on + /// + /// "Body steps compare bytes" (13 ▸ Rules), so the expectation is the whole body span as this + /// session left it — a foreign editor that changed one character of it skips the step rather than + /// throwing that character away. The liveness half is captured rather than assumed, for the same + /// reason this method resolves its folder blind: a session that ended *because* the card was + /// tombstoned belongs to a tombstoned card, and demanding a live one would make its own undo + /// stale the instant it was registered. public func registerBodyEdit(inCard cardID: ItemID, priorBody: String, newBody: String) { guard priorBody != newBody, let target = Self.cardBodyTarget(cardID, in: snapshot) else { return } let folder = rootURL .appendingPathComponent(target.laneID.rawValue, isDirectory: true) .appendingPathComponent(target.cardID.rawValue, isDirectory: true) - registerStep(HistoryPhrase.name(.edit, kind: .card)) { _ in + let lane = snapshot.lanes.first { $0.id == target.laneID } + let card = lane?.cards.first { $0.id == target.cardID } + let tombstoned = lane?.isDeleted == true || card?.isDeleted == true + + registerStep( + HistoryPhrase.name(.edit, kind: .card), + subject: card?.title.value, + undoExpects: [.item(folder, tombstoned: tombstoned, .body(newBody))], + redoExpects: [.item(folder, tombstoned: tombstoned, .body(priorBody))] + ) { _ in _ = try BoardWriter.writeBody(inItemFolder: folder, body: priorBody) } redo: { _ in _ = try BoardWriter.writeBody(inItemFolder: folder, body: newBody) @@ -1599,8 +1660,15 @@ public final class BoardStore { } guard landed != nil else { return } - // rename → restore title, at the one level with no item to aim at. - registerStep(HistoryPhrase.name(.rename, kind: .board)) { _ in + // rename → restore title, at the one level with no item to aim at. The board root is never + // tombstoned however its frontmatter reads (a board-level `deleted:` is a tolerated load + // warning), so `.live` here means exactly "the root is still readable". + registerStep( + HistoryPhrase.name(.rename, kind: .board), + subject: newTitle ?? priorTitle, + undoExpects: [.live(folder, .title(newTitle))], + redoExpects: [.live(folder, .title(priorTitle))] + ) { _ in try Self.setTitle(priorTitle, at: folder) } redo: { _ in try Self.setTitle(newTitle, at: folder) @@ -1669,7 +1737,12 @@ public final class BoardStore { // parent — the board root is the only one there is — so 06's vocabulary word for it is // Reorder, not Move. let restored = priorOrder - registerStep(HistoryPhrase.name(.reorder, kind: .lane)) { _ in + registerStep( + HistoryPhrase.name(.reorder, kind: .lane), + subject: lanes[from].title.value, + undoExpects: [.live(folder, .order(newOrder))], + redoExpects: [.live(folder, .order(restored))] + ) { _ in try Self.setOrder(restored, at: folder) } redo: { _ in try Self.setOrder(newOrder, at: folder) @@ -1745,7 +1818,12 @@ public final class BoardStore { // per gesture whatever the set's size" is the same sentence as one gesture, one undo step. let inverse = Array(zip(rewrites.map(\.folder), priorOrders)) let forward = rewrites - registerStep(HistoryPhrase.name(.reorder, kind: .lane, count: forward.count)) { _ in + registerStep( + HistoryPhrase.name(.reorder, kind: .lane, count: forward.count), + subject: members.count == 1 ? members[0].title.value : nil, + undoExpects: forward.map { .live($0.folder, .order($0.order)) }, + redoExpects: inverse.map { .live($0.0, .order($0.1)) } + ) { _ in for (folder, order) in inverse { try Self.setOrder(order, at: folder) } @@ -1788,6 +1866,9 @@ public final class BoardStore { let id: ItemID let laneID: ItemID let order: Double + /// What it is called, for the skip banner a stale step would raise — read here because the + /// snapshot this resolves against is the pre-write one, which is where a title still is. + let title: String? } /// `ids` narrowed to live cards under live lanes and sorted into **flatten order** — "lane @@ -1798,15 +1879,17 @@ public final class BoardStore { /// membership is a UUID set that vanished items leave silently (02-architecture.md), and "partial /// vanishing drops the survivors" is the design's own wording. private func draggedCards(_ ids: Set) -> [DraggedCard] { - var homes: [ItemID: (lane: ItemID, order: Double)] = [:] + var homes: [ItemID: (lane: ItemID, order: Double, title: String?)] = [:] for lane in snapshot.lanes where !lane.isDeleted { for card in lane.cards where !card.isDeleted { - homes[card.id] = (lane.id, card.order) + homes[card.id] = (lane.id, card.order, card.title.value) } } return SelectionGrammar.liveCards(in: snapshot) .filter { ids.contains($0) } - .compactMap { id in homes[id].map { DraggedCard(id: id, laneID: $0.lane, order: $0.order) } } + .compactMap { id in + homes[id].map { DraggedCard(id: id, laneID: $0.lane, order: $0.order, title: $0.title) } + } } /// The within-board card drop: `ids` land contiguously at logical position `index` among @@ -1899,7 +1982,17 @@ public final class BoardStore { ) } let crossedLanes = members.contains { $0.laneID != laneID } - registerStep(HistoryPhrase.name(crossedLanes ? .move : .reorder, kind: .card, count: arrivals.count)) { _ in + // The two lists are index-aligned mirror images — `inverse[i].from` is where the card is now + // and `forward[i].from` is where it was — so the expectations read as one swap: **the undo + // wants the card at its destination holding the rank the drop gave it; the redo wants it back + // at its origin holding the rank it left.** The destination *path* is the lane check: a card + // a foreign writer moved elsewhere leaves nothing there to validate. + registerStep( + HistoryPhrase.name(crossedLanes ? .move : .reorder, kind: .card, count: arrivals.count), + subject: members.count == 1 ? members[0].title : nil, + undoExpects: zip(inverse, forward).map { .live($0.from, .order($1.order)) }, + redoExpects: zip(inverse, forward).map { .live($1.from, .order($0.order)) } + ) { _ in for step in inverse { _ = try BoardWriter.moveItem( at: step.from, @@ -2561,7 +2654,11 @@ public final class BoardStore { // from what actually landed rather than from `urls`, so a batch that failed halfway still // hands ⌘Z exactly the cards it left behind. let items = created.compactMap { createdItem(at: $0.folder, kind: .card, attachments: [$0.source]) } - registerCreation(items, kind: .card) + registerCreation( + items, + kind: .card, + subject: created.count == 1 ? Self.cardTitle(forFile: created[0].source) : nil + ) } /// The title a dropped file's card takes: **the filename without its extension** @@ -2662,8 +2759,18 @@ public final class BoardStore { // reorder → restore original `order` (13-native-undo.md ▸ Rules). The step is named for the // *gesture's* subject — the cards the user was moving — not for every sibling the permutation // displaced, which is the same rule 06 applies to a commit subject. + // + // Every rewritten rank is validated, the displaced siblings' included: they are what this + // permutation wrote, so they are what it must find unchanged — the step is *named* for the + // gesture's subject and *validated* over its whole write. let steps = rewrites - registerStep(HistoryPhrase.name(.reorder, kind: .card, count: selection.ids.count)) { _ in + let moved = selection.ids + registerStep( + HistoryPhrase.name(.reorder, kind: .card, count: moved.count), + subject: moved.count == 1 ? moved.first.flatMap { Self.liveItem($0, in: snapshot)?.title } : nil, + undoExpects: steps.map { .live($0.folder, .order($0.to)) }, + redoExpects: steps.map { .live($0.folder, .order($0.from)) } + ) { _ in for step in steps { try Self.setOrder(step.from, at: step.folder) } @@ -2826,8 +2933,19 @@ public final class BoardStore { // tombstone → restore. The undo is Put Back's own write, which is what makes 13's "the stack // and the trash are two doors to the same tombstone state" true on disk rather than by // agreement — undoing a delete is *identical* in effect to Put Back. + // + // **Existence and liveness, no fields** (13: "existence/liveness for ... delete ... steps"): + // what a tombstone writes is the item's side of the trash, so that is the whole after-value. + // The `deleted` timestamp is deliberately *not* compared — it is a machine stamp rather than + // a decision, and an item somebody put back and re-deleted is still on the side this step + // left it on. let kind: HistoryPhrase.Kind = paths.contains(where: \.isLane) ? .lane : .card - registerStep(HistoryPhrase.name(.delete, kind: kind, count: folders.count)) { _ in + registerStep( + HistoryPhrase.name(.delete, kind: kind, count: folders.count), + subject: paths.count == 1 ? title(at: paths[0]) : nil, + undoExpects: folders.map { .tombstoned($0) }, + redoExpects: folders.map { .live($0) } + ) { _ in for folder in folders { try BoardWriter.restoreItem(at: folder) } @@ -2875,7 +2993,12 @@ public final class BoardStore { // restore (Put Back) → tombstone (13-native-undo.md ▸ Rules) — the trash pair read the other // way round from `tombstone(_:)`'s step. let kind: HistoryPhrase.Kind = paths.contains(where: \.isLane) ? .lane : .card - registerStep(HistoryPhrase.name(.restore, kind: kind, count: restored.count)) { _ in + registerStep( + HistoryPhrase.name(.restore, kind: kind, count: restored.count), + subject: paths.count == 1 ? title(at: paths[0]) : nil, + undoExpects: restored.map { .live($0.folder) }, + redoExpects: restored.map { .tombstoned($0.folder) } + ) { _ in for item in restored { try BoardWriter.updateIndex(inItemFolder: item.folder, operation: .delete(title: nil)) { document in Self.restoreTombstone(item.deleted, in: &document) @@ -2891,6 +3014,16 @@ public final class BoardStore { /// The `deleted` value a trash row currently carries — the one field a Put Back's inverse has to /// carry forward, and one the `ItemPath` vocabulary deliberately does not (a path is a location, /// not a reading of the file there). + /// What a trash-pair step's one item is called — the skip banner's quoted subject, `nil` for an + /// untitled item (which falls back to the step's own phrase) and for an id the snapshot has + /// already lost. A path is a location, not a reading of the file there, so this is the same + /// deliberate lookup `deletedField(at:)` is. + private func title(at path: TrashModel.ItemPath) -> String? { + guard let lane = snapshot.lanes.first(where: { $0.id == path.laneID }) else { return nil } + guard let cardID = path.cardID else { return lane.title.value } + return lane.cards.first(where: { $0.id == cardID })?.title.value + } + private func deletedField(at path: TrashModel.ItemPath) -> FieldValue { guard let lane = snapshot.lanes.first(where: { $0.id == path.laneID }) else { return .missing } guard let cardID = path.cardID else { return lane.deleted } @@ -3069,8 +3202,26 @@ public final class BoardStore { // restore → tombstone (13-native-undo.md ▸ Rules), with the position half of the gesture // walked back too: the row returns to the lane it was trashed in, at the rank it was trashed // holding, under the timestamp it was trashed at — which is exactly where its trash row was. + // + // Both halves of the gesture are validated, because both were written: the card must be live + // in the destination lane at the rank the drop gave it, and — the other way round — back in + // the lane it was trashed in, tombstoned, at the rank it was trashed holding. The same-lane + // case collapses to one folder and is still exactly this: the write that set no `order` set + // it to the value it already had. let steps = moves - registerStep(HistoryPhrase.name(.restore, kind: .card, count: steps.count)) { _ in + registerStep( + HistoryPhrase.name(.restore, kind: .card, count: steps.count), + subject: rows.count == 1 ? rows[0].card.title.value : nil, + undoExpects: steps.map { + .live(laneFolder.appendingPathComponent($0.cardID.rawValue, isDirectory: true), .order($0.order)) + }, + redoExpects: steps.map { + .tombstoned( + TrashModel.ItemPath(laneID: $0.laneID, cardID: $0.cardID).folder(under: root), + .order($0.priorOrder) + ) + } + ) { _ in for step in steps { let priorFolder = TrashModel.ItemPath(laneID: step.laneID, cardID: step.cardID).folder(under: root) if step.laneID != laneID { diff --git a/KanbanTests/HistoryProviderTests.swift b/KanbanTests/HistoryProviderTests.swift index 2018770..4eb1e2d 100644 --- a/KanbanTests/HistoryProviderTests.swift +++ b/KanbanTests/HistoryProviderTests.swift @@ -20,25 +20,35 @@ private final class StepLog { private(set) var crossings: [String] = [] + /// The direction each crossing was told it was — the argument the skip banner's verb comes from. + private(set) var directions: [HistoryDirection] = [] + /// A step that applies in both directions — the ordinary case. func step(_ name: String) -> HistoryStep { step(name, undo: .applied, redo: .applied) } - /// A step whose inverse declines — 13's staleness skip, without needing a foreign writer. + /// A step whose inverse declines as stale — 13's skip, without needing a foreign writer. func staleStep(_ name: String) -> HistoryStep { step(name, undo: .skipped, redo: .applied) } + /// A step whose inverse could not be written — the disk-error fate, which is not staleness. + func failingStep(_ name: String) -> HistoryStep { + step(name, undo: .failed, redo: .applied) + } + func step(_ name: String, undo: HistoryStepOutcome, redo: HistoryStepOutcome) -> HistoryStep { HistoryStep( name: name, - undo: { [weak self] in + undo: { [weak self] direction in self?.crossings.append("undo \(name)") + self?.directions.append(direction) return undo }, - redo: { [weak self] in + redo: { [weak self] direction in self?.crossings.append("redo \(name)") + self?.directions.append(direction) return redo } ) @@ -172,6 +182,55 @@ struct NativeHistoryProviderTests { #expect(provider.redoActionName == "Move Card") } + @Test("A step whose write failed stays on the stack, and the crossing stops there") + func aFailedStepStaysAndStopsTheCrossing() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Move Card")) + provider.register(log.failingStep("Rename Card")) + + provider.undo() + + // It was reached and it declined — and unlike a stale step it is still there to retry, with + // the step below it untouched underneath. + #expect(log.crossings == ["undo Rename Card"], "no fall-through: a refused disk is not a reason to try more") + #expect(provider.canUndo) + #expect(provider.undoActionName == "Rename Card") + #expect(provider.canRedo == false, "nothing landed, so nothing is redoable") + + provider.undo() + #expect(log.crossings == ["undo Rename Card", "undo Rename Card"], "⌘Z can retry it") + } + + @Test("A failed redo leaves the redo stack alone too") + func aFailedRedoStays() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Move Card", undo: .applied, redo: .failed)) + + provider.undo() + provider.redo() + + #expect(provider.canRedo, "still there to retry") + #expect(provider.redoActionName == "Move Card") + #expect(provider.canUndo == false) + } + + @Test("A step is told which command it is being crossed by, not which half is running") + func stepsAreToldTheDirection() { + let log = StepLog() + let provider = NativeHistoryProvider() + provider.register(log.step("Rename Card")) + + provider.undo() + provider.redo() + provider.undo() + + // The third crossing runs the *undo* half again, and the second runs the half registered as + // `redo` — what each is told is ⌘Z, ⇧⌘Z, ⌘Z, which is what the skip banner has to say. + #expect(log.directions == [.undo, .redo, .undo]) + } + @Test("A stack of nothing but stale steps empties itself and stops") func anEntirelyStaleStackEmptiesItself() { let log = StepLog() @@ -283,6 +342,26 @@ struct BoardUndoManagerTests { #expect(manager.redoMenuItemTitle == "Redo Rename Lane") } + @Test("A read-only board disables both directions, whatever the stack holds") + func theLockDisablesEnablement() { + final class Lock { var isOn = false } + let lock = Lock() + let provider = FakeHistoryProvider() + let manager = BoardUndoManager(history: provider, isReadOnly: { lock.isOn }) + provider.canUndo = true + provider.canRedo = true + #expect(manager.canUndo) + + lock.isOn = true + + #expect(manager.canUndo == false, "disabled with every other mutating command") + #expect(manager.canRedo == false) + #expect(provider.canUndo, "an enablement answer, not a clearing — the stack survives") + + lock.isOn = false + #expect(manager.canUndo, "and resumes when the lock clears") + } + @Test("Crossing forwards to the provider — what ⌘Z and the toolbar item actually reach") func crossingForwards() { let provider = FakeHistoryProvider() @@ -476,6 +555,27 @@ struct BoardSessionHistoryTests { #expect(log.crossings.isEmpty) } + @Test("The session's manager answers this board's own read-only lock") + func theSessionWiresTheLock() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (model, tearDown) = try makeModel() + defer { tearDown() } + + let ref = try openBoard(model, at: fixture.root) + let session = try #require(model.session(for: ref)) + session.history.register(StepLog().step("Move Card")) + #expect(session.undoManager.canUndo) + + session.store.enterVanishedRootLock() + #expect(session.undoManager.canUndo == false) + #expect(session.history.canUndo, "the stack itself survives the lock") + + session.store.handleWatcherEvent(.treeChanged(.appMediated)) + await session.store.awaitQuiescence() + #expect(session.undoManager.canUndo, "and resumes when it clears") + } + @Test("The composition root decides which provider a session gets") func theProviderIsBoundAtComposition() throws { let fixture = try makeBoard() diff --git a/KanbanTests/UndoWriteTests.swift b/KanbanTests/UndoWriteTests.swift index c60522f..404aa97 100644 --- a/KanbanTests/UndoWriteTests.swift +++ b/KanbanTests/UndoWriteTests.swift @@ -775,6 +775,477 @@ struct CrossingIsAWriteTests { } } +// MARK: - Staleness + +/// A writer that is not the app: `BoardWriter` reached **around** the store, which is exactly what an +/// agent, a hand edit or another editor is from the stack's point of view — a change no step was +/// registered for (13-native-undo.md ▸ Rules: "Foreign writes never join the stack ... collisions are +/// handled lazily, per step, by validation"). +private enum Foreign { + + static func rename(_ fixture: WriterFixture, _ path: String, to title: String) throws { + try BoardWriter.updateIndex(inItemFolder: fixture.url(path), operation: .rename(title: nil)) { document in + document.set(FrontmatterKeys.title, to: .string(title)) + } + } + + static func restyle(_ fixture: WriterFixture, _ path: String, background: String) throws { + try BoardWriter.updateIndex(inItemFolder: fixture.url(path), operation: .style(title: nil)) { document in + document.set(FrontmatterKeys.background, to: .string(background)) + } + } + + static func setOrder(_ fixture: WriterFixture, _ path: String, to order: Double) throws { + try BoardWriter.updateIndex(inItemFolder: fixture.url(path), operation: .reorder(title: nil)) { document in + document.set(FrontmatterKeys.order, to: .double(order)) + } + } + + static func setBody(_ fixture: WriterFixture, _ path: String, to body: String) throws { + _ = try BoardWriter.writeBody(inItemFolder: fixture.url(path), body: body) + } + + static func delete(_ fixture: WriterFixture, _ path: String) throws { + try BoardWriter.deleteItem(at: fixture.url(path)) + } + + static func purge(_ fixture: WriterFixture, _ path: String) throws { + try BoardWriter.purgeItem(at: fixture.url(path)) + } + + static func move(_ fixture: WriterFixture, _ path: String, toLane lane: String, order: Double) throws { + _ = try BoardWriter.moveItem( + at: fixture.url(path), + toParent: fixture.url(lane), + sourceBoardRoot: fixture.root, + destinationBoardRoot: fixture.root, + order: order + ) + } +} + +/// The staleness predicate at ⌘Z time (13-native-undo.md ▸ Rules ▸ staleness validation): "an inverse +/// operation re-checks its target against the current snapshot at ⌘Z time ... Target folder gone, or +/// the field no longer holding the step's after-value → the step is **skipped, not applied**: popped +/// from the stack with an info-tone banner ... and ⌘Z falls through to the next step." +/// +/// Every test here is the same hostile shape: perform a gesture, let somebody else write to the board +/// behind the app's back, then press ⌘Z and read the **file**. What must never happen is the inverse +/// landing on top of the foreign write. +@MainActor +@Suite("Undo ▸ staleness") +struct StaleStepTests { + + // MARK: Existence and liveness + + @Test("A foreign delete of the target skips its step — and ⌘Z falls through to the next one") + func aForeignDeleteSkipsAndFallsThrough() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + store.transient.beginRename(of: card2, currentTitle: "Second") + store.transient.updateRenameDraft("Second!") + store.commitRename() + + try Foreign.delete(fixture, card2Path) + history.undo() + + // The top step's card is in the trash now, so its rename is not ours to walk back; the one + // below it is untouched and applies in the same ⌘Z. + #expect(try document(fixture, card2Path).title.value == "Second!", "the foreign writer's board, left alone") + #expect(try document(fixture, card1Path).title.value == "First", "⌘Z fell through and did something") + #expect(store.banners.signposts.map(\.message) + == ["Undo skipped — 'Second!' changed outside Lanework"]) + #expect(history.canUndo == false, "both steps were consumed — one skipped, one applied") + #expect(history.redoActionName == "Rename Card", "only the step that ran is redoable") + } + + @Test("A target the foreign writer removed outright skips rather than failing") + func aVanishedTargetSkips() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.delete([card1]) + try Foreign.purge(fixture, card1Path) + + history.undo() + + #expect(fixture.exists(card1Path) == false) + #expect(store.banners.signposts.count == 1) + #expect(store.banners.oneShots.isEmpty, "a skip is not a write failure — no error row") + #expect(history.canUndo == false) + #expect(history.canRedo == false, "a skipped step leaves nothing behind") + } + + @Test("A foreign Put Back skips the delete's undo — the item is not on the side we left it") + func aForeignRestoreSkipsTheDeleteStep() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.delete([card1]) + try BoardWriter.restoreItem(at: fixture.url(card1Path)) + + history.undo() + + #expect(try document(fixture, card1Path).deleted.isMissing, "still live, as they left it") + #expect(store.banners.signposts.map(\.message) == ["Undo skipped — 'First' changed outside Lanework"]) + } + + @Test("A card a foreign writer moved away leaves nothing at the destination to walk back") + func aForeignMoveSkipsTheMoveStep() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.moveCards([card1], toLane: lane2, at: 0) + try Foreign.move(fixture, "\(Ident.lane2)/\(Ident.card1)", toLane: Ident.lane1, order: 5000) + + history.undo() + + #expect(fixture.exists(card1Path), "where the foreign writer put it") + #expect(try document(fixture, card1Path).order.value == 5000, "at the rank they gave it, not ours") + #expect(store.banners.signposts.count == 1) + #expect(history.canUndo == false) + } + + @Test("A tombstoned card is stale for a field edit, even with the field itself untouched") + func aTombstonedTargetIsStaleForAFieldEdit() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.setLaneWidth(lane1, units: 3) + try Foreign.delete(fixture, Ident.lane1) + + history.undo() + + #expect(try document(fixture, Ident.lane1).width.value == 3, "not resized inside the trash") + #expect(store.banners.signposts.count == 1) + } + + @Test("A card under a foreign-tombstoned lane is stale too — liveness is ancestor-walked") + func anAncestorTombstoneIsStale() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + try Foreign.delete(fixture, Ident.lane1) + + history.undo() + + #expect(try document(fixture, card1Path).title.value == "Renamed", "the card renders nowhere; nothing was written") + #expect(store.banners.signposts.count == 1) + } + + // MARK: The field-level predicate + + @Test("A foreign edit of the very field the step wrote skips it") + func aForeignFieldEditSkips() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + + try Foreign.rename(fixture, card1Path, to: "Theirs") + history.undo() + + #expect(try document(fixture, card1Path).title.value == "Theirs", + "never apply a stale inverse on top of someone else's newer write") + #expect(store.banners.signposts.map(\.message) == ["Undo skipped — 'Renamed' changed outside Lanework"]) + #expect(history.canUndo == false) + #expect(history.canRedo == false) + } + + @Test("A foreign change to an unrelated item skips nothing — the predicate is per target") + func anUnrelatedForeignChangeAppliesNormally() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + + try Foreign.rename(fixture, card2Path, to: "Theirs") + try Foreign.delete(fixture, card3Path) + history.undo() + + #expect(try document(fixture, card1Path).title.value == "First", "the step applied") + #expect(try document(fixture, card2Path).title.value == "Theirs", "and left the neighbours alone") + #expect(store.banners.signposts.isEmpty, "nothing to explain") + #expect(history.redoActionName == "Rename Card") + } + + @Test("A foreign change to another field of the same item skips nothing either") + func anUnrelatedFieldOfTheSameItemApplies() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + + try Foreign.restyle(fixture, card1Path, background: "red") + history.undo() + + let undone = try document(fixture, card1Path) + #expect(undone.title.value == "First", "the rename walked back") + #expect(undone.background.value == "red", "their colour survived it") + #expect(store.banners.signposts.isEmpty) + } + + @Test("A foreign rank change skips a reorder") + func aForeignRankSkipsAReorder() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.moveLane(lane1, toIndex: 1) + try Foreign.setOrder(fixture, Ident.lane1, to: 9000) + + history.undo() + + #expect(try document(fixture, Ident.lane1).order.value == 9000) + #expect(store.banners.signposts.map(\.message) == ["Undo skipped — 'Todo' changed outside Lanework"]) + } + + @Test("A foreign body edit skips the Edit session's step — body steps compare bytes") + func aForeignBodyEditSkipsTheSessionStep() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + let original = try FrontmatterDocument.parse(fixture.indexText(card1Path)).body + + let session = CardBodyEditSession() + session.save = { [weak store] text in store?.writeCardBody(inCard: card1, body: text) ?? .vanished } + session.registerUndo = { [weak store] prior, new in + store?.registerBodyEdit(inCard: card1, priorBody: prior, newBody: new) + } + session.adopt(diskBody: original) + session.edited("Mine.\n") + _ = session.flush() + session.endEditSession() + #expect(history.undoActionName == "Edit Card") + + // One character of difference is a different body: the step wrote every byte of the span. + try Foreign.setBody(fixture, card1Path, to: "Mine.\nAnd theirs.\n") + history.undo() + + #expect(try FrontmatterDocument.parse(fixture.indexText(card1Path)).body == "Mine.\nAnd theirs.\n") + #expect(store.banners.signposts.map(\.message) == ["Undo skipped — 'First' changed outside Lanework"]) + } + + // MARK: The banner + + @Test("A batch names the step, since there is no single item to name") + func aBatchStepNamesItself() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.applyStyle(to: .items([card1, card2, card3]), background: .set("red")) + try Foreign.restyle(fixture, card2Path, background: "green") + + history.undo() + + #expect(store.banners.signposts.map(\.message) + == ["Undo skipped — 'Restyle 3 Cards' changed outside Lanework"]) + #expect(try document(fixture, card1Path).background.value == "red", "all or nothing: no half-applied batch") + } + + @Test("The skip row is an info-tone signpost — dismissable, and never an error") + func theSkipRowIsASignpost() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.delete([card1]) + try Foreign.purge(fixture, card1Path) + history.undo() + + let row = try #require(store.bannerRows.last) + #expect(row.tone == .info) + #expect(row.dismissID != nil, "one-shot lifecycle: the user clears it, nothing expires it") + #expect(store.banners.oneShots.isEmpty) + #expect(store.banners.losses.isEmpty) + } + + // MARK: Redo + + @Test("Redo validates the same way, and says so in its own verb") + func redoStalenessIsSymmetric() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + history.undo() + #expect(try document(fixture, card1Path).title.value == "First") + + // Somebody writes over the state the undo left, so the *forward* write is now the stale one. + try Foreign.rename(fixture, card1Path, to: "Theirs") + history.redo() + + #expect(try document(fixture, card1Path).title.value == "Theirs") + #expect(store.banners.signposts.map(\.message) + == ["Redo skipped — 'Renamed' changed outside Lanework"]) + #expect(history.canRedo == false) + } + + @Test("An undone create redoes only onto the hole it left") + func redoOfACreateChecksTheHole() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + let before = try fixture.entryNames("") + + store.createLane() + let created = try #require(try fixture.entryNames("").first { !before.contains($0) }) + history.undo() + #expect(fixture.exists(created) == false) + + // Somebody puts a folder back at that identity — the redo's `recreateItem` refuses to + // clobber, so validation is what turns that into a skip rather than a write failure. + try fixture.item(created, Item.rich(order: "4096", title: "Theirs")) + history.redo() + + #expect(try document(fixture, created).title.value == "Theirs") + #expect(store.banners.signposts.count == 1) + #expect(store.banners.oneShots.isEmpty, "skipped before the Writer was ever reached") + } +} + +// MARK: - Stale versus failed + +/// The distinction 13 leaves to the implementation and this milestone settles: a **stale** step is +/// one the board has moved past — dropped, explained by the info row, ⌘Z falls through. A **failed** +/// one is a step the user still means to cross, refused by a condition that is usually momentary — +/// so it stays put, the ordinary write-failure error row says why, and ⌘Z retries it. +@MainActor +@Suite("Undo ▸ stale versus failed") +struct FailedCrossingTests { + + @Test("An inverse that cannot be written keeps its step, and banners as a write failure") + func aFailedInverseKeepsItsStep() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + + // The read and the parse succeed — so validation passes, and the step is genuinely current — + // and then the atomic replace has nowhere to land its temp file. + try FileManager.default.setAttributes([.posixPermissions: 0o500], ofItemAtPath: fixture.url(card1Path).path) + history.undo() + + #expect(try document(fixture, card1Path).title.value == "Renamed", "nothing landed") + #expect(store.banners.oneShots.count == 1, "the ordinary write-failure row, not a skip") + #expect(store.banners.signposts.isEmpty) + #expect(history.canUndo, "the step stays: a full disk is not a reason to lose the way back") + #expect(history.undoActionName == "Rename Card") + #expect(history.canRedo == false) + + // And when the condition clears, the same ⌘Z works. + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: fixture.url(card1Path).path) + history.undo() + + #expect(try document(fixture, card1Path).title.value == "First") + #expect(history.canRedo) + } + + @Test("A failed crossing stops rather than falling through to the steps below it") + func aFailedCrossingDoesNotFallThrough() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.delete([card2]) + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + + try FileManager.default.setAttributes([.posixPermissions: 0o500], ofItemAtPath: fixture.url(card1Path).path) + history.undo() + + #expect(try document(fixture, card2Path).deleted.value != nil, + "the step below was never reached — a refused disk is not a reason to attempt more") + #expect(history.undoActionName == "Rename Card") + } +} + +// MARK: - The read-only lock + +/// "Every read-only lock ... disables Undo/Redo with the other mutating commands; the stack itself +/// survives the lock and resumes when it clears" (13-native-undo.md ▸ Rules). +@MainActor +@Suite("Undo ▸ the read-only lock") +struct LockedBoardUndoTests { + + @Test("A locked board disables Undo and Redo — and the stack is still there when it clears") + func theLockDisablesAndTheStackSurvives() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + let manager = BoardUndoManager(history: history, isReadOnly: { [weak store] in store?.isReadOnly ?? false }) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + #expect(manager.canUndo) + + store.enterVanishedRootLock() + + #expect(manager.canUndo == false, "disabled with every other mutating command") + #expect(manager.canRedo == false) + #expect(history.canUndo, "the stack itself survives the lock") + #expect(manager.undoMenuItemTitle == "Undo Rename Card", "a disabled row keeps its name") + + // The lock clears on the next successful reload, and the same ⌘Z crosses the same step. + await reload(store) + #expect(store.isReadOnly == false) + #expect(manager.canUndo) + + manager.undo() + #expect(try document(fixture, card1Path).title.value == "First") + } + + @Test("A crossing that starts anyway is refused without losing the step") + func aCrossingUnderTheLockLosesNothing() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let (store, history) = try makeStore(fixture) + + store.transient.beginRename(of: card1, currentTitle: "First") + store.transient.updateRenameDraft("Renamed") + store.commitRename() + store.enterVanishedRootLock() + + history.undo() + + #expect(try document(fixture, card1Path).title.value == "Renamed", "the lock refused the write") + #expect(history.canUndo, "a refusal is a failure, not a staleness — the step stays") + #expect(store.banners.signposts.isEmpty, "the standing lock row is the message") + #expect(store.banners.oneShots.isEmpty, "and a refused write posts nothing of its own") + } +} + // MARK: - The phrase vocabulary @Suite("Undo ▸ the phrase vocabulary")