From 89d4d983e6314ecd2b2697b92b7e72c05f78282c Mon Sep 17 00:00:00 2001 From: rzen Date: Wed, 29 Jul 2026 12:34:55 -0400 Subject: [PATCH] Wire the open-time writability probe and read-only lock Closes the gap found at m10: enterUnwritableLock existed with zero call sites. WritabilityProbe classifies the cause volume-first - a board on a read-only DMG is also permission-denied by access(2), and "you don't have permission" would send the user to a Get Info panel that cannot help - with a pure classify(volumeIsReadOnly:isWritable:) truth table and a two-syscall probe that rebuilds its URL to defeat NSURL resource caching. ReadOnlyLockReason.unwritableLocation now carries the cause; BannerCenter phrases the two ("this board's volume is read-only" vs "you don't have permission to change this folder"). The probe wires once in BoardStoreRegistry.acquire, immediately after the store loads - every open path funnels through it, and running before the loose-file relocation and agent-guide hooks makes the skipped-with-log guide write true by construction (its isWritableFile pre-check demotes to second line of defense). The board still opens: lock, not refusal. The reconciling re-probe is now symmetric per 02's settled text - a volume gone read-only mid-session raises the lock at the next probe (sibling locks settle first, so a root returning read-only lands the honest lock); the stale "deliberately one-way" comment and its pinning test are gone. Save as Template's carve-out predicate extracted to a testable allowsSave (behavior unchanged); Duplicate stays disabled. 11 tests added. 1649 green on both schemes. Claude-Session: https://claude.ai/code/session_01SR4XGjmBE16ZUYWpfFHXwY --- Kanban/App/AppCommands.swift | 29 ++- Kanban/LiveStore/BannerCenter.swift | 16 +- Kanban/LiveStore/BoardStore.swift | 155 +++++++++---- Kanban/LiveStore/BoardStoreRegistry.swift | 16 ++ Kanban/LiveStore/WritabilityProbe.swift | 82 +++++++ KanbanTests/AgentGuideTests.swift | 2 +- KanbanTests/BannerCenterTests.swift | 21 +- KanbanTests/BoardAnnouncerTests.swift | 2 +- KanbanTests/CardBodyEditSessionTests.swift | 4 +- KanbanTests/LooseFileRelocationTests.swift | 2 +- KanbanTests/RootRecoveryTests.swift | 241 ++++++++++++++++++++- 11 files changed, 499 insertions(+), 71 deletions(-) create mode 100644 Kanban/LiveStore/WritabilityProbe.swift diff --git a/Kanban/App/AppCommands.swift b/Kanban/App/AppCommands.swift index e6b190a..6311322 100644 --- a/Kanban/App/AppCommands.swift +++ b/Kanban/App/AppCommands.swift @@ -339,12 +339,35 @@ struct SaveAsTemplateCommand: View { } private var canSave: Bool { - guard let store, let ref, !store.isEditingInline else { return false } - switch store.readOnlyLock { + guard let store, let ref else { return false } + return Self.allowsSave( + lock: store.readOnlyLock, + isEditingInline: store.isEditingInline, + hasUnsavedCardContent: appModel.hasUnsavedCardContent(for: ref) + ) + } + + /// The item's validation as a pure function of the three facts it turns on — extracted from + /// `canSave` so the carve-out can be tested at every combination rather than only through a + /// menu. + /// + /// The carve-out itself is 02-architecture.md ▸ Live-reload resilience, settled: under the + /// **unwritable-location lock alone** this stays live (copy-out is a read — archiving the + /// read-only DMG board being inspected is a legitimate errand), and it "gates on the hazard + /// itself, open sessions, not on lock provenance" — so an Edit or raw-source session holding + /// unsaved content disables it, whether the lock arrived at open or from the symmetric probe + /// mid-session, and nothing here asks which. The other two locks disable it outright. + nonisolated static func allowsSave( + lock: ReadOnlyLockReason?, + isEditingInline: Bool, + hasUnsavedCardContent: Bool + ) -> Bool { + guard !isEditingInline else { return false } + switch lock { case .none: return true case .unwritableLocation: - return !appModel.hasUnsavedCardContent(for: ref) + return !hasUnsavedCardContent case .vanishedRoot, .bracketedReloadFailed: return false } diff --git a/Kanban/LiveStore/BannerCenter.swift b/Kanban/LiveStore/BannerCenter.swift index 648d167..4d56137 100644 --- a/Kanban/LiveStore/BannerCenter.swift +++ b/Kanban/LiveStore/BannerCenter.swift @@ -165,8 +165,9 @@ public struct InProgressOperation: Identifiable, Sendable { /// and a changing *reason* updates the row rather than replacing it — no view churn, no lost /// animation, and no diffing surprise when a lock's cause changes underneath a standing row. public enum BannerRow: Identifiable, Sendable { - /// The board refuses writes. Condition, error tone. Producers: the failed bracketed reload - /// (built), the vanished root (this milestone), the open-time writability probe (m4). + /// The board refuses writes. Condition, error tone. Producers: the failed bracketed reload, the + /// vanished root, and the writability probe — at open and, symmetrically, on every reconciling + /// reload thereafter. case readOnlyLock(ReadOnlyLockReason) /// A reload failed and the last good snapshot is still on screen. Condition, error tone. case reloadBreakage(BoardLoadError) @@ -804,14 +805,21 @@ public final class BannerCenter { /// what is on screen is still the last good view — because the lock's whole promise is that /// nothing was lost: reading, selecting, searching and copying out all stay live (02 § "The /// lock's scope"). + /// + /// **The unwritable location gets two lines, not one shared one** (02 § Write-failure + /// surfacing, settled): "which specific cause, not a shared line … the fixes being different + /// acts". Ejecting a DMG or copying the board off it is not the same repair as a `chmod` or a + /// Get Info panel, and a line that covered both would name neither. public nonisolated static func headline(for lock: ReadOnlyLockReason) -> String { switch lock { case .bracketedReloadFailed: "This board couldn't be re-read after the last operation — showing the last good view, read-only" case .vanishedRoot: "This board's folder is gone — showing the last good view, read-only" - case .unwritableLocation: - "This board's location can't be written to — showing the last good view, read-only" + case .unwritableLocation(.readOnlyVolume): + "This board's volume is read-only — showing the last good view, read-only" + case .unwritableLocation(.permissionDenied): + "You don't have permission to change this folder — showing the last good view, read-only" } } diff --git a/Kanban/LiveStore/BoardStore.swift b/Kanban/LiveStore/BoardStore.swift index 2b97ffa..c672045 100644 --- a/Kanban/LiveStore/BoardStore.swift +++ b/Kanban/LiveStore/BoardStore.swift @@ -38,17 +38,24 @@ public enum ReadOnlyLockReason: Sendable, Equatable { /// normally. case vanishedRoot - /// The board opened somewhere it cannot be written: a read-only volume (a DMG, a snapshot, a + /// The board is 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 + /// **It carries which** (settled): "the probe distinguishes read-only volume from + /// permission-denied folder and the lock reason carries it … the fixes being different acts". + /// The payload is the *only* thing the two spellings of this lock differ in — same scope, same + /// clearing rule — so it is an associated value rather than two cases, and `BannerCenter` turns + /// it into the one line the user reads. + /// + /// **Raised and cleared by the probe, not by the reload's success** — 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 changed, in *either* + /// direction: it clears a lock whose cause is gone and raises one whose cause has appeared + /// mid-session (§ "the probe is symmetric"). + case unwritableLocation(UnwritableCause) } /// The refusal `BoardStore.performWrite` throws when the board is locked read-only. @@ -760,18 +767,21 @@ public final class BoardStore { reloadFailure = nil looseCardFiles = result.looseCardFiles legacyTombstones = result.legacyTombstones - clearLockIfDisproved(by: origin) + reconcileLock(after: origin) // The registry write-through, for the same "not board structure" reason the lock // clearing sits out here: whether this board's row needs a new title, icon, or // iconColor is the registry's question to answer (`syncDisplayState`'s own no-op // guard), not a decision this store makes by comparing against its own prior // snapshot. displayStateDelegate?() - // Last, and after `clearLockIfDisproved` deliberately: this is the seam the two - // deferred app-initiated writes are armed on. A board that was locked read-only - // tolerated its loose files and its legacy tombstones for exactly as long as the lock - // stood, and the reload that clears the lock is the reload that lets them move — see - // `relocateLooseCardFiles()` and `migrateLegacyTombstones()`. + // Last, and after `reconcileLock` deliberately: this is the seam the two deferred + // app-initiated writes are armed on. A board that was locked read-only tolerated its + // loose files and its legacy tombstones for exactly as long as the lock stood, and the + // reload that clears the lock is the reload that lets them move — see + // `relocateLooseCardFiles()` and `migrateLegacyTombstones()`. The ordering cuts the + // other way too now that the probe is symmetric: a reconciling reload that *raises* the + // lock raises it before these three run, so none of them writes into a location the + // same reload just learned is read-only. // // The migration goes second only because the relocation is the older rule; they touch // disjoint files (loose files beside an `index.md` vs the `deleted:` key inside one) and @@ -890,9 +900,10 @@ public final class BoardStore { startReload(origin) } - // MARK: - The lock's clearing rules + // MARK: - The lock's reconciliation rules - /// Clears the read-only lock if this successful reload actually disproved its cause. + /// Brings the read-only lock into line with what this successful reload — and, on a reconciling + /// one, a fresh writability probe — actually proves. /// /// **Reason-specific, because the causes are not alike** (02-architecture.md § Write-failure /// surfacing): @@ -902,28 +913,54 @@ public final class BoardStore { /// "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"). + /// loading proves nothing about writing. Only the probe can speak to it, and the probe runs on + /// **reconciling** reloads — wake, activation, a stream re-creation — because those are the + /// reloads that admit a blind window ("Writability re-probes on every reconciling reload"). /// - /// `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*. + /// ### The probe is symmetric (settled) /// - /// 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) { + /// It clears *and* raises. "A rewritable remount or fixed permission clears the lock without + /// ceremony, and a volume gone read-only mid-session *raises* it at the next probe — banner up + /// front, not every gesture failing one at a time (the lock's own founding rationale)." Between + /// probes a write that hits the newly read-only volume fails as an ordinary one-shot; this is + /// the line that converts that condition into the standing lock. + /// + /// A raise here is deliberately **not** routed through `enterUnwritableLock(_:)`: that method + /// speaks its own sentence, and this runs inside `land`, which posts exactly one announcement + /// per reload from the before/after pictures it already holds. Two voices for one lock is the + /// bug `announceLockChange` exists to avoid. + /// + /// The sibling locks are settled *before* the probe, so a reconciling reload that clears a + /// vanished root on a volume that came back read-only ends with the honest lock rather than no + /// lock at all. And a standing `.unwritableLocation` whose cause *changed* — a permission-denied + /// folder whose volume was then remounted read-only — re-lands with the new cause, updating the + /// row's line rather than replacing the row (`BannerRow.id` is constant per condition). + private func reconcileLock(after 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 } + case .unwritableLocation, nil: + break + } + + guard origin == .reconciling else { return } + + switch (readOnlyLock, WritabilityProbe.probe(rootURL)) { + case (.unwritableLocation, nil): Self.logger.debug("writability re-probe passed — the unwritable-location lock clears") readOnlyLock = nil + case let (.unwritableLocation, .some(cause)): + // Still unwritable. The assignment is not a no-op only when the *cause* moved. + readOnlyLock = .unwritableLocation(cause) + case let (nil, .some(cause)): + Self.logger.error("writability re-probe failed (\(cause.rawValue, privacy: .public)) — the read-only lock rises") + readOnlyLock = .unwritableLocation(cause) + case (nil, nil): + break + // Cleared above, so unreachable — spelled so a new lock reason is a compile error here + // rather than a silent fall-through past the probe. + case (.bracketedReloadFailed, _), (.vanishedRoot, _): + break } } @@ -960,18 +997,37 @@ public final class BoardStore { announceLockChange(from: before) } - /// 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. + /// **The open-time writability probe** (02 § "An unwritable board location enters the read-only + /// lock at open") — `BoardStoreRegistry.acquire`'s call, and the only place the lock is raised + /// outside a reconciling reload. + /// + /// A no-op on a writable board, which is the overwhelming case, and one `access(2)` plus one + /// volume resource value when it is not — cheap enough to sit unconditionally on the open path. + /// + /// **The open still succeeds.** Nothing here refuses the board or throws: the lock's read + /// affordances stay live as always, because "inspecting an archived board on a DMG is a + /// legitimate errand, and viewing-first is the point". All that changes is that every mutating + /// entry point now consults a predicate that is already `true` before the window can be acted + /// on — the lock is up *before* the user's first gesture, which is the whole of "fail loudly, + /// specifically, once". + public func probeWritabilityAtOpen() { + guard let cause = WritabilityProbe.probe(rootURL) else { return } + enterUnwritableLock(cause) + } + + /// Raises the unwritable-location read-only lock with the cause the probe found. + /// + /// Public because the probe is not the only conceivable producer and because tests arm it + /// directly; `probeWritabilityAtOpen()` is the app's own path to it. /// /// 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() { + /// of those clear on a success that would then re-probe anyway (`reconcileLock(after:)` settles + /// the siblings first, then probes, precisely so that reload lands on the right answer). + public func enterUnwritableLock(_ cause: UnwritableCause) { guard readOnlyLock == nil else { return } - Self.logger.error("board location is not writable — entering the read-only lock") - readOnlyLock = .unwritableLocation + Self.logger.error("board location is not writable (\(cause.rawValue, privacy: .public)) — entering the read-only lock") + readOnlyLock = .unwritableLocation(cause) announceLockChange(from: nil) } @@ -2756,8 +2812,10 @@ public final class BoardStore { /// locked board returns here having written nothing **and having remembered nothing**, so the /// next attempt is a fresh one. The arming seam is `land(_:generation:origin:)`: every lock /// clears on a successful reload and nowhere else, and this runs at the end of every successful - /// reload, after `clearLockIfDisproved` — so the reload that lifts the lock is the reload that - /// performs the relocation, with no timer, no queue, and no second state to keep in step. + /// reload, after `reconcileLock(after:)` — so the reload that lifts the lock is the reload that + /// performs the relocation, with no timer, no queue, and no second state to keep in step. The + /// same ordering covers the other direction, now that the probe is symmetric: a reconciling + /// reload that *raises* the unwritable-location lock raises it before this runs. /// /// ### It cannot hot-loop /// @@ -3581,12 +3639,17 @@ public final class BoardStore { /// /// Two gates, because two different things can be true. `performWrite` would refuse under the /// read-only lock on its own, but that refusal is a thrown error and this is not a gesture — so - /// the lock is checked first, the relocation's own deferral idiom. The writability probe beside - /// it covers the case the lock does not: 02-architecture.md's open-time unwritable-root lock is a - /// separate card, and until it lands a board on a read-only volume would reach the Writer, fail, - /// and post a banner about a file the user never asked for. 02 settles that exact case the other - /// way — "the open-time agent-guide write is skipped-with-log, the `CLAUDE.user.md`-taken - /// precedent" — so it is skipped with a log. + /// the lock is checked first, the relocation's own deferral idiom. That gate is now the one that + /// actually fires on an unwritable board: `BoardStoreRegistry.acquire` probes writability + /// *before* it calls this method, so 02-architecture.md's "the open-time agent-guide write is + /// skipped-with-log, the `CLAUDE.user.md`-taken precedent" is honored by the lock being up + /// rather than by this method noticing on its own. + /// + /// The `access(2)` check below stays anyway, as the second line of defense: it is the only gate + /// covering the window *between* probes — a volume remounted read-only mid-session raises the + /// lock at the next reconciling reload, and a reload landing in that window would otherwise + /// reach the Writer, fail, and post a banner about a file the user never asked for. Both skips + /// are logged and neither is ever surfaced. /// /// ### It cannot hot-loop /// diff --git a/Kanban/LiveStore/BoardStoreRegistry.swift b/Kanban/LiveStore/BoardStoreRegistry.swift index 2247df7..a5aecd7 100644 --- a/Kanban/LiveStore/BoardStoreRegistry.swift +++ b/Kanban/LiveStore/BoardStoreRegistry.swift @@ -146,6 +146,22 @@ public final class BoardStoreRegistry { let store = try BoardStore(rootURL: rootURL) + // **The open-time writability probe** (02-architecture.md § Write-failure surfacing, "An + // unwritable board location enters the read-only lock at open"), and this is the seam for + // it: every way a board opens — welcome's recents, a Finder open, File ▸ Open, restoration, + // a card window arriving first — funnels through `acquire`, so the probe is wired once here + // instead of at each caller, and no future open path can forget it. + // + // **First, immediately after the load.** The lock has to be standing before anything else + // in this method can act on the board, and two things below would otherwise write into a + // location this line already knows is read-only: the loose-file relocation and the agent + // guide. Ordering them after the probe is what makes 02's "the open-time agent-guide write + // is skipped-with-log" true by construction rather than by that method's own second gate. + // + // The board still opens. This is a lock, not a refusal: the window comes up, the snapshot + // renders, and reading, selecting, searching and copying out all stay live. + store.probeWritabilityAtOpen() + // Unreachable in practice — the load above just walked this directory — but the alternative // is a force-unwrap on a resource value the filesystem is free to refuse, so it is spelled // out. The loader's own vocabulary says it; no new error path is invented for a case that diff --git a/Kanban/LiveStore/WritabilityProbe.swift b/Kanban/LiveStore/WritabilityProbe.swift new file mode 100644 index 0000000..91d88b6 --- /dev/null +++ b/Kanban/LiveStore/WritabilityProbe.swift @@ -0,0 +1,82 @@ +import Foundation + +// MARK: - UnwritableCause + +/// **Why** a board's location refuses writes — the half of `ReadOnlyLockReason.unwritableLocation` +/// the user is actually told about (02-architecture.md § Write-failure surfacing, settled: "the +/// probe distinguishes read-only volume from permission-denied folder and the lock reason carries +/// it … the fixes being different acts"). +/// +/// Two cases and no `.unknown`: the probe below is total — it asks two questions of the filesystem +/// and every answer maps onto one of these or onto "writable". A third case would be a shrug in the +/// one place the design forbids one, since the whole point of naming the cause is that ejecting a +/// DMG and running `chmod` are not the same repair. +public enum UnwritableCause: String, Sendable, Equatable, CaseIterable { + /// The volume the board sits on is mounted read-only — a DMG, an APFS snapshot, a read-only + /// share. Nothing on it can be written, by anybody, until it is remounted. + case readOnlyVolume + + /// The volume is writable; this *folder* is not — POSIX permissions, an ACL, an owner that is + /// not us. A `chmod`, a Get Info panel, or a move somewhere else fixes it. + case permissionDenied +} + +// MARK: - WritabilityProbe + +/// The board root's writability, asked of the filesystem and classified — the open-time probe of +/// 02-architecture.md § Write-failure surfacing ("An unwritable board location enters the read-only +/// lock at open"), re-run on every reconciling reload because that rule is **symmetric**: a fixed +/// permission or a rewritable remount clears the lock, and a volume gone read-only mid-session +/// raises it. +/// +/// ### Two questions, and the order they are asked in matters +/// +/// `FileManager.isWritableFile(atPath:)` is `access(2)` with `W_OK`: a real-uid permission question +/// answered by the filesystem itself, which is why it is trustworthy where a stat of the mode bits +/// would not be (ACLs, sandbox denials, and read-only mounts all show up in it). But it answers +/// `false` for **both** halves of this case — a read-only volume fails `access(2)` exactly like a +/// `r-x` folder does — so on its own it can only say "no", never "why". +/// +/// `URLResourceKey.volumeIsReadOnlyKey` is what separates them, and it is therefore asked **first**. +/// The precedence is not a tie-break: a board on a mounted DMG *is* on a read-only volume and it is +/// also permission-denied by `access(2)`, and telling the user "you don't have permission to change +/// this folder" would send them to a Get Info panel that cannot help. The volume's answer is the +/// more fundamental fact and it names the repair that works, so it wins whenever it is `true`. +/// +/// A volume that will not answer at all (`nil` — an exotic filesystem, a URL whose volume is gone) +/// falls through to `access(2)`, which is the honest degradation: we still know whether the board +/// can be written, we just describe it as the folder's doing. +/// +/// ### Why the classifier is separate from the I/O +/// +/// `classify(volumeIsReadOnly:isWritable:)` is a pure function of the two answers, so the truth +/// table — including the read-only-volume-that-somehow-passes-`access` row, which `root` can +/// produce — is testable without a DMG. `probe(_:)` is only the two syscalls and this call. +public enum WritabilityProbe { + + /// The cause, or `nil` when the location accepts writes. + /// + /// Deliberately optional-returning rather than `Bool`-returning: every caller either raises a + /// lock carrying the cause or clears one, and a `Bool` would force a second question at each + /// site to find out which line to show. + public nonisolated static func classify(volumeIsReadOnly: Bool?, isWritable: Bool) -> UnwritableCause? { + if volumeIsReadOnly == true { return .readOnlyVolume } + return isWritable ? nil : .permissionDenied + } + + /// Asks the filesystem about `root` and classifies the answer. + /// + /// **The URL is rebuilt from its path** before the resource value is read. `URL` bridges to + /// `NSURL`, which caches resource values it has already been asked for — and this probe's whole + /// job is to notice that an answer *changed* since last time. A cached `volumeIsReadOnly` would + /// make the symmetric rule silently one-way again, in a way no test on a fresh URL would catch. + /// `access(2)` has no such cache, which is why only the resource value needs the ceremony. + public nonisolated static func probe(_ root: URL) -> UnwritableCause? { + let fresh = URL(fileURLWithPath: root.path, isDirectory: true) + let volumeIsReadOnly = (try? fresh.resourceValues(forKeys: [.volumeIsReadOnlyKey]))?.volumeIsReadOnly + return classify( + volumeIsReadOnly: volumeIsReadOnly, + isWritable: FileManager.default.isWritableFile(atPath: root.path) + ) + } +} diff --git a/KanbanTests/AgentGuideTests.swift b/KanbanTests/AgentGuideTests.swift index 00901de..5137531 100644 --- a/KanbanTests/AgentGuideTests.swift +++ b/KanbanTests/AgentGuideTests.swift @@ -429,7 +429,7 @@ struct AgentGuideStoreTests { let fixture = try makeBoard() defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) - store.enterUnwritableLock() + store.enterUnwritableLock(.permissionDenied) store.refreshAgentGuide() #expect(!fixture.exists(AgentGuide.filename)) diff --git a/KanbanTests/BannerCenterTests.swift b/KanbanTests/BannerCenterTests.swift index c66175b..70ee18b 100644 --- a/KanbanTests/BannerCenterTests.swift +++ b/KanbanTests/BannerCenterTests.swift @@ -614,7 +614,8 @@ struct BannerCenterPhrasingTests { @Test("Every lock reason says what is wrong and that the view is still the last good one") func lockHeadlinesReassure() { - let reasons: [ReadOnlyLockReason] = [.bracketedReloadFailed, .vanishedRoot, .unwritableLocation] + let reasons: [ReadOnlyLockReason] = [.bracketedReloadFailed, .vanishedRoot] + + UnwritableCause.allCases.map(ReadOnlyLockReason.unwritableLocation) let headlines = reasons.map(BannerCenter.headline(for:)) for headline in headlines { @@ -624,6 +625,22 @@ struct BannerCenterPhrasingTests { #expect(Set(headlines).count == reasons.count) } + /// "Which specific cause, not a shared line" (02 § Write-failure surfacing, settled): the two + /// halves of the unwritable location name **different repairs**, so they get different lines. + @Test("The unwritable location names which cause it is") + func unwritableCausesGetTheirOwnLines() { + let volume = BannerCenter.headline(for: .unwritableLocation(.readOnlyVolume)) + let folder = BannerCenter.headline(for: .unwritableLocation(.permissionDenied)) + + #expect(volume == "This board's volume is read-only — showing the last good view, read-only") + #expect(folder == "You don't have permission to change this folder — showing the last good view, read-only") + + // The distinction the design spends the extra line on: one says volume, the other says this + // folder, and neither says the other's word. + #expect(volume.contains("volume") && !volume.contains("permission")) + #expect(folder.contains("permission") && !folder.contains("volume")) + } + @Test("Reload breakage carries fail-fast's specifics — the path and what is wrong with it") func breakageHeadlineNamesThePath() { let headline = BannerCenter.headline( @@ -724,7 +741,7 @@ struct BannerCenterStoreTests { defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) - store.enterUnwritableLock() + store.enterUnwritableLock(.permissionDenied) store.banners.post(BoardWriteError(operation: .createCard, path: "/x", reason: .io(message: "the disk is full"))) store.banners.postLoss("Pasted 'Fix login' without its 3 attachments") store.banners.suspendHistory(reason: "the disk is full") diff --git a/KanbanTests/BoardAnnouncerTests.swift b/KanbanTests/BoardAnnouncerTests.swift index 7d31419..5235294 100644 --- a/KanbanTests/BoardAnnouncerTests.swift +++ b/KanbanTests/BoardAnnouncerTests.swift @@ -421,7 +421,7 @@ struct BoardAnnouncerSpeechTests { @Test("A lock whose cause changed is news again") func changedLockCauseSpeaksAgain() { var facts = BoardAnnouncer.ReloadFacts() - facts.lockBefore = .unwritableLocation + facts.lockBefore = .unwritableLocation(.permissionDenied) facts.lockAfter = .vanishedRoot #expect( diff --git a/KanbanTests/CardBodyEditSessionTests.swift b/KanbanTests/CardBodyEditSessionTests.swift index 388d227..5101c0c 100644 --- a/KanbanTests/CardBodyEditSessionTests.swift +++ b/KanbanTests/CardBodyEditSessionTests.swift @@ -199,10 +199,10 @@ struct CardBodyDirtyBufferTests { func aSuspendedSaveKeepsTheBuffer() throws { let spy = SaveSpy() let session = makeSession(spy) - spy.outcome = .suspended(.unwritableLocation) + spy.outcome = .suspended(.unwritableLocation(.permissionDenied)) session.edited("held\n") - #expect(session.flush() == .suspended(.unwritableLocation)) + #expect(session.flush() == .suspended(.unwritableLocation(.permissionDenied))) #expect(session.isDirty) // The close-time guard treats it as a non-failure: no write was attempted, the lock row has // been standing all along, and a modal offering Try Again could only fail again. diff --git a/KanbanTests/LooseFileRelocationTests.swift b/KanbanTests/LooseFileRelocationTests.swift index 49fdd61..fa03b8a 100644 --- a/KanbanTests/LooseFileRelocationTests.swift +++ b/KanbanTests/LooseFileRelocationTests.swift @@ -439,7 +439,7 @@ struct LooseFileStoreTests { let store = try BoardStore(rootURL: fixture.root) let brackets = RelocationBracketLog() brackets.attach(to: store) - store.enterUnwritableLock() + store.enterUnwritableLock(.permissionDenied) store.relocateLooseCardFiles() store.handleWatcherEvent(.treeChanged(.foreign)) diff --git a/KanbanTests/RootRecoveryTests.swift b/KanbanTests/RootRecoveryTests.swift index 651c526..f1de9b3 100644 --- a/KanbanTests/RootRecoveryTests.swift +++ b/KanbanTests/RootRecoveryTests.swift @@ -221,7 +221,7 @@ struct RootRecoveryTests { #expect(store.banners.oneShots.isEmpty) } - // MARK: The writability clearing rule + // MARK: The writability probe @Test("The unwritable-location lock clears only on a reconciling reload whose probe passes") func unwritableLockClearsOnlyOnAReconcilingProbe() async throws { @@ -232,20 +232,20 @@ struct RootRecoveryTests { // The probe has to be honest, so the root is made genuinely unwritable — `r-x`, which still // reads perfectly. That is the whole difficulty of this case: the board loads fine. try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) - store.enterUnwritableLock() - #expect(store.readOnlyLock == .unwritableLocation) + store.enterUnwritableLock(.permissionDenied) + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied)) // A foreign reload succeeds — and clears nothing. Loading proves nothing about writing, // which is exactly why this lock's clearing rule is not the other two's. store.handleWatcherEvent(.treeChanged(.foreign)) await store.awaitQuiescence() #expect(store.reloadFailure == nil, "an unwritable root still reads") - #expect(store.readOnlyLock == .unwritableLocation) + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied)) // Neither does a reconciling one while the permission is still what it was. store.handleWatcherEvent(.treeChanged(.reconciling)) await store.awaitQuiescence() - #expect(store.readOnlyLock == .unwritableLocation) + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied)) // The permission is fixed. Nothing announces that — a `chmod` in a terminal fires no event // the board would act on — so the lock stands until the next reconciling sweep (wake, app @@ -253,7 +253,7 @@ struct RootRecoveryTests { try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: fixture.root.path) store.handleWatcherEvent(.treeChanged(.foreign)) await store.awaitQuiescence() - #expect(store.readOnlyLock == .unwritableLocation, "only a reconciling reload re-probes") + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied), "only a reconciling reload re-probes") store.handleWatcherEvent(.treeChanged(.reconciling)) await store.awaitQuiescence() @@ -261,20 +261,239 @@ struct RootRecoveryTests { #expect(store.bannerRows.isEmpty) } - @Test("A reconciling reload that finds the root unwritable does not raise the lock by itself") - func theProbeOnlyClears() async throws { + /// "The probe is symmetric (settled): … a volume gone read-only mid-session *raises* it at the + /// next probe — banner up front, not every gesture failing one at a time." + @Test("A reconciling reload that finds the root unwritable raises the lock") + func theProbeIsSymmetric() async throws { let fixture = try makeBoard() defer { fixture.tearDown() } let store = try BoardStore(rootURL: fixture.root) + #expect(store.readOnlyLock == nil, "a writable board opens unlocked") + + // The root goes read-only under the open board. Nothing announces it: a `chmod` in a + // terminal is not a tree change, and a mid-session remount is not one either. try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) + // A foreign reload is not a probe. Between probes, "a write that hits the newly read-only + // volume fails as an ordinary one-shot" — the condition is not yet standing. + store.handleWatcherEvent(.treeChanged(.foreign)) + await store.awaitQuiescence() + #expect(store.reloadFailure == nil, "an unwritable root still reads") + #expect(store.readOnlyLock == nil, "only a reconciling reload probes") + + // The next reconciliation converts the condition into the standing lock, cause and all. + store.handleWatcherEvent(.treeChanged(.reconciling)) + await store.awaitQuiescence() + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied)) + #expect(store.bannerRows.map(\.id) == ["read-only-lock"]) + #expect(store.isReadOnly) + + // And it is a *lock*, not a broken board: the snapshot is intact and reads stay live. + #expect(store.reloadFailure == nil) + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["First"]) + + // Symmetric in the other direction, from a lock this probe raised rather than one the open + // flow armed — the same rule, so the same clearing. + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: fixture.root.path) + store.handleWatcherEvent(.treeChanged(.reconciling)) + await store.awaitQuiescence() + #expect(store.readOnlyLock == nil, "a fixed permission clears the lock without ceremony") + #expect(store.bannerRows.isEmpty) + } + + /// The raise is announced exactly once, in the banner's own words, through the single + /// per-reload sentence `land` posts — not a second voice of the probe's own. + @Test("A probe-raised lock speaks the banner's line, once") + func theRaiseIsAnnouncedOnce() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + var spoken: [String] = [] + store.announce = { if let phrase = $0 { spoken.append(phrase) } } + + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) store.handleWatcherEvent(.treeChanged(.reconciling)) await store.awaitQuiescence() - // Arming the lock is the open flow's job (m4). Inferring it from a probe here would be a - // policy decision this layer has not been asked to make — recorded as a test so the - // asymmetry is deliberate rather than forgotten. + #expect(spoken == ["Error: You don't have permission to change this folder — showing the last good view, read-only"]) + + // A second reconciling reload finds the same condition: the row is already standing, so + // nothing is said again. + store.handleWatcherEvent(.treeChanged(.reconciling)) + await store.awaitQuiescence() + #expect(spoken.count == 1) + } + + /// A vanished root that comes back on a read-only volume must not end up *unlocked*: the + /// sibling lock clears on the reload's success, and the probe in the same pass raises the honest + /// one. This drives the two halves directly, since a real remount is not a headless act. + @Test("A sibling lock clearing does not leave an unwritable root unlocked") + func aSiblingLockYieldsToTheProbe() async throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + store.enterVanishedRootLock() + #expect(store.readOnlyLock == .vanishedRoot) + + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) + store.handleWatcherEvent(.treeChanged(.reconciling)) + await store.awaitQuiescence() + + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied)) + } + + // MARK: The probe's classification + + /// The classifier is pure, so the row `access(2)` alone cannot distinguish — a read-only volume, + /// which fails `access(2)` exactly like a `r-x` folder does — is testable without a DMG. + @Test("A read-only volume outranks permission denial, whatever access(2) says") + func theVolumeAnswerWins() { + #expect(WritabilityProbe.classify(volumeIsReadOnly: true, isWritable: false) == .readOnlyVolume) + // The row that matters: on a mounted DMG both facts are true at once, and naming the folder + // would send the user to a Get Info panel that cannot help. + #expect(WritabilityProbe.classify(volumeIsReadOnly: true, isWritable: true) == .readOnlyVolume) + } + + @Test("A writable volume leaves the folder to answer for itself") + func thePermissionAnswerIsTheFallback() { + #expect(WritabilityProbe.classify(volumeIsReadOnly: false, isWritable: false) == .permissionDenied) + #expect(WritabilityProbe.classify(volumeIsReadOnly: false, isWritable: true) == nil) + // A volume that will not answer degrades to access(2) — still honest about *whether*, and + // it describes the refusal as the folder's doing. + #expect(WritabilityProbe.classify(volumeIsReadOnly: nil, isWritable: false) == .permissionDenied) + #expect(WritabilityProbe.classify(volumeIsReadOnly: nil, isWritable: true) == nil) + } + + @Test("The live probe reads the filesystem, not a cached resource value") + func theLiveProbeSeesChanges() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + + #expect(WritabilityProbe.probe(fixture.root) == nil) + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) + #expect(WritabilityProbe.probe(fixture.root) == .permissionDenied) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: fixture.root.path) + #expect(WritabilityProbe.probe(fixture.root) == nil) + } + + // MARK: The probe at open + + /// "An unwritable board location enters the read-only lock at open" — through the registry, + /// which is the seam every open path funnels through. + @Test("Acquiring an unwritable board opens it, locked") + func openTimeProbeRaisesTheLock() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) + + let registry = BoardStoreRegistry() + let store = try registry.acquire(fixture.root) + defer { registry.release(store) } + + // The open **succeeded** — the lock is not a refusal, and viewing an archived board is the + // legitimate errand the read affordances exist for. + #expect(cardTitles(inLane: Ident.lane1, of: store.snapshot) == ["First"]) + #expect(store.reloadFailure == nil) + + // And the lock was up before anything could act on it. + #expect(store.readOnlyLock == .unwritableLocation(.permissionDenied)) + #expect(store.bannerRows.map(\.id) == ["read-only-lock"]) + + // Writes are refused as a policy refusal, not as an I/O failure, and nothing is posted on + // top of the standing row. + #expect(throws: BoardStoreWriteRefusal.readOnlyLocked(.unwritableLocation(.permissionDenied))) { + try store.performWrite { () throws(BoardWriteError) -> Void in } + } + #expect(store.banners.oneShots.isEmpty) + } + + /// "The open-time agent-guide write is skipped-with-log, the `CLAUDE.user.md`-taken precedent." + /// The lock is what skips it: `acquire` probes before it calls `refreshAgentGuide()`. + @Test("The open-time agent-guide write is skipped under the lock") + func openTimeGuideWriteIsSkipped() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: fixture.root.path) + + let registry = BoardStoreRegistry() + let store = try registry.acquire(fixture.root) + defer { registry.release(store) } + + #expect(!FileManager.default.fileExists(atPath: fixture.root.appendingPathComponent(AgentGuide.filename).path)) + // Skipped with a log, never with a banner: the user did not ask for this file. + #expect(store.banners.oneShots.isEmpty) + #expect(store.bannerRows.map(\.id) == ["read-only-lock"]) + } + + @Test("A writable board acquires unlocked, and gets its guide") + func openTimeProbePassesQuietly() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + + let registry = BoardStoreRegistry() + let store = try registry.acquire(fixture.root) + defer { registry.release(store) } + #expect(store.readOnlyLock == nil) + #expect(store.bannerRows.isEmpty) + #expect(FileManager.default.fileExists(atPath: fixture.root.appendingPathComponent(AgentGuide.filename).path)) + } + + // MARK: The lock's scope, at this cause + + /// The settled carve-out: "under the unwritable-location lock alone, Save as Template stays + /// live" — copy-out is a read — "unless an open Edit or raw-source session holds unsaved + /// content", and Duplicate stays disabled even there. + @Test("Save as Template survives this lock alone; the other two disable it") + func saveAsTemplateCarveOut() { + let live = SaveAsTemplateCommand.allowsSave( + lock: .unwritableLocation(.readOnlyVolume), + isEditingInline: false, + hasUnsavedCardContent: false + ) + #expect(live, "archiving the read-only DMG board being inspected is a legitimate errand") + + // The gate is the hazard, not the provenance: unsaved content the lock's suspended saves + // cannot flush would be silently missed by the template. + #expect(!SaveAsTemplateCommand.allowsSave( + lock: .unwritableLocation(.readOnlyVolume), + isEditingInline: false, + hasUnsavedCardContent: true + )) + // Both causes are the same lock, so both carve out. + #expect(SaveAsTemplateCommand.allowsSave( + lock: .unwritableLocation(.permissionDenied), + isEditingInline: false, + hasUnsavedCardContent: false + )) + // An open inline title editor holds a pending change no flush can reach. + #expect(!SaveAsTemplateCommand.allowsSave( + lock: .unwritableLocation(.permissionDenied), + isEditingInline: true, + hasUnsavedCardContent: false + )) + // The other two locks disable it outright, unsaved content or not. + for lock: ReadOnlyLockReason in [.vanishedRoot, .bracketedReloadFailed] { + #expect(!SaveAsTemplateCommand.allowsSave( + lock: lock, + isEditingInline: false, + hasUnsavedCardContent: false + )) + } + #expect(SaveAsTemplateCommand.allowsSave(lock: nil, isEditingInline: false, hasUnsavedCardContent: false)) + } + + /// Duplicate is `acceptsBoardMutations`, which is the bare lock — so it stays disabled under + /// this cause too ("its destination is the same unwritable parent"). + @Test("Duplicate stays disabled under the unwritable-location lock") + func duplicateStaysDisabled() throws { + let fixture = try makeBoard() + defer { fixture.tearDown() } + let store = try BoardStore(rootURL: fixture.root) + + #expect(store.acceptsBoardMutations) + store.enterUnwritableLock(.readOnlyVolume) + #expect(!store.acceptsBoardMutations) } }