Surface write failures — banners, locks, one modal
The banner surface as one vocabulary (Kanban/LiveStore/BannerCenter, Kanban/UI/BannerStripView): a pure precedence rule — in-progress pinned above the collapse (ratified mid-build), lock > breakage > one-shot write failures > commit+attachment, signposts last — with all user-facing phrasing owned here via exhaustive switches over the closed WriteOperation enum; free-form English survives only in diagnostics. performWrite posts its failures before rethrowing, so no one-shot can bypass the strip; refusals under lock post nothing. The lock vocabulary completes: vanishedRoot and unwritableLocation join bracketedReloadFailed, each with its own clearing rule (unwritable clears only on a reconciling reload's writability re-probe). The registry now owns root recovery: bookmark re-resolution absorbs renames transparently, a dead root locks read-only and re-arms FSEvents on the gone path so the root's return round-trips back through rootChanged, re-minting and re-keying on the way. DirtyBufferGuard is the one modal moment, retry / save a copy / discard, no fourth button. 36 new tests; full suite 333 tests in 62 suites green. Five findings filed on the Redesign board. Claude-Session: https://claude.ai/code/session_018BjQRYBR6jQja3jCRi5S3A
This commit is contained in:
@@ -4,21 +4,47 @@ import os
|
||||
|
||||
// MARK: - Vocabulary
|
||||
|
||||
/// Why a board is refusing writes — the read-only lock's cause, and the whole of its vocabulary
|
||||
/// today.
|
||||
/// Why a board is refusing writes — the read-only lock's cause, and now the whole of the
|
||||
/// vocabulary 02-architecture.md names.
|
||||
///
|
||||
/// 02-architecture.md names three ways a board enters the lock and only the first is built here.
|
||||
/// `vanishedRoot` (§ Write-failure surfacing — a root that is gone, after bookmark re-resolution
|
||||
/// found nothing) and `unwritableLocation` (§ — a read-only volume or a permission-denied folder,
|
||||
/// probed at open) arrive with the cards that implement them. The banner turns a case of this into
|
||||
/// user-facing phrasing (§ The banner surface); the store only owns the truth of it.
|
||||
/// The three cases share one *scope* (§ "The lock's scope") — every mutating command disabled
|
||||
/// across every window sharing the store, drops refused, ⌘-drag moves degraded to copies, editor
|
||||
/// buffers kept but their debounced saves suspended — and differ only in cause and in **what
|
||||
/// clears them**, which is the one thing this enum's cases are actually asked about (see
|
||||
/// `BoardStore.land(_:generation:origin:)`). The banner turns a case of this into user-facing
|
||||
/// phrasing (§ The banner surface); the store only owns the truth of it.
|
||||
public enum ReadOnlyLockReason: Sendable, Equatable {
|
||||
/// A reload that followed a bracketed wholesale operation failed, so the last-good snapshot on
|
||||
/// screen may describe a tree that no longer exists — after a branch switch, a different branch
|
||||
/// entirely. Writes derived from it would land nonsense, so every write is refused until a
|
||||
/// reload succeeds (02-architecture.md § Live-reload resilience, "A failed reload after a
|
||||
/// bracketed operation locks the board read-only").
|
||||
///
|
||||
/// **Clears on the next successful reload, whatever its origin** — typically once the offending
|
||||
/// file is fixed.
|
||||
case bracketedReloadFailed
|
||||
|
||||
/// The board's root is gone and its bookmark re-resolution found nothing: the volume unmounted,
|
||||
/// or the folder was deleted in Finder while the board was open (02-architecture.md §
|
||||
/// Write-failure surfacing, "A vanished board root locks the board read-only"). Every write
|
||||
/// would land nowhere, so the last-good snapshot stays on screen, read-only.
|
||||
///
|
||||
/// **Clears on the next successful reload, whatever its origin**: a reload can only succeed if
|
||||
/// the root is back, so success *is* the return signal. Pending dirty buffers then save
|
||||
/// normally.
|
||||
case vanishedRoot
|
||||
|
||||
/// The board opened somewhere it cannot be written: a read-only volume (a DMG, a snapshot, a
|
||||
/// read-only share) or a permission-denied folder (02-architecture.md § Write-failure
|
||||
/// surfacing, "An unwritable board location enters the read-only lock at open"). Failing
|
||||
/// loudly, specifically, *once* beats letting every gesture fail one at a time.
|
||||
///
|
||||
/// **Clears only on a successful *reconciling* reload whose writability re-probe passes** —
|
||||
/// unlike its two siblings, whose cause a successful reload disproves by itself. A board on a
|
||||
/// read-only DMG reloads perfectly all day long; only the probe (§ "Writability re-probes on
|
||||
/// every reconciling reload" — wake, activation) can tell that the permission or the mount
|
||||
/// actually changed.
|
||||
case unwritableLocation
|
||||
}
|
||||
|
||||
/// The refusal `BoardStore.performWrite` throws when the board is locked read-only.
|
||||
@@ -185,11 +211,50 @@ public final class BoardStore {
|
||||
/// The board's selection, re-resolved against every snapshot this store applies.
|
||||
public private(set) var selection: Selection
|
||||
|
||||
/// The board root this store was opened on. Constant for now: absorbing a rename by
|
||||
/// re-resolving the security-scoped bookmark (02-architecture.md § Write-failure surfacing) is
|
||||
/// the registry's job and a later card's, and it will arrive together with `.rootChanged`
|
||||
/// handling.
|
||||
public let rootURL: URL
|
||||
/// Where the board is **now**. Follows the folder: a rename or a move absorbed through
|
||||
/// `relocate(to:)` updates it, so every URL derived from it — the Writer's paths, card-window
|
||||
/// keys, Reveal in Finder — re-derives at the new location (02-architecture.md §
|
||||
/// Write-failure surfacing, "A renamed or moved board root follows its file identity").
|
||||
///
|
||||
/// Observed, deliberately: the window title's folder-name fallback reads this, and a rename in
|
||||
/// Finder should be visible in the title bar without anything else being told.
|
||||
///
|
||||
/// `snapshot.rootURL` is the root the *last successful reload* walked, and therefore lags this
|
||||
/// by exactly one reload during an absorption. That is not a second source of truth: the
|
||||
/// relocation is always followed by the watcher reattach whose reconciling reload rebuilds the
|
||||
/// snapshot at the new root, after which the two agree again.
|
||||
public private(set) var rootURL: URL
|
||||
|
||||
// MARK: Banners
|
||||
|
||||
/// The board window's banner strip, as a model (02-architecture.md § The banner surface).
|
||||
///
|
||||
/// **Owned, not injected**, and the reason is the hosting rule: the strip is "hosted by the
|
||||
/// window of origin", and a board window's strip has exactly one lifetime — this store's. A
|
||||
/// card window (m6) gets its *own* center for its own save, attachment, and raw-source Apply
|
||||
/// failures, and re-homes those rows here when it closes; injecting a shared center would
|
||||
/// erase precisely that distinction.
|
||||
///
|
||||
/// It holds only what nothing else does — one-shot write failures, the history suspension,
|
||||
/// in-progress operations, passive signposts. The lock and the reload breakage stay this
|
||||
/// store's own state and are composed in at render time by `bannerRows`.
|
||||
public let banners = BannerCenter()
|
||||
|
||||
/// The rows the board window's strip renders, in precedence order.
|
||||
///
|
||||
/// Composed rather than stored: `readOnlyLock` and `reloadFailure` are the store's truths and
|
||||
/// `banners` holds the rest, so a stored array would be a third copy waiting to go stale. The
|
||||
/// ordering rule itself lives in `BannerCenter.rows(...)`, which is pure and tested on its own.
|
||||
public var bannerRows: [BannerRow] {
|
||||
BannerCenter.rows(
|
||||
lock: readOnlyLock,
|
||||
breakage: reloadFailure,
|
||||
oneShots: banners.oneShots,
|
||||
suspension: banners.historySuspension,
|
||||
operations: banners.operations,
|
||||
signposts: banners.signposts
|
||||
)
|
||||
}
|
||||
|
||||
// MARK: Wiring
|
||||
|
||||
@@ -200,6 +265,18 @@ public final class BoardStore {
|
||||
@ObservationIgnored
|
||||
public var watcherBrackets: (begin: @MainActor () -> Void, end: @MainActor () -> Void)?
|
||||
|
||||
/// What to do when the watched root changes identity — injected for the same reason the
|
||||
/// brackets are: the response needs the board's **security-scoped bookmark**, and this store
|
||||
/// does not own one (the registry does, along with the watcher that must be re-attached and the
|
||||
/// last-known path that arms the return detection). A store that reached for a bookmark it did
|
||||
/// not hold could not be built or tested without a registry.
|
||||
///
|
||||
/// `nil` keeps the documented no-op: the last-good snapshot stays on screen, which is what
|
||||
/// every other failure path here does, and which is exactly the shape unit tests and any
|
||||
/// storeless use want. `BoardStoreRegistry` wires it to its own recovery loop.
|
||||
@ObservationIgnored
|
||||
public var rootChangeDelegate: (@MainActor () -> Void)?
|
||||
|
||||
// MARK: Reload machinery
|
||||
|
||||
/// Monotonic id of the most recently *started* reload — and therefore also the number of tree
|
||||
@@ -295,16 +372,22 @@ public final class BoardStore {
|
||||
requestReload(origin)
|
||||
|
||||
case .rootChanged:
|
||||
// Stubbed on purpose, and the stub is the whole of the honest answer today: the settled
|
||||
// response is to re-resolve the board's security-scoped bookmark and either
|
||||
// `reattach(to:)` the watcher at the new location — a rename absorbed with no banner and
|
||||
// no lock — or enter the vanished-root read-only lock (02-architecture.md §
|
||||
// Write-failure surfacing). Both halves need the registry's bookmark, which this store
|
||||
// deliberately does not own yet, and a guess here would be a *wrong* guess: treating a
|
||||
// rename as a vanish would lock a board that is merely somewhere else. The card that
|
||||
// brings the bookmark replaces this case; until then a root change simply leaves the
|
||||
// last-good snapshot on screen, which is the same thing every failure path does.
|
||||
Self.logger.debug("rootChanged ignored — bookmark re-resolution is not built yet")
|
||||
// Delegated, never guessed at. The settled response is to re-resolve the board's
|
||||
// security-scoped bookmark and either `reattach(to:)` the watcher at the new location —
|
||||
// a rename absorbed with no banner and no lock — or enter the vanished-root read-only
|
||||
// lock (02-architecture.md § Write-failure surfacing). Both halves need a bookmark this
|
||||
// store does not own, and a guess here would be a *wrong* guess: treating a rename as a
|
||||
// vanish would lock a board that is merely somewhere else.
|
||||
//
|
||||
// No reload is started either way. The delegate's two outcomes both end in one —
|
||||
// `reattach(to:)`'s reconciling reload at the re-resolved root, or the vanished-root
|
||||
// lock's eventual clearance when the root returns — and a reload fired from here would
|
||||
// walk a path that just stopped being the board.
|
||||
guard let rootChangeDelegate else {
|
||||
Self.logger.debug("rootChanged ignored — no delegate is wired (storeless use)")
|
||||
return
|
||||
}
|
||||
rootChangeDelegate()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -350,26 +433,26 @@ public final class BoardStore {
|
||||
outcome = .failure(error)
|
||||
}
|
||||
await barrier?()
|
||||
await self?.apply(outcome, generation: generation)
|
||||
await self?.apply(outcome, generation: generation, origin: origin)
|
||||
}
|
||||
}
|
||||
|
||||
/// Lands one walk's result and starts whatever it uncovered.
|
||||
private func apply(_ outcome: Result<LoadResult, BoardLoadError>, generation: Int) {
|
||||
private func apply(_ outcome: Result<LoadResult, BoardLoadError>, generation: Int, origin: WatchOrigin) {
|
||||
reloadInFlight = false
|
||||
|
||||
// The stale-apply guard. Serialization means this should not trigger today, but "only the
|
||||
// newest result applies" is the rule the wholesale floor and every future overlapping-load
|
||||
// change are written against, so it is enforced rather than assumed.
|
||||
if generation == reloadGeneration {
|
||||
land(outcome, generation: generation)
|
||||
land(outcome, generation: generation, origin: origin)
|
||||
}
|
||||
|
||||
startPendingReload()
|
||||
resumeQuiescenceWaitersIfQuiet()
|
||||
}
|
||||
|
||||
private func land(_ outcome: Result<LoadResult, BoardLoadError>, generation: Int) {
|
||||
private func land(_ outcome: Result<LoadResult, BoardLoadError>, generation: Int, origin: WatchOrigin) {
|
||||
// Consumed here, before the branch, because *both* outcomes end the expectation: a wholesale
|
||||
// operation gets exactly one reload to prove itself, and a second failure after it is
|
||||
// ordinary per-file breakage again.
|
||||
@@ -385,10 +468,10 @@ public final class BoardStore {
|
||||
case let .success(result):
|
||||
snapshot = result.model
|
||||
loadWarnings = result.warnings
|
||||
// One success clears both conditions — transient breakage self-heals and a locked board
|
||||
// unlocks without the user doing anything but fixing the file.
|
||||
// Breakage always heals on a success — it *is* the claim "the last reload failed", and
|
||||
// this one did not.
|
||||
reloadFailure = nil
|
||||
readOnlyLock = nil
|
||||
clearLockIfDisproved(by: origin)
|
||||
selection = selection.resolved(against: result.model)
|
||||
|
||||
case let .failure(error):
|
||||
@@ -396,7 +479,13 @@ public final class BoardStore {
|
||||
// replaces a good snapshot, and a selection over a snapshot that did not change has
|
||||
// nothing to re-resolve against.
|
||||
reloadFailure = error
|
||||
if endsWholesaleOperation {
|
||||
// `readOnlyLock == nil` rather than an unconditional assignment: a root that vanished
|
||||
// mid-bracket already raised its own, truer lock, and 02-architecture.md is explicit
|
||||
// that the bracket's final reload becomes a no-op there rather than a redundant
|
||||
// failure. Overwriting `.vanishedRoot` with `.bracketedReloadFailed` would also break
|
||||
// the clearing rules — the vanished root's lock must not clear on a reload that never
|
||||
// proves the root came back.
|
||||
if endsWholesaleOperation, readOnlyLock == nil {
|
||||
readOnlyLock = .bracketedReloadFailed
|
||||
}
|
||||
Self.logger.error("reload \(generation, privacy: .public) failed: \(error.description, privacy: .public)")
|
||||
@@ -409,6 +498,88 @@ public final class BoardStore {
|
||||
startReload(origin)
|
||||
}
|
||||
|
||||
// MARK: - The lock's clearing rules
|
||||
|
||||
/// Clears the read-only lock if this successful reload actually disproved its cause.
|
||||
///
|
||||
/// **Reason-specific, because the causes are not alike** (02-architecture.md § Write-failure
|
||||
/// surfacing):
|
||||
///
|
||||
/// - `.bracketedReloadFailed` and `.vanishedRoot` are *disproved by the success itself*. The
|
||||
/// first says "the tree could not be re-read after a wholesale change" and the second says
|
||||
/// "the root is gone" — a completed tree walk at the root contradicts both, whatever origin
|
||||
/// asked for it, so any success clears them.
|
||||
/// - `.unwritableLocation` is not. A board on a read-only DMG reloads flawlessly forever;
|
||||
/// loading proves nothing about writing. It clears only when a **reconciling** reload — wake,
|
||||
/// activation, a stream re-creation — re-probes writability and finds it changed ("Writability
|
||||
/// re-probes on every reconciling reload, so a fixed permission or rewritable remount clears
|
||||
/// the lock without ceremony").
|
||||
///
|
||||
/// `FileManager.isWritableFile(atPath:)` is `access(2)` on the root directory: a real-uid
|
||||
/// permission question asked of the filesystem, which is what makes it answer correctly for
|
||||
/// both halves of the case — a read-only *mount* and a permission-denied *folder*.
|
||||
///
|
||||
/// Deliberately **one-way**: a reconciling reload that finds the root unwritable does not
|
||||
/// *raise* the lock. Arming it is the open flow's job (`enterUnwritableLock()`), and inferring
|
||||
/// a lock from a probe here would be a policy decision this milestone was not asked to make.
|
||||
private func clearLockIfDisproved(by origin: WatchOrigin) {
|
||||
switch readOnlyLock {
|
||||
case nil:
|
||||
break
|
||||
case .bracketedReloadFailed, .vanishedRoot:
|
||||
readOnlyLock = nil
|
||||
case .unwritableLocation:
|
||||
guard origin == .reconciling, FileManager.default.isWritableFile(atPath: rootURL.path) else { return }
|
||||
Self.logger.debug("writability re-probe passed — the unwritable-location lock clears")
|
||||
readOnlyLock = nil
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Root identity and explicit locks
|
||||
|
||||
/// Points this store at the board's new location, absorbing a rename or a move.
|
||||
///
|
||||
/// Called by the registry when a `.rootChanged` re-resolved the board's bookmark somewhere else
|
||||
/// (02-architecture.md § Write-failure surfacing, "A renamed or moved board root follows its
|
||||
/// file identity"): the board the app has open is the *file*, not the path string, so this is
|
||||
/// bookkeeping rather than an event — **no banner, no lock, nothing was ever wrong**.
|
||||
///
|
||||
/// It deliberately starts **no reload**. The caller follows this with the watcher's
|
||||
/// `reattach(to:)`, whose reconciling reload is the one that rebuilds the snapshot at the new
|
||||
/// root; a reload fired from here would be a second walk racing that one for no gain. Until it
|
||||
/// lands, `rootURL` is the new location and `snapshot.rootURL` is still the old — see
|
||||
/// `rootURL`'s note.
|
||||
public func relocate(to newRoot: URL) {
|
||||
guard newRoot != rootURL else { return }
|
||||
Self.logger.debug("board root relocated; Writer URLs now derive from the new location")
|
||||
rootURL = newRoot
|
||||
}
|
||||
|
||||
/// Raises the vanished-root read-only lock — the registry's call, after bookmark re-resolution
|
||||
/// found nothing and the last-known path is not there either.
|
||||
///
|
||||
/// Overwrites whatever lock was standing: a root that is gone is the most current and most
|
||||
/// specific truth about why writes are refused, and its clearing rule (a successful reload,
|
||||
/// which can only happen if the root came back) is strictly the safer one to be holding.
|
||||
public func enterVanishedRootLock() {
|
||||
Self.logger.error("board root vanished — entering the read-only lock")
|
||||
readOnlyLock = .vanishedRoot
|
||||
}
|
||||
|
||||
/// Raises the unwritable-location read-only lock — the open flow's call, after probing the
|
||||
/// root's writability (02 § "An unwritable board location enters the read-only lock at open").
|
||||
/// Public now so the vocabulary and its clearing rule ship together; m4's open flow is the
|
||||
/// producer.
|
||||
///
|
||||
/// Does **not** overwrite a standing lock: a board that is already locked for a vanished root
|
||||
/// or a failed bracketed reload has a cause that outranks "and it is also read-only", and both
|
||||
/// of those clear on a success that would then re-probe anyway.
|
||||
public func enterUnwritableLock() {
|
||||
guard readOnlyLock == nil else { return }
|
||||
Self.logger.error("board location is not writable — entering the read-only lock")
|
||||
readOnlyLock = .unwritableLocation
|
||||
}
|
||||
|
||||
// MARK: - Write gate
|
||||
|
||||
/// Runs a synchronous Writer operation inside the watcher bracket, so the churn it produces
|
||||
@@ -418,6 +589,13 @@ public final class BoardStore {
|
||||
/// disk; the watcher notices; the reload applies. That indirection is the one-way flow, and it is
|
||||
/// why this method's only jobs are the gate and the bracket.
|
||||
///
|
||||
/// **A failure posts to the banner before it is rethrown** (02-architecture.md § Write-failure
|
||||
/// surfacing): the strip is how the one-way flow keeps its honesty — the action visibly did not
|
||||
/// happen, and the banner is the only thing that says why — so no call site is trusted to
|
||||
/// remember, and a `try?` at some future call site cannot make a failure silent. The refusal
|
||||
/// below is deliberately *not* posted: the lock row is already standing, and a second row per
|
||||
/// refused gesture would bury it under echoes of itself.
|
||||
///
|
||||
/// - Throws: `BoardStoreWriteRefusal.readOnlyLocked` if the board is locked read-only — checked
|
||||
/// *before* the bracket opens, so a refusal costs no suspended watcher and no owed reload.
|
||||
/// Otherwise rethrows whatever `operation` threw, which is a `BoardWriteError`. The untyped
|
||||
@@ -438,7 +616,14 @@ public final class BoardStore {
|
||||
// `defer`, not a trailing call: a Writer operation that fails partway has still touched disk,
|
||||
// and an unbalanced bracket would leave the watcher suspended for the rest of the session.
|
||||
defer { watcherBrackets?.end() }
|
||||
return try operation()
|
||||
// `do throws(BoardWriteError)`: without the annotation the `catch` widens to `any Error` and
|
||||
// the Writer's typed error is lost on the way to the banner.
|
||||
do throws(BoardWriteError) {
|
||||
return try operation()
|
||||
} catch {
|
||||
banners.post(error)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
/// Runs an operation that rewrites the tree **wholesale** — pull-rebase, branch switch, undo
|
||||
@@ -474,7 +659,18 @@ public final class BoardStore {
|
||||
wholesaleReloadFloor = reloadGeneration + 1
|
||||
watcherBrackets?.end()
|
||||
}
|
||||
try operation()
|
||||
do {
|
||||
try operation()
|
||||
} catch let error as BoardWriteError {
|
||||
// Same honesty rule as `performWrite`, applied to the one error type the banner has
|
||||
// phrasing for. A wholesale operation is usually git's (m7), whose own failure
|
||||
// vocabulary is not `BoardWriteError` and whose surfacing — the suspended-history
|
||||
// condition, the in-progress row swapping for an error — is the committer's to drive;
|
||||
// but a `BoardWriteError` escaping here is an ordinary failed write and may no more
|
||||
// bypass the strip than one from `performWrite`.
|
||||
banners.post(error)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Selection
|
||||
|
||||
Reference in New Issue
Block a user