From 840528d14c076d457e4489e693047331dbb3cf16 Mon Sep 17 00:00:00 2001 From: rzen Date: Sun, 26 Jul 2026 17:48:46 -0400 Subject: [PATCH] Implement attachment import and listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit importAttachments lands files into a card's attachments/ (created on first import — the one folder the app ever makes there), never overwriting and never refusing a name: collisions auto-rename Finder-style, counting up from 2, with a same-named subfolder blocking a name exactly like a file. Sources are validated before any copy; a failed copy removes the partial destination and throws naming the source file; a multi-file batch stops at the first failure with earlier files staying landed. listAttachments is the flat surface: top-level regular files only, Finder-sorted, subfolders and hidden entries excluded and untouched; a missing attachments/ lists empty. 11 new unit tests; 245 total green. Claude-Session: https://claude.ai/code/session_018BjQRYBR6jQja3jCRi5S3A --- Kanban/Storage/BoardWriter.swift | 171 ++++++++++++++++++++++ KanbanTests/BoardWriterTests.swift | 225 +++++++++++++++++++++++++++++ 2 files changed, 396 insertions(+) diff --git a/Kanban/Storage/BoardWriter.swift b/Kanban/Storage/BoardWriter.swift index 04430e2..26bc01f 100644 --- a/Kanban/Storage/BoardWriter.swift +++ b/Kanban/Storage/BoardWriter.swift @@ -814,6 +814,165 @@ public enum BoardWriter: Sendable { } } + // MARK: - Attachments + + /// The one folder this app ever creates under a card — every other subfolder under + /// `attachments/` is tolerated but never made or named by the app (01-storage-format.md § + /// Attachments). Internal rather than `private`: `importAttachments` and `listAttachments` + /// must never disagree about which folder holds a card's files. + static let attachmentsFolderName = "attachments" + + /// Imports files into a card's `attachments/` folder, creating it on first import — the + /// write side of 01-storage-format.md § Attachments. **Never refuses the drop**: a name + /// already taken is auto-renamed Finder-style (`shot.png` → `shot 2.png` → `shot 3.png`, …) + /// rather than overwritten or bounced. + /// + /// A multi-file drop imports **in order, one finished copy at a time** — not a transaction + /// across the batch: the first failure stops it and throws naming the failing source file, + /// but every file already imported stays landed, because each import already completed as + /// its own write before the next one starts. Returns the landed names, in input order, for + /// exactly the files that made it in. + /// + /// The order of checks is the contract: + /// + /// 1. **`cardFolder` must be an existing, UUID-shaped directory** — attachments belong to + /// cards, the same shape guard `deleteItem`/`restoreItem`/`purgeItem` lean on + /// (`checkIsUUIDShaped`): a lane or a board root is refused before anything else happens. + /// 2. **`attachments/` is created if missing** (`.io` naming `cardFolder` on failure) — the + /// one exception to "subfolders are never created by the app" (§ Attachments); every + /// other folder under it is the user's or a hand-editor's, left alone. + /// 3. **Each source is validated before it is touched**: must exist and be a regular file — + /// not a directory, which is out of this call's scope entirely — refused `.unreadable` + /// naming the source *before* any copy for that file is attempted, so a batch never + /// partially copies something it was about to refuse. + /// 4. **The collision-free name is decided, then copied to directly** — no temp name and + /// rename, unlike `atomicReplace`: `FileManager.copyItem` lands the bytes straight under + /// the final name. `copyItem` is not atomic, so on any failure the partial destination is + /// removed best-effort and `.io` is thrown naming the source file — "no half-copied + /// attachment is ever left in `attachments/`" (02-architecture.md § Write-failure + /// surfacing) is a promise about the *failure path*, not a stronger atomicity claim + /// `copyItem` cannot make. A crash mid-copy (process death, not a caught `Error`) can + /// still leave a partial file on disk — the accepted limit of a non-atomic copy, the same + /// one an ordinary Finder copy has. + public static func importAttachments( + _ sourceURLs: [URL], + intoCard cardFolder: URL + ) throws(BoardWriteError) -> [ImportedAttachment] { + let operation = "import attachment" + try checkIsDirectory(cardFolder, describedAs: "card folder", operation: operation) + try checkIsUUIDShaped(cardFolder, operation: operation) + + let attachmentsFolder = cardFolder.appendingPathComponent(attachmentsFolderName, isDirectory: true) + do { + try FileManager.default.createDirectory(at: attachmentsFolder, withIntermediateDirectories: true) + } catch { + throw BoardWriteError( + operation: operation, + path: cardFolder.path, + reason: .io(message: "could not create attachments folder: \(error.localizedDescription)") + ) + } + + var landed: [ImportedAttachment] = [] + for sourceURL in sourceURLs { + var isDirectory: ObjCBool = false + guard FileManager.default.fileExists(atPath: sourceURL.path, isDirectory: &isDirectory) else { + throw BoardWriteError( + operation: operation, + path: sourceURL.path, + reason: .unreadable(message: "file does not exist") + ) + } + guard !isDirectory.boolValue else { + throw BoardWriteError( + operation: operation, + path: sourceURL.path, + reason: .unreadable(message: "is a folder, not a file — importing a folder is not supported") + ) + } + + let name = freshAttachmentName(for: sourceURL.lastPathComponent, in: attachmentsFolder) + let destinationURL = attachmentsFolder.appendingPathComponent(name) + do { + try FileManager.default.copyItem(at: sourceURL, to: destinationURL) + } catch { + try? FileManager.default.removeItem(at: destinationURL) + throw BoardWriteError( + operation: operation, + path: sourceURL.path, + reason: .io(message: "could not copy file: \(error.localizedDescription)") + ) + } + landed.append(ImportedAttachment(sourceURL: sourceURL, fileName: name)) + } + return landed + } + + /// The Finder-style collision-free name for `originalName` landing in `folder`: the name + /// itself when nothing on disk claims it yet, else the base name suffixed `" 2"`, `" 3"`, … + /// — counting up from 2 against what is on disk *at decision time*, one collision at a time. + /// + /// Splits `originalName` the way `URL` itself does (`deletingPathExtension`/ + /// `pathExtension`), which is also exactly how Finder does it: an extension-less name + /// suffixes directly (`"notes"` → `"notes 2"`), and a multi-dot name splits after the + /// *last* dot (`"archive.tar.gz"` → `"archive.tar 2.gz"`, not `"archive 2.tar.gz"`) — both + /// accepted as what Finder itself produces, not worked around. + /// + /// `fileExists` is the one test, and it is true for a directory as much as a file — a + /// same-named *subfolder* blocks the name exactly like a file would, so an import never + /// overwrites, renames, or descends into one; it just renames the incoming file instead. + private static func freshAttachmentName(for originalName: String, in folder: URL) -> String { + guard FileManager.default.fileExists(atPath: folder.appendingPathComponent(originalName).path) else { + return originalName + } + + let nameURL = URL(fileURLWithPath: originalName) + let base = nameURL.deletingPathExtension().lastPathComponent + let ext = nameURL.pathExtension + + var counter = 2 + while true { + let candidate = ext.isEmpty ? "\(base) \(counter)" : "\(base) \(counter).\(ext)" + guard FileManager.default.fileExists(atPath: folder.appendingPathComponent(candidate).path) else { + return candidate + } + counter += 1 + } + } + + /// The card's flat attachment listing (01-storage-format.md § Attachments, "the app's + /// attachment surfaces … are flat: top-level files only"): the top-level *files* of + /// `attachments/`, subfolders and hidden files excluded, symlinks excluded, sorted + /// `localizedStandardCompare` — Finder order, so `"shot 2.png"` sorts before `"shot + /// 10.png"` rather than after it. + /// + /// A **missing** `attachments/` lists as `[]`, not an error: nothing has been imported yet + /// is an ordinary state, not a malformed one. `cardFolder` itself missing, or not a + /// directory, *is* `.unreadable` — that is a caller asking about a card that isn't there, + /// not an empty listing. Purely a read: nothing here ever creates `attachments/` or + /// disturbs anything inside it, subfolders included. + public static func listAttachments(ofCard cardFolder: URL) throws(BoardWriteError) -> [String] { + let operation = "list attachments" + try checkIsDirectory(cardFolder, describedAs: "card folder", operation: operation) + + let attachmentsFolder = cardFolder.appendingPathComponent(attachmentsFolderName, isDirectory: true) + guard let entries = try? FileManager.default.contentsOfDirectory( + at: attachmentsFolder, + includingPropertiesForKeys: [.isRegularFileKey, .isSymbolicLinkKey], + options: [.skipsHiddenFiles] + ) else { + return [] + } + + let files = entries.filter { url in + guard let values = try? url.resourceValues(forKeys: [.isRegularFileKey, .isSymbolicLinkKey]) else { + return false + } + return values.isRegularFile == true && values.isSymbolicLink != true + } + return files.map(\.lastPathComponent).sorted { $0.localizedStandardCompare($1) == .orderedAscending } + } + // MARK: - Move/copy pre-flight /// The rank a moved or copied root lands on: the caller's explicit value — a drop between @@ -941,6 +1100,18 @@ public enum CopyStamps: Sendable { case born } +// MARK: - Attachment vocabulary + +/// One imported attachment: where it came from and the name it landed under. `fileName` is the +/// post-collision-rename name — `sourceURL.lastPathComponent` when nothing on disk claimed it, +/// the Finder-style `"… 2"` (or higher) variant otherwise (`importAttachments`). `sourceURL` is +/// carried through unchanged so a caller can report the batch (which files came from where) +/// without re-deriving it from input order. +public struct ImportedAttachment: Sendable, Equatable { + public let sourceURL: URL + public let fileName: String +} + // MARK: - Error /// A write that did not happen, said out loud: which operation, which file, and why — diff --git a/KanbanTests/BoardWriterTests.swift b/KanbanTests/BoardWriterTests.swift index f195e11..84f7b3b 100644 --- a/KanbanTests/BoardWriterTests.swift +++ b/KanbanTests/BoardWriterTests.swift @@ -1909,3 +1909,228 @@ struct BoardWriterSameParentMoveTests { #expect(try fixture.entryNames("A.kanban/\(Ident.lane1)").contains(Ident.card1)) } } + +// MARK: - Import attachments + +/// `BoardWriter.importAttachments`: the write side of 01-storage-format.md § Attachments — +/// never refuses a drop, auto-renames Finder-style on collision, and stops a multi-file batch +/// at the first failure without undoing what already landed. +struct BoardWriterImportAttachmentsTests { + /// A bare UUID-shaped card folder — `importAttachments` never reads its `index.md`, so a + /// minimal one (like every other card these suites mint) is as good a target as a fully + /// loaded one. + private func card(_ fixture: WriterFixture) throws -> URL { + try fixture.item(Ident.card1, Item.rich(order: "1024", title: "Card")) + } + + @Test func firstImportCreatesAttachmentsAndLandsTheOriginalName() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + let bytes = Data([0x89, 0x50, 0x4E, 0x47, 0x00, 0xFF]) + let source = try fixture.file("sources/shot.png", bytes) + + let landed = try BoardWriter.importAttachments([source], intoCard: card) + + #expect(landed == [ImportedAttachment(sourceURL: source, fileName: "shot.png")]) + #expect(try fixture.data("\(Ident.card1)/attachments/shot.png") == bytes) + // The source is a copy's origin, never its casualty. + #expect(try fixture.data("sources/shot.png") == bytes) + } + + /// Three separate drops of a file named `shot.png`: never overwritten, never refused — each + /// lands under the next free Finder-style name, and all three coexist afterwards. + @Test func collidingImportsAutoRenameFinderStyle() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + let first = try fixture.file("sources/1/shot.png", Data([0x01])) + let second = try fixture.file("sources/2/shot.png", Data([0x02])) + let third = try fixture.file("sources/3/shot.png", Data([0x03])) + + let landed1 = try BoardWriter.importAttachments([first], intoCard: card) + let landed2 = try BoardWriter.importAttachments([second], intoCard: card) + let landed3 = try BoardWriter.importAttachments([third], intoCard: card) + + #expect(landed1.map(\.fileName) == ["shot.png"]) + #expect(landed2.map(\.fileName) == ["shot 2.png"]) + #expect(landed3.map(\.fileName) == ["shot 3.png"]) + #expect(try fixture.entryNames("\(Ident.card1)/attachments") == ["shot 2.png", "shot 3.png", "shot.png"]) + #expect(try fixture.data("\(Ident.card1)/attachments/shot.png") == Data([0x01])) + #expect(try fixture.data("\(Ident.card1)/attachments/shot 2.png") == Data([0x02])) + #expect(try fixture.data("\(Ident.card1)/attachments/shot 3.png") == Data([0x03])) + } + + /// A name with no extension at all suffixes directly — `"notes"` → `"notes 2"`, not + /// `"notes 2."` or some other extension-shaped artifact. + @Test func anExtensionLessCollisionRenamesToNotes2() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + let first = try fixture.file("sources/1/notes", Data("first".utf8)) + let second = try fixture.file("sources/2/notes", Data("second".utf8)) + + _ = try BoardWriter.importAttachments([first], intoCard: card) + let landed = try BoardWriter.importAttachments([second], intoCard: card) + + #expect(landed.map(\.fileName) == ["notes 2"]) + #expect(try fixture.entryNames("\(Ident.card1)/attachments") == ["notes", "notes 2"]) + } + + /// `fileExists` is true for a directory as much as a file: a subfolder that happens to share + /// the incoming name blocks it exactly like a file would, and is never touched by the import. + @Test func aFileNamedLikeAnExistingSubfolderAlsoRenames() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + try fixture.file("\(Ident.card1)/attachments/shot.png/inner.txt", Data("hand-made".utf8)) + let source = try fixture.file("sources/shot.png", Data([0x09])) + + let landed = try BoardWriter.importAttachments([source], intoCard: card) + + #expect(landed.map(\.fileName) == ["shot 2.png"]) + #expect(try fixture.data("\(Ident.card1)/attachments/shot 2.png") == Data([0x09])) + #expect(try fixture.entryNames("\(Ident.card1)/attachments/shot.png") == ["inner.txt"]) + } + + /// One call, three sources, two of which collide with what is already there and with each + /// other in turn — the returned names track input order, not landing order. + @Test func multiFileImportInOneCallReturnsLandedNamesInInputOrder() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + try fixture.file("\(Ident.card1)/attachments/shot.png", Data([0x00])) + let shot = try fixture.file("sources/1/shot.png", Data([0x01])) + let notes = try fixture.file("sources/2/notes.txt", Data([0x02])) + let shotAgain = try fixture.file("sources/3/shot.png", Data([0x03])) + + let landed = try BoardWriter.importAttachments([shot, notes, shotAgain], intoCard: card) + + #expect(landed.map(\.fileName) == ["shot 2.png", "notes.txt", "shot 3.png"]) + #expect(landed.map(\.sourceURL) == [shot, notes, shotAgain]) + #expect(try fixture.entryNames("\(Ident.card1)/attachments") + == ["notes.txt", "shot 2.png", "shot 3.png", "shot.png"]) + } + + /// Refused before any copy for that file: the failing source is named, nothing of its lands, + /// and — because each import is its own completed write — the file that landed *before* it + /// in the same batch stays landed rather than being rolled back. + @Test func anUnreadableSourceRefusesNamingItAndStopsTheBatch() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + let first = try fixture.file("sources/shot.png", Data([0x01])) + let missing = fixture.url("sources").appendingPathComponent("missing.png") + let third = try fixture.file("sources/third.png", Data([0x03])) + + let error = writeFailure { + _ = try BoardWriter.importAttachments([first, missing, third], intoCard: card) + } + guard case .unreadable = error?.reason else { + Issue.record("expected .unreadable, got \(String(describing: error?.reason))") + return + } + #expect(error?.path == missing.path) + #expect(error?.operation == "import attachment") + #expect(try fixture.entryNames("\(Ident.card1)/attachments") == ["shot.png"]) + } + + /// Attachments belong to cards: the shape guard `deleteItem`/`restoreItem`/`purgeItem` share + /// refuses a board root (or any non-UUID-shaped folder) before `attachments/` is even + /// considered. + @Test func importIntoANonUUIDShapedFolderIsRefused() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let root = try fixture.item("A.kanban", Item.board) + let source = try fixture.file("sources/shot.png", Data([0x01])) + + let error = writeFailure { + _ = try BoardWriter.importAttachments([source], intoCard: root) + } + guard case let .unreadable(message) = error?.reason else { + Issue.record("expected .unreadable, got \(String(describing: error?.reason))") + return + } + #expect(message.contains("UUID-shaped")) + #expect(!fixture.exists("A.kanban/attachments")) + } + + /// "Never refuses the drop" is policy, not an I/O guarantee: a copy that genuinely cannot + /// land — `attachments/` made unwritable here, standing in for a full disk or a permissions + /// error — surfaces as `.io` naming the source file, and no partial file is left behind. + @Test func aFailedCopyLeavesNoPartialAttachment() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + let existing = try fixture.file("sources/existing.png", Data([0x00])) + _ = try BoardWriter.importAttachments([existing], intoCard: card) + let attachmentsFolder = fixture.url(Ident.card1).appendingPathComponent("attachments") + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: attachmentsFolder.path) + + let source = try fixture.file("sources/shot.png", Data([0x01])) + let error = writeFailure { + _ = try BoardWriter.importAttachments([source], intoCard: card) + } + guard case .io = error?.reason else { + Issue.record("expected .io, got \(String(describing: error?.reason))") + return + } + #expect(error?.path == source.path) + #expect(try fixture.entryNames("\(Ident.card1)/attachments") == ["existing.png"]) + } +} + +// MARK: - List attachments + +/// `BoardWriter.listAttachments`: the flat, top-level-files-only read (01-storage-format.md § +/// Attachments) that never writes anything, not even `attachments/` itself. +struct BoardWriterListAttachmentsTests { + private func card(_ fixture: WriterFixture) throws -> URL { + try fixture.item(Ident.card1, Item.rich(order: "1024", title: "Card")) + } + + /// Subfolders and hidden files never surface, and survive both the import and the listing + /// untouched; the sort is Finder's numeric order, not plain string order (`"shot 2.png"` + /// before `"shot 10.png"`, which a byte-wise sort would put the other way). + @Test func listingIsFlatExcludesHiddenFilesAndSortsFinderStyle() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + let shot2 = try fixture.file("sources/1/shot 2.png", Data([0x01])) + let shot10 = try fixture.file("sources/2/shot 10.png", Data([0x02])) + _ = try BoardWriter.importAttachments([shot2, shot10], intoCard: card) + try fixture.file("\(Ident.card1)/attachments/sub/inside.txt", Data("hand-made".utf8)) + try fixture.file("\(Ident.card1)/attachments/.DS_Store", Data("hidden".utf8)) + + let listing = try BoardWriter.listAttachments(ofCard: card) + + #expect(listing == ["shot 2.png", "shot 10.png"]) + #expect(try fixture.entryNames("\(Ident.card1)/attachments/sub") == ["inside.txt"]) + #expect(fixture.exists("\(Ident.card1)/attachments/.DS_Store")) + } + + /// Nothing imported yet is an ordinary state, not an error — `attachments/` need not exist + /// for a card to list cleanly as empty. + @Test func aMissingAttachmentsFolderListsAsEmpty() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let card = try card(fixture) + + #expect(try BoardWriter.listAttachments(ofCard: card) == []) + #expect(!fixture.exists("\(Ident.card1)/attachments")) + } + + /// Unlike a missing `attachments/`, a missing *card* is a loud error — the caller asked + /// about something that is not there. + @Test func aMissingCardFolderIsALoudUnreadableError() throws { + let fixture = try WriterFixture() + defer { fixture.tearDown() } + let missing = fixture.url(Ident.card1) + + let error = writeFailure { _ = try BoardWriter.listAttachments(ofCard: missing) } + guard case .unreadable = error?.reason else { + Issue.record("expected .unreadable, got \(String(describing: error?.reason))") + return + } + } +}