From bf18512abc0c37e2b769ae95971b3d699fb8b09d Mon Sep 17 00:00:00 2001 From: rzen Date: Tue, 28 Jul 2026 07:20:14 -0400 Subject: [PATCH] Make Duplicate a cancellable per-item walk with a save-panel fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The copy now walks the source tree item by item, checking cancellation between items, and the in-progress banner row carries its promised Cancel — a cancelled duplicate removes the partial sibling and never happened (DESIGN/03 > File menu). A sandbox permission refusal of the silent Finder-style sibling falls back to an NSSavePanel pre-filled with the parent folder and the copy name — the panel's grant is the sandbox's own answer; cancelling the panel cancels quietly, and non-permission failures keep the ordinary one-shot banner. Refusal classification is deliberately narrow (NSFileWriteNoPermissionError itself, no underlying- chain walk) so an unreadable source never masquerades as a destination refusal. Directories are created writable first with mode and timestamps restored after the subtree lands, so a read-only source folder can't strand its own copy. BoardDuplicatorTests grows from 9 to 18 tests. Claude-Session: https://claude.ai/code/session_01SR4XGjmBE16ZUYWpfFHXwY --- Kanban/App/AppCommands.swift | 165 +++++++++++-- Kanban/App/BoardDuplicator.swift | 313 +++++++++++++++++++++++-- KanbanTests/BoardDuplicatorTests.swift | 224 +++++++++++++++++- 3 files changed, 660 insertions(+), 42 deletions(-) diff --git a/Kanban/App/AppCommands.swift b/Kanban/App/AppCommands.swift index ca83637..5623034 100644 --- a/Kanban/App/AppCommands.swift +++ b/Kanban/App/AppCommands.swift @@ -108,10 +108,27 @@ struct OpenRecentMenu: View { /// copied history misses pending work". Not a *close*: 09-templates.md states the rule with its /// exception attached ("sessions staying open"), and 03 is explicit that "the original stays open /// too". `AppModel.flushPendingWork(for:)` is that step of the sequence, run on its own. -/// 2. **The copy** — `BoardDuplicator`, off the main actor so the spinner can spin. -/// 3. **The copy opens in its own board window** — "macOS Duplicate convention" — through the +/// 2. **The copy** — `BoardDuplicator`, off the main actor so the spinner can spin, and cancellable: +/// the in-progress row carries Cancel, which cancels the copy task, and the walk removes its own +/// partial sibling on the way out ("a cancelled duplicate never happened"). +/// 3. **The save panel, but only on a refusal** — "the silent Finder-style sibling is attempted +/// first; on a permission refusal a save panel opens pre-filled with the parent folder and the +/// 'copy' name — the panel's grant is the sandbox's own answer, and it doubles as a +/// choose-another-location affordance" (03, settled). The board's bookmark grants its own subtree, +/// not its parent, so the sibling may simply be unwritable; that is a question about *where*, and +/// the panel is where the sandbox answers it. +/// 4. **The copy opens in its own board window** — "macOS Duplicate convention" — through the /// ordinary open path, so it registers, bookmarks, and titles itself like any other board. /// +/// ### Three endings, and only one of them speaks +/// +/// A copy that lands opens. A copy the user **cancelled** — the row's Cancel, or the save panel's — +/// says nothing at all: "cancelling the panel cancels the duplicate quietly (no banner — the user +/// declined, nothing failed)", and the row's Cancel is the same sentence about the same gesture. +/// Everything else — a full disk, a name already taken — is the ordinary one-shot banner +/// (02-architecture.md § Write-failure surfacing). `BoardDuplicator.Failure`'s three cases are those +/// three endings, switched exhaustively below so a fourth could not be forgotten. +/// /// ### Validation /// /// Board window only, so a welcome-selected recent can never be duplicated by accident — 03 says it @@ -151,37 +168,141 @@ struct DuplicateBoardCommand: View { let source = store.rootURL Task { @MainActor in + let cancellation = DuplicateCancellation() // The in-progress row 02 § The banner surface names for "big-board Duplicate": info - // tone, pinned, cleared on completion, swapped for the error row on failure. - // - // No Cancel yet. 02 promises one on copy-shaped work ("remove the partial copy, nothing - // lost"), which needs a cooperatively cancellable copy and a cleanup of the partial - // destination; `beginOperation`'s `cancel` slot is where it plugs in. - let operation = store.banners.beginOperation(label: "Duplicating '\(name)'…") + // tone, pinned, cleared on completion, swapped for the error row on failure — and + // carrying Cancel, which 02 promises on copy-shaped work ("remove the partial copy, + // nothing lost") and 03 spends on this command by name. + let operation = store.banners.beginOperation( + label: "Duplicating '\(name)'…", + cancel: { cancellation.cancel() } + ) defer { store.banners.endOperation(operation) } await appModel.flushPendingWork(for: ref) - do { - // Off the main actor: the copy is real I/O on a board that may carry a large `.git`, - // and a spinner drawn by a blocked main thread is a still picture. See - // `BoardDuplicator` for why that is safe here. - let copy = try await Task.detached(priority: .userInitiated) { - try BoardDuplicator.duplicate(boardAt: source, titled: name) - }.value + // 1. The silent Finder-style sibling. + var outcome = await copy(source, titled: name, into: nil, cancellation: cancellation) + + // 2. The save panel, and only where 03 puts it: a *permission* refusal of that sibling. + if case let .failure(.refused(refusal)) = outcome { + Self.logger.notice("duplicate refused: \(refusal.description, privacy: .public)") + guard let chosen = Self.chooseDestination(for: source) else { + // The user declined. Nothing failed, so nothing is said. + return + } + outcome = await copy(source, titled: name, into: chosen, cancellation: cancellation) + } + + switch outcome { + case let .success(copy): appModel.openBoard(at: copy) - } catch let error as BoardWriteError { + case .failure(.cancelled): + // The walk removed its own partial sibling; the row leaves with the `defer`. + Self.logger.notice("duplicate cancelled — the partial copy was removed") + // `.refused` cannot reach here (the panel path either returns above or asks a + // destination that never refuses again), but it carries a real failure, so if it ever + // did, it would be said out loud rather than swallowed. + case let .failure(.failed(error)), let .failure(.refused(error)): Self.logger.error("duplicate failed: \(error.description, privacy: .public)") store.banners.post(error) - } catch { - store.banners.post(BoardWriteError( - operation: .duplicateBoard(title: name), - path: source.path, - reason: .io(message: error.localizedDescription) - )) } } } + + /// One attempt at the copy — off the main actor, wired to the banner row's Cancel. + /// + /// `Task.detached` rather than a child task, for both halves of the reason: the copy is real I/O + /// on a board that may carry a large `.git`, and a spinner drawn by a blocked main thread is a + /// still picture (see `BoardDuplicator` for why running it off the actor is safe); and a detached + /// task's cancellation is *only* the Cancel button's, never something inherited from whatever + /// else the enclosing task is doing. + /// + /// `destination` is `nil` for the Finder-style sibling and the panel's answer otherwise — the two + /// `BoardDuplicator` entry points, which differ only in who chose the location and therefore in + /// whether a permission failure is a question or an answer. + private func copy( + _ source: URL, + titled name: String, + into destination: URL?, + cancellation: DuplicateCancellation + ) async -> Result { + // Cancelled during the flush: the copy never starts, rather than starting and being told to + // stop — the same outcome, reached without making a folder to delete. + guard !cancellation.isCancelled else { return .failure(.cancelled) } + + let task = Task.detached(priority: .userInitiated) { + if let destination { + return try BoardDuplicator.duplicate(boardAt: source, titled: name, into: destination) + } + return try BoardDuplicator.duplicate(boardAt: source, titled: name) + } + cancellation.attach(task) + + do { + return .success(try await task.value) + } catch let failure as BoardDuplicator.Failure { + return .failure(failure) + } catch { + return .failure(.failed(BoardWriteError( + operation: .duplicateBoard(title: name), + path: source.path, + reason: .io(message: error.localizedDescription) + ))) + } + } + + /// The save panel a refusal hands the question to (03, settled) — "pre-filled with the parent + /// folder and the 'copy' name". + /// + /// Both pre-fills are the sibling the app just failed to write, so the panel opens showing + /// exactly what would have happened silently, and one Return makes it happen. Whatever the user + /// changes is then honored verbatim (`BoardDuplicator.duplicate(boardAt:titled:into:)`): the + /// panel is a location grant *and* a choose-another-location affordance, and second-guessing the + /// name it returns would break the second half. + /// + /// `nil` is the user declining, which this command answers with silence. The panel is modal, + /// like the template chooser's — the in-progress row stays up behind it, because the duplicate + /// genuinely is still in progress. + private static func chooseDestination(for source: URL) -> URL? { + let panel = NSSavePanel() + panel.directoryURL = source.deletingLastPathComponent() + panel.nameFieldStringValue = BoardDuplicator.copyDestination(for: source).lastPathComponent + panel.canCreateDirectories = true + panel.isExtensionHidden = false + panel.allowsOtherFileTypes = true + panel.prompt = "Duplicate" + panel.message = "Choose where to keep the duplicate." + + guard panel.runModal() == .OK, let url = panel.url else { return nil } + return url + } +} + +/// The Cancel button's end of a running duplicate: the one piece of state the banner row's `cancel` +/// closure and the copy task have to share. +/// +/// **A main-actor box rather than a lock**, because there is nothing here to race over: the row's +/// `cancel` is `@MainActor @Sendable`, and the task is created and attached on the same actor. The +/// walk itself reads no shared state at all — it reads its own `Task.isCancelled`, which `cancel()` +/// sets by cancelling the task — so this type exists only to close the window between the row +/// appearing and the copy task existing. A Cancel pressed during the flush must not be forgotten by +/// the task that starts after it, which is what `isCancelled` is for. +@MainActor +private final class DuplicateCancellation { + + private var task: Task? + private(set) var isCancelled = false + + func attach(_ task: Task) { + self.task = task + if isCancelled { task.cancel() } + } + + func cancel() { + isCancelled = true + task?.cancel() + } } // MARK: - Reveal in Finder diff --git a/Kanban/App/BoardDuplicator.swift b/Kanban/App/BoardDuplicator.swift index 621c7e6..5a31876 100644 --- a/Kanban/App/BoardDuplicator.swift +++ b/Kanban/App/BoardDuplicator.swift @@ -18,9 +18,34 @@ import Foundation /// - Timestamps, unknown keys, strays, `CLAUDE.user.md`, attachments: verbatim, for the same reason. /// **Nothing here reads a board file at all.** /// -/// `FileManager.copyItem` rather than a walk through `BoardWriter.copyItem`: the Writer's copy path -/// exists to remint identities and restamp frontmatter at an import boundary, and this operation is -/// defined by doing neither of those things. +/// Not `BoardWriter.copyItem`, whose copy path exists to remint identities and restamp frontmatter +/// at an import boundary — this operation is defined by doing neither of those things. +/// +/// ### A per-item walk, because the copy is cancellable +/// +/// 03 settles the shape as well as the promise: "the copy runs as a per-item file walk that checks +/// cancellation between items — **never one monolithic `copyItem`** — and Cancel removes the partial +/// sibling before dismissing the banner (the attachment partial-cleanup precedent): a cancelled +/// duplicate never happened." +/// +/// So the walk *is* the promise: one `FileManager.copyItem` per file — bytes and their metadata +/// copied by the file system, never re-encoded, symlinks copied as symlinks — folders recreated by +/// hand, and a cancellation read **between** items, never mid-file (a copy interrupted inside a +/// 200 MB pack file leaves rubble that is harder to reason about than one more file's wait). The +/// entries of every folder are walked in name order, so the same cancellation removes the same +/// partial tree every time and a failure names the same file twice running. +/// +/// **The partial goes on both exits.** Cancel promises removal; a failure gets it too, because a +/// half-copied board is pure residue — nothing was there before, so there is no true state for it to +/// be (`BoardWriter.copyItem`'s all-or-nothing posture, and 02-architecture.md's "the action visibly +/// doesn't happen"). Only the silence differs: `.cancelled` says nothing, `.failed` gets the banner. +/// The one thing never removed is a destination this walk did not create — an existing name is the +/// user's, and refusing to clobber it is the same stance `BoardWriter.createBoard` takes. +/// +/// The accepted cost of building folders by hand rather than handing the tree to one `copyItem`: +/// **extended attributes on folders do not carry** (files keep theirs — each is still copied by +/// `copyItem` — and POSIX permissions and timestamps are carried explicitly below). Nothing the app, +/// the storage format, or git keeps lives in a folder xattr. /// /// ### Not `@MainActor` /// @@ -32,6 +57,43 @@ import Foundation /// session holds open for the window's whole life, not a per-thread one. enum BoardDuplicator { + // MARK: - Outcomes + + /// The three ways a duplicate ends without a copy — which are also, one for one, the three + /// surfaces 03 gives them: silence, a save panel, a banner. + /// + /// An enum rather than a bare `BoardWriteError` because the command's branch has to be + /// exhaustive: "cancelling the panel cancels the duplicate quietly (no banner — the user + /// declined, nothing failed)" and "non-permission failures (disk full, …) keep the ordinary + /// one-shot banner" are different outcomes of the same call, and a caller that forgot one of + /// them should not compile. + enum Failure: Error, Sendable, Equatable { + + /// The user pressed Cancel on the in-progress row. The partial sibling is already gone and + /// there is nothing to report — "a cancelled duplicate never happened". + case cancelled + + /// The sandbox refused the destination: "the board's security-scoped bookmark grants its + /// subtree, not its parent, so the sibling destination may be unwritable" (03, settled). + /// The caller's cue to open the save panel — the grant *is* the sandbox's own answer — and + /// never a banner on its own. It still carries the write error, so a caller with no panel to + /// offer (and the log) has the specifics. + case refused(BoardWriteError) + + /// Anything else: a full disk, an unreadable source, a name already taken. The ordinary + /// one-shot banner, in the vocabulary every other failed write in the app speaks. + case failed(BoardWriteError) + } + + /// Why a walk stopped and on which item — the recursion's private currency, converted to a + /// `Failure` (and the partial removed) the moment it surfaces. + private enum WalkStop: Error { + case cancelled + case failed(url: URL, error: any Error) + } + + // MARK: - Where the copy lands + /// The Finder-style destination for duplicating `rootURL`: `"Board copy"`, then `"Board copy 2"`, /// `"Board copy 3"`, … — Finder's own ladder, counting up from 2 against what is on disk at /// decision time, one collision at a time. @@ -47,6 +109,10 @@ enum BoardDuplicator { /// /// `fileExists` is the one test, and it is true for a file as much as a folder: anything already /// wearing the name blocks it, which is what keeps a duplicate from ever overwriting something. + /// + /// It doubles as the **save panel's suggested name** when the sibling is refused (03): a parent + /// the sandbox will not let us write is usually one we cannot list either, so the ladder simply + /// finds no collision and suggests `"Board copy"` — the right pre-fill, arrived at honestly. static func copyDestination(for rootURL: URL) -> URL { let parent = rootURL.deletingLastPathComponent() let base = rootURL.deletingPathExtension().lastPathComponent @@ -65,27 +131,109 @@ enum BoardDuplicator { return candidate(name) } - /// Copies the board at `rootURL` to its Finder-style sibling and answers where it landed. + // MARK: - Duplicating + + /// Copies the board at `rootURL` to its Finder-style sibling and answers where it landed — the + /// silent first attempt 03 asks for ("the silent Finder-style sibling is attempted first"). /// /// `title` is the board's display name, carried only so a failure can name the board the user /// pressed Duplicate on — this function never reads it off disk, which is the whole point. /// - /// The failure is a `BoardWriteError` like every other write in the app, so the board window's - /// banner renders it in the vocabulary it already speaks. A copy that fails part-way leaves a - /// partial folder behind; `FileManager` cleans up its own destination on most failures, and the - /// residue that survives is a folder the user can see and delete — the honest outcome, and the - /// one 02's "remove the partial copy" Cancel affordance would formalise when it lands. - static func duplicate(boardAt rootURL: URL, titled title: String?) throws(BoardWriteError) -> URL { - let destination = copyDestination(for: rootURL) + /// **This is the one entry point that can answer `.refused`**, because it is the one that chose + /// the destination: a permission failure here is a question about *where*, and 03 hands that + /// question to a save panel rather than to a banner. + /// + /// `isCancelled` is read between items and nowhere else. Its default is the ambient task's own + /// cancellation, so the caller cancels a duplicate the way it cancels anything else — by + /// cancelling the task doing it — and a test can trip it deterministically at item *N*. + static func duplicate( + boardAt rootURL: URL, + titled title: String?, + isCancelled: () -> Bool = { Task.isCancelled } + ) throws(Failure) -> URL { + try copyBoard(at: rootURL, titled: title, to: copyDestination(for: rootURL), isCancelled: isCancelled) + } + + /// Copies the board at `rootURL` to `destination` — the panel's answer, honored verbatim. + /// + /// The user picked a name and a folder, so neither is second-guessed: no `copy` ladder, no + /// extension repair, and nothing overwritten (an existing name fails rather than being replaced, + /// `BoardWriter.createBoard`'s own refusal — the panel's replace prompt grants access, it does + /// not delete anything). + /// + /// **A permission failure here is an ordinary failure, not a refusal.** The panel *was* the + /// sandbox's answer; asking the same question twice would be a loop, so this one gets the banner. + static func duplicate( + boardAt rootURL: URL, + titled title: String?, + into destination: URL, + isCancelled: () -> Bool = { Task.isCancelled } + ) throws(Failure) -> URL { do { - try FileManager.default.copyItem(at: rootURL, to: destination) + return try copyBoard(at: rootURL, titled: title, to: destination, isCancelled: isCancelled) } catch { - throw BoardWriteError( - operation: .duplicateBoard(title: title), - path: destination.path, + if case let .refused(write) = error { throw .failed(write) } + throw error + } + } + + /// The copy, once the destination is decided: root, walk, restore — with the partial removed on + /// the way out of either exit that is not a copy. + private static func copyBoard( + at rootURL: URL, + titled title: String?, + to destination: URL, + isCancelled: () -> Bool + ) throws(Failure) -> URL { + let operation = WriteOperation.duplicateBoard(title: title) + + /// Every failure in one shape, and the refusal/ordinary split decided in one place. + func failure(at url: URL, _ error: any Error) -> Failure { + let write = BoardWriteError( + operation: operation, + path: url.path, reason: .io(message: error.localizedDescription) ) + return isPermissionRefusal(error) ? .refused(write) : .failed(write) } + + // The source's own attributes, read before anything is created — and the first thing that + // fails when the board root isn't there at all, which is why that failure names the source. + let rootAttributes: [FileAttributeKey: Any] + do { + rootAttributes = try FileManager.default.attributesOfItem(atPath: rootURL.path) + } catch { + throw failure(at: rootURL, error) + } + + // A duplicate cancelled before it began is still a cancelled duplicate; answering here keeps + // the empty destination from ever existing. + if isCancelled() { throw .cancelled } + + // The root first, and **nothing is cleaned up if this fails**: a name already on disk is not + // ours to remove, and this is also where the sandbox says "not here" — the refusal the save + // panel answers, raised before a single byte has been copied. + do { + try createDirectory(at: destination) + } catch { + throw failure(at: destination, error) + } + + do { + try copyContents(of: rootURL, into: destination, isCancelled: isCancelled) + } catch { + // Cancelled or failed, the partial sibling goes: the tree exists only because this call + // made it, and half a board is not a state anything should have to render. + try? FileManager.default.removeItem(at: destination) + switch error { + case .cancelled: + throw .cancelled + case let .failed(url, underlying): + throw failure(at: url, underlying) + } + } + + restoreAttributes(from: rootAttributes, onto: destination) // m7-git: strip the copy's remote configuration — "the duplicate keeps `.git` but has its // remote configuration stripped ... it must not silently push into the original's remote" // (03-board-ui.md). Remotes only: the repo-local `user.name`/`user.email` survives, so the @@ -93,4 +241,139 @@ enum BoardDuplicator { // nothing here — it lives on the registry record, and the copy's record is born fresh. return destination } + + // MARK: - The walk + + /// Copies everything inside `source` into the already-created `destination`, one item at a time. + /// + /// Name order, hidden entries included (no `.skipsHiddenFiles`): `.git`, `.DS_Store` and every + /// other dotfile are part of the fork, and a deterministic order is what makes a cancellation + /// reproducible. + /// + /// Each entry's type comes from `attributesOfItem`, which does **not** traverse symlinks — so a + /// link is copied as a link (by `copyItem`, which does not follow it either) rather than being + /// mistaken for the folder it points at and walked into. + private static func copyContents( + of source: URL, + into destination: URL, + isCancelled: () -> Bool + ) throws(WalkStop) { + let entries: [URL] + do { + entries = try FileManager.default.contentsOfDirectory( + at: source, + includingPropertiesForKeys: nil, + options: [] + ) + } catch { + throw .failed(url: source, error: error) + } + + for entry in entries.sorted(by: { $0.lastPathComponent < $1.lastPathComponent }) { + // Between items, never mid-item: this is the whole of "checks cancellation between + // items", and the reason the copy is a walk at all. + if isCancelled() { throw .cancelled } + + let attributes: [FileAttributeKey: Any] + do { + attributes = try FileManager.default.attributesOfItem(atPath: entry.path) + } catch { + throw .failed(url: entry, error: error) + } + + let isDirectory = attributes[.type] as? FileAttributeType == .typeDirectory + let target = destination.appendingPathComponent(entry.lastPathComponent, isDirectory: isDirectory) + + guard isDirectory else { + // Files, symlinks, and whatever else the file system holds: `copyItem` lands the + // bytes and the metadata that rides with them, byte-for-byte, unread. + do { + try FileManager.default.copyItem(at: entry, to: target) + } catch { + throw .failed(url: entry, error: error) + } + continue + } + + do { + try createDirectory(at: target) + } catch { + throw .failed(url: entry, error: error) + } + try copyContents(of: entry, into: target, isCancelled: isCancelled) + restoreAttributes(from: attributes, onto: target) + } + } + + /// Creates `url` as a plain directory, wearing the process's own default permissions until its + /// contents have landed (`restoreAttributes(from:onto:)` puts the source's back afterwards). + /// + /// **Default permissions first, the source's last**, because a folder is not only a thing being + /// copied but the thing being copied *into*: a source folder that is read-only, or unreadable, + /// would otherwise be reproduced as a destination this walk cannot write its own children into — + /// and, when something later fails, as a partial tree the cleanup cannot remove either. `cp -R` + /// defers the mode for the same reason. + /// + /// `withIntermediateDirectories: false` throughout: every parent either exists already (the walk + /// just made it) or is the one the user pointed at, and inventing a missing folder would be this + /// function deciding where a board lives. + private static func createDirectory(at url: URL) throws { + try FileManager.default.createDirectory(at: url, withIntermediateDirectories: false) + } + + /// Puts a folder's POSIX permissions and its creation and modification dates back, **after** its + /// contents have landed — writing into a folder is itself a modification, and its mode may be + /// what stops the writing (above), so both wait for the subtree to be finished. + /// + /// Best effort by design: a volume that will not take a date back (or a file system with no + /// creation dates at all) is not a reason to fail a duplicate that otherwise worked, and neither + /// a folder's timestamp nor its mode is something the storage format reads. + private static func restoreAttributes(from attributes: [FileAttributeKey: Any], onto url: URL) { + var carried: [FileAttributeKey: Any] = [:] + if let permissions = attributes[.posixPermissions] { + carried[.posixPermissions] = permissions + } + if let created = attributes[.creationDate] { + carried[.creationDate] = created + } + if let modified = attributes[.modificationDate] { + carried[.modificationDate] = modified + } + guard !carried.isEmpty else { return } + try? FileManager.default.setAttributes(carried, ofItemAtPath: url.path) + } + + // MARK: - Classifying a refusal + + /// Whether `error` is the sandbox saying **not here** — the one failure 03 answers with a save + /// panel instead of a banner ("the board's security-scoped bookmark grants its subtree, not its + /// parent"). + /// + /// Pure, and deliberately narrow on both axes: + /// + /// - **Write permission only.** `NSFileWriteNoPermissionError` is what `FileManager` reports for + /// a create it is not allowed to make, and `EACCES`/`EPERM` is what that wraps when the + /// underlying error survives. A *read* refusal is not in it: the source is inside the grant the + /// window already holds, so a board we cannot read is a broken board, not a question about + /// where the copy should go — re-pointing the destination would not help it. + /// - **Permission only.** A full disk or a read-only volume is a genuine failure with a banner to + /// its name; offering a save panel for it would be the app pretending the user chose wrong. + /// + /// **A Cocoa error answers for itself and its chain is not consulted**, which is the subtle half: + /// Foundation hangs the POSIX `EACCES` under a *read* denial as readily as under a write one, so + /// a walk that went looking for `EACCES` anywhere would call an unreadable source folder a + /// question about the destination. Cocoa has already drawn the read/write line; this trusts it. + /// The underlying chain is only followed out of domains that have not classified anything — + /// where a bare POSIX `EACCES`/`EPERM` is all the refusal there is to find. + static func isPermissionRefusal(_ error: any Error) -> Bool { + let nsError = error as NSError + if nsError.domain == NSCocoaErrorDomain { + return nsError.code == NSFileWriteNoPermissionError + } + if nsError.domain == NSPOSIXErrorDomain { + return nsError.code == Int(EACCES) || nsError.code == Int(EPERM) + } + guard let underlying = nsError.userInfo[NSUnderlyingErrorKey] as? NSError else { return false } + return isPermissionRefusal(underlying) + } } diff --git a/KanbanTests/BoardDuplicatorTests.swift b/KanbanTests/BoardDuplicatorTests.swift index 0f0fecc..de68314 100644 --- a/KanbanTests/BoardDuplicatorTests.swift +++ b/KanbanTests/BoardDuplicatorTests.swift @@ -12,9 +12,13 @@ import Testing /// born exactly matching its history"; /// - it doesn't overwrite — the Finder-style `copy` ladder renames instead. /// +/// And a fourth, which the per-item walk exists for: **it doesn't leave residue** — a cancelled or +/// failed copy removes its own partial sibling, "a cancelled duplicate never happened". +/// /// Tested against real folders, because every one of those is a claim about bytes on disk. The -/// window flow around the copy (the flush, the banner row, the open that follows) is the command's, -/// not this type's. +/// window flow around the copy (the flush, the banner row, the save panel, the open that follows) is +/// the command's, not this type's — what belongs here is the vocabulary that flow branches on: +/// cancelled, refused, failed. // MARK: - Fixtures @@ -54,6 +58,21 @@ private func tree(of root: URL) throws -> Set { return paths } +/// `writeFailure`'s twin for the duplicate's own three-outcome vocabulary: the copy answers +/// `.cancelled`, `.refused` or `.failed`, and which one is the whole question every test below asks. +private func duplicateFailure(_ operation: () throws -> Void) -> BoardDuplicator.Failure? { + do { + try operation() + Issue.record("expected the duplicate to fail, but it succeeded") + return nil + } catch let failure as BoardDuplicator.Failure { + return failure + } catch { + Issue.record("expected a BoardDuplicator.Failure, got \(error)") + return nil + } +} + // MARK: - Tests @MainActor @@ -180,10 +199,205 @@ struct BoardDuplicatorTests { defer { board.fixture.tearDown() } let missing = board.fixture.url("Never.kanban") - let error = writeFailure { _ = try BoardDuplicator.duplicate(boardAt: missing, titled: "Never") } + let failure = duplicateFailure { _ = try BoardDuplicator.duplicate(boardAt: missing, titled: "Never") } - #expect(error?.operation == .duplicateBoard(title: "Never"), + // An ordinary failure, not a refusal: a source that isn't there is not a question about + // where the copy should go, so no save panel could help it. + guard case let .failed(error) = failure else { + Issue.record("expected an ordinary failure, got \(String(describing: failure))") + return + } + #expect(error.operation == .duplicateBoard(title: "Never"), "the user pressed Duplicate; the banner must say so") - #expect(BannerCenter.headline(for: try #require(error)).hasPrefix("Couldn't duplicate 'Never'")) + #expect(BannerCenter.headline(for: error).hasPrefix("Couldn't duplicate 'Never'")) + } + + // MARK: - The per-item walk + + @Test("Hidden folders, empty ones and nested strays all ride along — the walk is not the loader") + func theWalkCarriesEverythingATreeCanHold() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + // A `.git`-shaped hidden folder with a file in it, an empty folder inside that (the shape a + // fresh repository actually has), and a stray beside a card's index. + try board.fixture.file("Roadmap.kanban/.git/HEAD", Data("ref: refs/heads/main\n".utf8)) + try FileManager.default.createDirectory( + at: board.root.appendingPathComponent(".git/objects", isDirectory: true), + withIntermediateDirectories: true + ) + try board.fixture.file("Roadmap.kanban/\(Ident.lane1)/\(Ident.card1)/attachments/shot.png", Data([0x01, 0x02])) + + let copy = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap") + + #expect(try tree(of: copy) == tree(of: board.root), + "an item-by-item walk carries exactly what one copyItem would have") + #expect(try Data(contentsOf: copy.appendingPathComponent(".git/HEAD")) + == Data("ref: refs/heads/main\n".utf8)) + var isDirectory: ObjCBool = false + #expect(FileManager.default.fileExists( + atPath: copy.appendingPathComponent(".git/objects").path, + isDirectory: &isDirectory + ) && isDirectory.boolValue, "an empty folder is a folder, not nothing") + } + + // MARK: - Cancel + + @Test("Cancelling between items removes the partial sibling — a cancelled duplicate never happened") + func cancellingRemovesThePartialSibling() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + let before = try tree(of: board.root) + + // Trips on the fourth read: the root folder exists by then and the walk is two levels deep + // into it, so there is a genuine partial tree to remove rather than nothing to clean up. + var reads = 0 + let failure = duplicateFailure { + _ = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap", isCancelled: { + reads += 1 + return reads > 3 + }) + } + + #expect(failure == BoardDuplicator.Failure.cancelled) + #expect(try Set(board.fixture.entryNames("")) == ["Roadmap.kanban"], + "no 'Roadmap copy.kanban' survives — the partial went with the cancellation") + #expect(try tree(of: board.root) == before, "and the original was never touched") + } + + @Test("A duplicate cancelled before it began never creates the destination at all") + func cancellingBeforeTheFirstItemCreatesNothing() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + + let failure = duplicateFailure { + _ = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap", isCancelled: { true }) + } + + #expect(failure == BoardDuplicator.Failure.cancelled) + #expect(try Set(board.fixture.entryNames("")) == ["Roadmap.kanban"]) + } + + @Test("A failure mid-walk removes the partial sibling too, and says what went wrong") + func aFailedWalkRemovesThePartialSibling() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + // A card folder the walk cannot list, standing in for any I/O that gives out part-way. + let unreadable = board.root.appendingPathComponent("\(Ident.lane1)/\(Ident.card1)") + try FileManager.default.setAttributes([.posixPermissions: 0o000], ofItemAtPath: unreadable.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: unreadable.path) } + + let failure = duplicateFailure { _ = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap") } + + guard case let .failed(error) = failure else { + Issue.record("expected an ordinary failure, got \(String(describing: failure))") + return + } + #expect(error.operation == .duplicateBoard(title: "Roadmap")) + #expect(error.path == unreadable.path, "fail-fast names the item it stopped on") + #expect(try Set(board.fixture.entryNames("")) == ["Roadmap.kanban"], + "a half-copied board is residue — it goes, and the banner is what remains") + } + + // MARK: - Refusal and the panel's answer + + @Test("A sandbox refusal of the sibling is a refusal, not a failure — the save panel's cue") + func anUnwritableParentIsARefusal() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + // The board's *parent* is unwritable while the board itself is not: exactly the shape of a + // bookmark that grants a subtree and not the folder above it. + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: board.fixture.root.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: board.fixture.root.path) } + + let failure = duplicateFailure { _ = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap") } + + guard case let .refused(error) = failure else { + Issue.record("expected a refusal, got \(String(describing: failure))") + return + } + #expect(error.operation == .duplicateBoard(title: "Roadmap")) + } + + @Test("A refusal at a destination the user chose is an ordinary failure — the panel isn't asked twice") + func aRefusalAtAnAnsweredDestinationIsAnOrdinaryFailure() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: board.fixture.root.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: board.fixture.root.path) } + + let chosen = board.fixture.url("Chosen.kanban") + let failure = duplicateFailure { + _ = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap", into: chosen) + } + + guard case .failed = failure else { + Issue.record("expected an ordinary failure, got \(String(describing: failure))") + return + } + } + + @Test("A chosen destination is honored verbatim, name and location alike") + func aChosenDestinationIsHonoredVerbatim() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + let elsewhere = board.fixture.url("Elsewhere") + try FileManager.default.createDirectory(at: elsewhere, withIntermediateDirectories: true) + // Not the "copy" ladder's answer at all: another folder, another name, no extension. + let chosen = elsewhere.appendingPathComponent("Fork", isDirectory: true) + + let copy = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap", into: chosen) + + #expect(copy == chosen) + #expect(try tree(of: copy) == tree(of: board.root)) + } + + @Test("A chosen name already on disk fails, and is never overwritten or removed") + func aChosenNameAlreadyTakenFails() throws { + let board = try makeBoard(named: "Roadmap.kanban") + defer { board.fixture.tearDown() } + let occupied = try board.fixture.file("Taken.kanban", Data("not a board, and not yours to delete".utf8)) + + let failure = duplicateFailure { + _ = try BoardDuplicator.duplicate(boardAt: board.root, titled: "Roadmap", into: occupied) + } + + guard case .failed = failure else { + Issue.record("expected an ordinary failure, got \(String(describing: failure))") + return + } + #expect(try board.fixture.data("Taken.kanban") == Data("not a board, and not yours to delete".utf8), + "the destination this call did not create is the one thing cleanup never removes") + } + + // MARK: - Classifying a refusal + + @Test("Only a write-permission denial is the panel's question; everything else is a banner") + func permissionRefusalsAreClassifiedNarrowly() { + #expect(BoardDuplicator.isPermissionRefusal( + NSError(domain: NSCocoaErrorDomain, code: NSFileWriteNoPermissionError) + )) + #expect(BoardDuplicator.isPermissionRefusal( + NSError(domain: NSPOSIXErrorDomain, code: Int(EACCES)) + )) + #expect(BoardDuplicator.isPermissionRefusal(NSError( + domain: "SomeWrapper", + code: 1, + userInfo: [NSUnderlyingErrorKey: NSError(domain: NSPOSIXErrorDomain, code: Int(EPERM))] + )), "an unclassified wrapper is followed through to the refusal underneath") + + #expect(!BoardDuplicator.isPermissionRefusal( + NSError(domain: NSCocoaErrorDomain, code: NSFileWriteOutOfSpaceError) + ), "a full disk is a real failure — a save panel would be the app blaming the user") + #expect(!BoardDuplicator.isPermissionRefusal( + NSError(domain: NSCocoaErrorDomain, code: NSFileWriteVolumeReadOnlyError) + )) + #expect(!BoardDuplicator.isPermissionRefusal(NSError( + domain: NSCocoaErrorDomain, + code: NSFileReadNoPermissionError, + userInfo: [NSUnderlyingErrorKey: NSError(domain: NSPOSIXErrorDomain, code: Int(EACCES))] + )), """ + a source we cannot read is a broken board, not a question about where the copy goes — and \ + Foundation hangs the same EACCES under it, which is why the chain is not searched blindly + """) } }