diff --git a/Info.plist b/Info.plist index 2755b1e..60782a9 100644 --- a/Info.plist +++ b/Info.plist @@ -13,9 +13,9 @@ CFBundlePackageType APPL CFBundleShortVersionString - 0.2.0 + 0.3.0 CFBundleVersion - 2 + 3 LSMinimumSystemVersion 14.0 LSUIElement diff --git a/Package.swift b/Package.swift index e8361c4..86f89ee 100644 --- a/Package.swift +++ b/Package.swift @@ -27,5 +27,14 @@ let package = Package( dependencies: ["ShotdeckCore"], swiftSettings: [.swiftLanguageMode(.v6)] ), + // Exercises the real Shotdeck-app-target wiring (AppDelegate.makeLaunchModel(), + // AppModel.bootstrap()) via @testable import — logic ShotdeckCoreTests cannot + // reach because it only depends on ShotdeckCore, not the Shotdeck executable + // target itself. See ReturnWatcherLaunchWiringTests.swift. + .testTarget( + name: "ShotdeckTests", + dependencies: ["Shotdeck", "ShotdeckCore"], + swiftSettings: [.swiftLanguageMode(.v6)] + ), ] ) diff --git a/Sources/Shotdeck/AppModel.swift b/Sources/Shotdeck/AppModel.swift index 47897ad..79888da 100644 --- a/Sources/Shotdeck/AppModel.swift +++ b/Sources/Shotdeck/AppModel.swift @@ -32,6 +32,14 @@ public final class AppModel { public private(set) var isSending: Bool = false public private(set) var outboxDisplayName: String public private(set) var watchFolderDisplayName: String + /// Live transport choice; WP-onedrive reads this to pick the send path and to drive + /// the Settings "Send via" picker and the menu's "Send…" label. + public private(set) var transport: SendTransport + /// Ground truth for the Settings OneDrive row: nil means "no OneDrive folder found". + /// Views read this instead of calling `OneDriveLocator.resolveOneDriveFolder()` + /// directly, so state (and testing with a fake home) flows through the model like + /// everything else — never a View reaching past the model for real UserDefaults/home. + public private(set) var resolvedOneDriveFolder: URL? /// Live outbox; WP-4b reads this (not `paths.outbox`) so Settings folder changes take effect. public private(set) var outboxURL: URL /// Live watch folder; WP-4c updates this alongside `ReturnWatcher.updateWatchFolder`. @@ -81,12 +89,16 @@ public final class AppModel { ) self.region = Self.loadPersistedRegion() self.screenRecordingGranted = ScreenCapturer.isScreenRecordingGranted - // Seeded from FolderSettings.resolve() via resolvedAppSupportPaths — never .standard(). - let folders = FolderSettings.resolve() + // Seeded from TransportSettings.effectiveFolders() — the one place that combines + // the transport choice with FolderSettings/OneDriveLocator. Never call + // FolderSettings.resolve() directly outside that function. + let folders = TransportSettings.effectiveFolders() self.outboxURL = folders.outbox self.watchFolderURL = folders.watch self.outboxDisplayName = folders.outbox.lastPathComponent self.watchFolderDisplayName = folders.watch.lastPathComponent + self.transport = folders.transport + self.resolvedOneDriveFolder = OneDriveLocator.resolveOneDriveFolder() self.captureHotkey = HotkeyPreference.load() self.updateChecker = UpdateChecker() self.updateChecker.onChecked = { [weak self] in @@ -118,6 +130,23 @@ public final class AppModel { watchFolderURL = watch setFolderDisplayNames(outbox: outbox.lastPathComponent, watch: watch.lastPathComponent) } + func setTransport(_ value: SendTransport) { transport = value } + func setResolvedOneDriveFolder(_ value: URL?) { resolvedOneDriveFolder = value } + + /// Bumped by chooseTransport/chooseOneDriveFolder (SettingsView.swift) before each + /// spawns its async watcher-reconcile Task; that Task checks its own snapshot + /// against the live value before every mutating step, so rapid toggling always + /// lets the LAST choice win instead of applying stale, superseded work. Not + /// `@Observable`-relevant state — pure internal bookkeeping, never read by a View. + var reconcileGeneration = 0 + /// The MOST RECENT watcher-reconcile Task spawned by chooseTransport/ + /// chooseOneDriveFolder, if one is still (or was just) in flight. send() awaits + /// this BEFORE snapshotting transport/folder, so a toggle immediately followed by + /// Send can never race ahead of the reconcile it depends on (the watcher's + /// recordUncommented flag briefly lagging the just-chosen transport, for example). + /// `Task` never throws; awaiting an already-completed task's `.value` + /// returns immediately. Not `@Observable`-relevant — pure internal bookkeeping. + var pendingReconcileTask: Task? func rememberLastComposedPDF(_ url: URL) { lastComposedPDFURL = url } /// True when a last-composed PDF path is known this run, or the newest @@ -185,7 +214,29 @@ public final class AppModel { // Empty ledger on first run is not an error. } + // OneDrive mode: outbox == watch folder, so a freshly written, unmarked PDF must + // never show up as a return; only a document that already carries a mark does. + // Also make sure the resolved OneDrive folder actually exists before the + // watcher starts watching it (bootstrap is the other creation trigger besides + // chooseTransport/chooseOneDriveFolder — see TransportSettings.effectiveFolders). + if transport == .oneDrive { + try? FileManager.default.createDirectory(at: outboxURL, withIntermediateDirectories: true) + } + setResolvedOneDriveFolder(OneDriveLocator.resolveOneDriveFolder()) + await watcher.setRecordUncommented(transport == .airDrop) + do { + // BLOCKER fix: reconcile the watcher's internal watchFolder with the live + // watchFolderURL UNCONDITIONALLY, before it ever starts. `paths` (and so the + // watcher's initial folder, set in its own init) now comes from the same + // transport-aware TransportSettings.effectiveFolders() as watchFolderURL, so + // in the normal case this is a no-op — but it is the only thing that would + // have caught the old bug (launch paths built AirDrop-only while OneDrive was + // the persisted transport, leaving the watcher's FSEvents stream pointed at a + // stale folder for the whole session) and it stays cheap insurance against + // that class of drift ever recurring. Calling it before start() only updates + // the stored folder — no FSEvents stream exists yet to restart. + try await watcher.updateWatchFolder(watchFolderURL) try await watcher.start { [weak self] _ in Task { @MainActor in guard let self else { return } @@ -198,17 +249,29 @@ public final class AppModel { setStatus((error as? ShotdeckError)?.errorDescription ?? "Could not watch the return folder.") } - let pref = HotkeyPreference.load() - captureHotkey = pref - if !bindCaptureHotkey(pref) { - setStatus("\(pref.displayString) is already used by another app — capture only works from the menu.") - } - - let skipSchedule = + // Env-var-flagged self-test/headless runs (PickerSelfTest's phases, PanelSnapshot, + // and the new ShotdeckTests launch-wiring regression test) skip two real-world + // side effects that are unsafe or meaningless in that context: the update-check + // schedule (a real network call), and binding the REAL, process-wide Carbon + // global hotkey — which is not safe to exercise in an automated/parallel test + // process (it can collide with ShotdeckCoreTests' own HotkeyCenterCarbonTests + // running in the same test binary) and was never meaningfully exercised by any + // self-test anyway. A real user launch never sets these env vars, so production + // behavior is unchanged. + let isSelfTestRun = ProcessInfo.processInfo.environment["SHOTDECK_PICKER_SELFTEST"] != nil || ProcessInfo.processInfo.environment["SHOTDECK_SNAPSHOT_DIR"] != nil || ProcessInfo.processInfo.environment["SHOTDECK_UPDATE_SELFTEST"] != nil - if !skipSchedule { + || ProcessInfo.processInfo.environment["SHOTDECK_ONEDRIVE_SELFTEST"] != nil + || ProcessInfo.processInfo.environment["REDLINE_SELFTEST_PHASE"] != nil + + let pref = HotkeyPreference.load() + captureHotkey = pref + if !isSelfTestRun, !bindCaptureHotkey(pref) { + setStatus("\(pref.displayString) is already used by another app — capture only works from the menu.") + } + + if !isSelfTestRun { updateChecker.startSchedule() } } diff --git a/Sources/Shotdeck/MenuBarView.swift b/Sources/Shotdeck/MenuBarView.swift index 5b239cb..cde9147 100644 --- a/Sources/Shotdeck/MenuBarView.swift +++ b/Sources/Shotdeck/MenuBarView.swift @@ -91,7 +91,7 @@ struct MenuBarView: View { model.setStatus("Send is not available in this build.") } } label: { - actionLabel("Send…") + actionLabel(model.transport == .oneDrive ? "Send to OneDrive" : "Send…") } .disabled(model.session.isEmpty || model.isSending) diff --git a/Sources/Shotdeck/PanelSnapshot.swift b/Sources/Shotdeck/PanelSnapshot.swift index e6580e3..b5a7640 100644 --- a/Sources/Shotdeck/PanelSnapshot.swift +++ b/Sources/Shotdeck/PanelSnapshot.swift @@ -61,6 +61,28 @@ enum PanelSnapshot { try renderMenuBar(model: model, to: directory, name: "03-empty-session") // 04 — three real PNGs in the temp spool so SessionStrip thumbnails decode. + try await addSampleCaptures(to: model) + try renderMenuBar(model: model, to: directory, name: "04-captures-present") + + // 05 — two inspected PDFs in the temp ledger, one marked / one not. + try await seedReturns(model: model) + try renderMenuBar(model: model, to: directory, name: "05-returns-present") + + // 06 — SettingsView against the same isolated model. + try render( + SettingsView().environment(model), + to: directory.appendingPathComponent("panel-06-settings.png") + ) + + // 07/08/09 — OneDrive-mode Settings + menu bar, on a SEPARATE isolated model so + // this transport switch never bleeds into the AirDrop-mode panels above. + try await captureOneDrivePanels(to: directory) + } + + /// Three real PNGs appended to the given model's temp spool so SessionStrip + /// thumbnails decode. Shared by panel 04 (AirDrop) and panel 09 (OneDrive). + @MainActor + private static func addSampleCaptures(to model: AppModel) async throws { let swatches: [(CGFloat, CGFloat, CGFloat)] = [ (0.85, 0.22, 0.18), (0.18, 0.62, 0.32), @@ -77,17 +99,62 @@ enum PanelSnapshot { ) } model.replaceSession(try await model.spool.currentSession()) - try renderMenuBar(model: model, to: directory, name: "04-captures-present") + } - // 05 — two inspected PDFs in the temp ledger, one marked / one not. - try await seedReturns(model: model) - try renderMenuBar(model: model, to: directory, name: "05-returns-present") + /// Panels 07-09: OneDrive transport, on its own isolated model/temp root so + /// switching transport here never touches the AirDrop-mode model above, the real + /// home directory, or UserDefaults.standard. The "resolved" and "not found" states + /// are produced by calling the real OneDriveLocator functions against fake home + /// trees built under this snapshot's own temp root — never a hand-typed path. + @MainActor + private static func captureOneDrivePanels(to directory: URL) async throws { + let (model, root) = try makeIsolatedModel() + defer { try? FileManager.default.removeItem(at: root) } + model.setTransport(.oneDrive) - // 06 — SettingsView against the same isolated model. + // 07 — a resolved OneDrive folder, shaped like the real default + // (…/Library/CloudStorage/OneDrive-MMDGROUP/Redline): a fake home tree with a + // real OneDrive-MMDGROUP directory under it, resolved via the same pure + // OneDriveLocator function production code uses — never a hand-typed path. + let fakeHomeWithOneDrive = root.appendingPathComponent("fake-home-with-onedrive", isDirectory: true) + let syncRoot = fakeHomeWithOneDrive + .appendingPathComponent("Library/CloudStorage/OneDrive-MMDGROUP", isDirectory: true) + try FileManager.default.createDirectory(at: syncRoot, withIntermediateDirectories: true) + guard let resolvedFolder = OneDriveLocator.defaultRedlineFolder( + home: fakeHomeWithOneDrive, fileManager: .default + ) else { + throw SnapshotError.oneDriveFixtureFailed("fake OneDrive-MMDGROUP root did not resolve") + } + model.setResolvedOneDriveFolder(resolvedFolder) try render( SettingsView().environment(model), - to: directory.appendingPathComponent("panel-06-settings.png") + to: directory.appendingPathComponent("panel-07-settings-onedrive.png") ) + + // 08 — no OneDrive folder found: a fake home with NO Library/CloudStorage at + // all, and a throwaway UserDefaults suite (never .standard, never touched + // before) so the stored-override check also legitimately finds nothing. + let fakeHomeWithoutOneDrive = root.appendingPathComponent("fake-home-without-onedrive", isDirectory: true) + try FileManager.default.createDirectory(at: fakeHomeWithoutOneDrive, withIntermediateDirectories: true) + let isolatedDefaults = try makeIsolatedDefaultsSuite() + defer { isolatedDefaults.defaults.removePersistentDomain(forName: isolatedDefaults.suiteName) } + let missingFolder = OneDriveLocator.resolveOneDriveFolder( + defaults: isolatedDefaults.defaults, home: fakeHomeWithoutOneDrive, fileManager: .default + ) + guard missingFolder == nil else { + throw SnapshotError.oneDriveFixtureFailed("fake home without OneDrive unexpectedly resolved") + } + model.setResolvedOneDriveFolder(nil) + try render( + SettingsView().environment(model), + to: directory.appendingPathComponent("panel-08-settings-onedrive-missing.png") + ) + + // 09 — menu bar panel, 3 captures present, OneDrive mode ("Send to OneDrive"). + model.snapshotSetScreenRecordingGranted(true) + model.replaceRegion(sampleRegion()) + try await addSampleCaptures(to: model) + try renderMenuBar(model: model, to: directory, name: "09-captures-present-onedrive") } @MainActor @@ -177,6 +244,23 @@ enum PanelSnapshot { return (model, root) } + /// A throwaway UserDefaults suite — never `.standard` — for the panel-08 fixture, + /// the same isolation pattern ShotdeckCoreTests uses for TransportSettings/ + /// OneDriveLocator tests. + private struct IsolatedDefaultsSuite { + let suiteName: String + let defaults: UserDefaults + } + + private static func makeIsolatedDefaultsSuite() throws -> IsolatedDefaultsSuite { + let suiteName = "shotdeck-panel-snapshot-\(UUID().uuidString)" + guard let defaults = UserDefaults(suiteName: suiteName) else { + throw SnapshotError.oneDriveFixtureFailed("could not create isolated UserDefaults suite") + } + defaults.removePersistentDomain(forName: suiteName) + return IsolatedDefaultsSuite(suiteName: suiteName, defaults: defaults) + } + @MainActor private static func seedReturns(model: AppModel) async throws { let watch = model.paths.watchFolder @@ -283,6 +367,7 @@ private enum SnapshotError: Error, CustomStringConvertible { case encodeFailed(String) case pngGenerationFailed case pdfWriteFailed(String) + case oneDriveFixtureFailed(String) var description: String { switch self { @@ -290,6 +375,7 @@ private enum SnapshotError: Error, CustomStringConvertible { case .encodeFailed(let name): return "PNG encode failed for \(name)" case .pngGenerationFailed: return "CoreGraphics PNG generation failed" case .pdfWriteFailed(let name): return "could not write \(name)" + case .oneDriveFixtureFailed(let detail): return "OneDrive snapshot fixture failed: \(detail)" } } } diff --git a/Sources/Shotdeck/PickerSelfTest.swift b/Sources/Shotdeck/PickerSelfTest.swift index b30c703..7b34186 100644 --- a/Sources/Shotdeck/PickerSelfTest.swift +++ b/Sources/Shotdeck/PickerSelfTest.swift @@ -3,6 +3,7 @@ import CoreGraphics import Darwin import Foundation import ImageIO +import PDFKit import ShotdeckCore /// In-process self-test for the region picker, driven by `SHOTDECK_PICKER_SELFTEST`. @@ -173,7 +174,7 @@ enum PickerSelfTest { try await executeSendTruth() print("SEND-TRUTH PASS") fflush(stdout) - if !startUpdateSelfTestIfRequested() { + if !startUpdateSelfTestIfRequested(), !startOneDriveSelfTestIfRequested() { exit(0) } } catch { @@ -222,7 +223,7 @@ enum PickerSelfTest { sendTruthFail("seeded session was empty") } - let pending = try await model.composePDFForSend() + let pending = try await model.composePDFForSend(outbox: model.outboxURL, transport: .airDrop) guard fm.fileExists(atPath: pending.fileURL.path) else { sendTruthFail("PDF was not written") } @@ -315,7 +316,9 @@ enum PickerSelfTest { try await runUpdateSelfTest(outputDirectory: output) print("UPDATE-SELFTEST PASS version=99.0.0") fflush(stdout) - exit(0) + if !startOneDriveSelfTestIfRequested() { + exit(0) + } } catch let error as UpdateSelfTestError { updateFail(error.description) } catch { @@ -473,6 +476,335 @@ enum PickerSelfTest { exit(1) } + /// Phase 5: proves the OneDrive transport end to end against a REAL sync root — + /// resolved at runtime via `OneDriveLocator.syncRoots()`, never a hardcoded path, so + /// this runs correctly on any Mac/account that has OneDrive signed in (MMD-named + /// root preferred, same as production). Triggered by `SHOTDECK_ONEDRIVE_SELFTEST` + /// when chained after PICKER/SEND-TRUTH/UPDATE-SELFTEST — the exact pattern + /// `startUpdateSelfTestIfRequested` uses for its own env var. Returns true when the + /// async phase was scheduled (it calls `exit` itself). + @discardableResult + private static func startOneDriveSelfTestIfRequested() -> Bool { + guard ProcessInfo.processInfo.environment["SHOTDECK_ONEDRIVE_SELFTEST"] != nil else { + return false + } + runOneDriveSelfTestAndExit() + return true + } + + /// Entry point for running ONLY this phase, bypassing the on-screen picker chain + /// entirely. The harness has no other per-phase selector, so this is the escape + /// hatch: `REDLINE_SELFTEST_PHASE=onedrive`. + static func runOneDriveOnlyIfRequested() { + guard ProcessInfo.processInfo.environment["REDLINE_SELFTEST_PHASE"] == "onedrive" else { + return + } + // Same hop as runIfRequested(): a plain main-queue turn after NSApp starts, so + // AppKit/PDFKit calls inside the phase are not racing app launch. + DispatchQueue.main.async { + MainActor.assumeIsolated { + runOneDriveSelfTestAndExit() + } + } + } + + private static func runOneDriveSelfTestAndExit() { + // Never a false PASS: no real OneDrive sync root on this machine/account is a + // SKIP (still non-zero exit), not silently treated as passing. + guard let syncRoot = OneDriveLocator.syncRoots().first else { + print("ONEDRIVE-SELFTEST SKIP no OneDrive sync root") + fflush(stdout) + exit(1) + } + Task { @MainActor in + do { + let folder = try await executeOneDriveSelfTest(syncRoot: syncRoot) + print("ONEDRIVE-SELFTEST PASS path=\(folder.path)") + fflush(stdout) + exit(0) + } catch let error as OneDriveSelfTestError { + oneDriveFail(error.description) + } catch { + oneDriveFail(String(describing: error)) + } + } + } + + /// Sub-step 1: builds a session, sends it through the OneDrive branch of + /// `send(anchor: nil)` against a NEW folder under `syncRoot`, confirms the watcher + /// does NOT report the freshly-written unmarked PDF as a return, then adds a real + /// PDFKit ink annotation in place (what the iPad does) and confirms the watcher now + /// reports it as commented. + /// + /// Sub-step 1b: rapid transport toggling (chooseTransport(.airDrop) immediately + /// followed by chooseTransport(.oneDrive), no await between them) must still end + /// with the watcher pointed at the OneDrive folder — proves the generation-guarded + /// reconcile in chooseTransport/chooseOneDriveFolder (SettingsView.swift) really + /// does let the last choice win instead of an earlier, superseded call applying its + /// stale folder after a later one already won. + /// + /// Sub-step 1c: the OTHER race — a toggle immediately followed by Send, with no + /// sleep at all before send() runs. Proves send() awaits `pendingReconcileTask` + /// before snapshotting transport/folder: a freshly-sent, still-unmarked PDF must + /// never be reported as an already-returned document (which is exactly what would + /// happen if send() raced ahead while recordUncommented was still `true`, stale + /// from the .airDrop leg of the toggle). + /// + /// Sub-step 2: relaunch simulation — the exact BLOCKER scenario this phase exists to + /// catch. OneDrive is still persisted in defaults from sub-step 1; builds a FRESH + /// model the same way the real app launches (`AppDelegate.makeLaunchModel()` itself, + /// not a reimplementation), bootstraps it, then marks a PDF in the folder in place — + /// the relaunched watcher must report it. A temp app-support root keeps this off the + /// real ~/Library/Application Support/Shotdeck. + /// + /// Never deletes anything under OneDrive — the created folder and PDFs are left in + /// place for Ben to inspect / for the real iPad round trip. + private static func executeOneDriveSelfTest(syncRoot: URL) async throws -> URL { + let fm = FileManager.default + + // UserDefaults.standard is the ONLY defaults instance send()/TransportSettings + // actually read at runtime (there is no defaults-threading through AppModel), so + // "isolated" here means snapshot-and-restore around the real keys — the same + // pattern runRegionPersistPhase already uses for CaptureRegion.defaultsKey. + let defaults = UserDefaults.standard + let previousTransport = defaults.string(forKey: TransportSettings.transportDefaultsKey) + let previousFolder = defaults.string(forKey: TransportSettings.oneDriveFolderDefaultsKey) + defer { + if let previousTransport { + defaults.set(previousTransport, forKey: TransportSettings.transportDefaultsKey) + } else { + defaults.removeObject(forKey: TransportSettings.transportDefaultsKey) + } + if let previousFolder { + defaults.set(previousFolder, forKey: TransportSettings.oneDriveFolderDefaultsKey) + } else { + defaults.removeObject(forKey: TransportSettings.oneDriveFolderDefaultsKey) + } + } + + let stamp = DubaiTime.fileStamp(Date()) + let selftestFolder = syncRoot.appendingPathComponent("Redline-selftest-\(stamp)", isDirectory: true) + try fm.createDirectory(at: selftestFolder, withIntermediateDirectories: true) + + TransportSettings.setTransport(.oneDrive, defaults: defaults) + TransportSettings.setOneDriveFolder(selftestFolder, defaults: defaults) + + // Local spool root only — the outbox/watch folder is the real OneDrive folder. + let spoolRoot = fm.temporaryDirectory + .appendingPathComponent("shotdeck-onedrive-selftest-\(UUID().uuidString)", isDirectory: true) + defer { try? fm.removeItem(at: spoolRoot) } + + let paths = try AppSupportPaths(root: spoolRoot, outbox: selftestFolder, watchFolder: selftestFolder) + let ledger = try ReturnLedger(paths: paths) + let watcher = ReturnWatcher(paths: paths, ledger: ledger) + await watcher.setRecordUncommented(false) // OneDrive mode: today's default is AirDrop's `true`. + + let model = AppModel( + paths: paths, + spool: try SpoolStore(paths: paths), + composer: PDFComposer(), + capturer: ScreenCapturer(), + hotkeys: HotkeyCenter(), + picker: RegionPickerController(), + ledger: ledger, + watcher: watcher + ) + model.setFolderURLs(outbox: selftestFolder, watch: selftestFolder) + model.setTransport(.oneDrive) + + let png = try makeTinyPNGData() + _ = try await model.spool.append( + pngData: png, pixelWidth: 64, pixelHeight: 48, scale: 1, capturedAt: Date() + ) + model.replaceSession(try await model.spool.currentSession()) + guard !model.session.isEmpty else { + throw OneDriveSelfTestError.detail("seeded session was empty") + } + + await model.send(anchor: nil) + + guard let status = model.statusLine, status.hasPrefix("Saved to OneDrive") else { + throw OneDriveSelfTestError.detail( + "status did not start with 'Saved to OneDrive': \(model.statusLine ?? "nil")" + ) + } + guard model.session.isEmpty else { + throw OneDriveSelfTestError.detail("session was not archived after the OneDrive send") + } + + let written = (try? fm.contentsOfDirectory(at: selftestFolder, includingPropertiesForKeys: nil)) ?? [] + guard let pdfURL = written.first(where: { $0.pathExtension.lowercased() == "pdf" }) else { + throw OneDriveSelfTestError.detail("no PDF found in \(selftestFolder.path)") + } + + // Unmarked so far: the watcher must not treat it as a return. + let beforeMarkup = try await watcher.scanNow() + guard !beforeMarkup.contains(where: { $0.fileURL == pdfURL }) else { + throw OneDriveSelfTestError.detail("unmarked PDF was reported as returned by scanNow") + } + let commentedBefore = try await ledger.commented() + guard !commentedBefore.contains(where: { $0.fileURL == pdfURL }) else { + throw OneDriveSelfTestError.detail("unmarked PDF was recorded as commented in the ledger") + } + + // What the iPad does: mark it up in place with a real ink annotation, then save. + try addInkMark(to: pdfURL) + + let afterMarkup = try await watcher.scanNow() + guard let recorded = afterMarkup.first(where: { $0.fileURL == pdfURL }), recorded.isCommented else { + throw OneDriveSelfTestError.detail("annotated PDF was not reported as commented by scanNow") + } + let commentedAfter = try await ledger.commented() + guard commentedAfter.contains(where: { $0.fileURL == pdfURL }) else { + throw OneDriveSelfTestError.detail("annotated PDF was not recorded in the ledger as commented") + } + + // Sub-step 1b: rapid toggle race — see the doc comment above this function. + model.chooseTransport(.airDrop) + model.chooseTransport(.oneDrive) // immediately superseding the call above + // The generation guard itself is what's under test, not this wait — it just + // gives the (already-guarded) reconcile Task a moment to settle either way. + try await Task.sleep(for: .milliseconds(500)) + guard model.transport == .oneDrive else { + throw OneDriveSelfTestError.detail( + "rapid toggle: model.transport ended as \(model.transport), expected .oneDrive" + ) + } + let racePDFURL = selftestFolder.appendingPathComponent("Redline-race-\(stamp).pdf") + try writeUnmarkedRedlinePDF(to: racePDFURL) + try addInkMark(to: racePDFURL) + let raceFound = try await model.watcher.scanNow() + guard raceFound.first(where: { $0.fileURL == racePDFURL })?.isCommented == true else { + throw OneDriveSelfTestError.detail( + "rapid toggle: watcher did not end up watching \(selftestFolder.path) — an earlier, superseded chooseTransport call won" + ) + } + + // Sub-step 1c: toggle-then-immediate-send race — see the doc comment above + // this function. No sleep here: this IS the exact race window finding #3 + // exists to close, so send() itself must wait out the pending reconcile. + let racePNG = try makeTinyPNGData() + _ = try await model.spool.append( + pngData: racePNG, pixelWidth: 64, pixelHeight: 48, scale: 1, capturedAt: Date() + ) + model.replaceSession(try await model.spool.currentSession()) + guard !model.session.isEmpty else { + throw OneDriveSelfTestError.detail("toggle-then-send: re-seeded session was empty") + } + + let knownBeforeToggleSend = Set( + ((try? fm.contentsOfDirectory(at: selftestFolder, includingPropertiesForKeys: nil)) ?? []) + .map(\.lastPathComponent) + ) + model.chooseTransport(.airDrop) + model.chooseTransport(.oneDrive) // immediately superseding, no sleep before send() + await model.send(anchor: nil) + + guard let toggleSendStatus = model.statusLine, toggleSendStatus.hasPrefix("Saved to OneDrive") else { + throw OneDriveSelfTestError.detail( + "toggle-then-send: status was \(model.statusLine ?? "nil"), expected 'Saved to OneDrive'" + ) + } + guard model.session.isEmpty else { + throw OneDriveSelfTestError.detail("toggle-then-send: session was not archived") + } + let filesAfterToggleSend = (try? fm.contentsOfDirectory( + at: selftestFolder, includingPropertiesForKeys: nil + )) ?? [] + guard let toggleSendPDFURL = filesAfterToggleSend.first(where: { + $0.pathExtension.lowercased() == "pdf" && !knownBeforeToggleSend.contains($0.lastPathComponent) + }) else { + throw OneDriveSelfTestError.detail("toggle-then-send: no new PDF found in \(selftestFolder.path)") + } + // The freshly-sent PDF is UNMARKED. If send() had raced ahead of the pending + // reconcile, recordUncommented could still have been (stale) true, and this + // scan would wrongly report it as already returned. + let scanAfterToggleSend = try await model.watcher.scanNow() + guard !scanAfterToggleSend.contains(where: { $0.fileURL == toggleSendPDFURL }) else { + throw OneDriveSelfTestError.detail( + "toggle-then-send: freshly-sent unmarked PDF at \(toggleSendPDFURL.path) was reported as returned — send() raced ahead of the pending reconcile" + ) + } + + // Sub-step 2: relaunch simulation — see the doc comment above this function. + let relaunchAppSupportRoot = fm.temporaryDirectory + .appendingPathComponent("shotdeck-onedrive-relaunch-\(UUID().uuidString)", isDirectory: true) + defer { try? fm.removeItem(at: relaunchAppSupportRoot) } + + let relaunchModel = AppDelegate.makeLaunchModel(appSupportRoot: relaunchAppSupportRoot) + guard relaunchModel.transport == .oneDrive else { + throw OneDriveSelfTestError.detail( + "relaunch: model transport was \(relaunchModel.transport), expected .oneDrive" + ) + } + guard relaunchModel.watchFolderURL.path == selftestFolder.path else { + throw OneDriveSelfTestError.detail( + "relaunch: model watchFolderURL was \(relaunchModel.watchFolderURL.path), expected \(selftestFolder.path) — this is the exact BLOCKER this phase guards against" + ) + } + + await relaunchModel.bootstrap() + + let relaunchPDFURL = selftestFolder.appendingPathComponent("Redline-relaunch-\(stamp).pdf") + try writeUnmarkedRedlinePDF(to: relaunchPDFURL) + try addInkMark(to: relaunchPDFURL) + + let relaunchFound = try await relaunchModel.watcher.scanNow() + guard relaunchFound.first(where: { $0.fileURL == relaunchPDFURL })?.isCommented == true else { + throw OneDriveSelfTestError.detail( + "relaunch: watcher did not report the marked PDF at \(relaunchPDFURL.path) as returned — it was watching the wrong folder after relaunch" + ) + } + await relaunchModel.watcher.stop() + + return selftestFolder + } + + /// Adds a real PDFKit ink annotation to the PDF at `url` in place and saves it — + /// exactly what the iPad does when marking up a page. + private static func addInkMark(to url: URL) throws { + guard let document = PDFDocument(url: url), let page = document.page(at: 0) else { + throw OneDriveSelfTestError.detail("could not reopen \(url.path) to annotate it") + } + let ink = PDFAnnotation( + bounds: CGRect(x: 20, y: 20, width: 60, height: 60), + forType: .ink, + withProperties: nil + ) + let stroke = NSBezierPath() + stroke.move(to: NSPoint(x: 20, y: 20)) + stroke.line(to: NSPoint(x: 80, y: 80)) + ink.add(stroke) + page.addAnnotation(ink) + guard document.write(to: url) else { + throw OneDriveSelfTestError.detail("could not save the annotated PDF back to \(url.path)") + } + } + + /// Writes a fresh, unmarked, single-page "Redline"-creator PDF straight to `url` — + /// standing in for a PDF that has just landed in the watch folder, before any + /// human mark. Used by the rapid-toggle and relaunch sub-steps, which don't need to + /// exercise send()/composePDFForSend() again (sub-step 1 already does). + private static func writeUnmarkedRedlinePDF(to url: URL) throws { + let document = PDFDocument() + let page = PDFPage() + page.setBounds(CGRect(x: 0, y: 0, width: 612, height: 792), for: .mediaBox) + document.insert(page, at: 0) + document.documentAttributes = [ + PDFDocumentAttribute.creatorAttribute: "Redline", + PDFDocumentAttribute.subjectAttribute: UUID().uuidString, + ] + guard document.write(to: url) else { + throw OneDriveSelfTestError.detail("could not write \(url.path)") + } + } + + private static func oneDriveFail(_ detail: String) -> Never { + print("ONEDRIVE-SELFTEST FAIL \(detail)") + fflush(stdout) + exit(1) + } + private static func interpolate(_ step: Int) -> NSPoint { let t = CGFloat(step) / CGFloat(dragSteps) return NSPoint( @@ -538,3 +870,12 @@ private enum UpdateSelfTestError: Error, CustomStringConvertible { } } } + +private enum OneDriveSelfTestError: Error, CustomStringConvertible { + case detail(String) + var description: String { + switch self { + case .detail(let s): return s + } + } +} diff --git a/Sources/Shotdeck/SendController.swift b/Sources/Shotdeck/SendController.swift index d4d818a..b033291 100644 --- a/Sources/Shotdeck/SendController.swift +++ b/Sources/Shotdeck/SendController.swift @@ -16,9 +16,66 @@ extension AppModel: SendCapable { guard !session.isEmpty, !isSending else { return } setSending(true) + // Wait for any IN-FLIGHT transport/folder reconcile (chooseTransport/ + // chooseOneDriveFolder in SettingsView.swift) to fully settle BEFORE + // snapshotting transport/folder below. Without this, a toggle immediately + // followed by Send could let send() read a state that is still mid-transition + // — e.g. the watcher's recordUncommented flag briefly lagging the just-chosen + // transport, so a freshly-sent unmarked OneDrive PDF gets misreported as an + // already-returned document. `Task.value` never throws, and + // awaiting nil is an immediate no-op (AirDrop mode, or no toggle in flight). + await pendingReconcileTask?.value + + // Snapshot BOTH the transport AND the destination folder into local `let`s + // ONCE, before any further `await` in this function. chooseTransport/ + // chooseOneDriveFolder also refuse outright (status "Finish the current send + // first.") while isSending is true, but this snapshot is the actual fix for the + // send-vs-switch race: even without that guard, everything below operates on + // these frozen values — composePDFForSend(outbox:transport:) takes both as + // parameters and never re-reads `self.outboxURL`/`self.transport` after a + // suspension point, so a concurrent transport switch mid-send can no longer + // land the PDF under one transport's folder while the archive/status branch + // runs the other's. + let transport = TransportSettings.transport() + let destinationFolder: URL + + // OneDrive mode: verify the real destination exists, is writable, AND actually + // accepts a real write RIGHT NOW, before composing anything. `isWritableDirectory` + // alone is not enough — a OneDrive Files-On-Demand directory whose provider + // domain is signed out can report as existing and POSIX-writable while an + // actual write fails, so `probeWritable` writes-fsyncs-removes a tiny real probe + // file to catch that. `outboxURL` is kept in sync with the resolved OneDrive + // folder by bootstrap/chooseTransport/chooseOneDriveFolder, but this is + // re-resolved fresh here (never trusted stale) so a folder that vanished or lost + // its permissions since then (OneDrive signed out, external volume unmounted, + // folder deleted, chmod'd unwritable) is caught instead of silently attempted + // and surfacing as a generic PDF-composition failure. + if transport == .oneDrive { + guard let folder = OneDriveLocator.resolveOneDriveFolder(), + OneDriveLocator.isWritableDirectory(at: folder), + OneDriveLocator.probeWritable(at: folder) + else { + let path = OneDriveLocator.resolveOneDriveFolder()?.path + ?? TransportSettings.storedOneDriveFolderPath() + ?? "no OneDrive folder found" + setResolvedOneDriveFolder(nil) + setStatus(ShotdeckError.oneDriveFolderUnavailable(path: path).errorDescription) + setSending(false) + return + } + destinationFolder = folder + setResolvedOneDriveFolder(folder) + if outboxURL != folder || watchFolderURL != folder { + setFolderURLs(outbox: folder, watch: folder) + try? await watcher.updateWatchFolder(folder) + } + } else { + destinationFolder = outboxURL + } + let pending: ComposedSend do { - pending = try await composePDFForSend() + pending = try await composePDFForSend(outbox: destinationFolder, transport: transport) } catch { // Never unlink the published PDF, and never unlink the temp file either: // a rename failure would leave the complete document at the temp name. @@ -27,39 +84,59 @@ extension AppModel: SendCapable { return } - guard let anchor else { - handleDidFailToShareItems(fileName: pending.fileName) + switch transport { + case .oneDrive: + // No AirDrop, no anchor needed — the PDF is already in the watched + // OneDrive folder. Archive immediately; the iPad marks it up in place. + await handleDidShareItems(fileName: pending.fileName, pageCount: pending.pageCount) + let pageWord = pending.pageCount == 1 ? "page" : "pages" + setStatus( + "Saved to OneDrive — \(pending.pageCount) \(pageWord). Open it in Files on your iPad." + ) setSending(false) - return - } - do { - try Sharing.airDrop(fileURL: pending.fileURL, from: anchor) { [weak self] success in - guard let self else { return } - if success { - await self.handleDidShareItems( - fileName: pending.fileName, - pageCount: pending.pageCount - ) - } else { - self.handleDidFailToShareItems(fileName: pending.fileName) + case .airDrop: + guard let anchor else { + handleDidFailToShareItems(fileName: pending.fileName) + setSending(false) + return + } + + do { + try Sharing.airDrop(fileURL: pending.fileURL, from: anchor) { [weak self] success in + guard let self else { return } + if success { + await self.handleDidShareItems( + fileName: pending.fileName, + pageCount: pending.pageCount + ) + } else { + self.handleDidFailToShareItems(fileName: pending.fileName) + } + self.setSending(false) } - self.setSending(false) + } catch { + // canPerform false, no service, or no visible window: same as cancel. + handleDidFailToShareItems(fileName: pending.fileName) + setSending(false) } - } catch { - // canPerform false, no service, or no visible window: same as cancel. - handleDidFailToShareItems(fileName: pending.fileName) - setSending(false) } } - /// Writes the PDF to the outbox and records its path. Does not archive the session + /// Writes the PDF to `outboxDir` and records its path. Does not archive the session /// and does not present AirDrop — that happens only after the share completes. - func composePDFForSend() async throws -> ComposedSend { + /// `outboxDir`/`transport` are passed in (values `send(anchor:)` snapshotted before + /// any await) rather than read from `self.outboxURL`/`self.transport` here, so a + /// concurrent transport switch mid-send can never redirect an in-flight compose to + /// a different folder. A write/rename failure specifically at the destination + /// folder (as opposed to composer.compose()'s own session/image-content failures) + /// is reported as `oneDriveFolderUnavailable` rather than the generic + /// `pdfCompositionFailed` when `transport == .oneDrive` — the File Provider edge + /// case where the folder looked writable moments ago in `send(anchor:)` but the + /// actual write still failed (e.g. OneDrive signed out mid-write). + func composePDFForSend(outbox outboxDir: URL, transport: SendTransport) async throws -> ComposedSend { let workingSession = session let composer = self.composer - // Live outbox (FolderSettings), not `paths.outbox` — Settings changes take effect. - let outboxDir = outboxURL let sourceDir = paths.sessionDirectory(workingSession.id) let fileName = PDFComposer.fileName(for: workingSession) let finalURL = outboxDir.appendingPathComponent(fileName) @@ -79,14 +156,27 @@ extension AppModel: SendCapable { // POSIX rename onto `finalURL` replaces any same-name file in one // directory operation; there is never a window where the PDF is gone. if Darwin.rename(tempURL.path, finalURL.path) != 0 { + if transport == .oneDrive { + throw ShotdeckError.oneDriveFolderUnavailable(path: outboxDir.path) + } throw ShotdeckError.pdfCompositionFailed( reason: "could not publish the PDF: \(String(cString: strerror(errno)))" ) } - try AtomicFile.fsyncDirectory(at: outboxDir) + do { + try AtomicFile.fsyncDirectory(at: outboxDir) + } catch { + if transport == .oneDrive { + throw ShotdeckError.oneDriveFolderUnavailable(path: outboxDir.path) + } + throw error + } }.value guard FileManager.default.fileExists(atPath: finalURL.path) else { + if transport == .oneDrive { + throw ShotdeckError.oneDriveFolderUnavailable(path: outboxDir.path) + } throw ShotdeckError.pdfCompositionFailed(reason: "the PDF was not written to disk") } rememberLastComposedPDF(finalURL) diff --git a/Sources/Shotdeck/SettingsView.swift b/Sources/Shotdeck/SettingsView.swift index 4f33f99..787332a 100644 --- a/Sources/Shotdeck/SettingsView.swift +++ b/Sources/Shotdeck/SettingsView.swift @@ -33,6 +33,25 @@ struct SettingsView: View { .frame(minHeight: 22) } + GridRow { + Text("Send via") + .font(.headline) + .frame(maxWidth: .infinity, alignment: .leading) + .gridCellColumns(2) + .padding(.top, 6) + } + + GridRow { + Picker("Send via", selection: transportBinding) { + ForEach(SendTransport.allCases, id: \.self) { transport in + Text(transport.displayName).tag(transport) + } + } + .labelsHidden() + .pickerStyle(.segmented) + .gridCellColumns(2) + } + GridRow { Text("Folders") .font(.headline) @@ -41,17 +60,47 @@ struct SettingsView: View { .padding(.top, 6) } - GridRow(alignment: .center) { - fieldLabel("Watch folder") - folderValue(path: model.watchFolderURL.path) { - model.chooseWatchFolder() + if model.transport == .airDrop { + GridRow(alignment: .center) { + fieldLabel("Watch folder") + folderValue(path: model.watchFolderURL.path) { + model.chooseWatchFolder() + } } - } - GridRow(alignment: .center) { - fieldLabel("Output folder") - folderValue(path: model.outboxURL.path) { - model.chooseOutboxFolder() + GridRow(alignment: .center) { + fieldLabel("Output folder") + folderValue(path: model.outboxURL.path) { + model.chooseOutboxFolder() + } + } + } else { + GridRow(alignment: .center) { + fieldLabel("OneDrive folder") + if let folder = model.resolvedOneDriveFolder { + folderValue(path: folder.path) { + model.chooseOneDriveFolder() + } + } else { + // One-line row, same shape as the normal path row: "Not found" + // where the path would be, Choose… stays live. The explanation + // moves to the caption below instead of wrapping this row. + folderValue(path: "Not found") { + model.chooseOneDriveFolder() + } + } + } + + GridRow { + Text( + model.resolvedOneDriveFolder != nil + ? "The PDF is saved here and this same folder is watched for the marked-up copy. On the iPad open it from Files > OneDrive." + : "No OneDrive folder found. Sign in to OneDrive, or choose a folder." + ) + .font(.caption) + .foregroundStyle(.secondary) + .fixedSize(horizontal: false, vertical: true) + .gridCellColumns(2) } } @@ -68,6 +117,10 @@ struct SettingsView: View { .onDisappear { disarmHotkeyRecorder() } } + private var transportBinding: Binding { + Binding(get: { model.transport }, set: { model.chooseTransport($0) }) + } + private func armHotkeyRecorder() { guard !isRecordingHotkey else { return } isRecordingHotkey = true @@ -190,6 +243,87 @@ extension AppModel: SettingsWindowPresenting { } } + /// Settings "Send via" picker action. Persists the choice, recomputes the effective + /// outbox/watch folder for the new transport, creates the OneDrive folder if it + /// doesn't exist yet, and re-points the running watcher (folder + recordUncommented) + /// at the new state. Switching back to AirDrop restores its own stored overrides + /// untouched, since AirDrop and OneDrive folder settings are stored under separate keys. + /// Refuses while a send is in flight (send() snapshots its own folder/transport, but + /// switching mid-send is still confusing UX — nothing to gain by allowing it). + /// The async reconcile below is generation-guarded: `reconcileGeneration` is bumped + /// synchronously before the Task starts, and the Task checks its own snapshot against + /// the live value before every mutating step, so rapid toggling (this function or + /// chooseOneDriveFolder, in any order) always lets the LAST choice win instead of an + /// earlier, superseded call applying its stale folder/flag after a later one already won. + /// The Task's handle is stored in `pendingReconcileTask` so send() can await its + /// completion before snapshotting transport/folder — closing the OTHER race, where a + /// toggle is immediately followed by Send before this reconcile has settled. + func chooseTransport(_ value: SendTransport) { + guard !isSending else { + setStatus("Finish the current send first.") + return + } + guard value != transport else { return } + TransportSettings.setTransport(value) + setTransport(value) + let folders = TransportSettings.effectiveFolders() + if value == .oneDrive { + try? FileManager.default.createDirectory( + at: folders.outbox, withIntermediateDirectories: true + ) + } + setFolderURLs(outbox: folders.outbox, watch: folders.watch) + setResolvedOneDriveFolder(OneDriveLocator.resolveOneDriveFolder()) + + reconcileGeneration += 1 + let generation = reconcileGeneration + pendingReconcileTask = Task { + guard generation == self.reconcileGeneration else { return } + await watcher.setRecordUncommented(value == .airDrop) + guard generation == self.reconcileGeneration else { return } + do { + try await watcher.updateWatchFolder(folders.watch) + } catch { + guard generation == self.reconcileGeneration else { return } + setStatus( + (error as? ShotdeckError)?.errorDescription ?? "Could not switch the watch folder." + ) + } + } + } + + /// Refuses while a send is in flight, same reasoning as chooseTransport. See + /// chooseTransport's doc comment for the generation-guard mechanism shared here. + func chooseOneDriveFolder() { + guard !isSending else { + setStatus("Finish the current send first.") + return + } + let start = resolvedOneDriveFolder ?? FileManager.default.homeDirectoryForCurrentUser + guard let url = chooseDirectory(startingAt: start) else { return } + TransportSettings.setOneDriveFolder(url) + try? FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + setResolvedOneDriveFolder(OneDriveLocator.resolveOneDriveFolder()) + guard transport == .oneDrive else { return } + setFolderURLs(outbox: url, watch: url) + + reconcileGeneration += 1 + let generation = reconcileGeneration + pendingReconcileTask = Task { + guard generation == self.reconcileGeneration else { return } + do { + try await watcher.updateWatchFolder(url) + guard generation == self.reconcileGeneration else { return } + setStatus("OneDrive folder set to \(url.lastPathComponent).") + } catch { + guard generation == self.reconcileGeneration else { return } + setStatus( + (error as? ShotdeckError)?.errorDescription ?? "Could not switch the watch folder." + ) + } + } + } + private func chooseDirectory(startingAt directory: URL) -> URL? { let panel = NSOpenPanel() panel.canChooseDirectories = true diff --git a/Sources/Shotdeck/main.swift b/Sources/Shotdeck/main.swift index 3b61e42..bf052db 100644 --- a/Sources/Shotdeck/main.swift +++ b/Sources/Shotdeck/main.swift @@ -7,6 +7,9 @@ import ShotdeckCore if ProcessInfo.processInfo.environment["SHOTDECK_SNAPSHOT_DIR"] != nil { MainActor.assumeIsolated { PanelSnapshot.runIfRequested() } } +if ProcessInfo.processInfo.environment["REDLINE_SELFTEST_PHASE"] == "onedrive" { + MainActor.assumeIsolated { PickerSelfTest.runOneDriveOnlyIfRequested() } +} if ProcessInfo.processInfo.environment["SHOTDECK_PICKER_SELFTEST"] != nil { MainActor.assumeIsolated { PickerSelfTest.runIfRequested() } } @@ -47,9 +50,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate { Task { await model.bootstrap() } } - private static func makeLaunchModel() -> AppModel { + /// Builds the model exactly the way the real app launches: paths come from + /// `TransportSettings.resolvedAppSupportPaths()` — transport-aware, so the watcher + /// this feeds is never seeded with a stale AirDrop folder while OneDrive is the + /// persisted transport (that was the BLOCKER this function used to have, when it + /// called the AirDrop-only `FolderSettings.resolvedAppSupportPaths()` instead). + /// `appSupportRoot` exists only so PickerSelfTest's relaunch-simulation sub-step can + /// point this at a temp directory instead of the real + /// ~/Library/Application Support/Shotdeck — production always calls this with no + /// argument (the real root). Internal, not private, for that same reason. + static func makeLaunchModel(appSupportRoot: URL? = nil) -> AppModel { do { - let paths = try FolderSettings.resolvedAppSupportPaths() + let paths = try TransportSettings.resolvedAppSupportPaths(root: appSupportRoot) return try makeModel(paths: paths) } catch { Log.ui.critical( diff --git a/Sources/ShotdeckCore/Model/ShotdeckError.swift b/Sources/ShotdeckCore/Model/ShotdeckError.swift index a188a64..ab5b063 100644 --- a/Sources/ShotdeckCore/Model/ShotdeckError.swift +++ b/Sources/ShotdeckCore/Model/ShotdeckError.swift @@ -10,6 +10,7 @@ public enum ShotdeckError: Error, LocalizedError, Sendable { case pdfCompositionFailed(reason: String) case airDropUnavailable case noCommentedReturns + case oneDriveFolderUnavailable(path: String) public var errorDescription: String? { switch self { @@ -31,6 +32,8 @@ public enum ShotdeckError: Error, LocalizedError, Sendable { return "AirDrop is not available right now." case .noCommentedReturns: return "None of the returned PDFs have comments on them." + case .oneDriveFolderUnavailable(let path): + return "Your OneDrive folder is not available: \(path). Check that OneDrive is signed in, or choose another folder in Settings." } } } diff --git a/Sources/ShotdeckCore/Returns/ReturnWatcher.swift b/Sources/ShotdeckCore/Returns/ReturnWatcher.swift index 829d3b8..36e03cd 100644 --- a/Sources/ShotdeckCore/Returns/ReturnWatcher.swift +++ b/Sources/ShotdeckCore/Returns/ReturnWatcher.swift @@ -10,6 +10,21 @@ public actor ReturnWatcher { private var bridge: FSEventBridge? private var pendingScanTask: Task? private let eventQueue = DispatchQueue(label: "ai.flowmaster.shotdeck.returns.fsevents") + /// When false, a document with zero human marks is neither recorded into the ledger + /// nor included in scanNow's/onChange's results — needed for OneDrive mode, where the + /// outbox and watch folder are the same folder and a freshly written, unmarked PDF + /// must not be treated as a return. Defaults to true (today's AirDrop behaviour). + /// A document that IS commented is always recorded, regardless of this flag. + public var recordUncommented: Bool = true + + /// The folder this watcher is CURRENTLY seeded to scan/watch — whatever `init` + /// last set it to, or `updateWatchFolder` since. Exposed so tests can observe the + /// watcher's seeded folder directly (e.g. right after construction, before + /// `start()`/`updateWatchFolder()` ever run) rather than only inferring it + /// indirectly through `scanNow()`'s behavior. + public var currentWatchFolder: URL { + watchFolder + } /// Watch folder is `paths.watchFolder`, which production constructs from /// `FolderSettings.resolve().watch`. This type never calls FolderSettings; @@ -29,6 +44,12 @@ public actor ReturnWatcher { onChange(found) } + /// Sets `recordUncommented`. A `func` (not a plain property set) only because + /// callers outside this actor must `await` it like any other actor mutation. + public func setRecordUncommented(_ value: Bool) { + recordUncommented = value + } + /// Idempotent. Stops and releases the FSEventStream if one is running; safe to call /// when never started or already stopped. Cancels any pending debounced scan. public func stop() { @@ -77,6 +98,7 @@ public actor ReturnWatcher { guard let document = PDFDocument(url: url), AnnotationInspector.isShotdeckDocument(document) else { continue } guard let inspected = try? AnnotationInspector.inspect(fileURL: url) else { continue } + if !recordUncommented, !inspected.isCommented { continue } try await ledger.record(inspected) results.append(inspected) } diff --git a/Sources/ShotdeckCore/Support/AppSupportPaths.swift b/Sources/ShotdeckCore/Support/AppSupportPaths.swift index 7f1dccf..8e584b4 100644 --- a/Sources/ShotdeckCore/Support/AppSupportPaths.swift +++ b/Sources/ShotdeckCore/Support/AppSupportPaths.swift @@ -11,12 +11,6 @@ public struct AppSupportPaths: Sendable { /// Production paths. public static func standard() throws -> AppSupportPaths { let fileManager = FileManager.default - let appSupportParent = try fileManager.url( - for: .applicationSupportDirectory, - in: .userDomainMask, - appropriateFor: nil, - create: true - ) let desktop = try fileManager.url( for: .desktopDirectory, in: .userDomainMask, @@ -29,10 +23,25 @@ public struct AppSupportPaths: Sendable { appropriateFor: nil, create: true ) - let root = appSupportParent.appendingPathComponent("Shotdeck", isDirectory: true) + let root = try standardRoot(fileManager: fileManager) return try AppSupportPaths(root: root, outbox: desktop, watchFolder: downloads) } + /// The standard `~/Library/Application Support/Shotdeck` root. Shared by + /// `standard()`, `FolderSettings.resolvedAppSupportPaths()`, and + /// `TransportSettings.resolvedAppSupportPaths()` so all three agree on where the + /// root lives — the folder-resolution logic (AirDrop-only vs transport-aware) + /// differs between those, the root computation never should. + public static func standardRoot(fileManager: FileManager = .default) throws -> URL { + let appSupportParent = try fileManager.url( + for: .applicationSupportDirectory, + in: .userDomainMask, + appropriateFor: nil, + create: true + ) + return appSupportParent.appendingPathComponent("Shotdeck", isDirectory: true) + } + /// Test paths rooted anywhere. Every directory is created if missing. public init(root: URL, outbox: URL, watchFolder: URL) throws { self.root = root diff --git a/Sources/ShotdeckCore/Support/FolderSettings.swift b/Sources/ShotdeckCore/Support/FolderSettings.swift index 618b1ae..136bd8e 100644 --- a/Sources/ShotdeckCore/Support/FolderSettings.swift +++ b/Sources/ShotdeckCore/Support/FolderSettings.swift @@ -70,15 +70,7 @@ public enum FolderSettings { defaults: UserDefaults = .standard, fileManager: FileManager = .default ) throws -> AppSupportPaths { - let resolvedRoot: URL - if let root { - resolvedRoot = root - } else { - let appSupportParent = try fileManager.url( - for: .applicationSupportDirectory, in: .userDomainMask, - appropriateFor: nil, create: true) - resolvedRoot = appSupportParent.appendingPathComponent("Shotdeck", isDirectory: true) - } + let resolvedRoot = try root ?? AppSupportPaths.standardRoot(fileManager: fileManager) let folders = resolve(defaults: defaults, fileManager: fileManager) return try AppSupportPaths(root: resolvedRoot, outbox: folders.outbox, watchFolder: folders.watch) } diff --git a/Sources/ShotdeckCore/Support/TransportSettings.swift b/Sources/ShotdeckCore/Support/TransportSettings.swift new file mode 100644 index 0000000..c278d61 --- /dev/null +++ b/Sources/ShotdeckCore/Support/TransportSettings.swift @@ -0,0 +1,221 @@ +import Foundation + +/// The two ways a composed PDF can reach the iPad and come back marked up. +public enum SendTransport: String, Codable, Sendable, CaseIterable { + case airDrop + case oneDrive + + public var displayName: String { + switch self { + case .airDrop: return "AirDrop" + case .oneDrive: return "OneDrive folder" + } + } +} + +/// User-configurable transport choice plus the OneDrive folder override, backed by +/// UserDefaults the same way `FolderSettings` is. See `FolderSettings` for why a plain +/// path (not a security-scoped bookmark) is correct for this unsandboxed app. +public enum TransportSettings { + public static let transportDefaultsKey = "ai.flowmaster.shotdeck.transport" + public static let oneDriveFolderDefaultsKey = "ai.flowmaster.shotdeck.oneDriveFolder" + + /// Defaults to `.airDrop` when unset or when the stored value cannot be parsed. + public static func transport(defaults: UserDefaults = .standard) -> SendTransport { + guard let raw = defaults.string(forKey: transportDefaultsKey), + let value = SendTransport(rawValue: raw) + else { return .airDrop } + return value + } + + public static func setTransport(_ value: SendTransport, defaults: UserDefaults = .standard) { + defaults.set(value.rawValue, forKey: transportDefaultsKey) + } + + /// Raw stored path (or nil if never set / cleared). Does NOT validate that the + /// directory still exists. + public static func storedOneDriveFolderPath(defaults: UserDefaults = .standard) -> String? { + defaults.string(forKey: oneDriveFolderDefaultsKey) + } + + public static func setOneDriveFolder(_ url: URL, defaults: UserDefaults = .standard) { + defaults.set(url.path, forKey: oneDriveFolderDefaultsKey) + } + + public static func resetOneDriveFolder(defaults: UserDefaults = .standard) { + defaults.removeObject(forKey: oneDriveFolderDefaultsKey) + } + + /// The outbox/watch folders Redline should actually use right now, for the current + /// transport. AirDrop mode delegates to `FolderSettings.resolve()` unchanged. + /// OneDrive mode uses the SAME folder for both outbox and watch — see + /// `OneDriveLocator.resolveOneDriveFolder`. When no OneDrive folder can be resolved + /// at all (no sync root, no override), this falls back to the AirDrop folders so the + /// app always has somewhere to write; `send(anchor:)` performs its own live + /// existence check before ever composing into a OneDrive send, so that fallback is + /// never mistaken for a valid OneDrive destination. + public static func effectiveFolders( + defaults: UserDefaults = .standard, + fileManager: FileManager = .default + ) -> (outbox: URL, watch: URL, transport: SendTransport) { + let transport = transport(defaults: defaults) + switch transport { + case .airDrop: + let folders = FolderSettings.resolve(defaults: defaults, fileManager: fileManager) + return (folders.outbox, folders.watch, transport) + case .oneDrive: + if let folder = OneDriveLocator.resolveOneDriveFolder( + defaults: defaults, + home: fileManager.homeDirectoryForCurrentUser, + fileManager: fileManager + ) { + return (folder, folder, transport) + } + let folders = FolderSettings.resolve(defaults: defaults, fileManager: fileManager) + return (folders.outbox, folders.watch, transport) + } + } + + /// Builds an `AppSupportPaths` using `root` (defaults to the standard + /// `~/Library/Application Support/Shotdeck` when nil) plus whatever + /// `effectiveFolders()` returns for outbox/watch. Unlike + /// `FolderSettings.resolvedAppSupportPaths()` (AirDrop-only), this is + /// transport-aware — it is the ONLY function launch code should use to build its + /// paths, so the watcher it feeds is never seeded with a stale AirDrop folder while + /// OneDrive is the persisted transport. `root` is exposed purely so tests (and the + /// ONEDRIVE-SELFTEST relaunch simulation) can point it at a temporary directory + /// instead of the user's real Application Support folder. + public static func resolvedAppSupportPaths( + root: URL? = nil, + defaults: UserDefaults = .standard, + fileManager: FileManager = .default + ) throws -> AppSupportPaths { + let resolvedRoot = try root ?? AppSupportPaths.standardRoot(fileManager: fileManager) + let folders = effectiveFolders(defaults: defaults, fileManager: fileManager) + return try AppSupportPaths(root: resolvedRoot, outbox: folders.outbox, watchFolder: folders.watch) + } +} + +/// Pure path logic for locating a OneDrive sync root under +/// `~/Library/CloudStorage` and the Redline folder inside it. No side effects — never +/// creates a directory. Fully unit-testable with a fake home tree. +public enum OneDriveLocator { + /// Every directory directly under `/Library/CloudStorage` whose name starts + /// with "OneDrive-", sorted so a name containing "MMD" (case-insensitive) sorts + /// first, then alphabetically. Empty when CloudStorage does not exist. + public static func syncRoots( + home: URL = FileManager.default.homeDirectoryForCurrentUser, + fileManager: FileManager = .default + ) -> [URL] { + let cloudStorage = home.appendingPathComponent("Library/CloudStorage", isDirectory: true) + var isDirectory: ObjCBool = false + guard fileManager.fileExists(atPath: cloudStorage.path, isDirectory: &isDirectory), + isDirectory.boolValue + else { return [] } + + let items = (try? fileManager.contentsOfDirectory( + at: cloudStorage, + includingPropertiesForKeys: [.isDirectoryKey], + options: [.skipsHiddenFiles] + )) ?? [] + + let roots = items.filter { url in + guard url.lastPathComponent.hasPrefix("OneDrive-") else { return false } + var itemIsDirectory: ObjCBool = false + let exists = fileManager.fileExists(atPath: url.path, isDirectory: &itemIsDirectory) + return exists && itemIsDirectory.boolValue + } + + return roots.sorted { a, b in + let aName = a.lastPathComponent + let bName = b.lastPathComponent + let aIsMMD = aName.localizedCaseInsensitiveContains("MMD") + let bIsMMD = bName.localizedCaseInsensitiveContains("MMD") + if aIsMMD != bIsMMD { return aIsMMD } + return aName.localizedStandardCompare(bName) == .orderedAscending + } + } + + /// First sync root's "Redline" subfolder, or nil when there is no sync root at all. + public static func defaultRedlineFolder( + home: URL = FileManager.default.homeDirectoryForCurrentUser, + fileManager: FileManager = .default + ) -> URL? { + guard let first = syncRoots(home: home, fileManager: fileManager).first else { return nil } + return first.appendingPathComponent("Redline", isDirectory: true) + } + + /// The stored override when it is set AND still exists as a directory; otherwise + /// `defaultRedlineFolder`. Never creates anything. + public static func resolveOneDriveFolder( + defaults: UserDefaults = .standard, + home: URL = FileManager.default.homeDirectoryForCurrentUser, + fileManager: FileManager = .default + ) -> URL? { + if let storedPath = TransportSettings.storedOneDriveFolderPath(defaults: defaults) { + var isDirectory: ObjCBool = false + let exists = fileManager.fileExists(atPath: storedPath, isDirectory: &isDirectory) + if exists, isDirectory.boolValue { + return URL(fileURLWithPath: storedPath, isDirectory: true) + } + } + return defaultRedlineFolder(home: home, fileManager: fileManager) + } + + /// True when `url` exists as a directory AND is writable by the current process. + /// The live check `send(anchor:)` performs before ever composing into a OneDrive + /// destination — a directory that exists but has had its permissions revoked (e.g. + /// `chmod 500`) must be treated as unavailable, not silently attempted and + /// surfaced as a generic PDF-composition failure. + public static func isWritableDirectory( + at url: URL, + fileManager: FileManager = .default + ) -> Bool { + var isDirectory: ObjCBool = false + let exists = fileManager.fileExists(atPath: url.path, isDirectory: &isDirectory) + guard exists, isDirectory.boolValue else { return false } + return fileManager.isWritableFile(atPath: url.path) + } + + /// Writes a tiny probe file into `folder`, fsyncs it, then removes it — the only + /// reliable way to catch a OneDrive Files-On-Demand directory whose provider domain + /// is signed out: such a directory can report as existing and POSIX-writable + /// (`isWritableDirectory` returns true) while an actual write fails. True only when + /// the write, fsync, AND removal of the probe file all succeed; any failure at any + /// of those steps means false, so the caller treats the folder as unavailable + /// rather than proceeding to compose a real PDF into it. + public static func probeWritable( + at folder: URL, + fileManager: FileManager = .default + ) -> Bool { + let probeURL = folder.appendingPathComponent(".redline-probe-\(UUID().uuidString)") + // Belt-and-suspenders cleanup, unconditional: AtomicFile.write renames the temp + // file onto probeURL and THEN fsyncs the containing directory — if that last + // fsync throws, the probe file already exists on disk but the catch below + // returns false before ever reaching the explicit removeItem call. And if the + // explicit removeItem itself throws, this is the only retry it gets. Either + // way, never leave the probe file behind just because we're about to return. + defer { + if fileManager.fileExists(atPath: probeURL.path) { + try? fileManager.removeItem(at: probeURL) + } + } + + do { + try AtomicFile.write(Data(), to: probeURL) + } catch { + return false + } + + do { + try fileManager.removeItem(at: probeURL) + } catch { + return false + } + + // Only true when the explicit removal above actually succeeded AND the file is + // confirmed gone — never trust a removeItem call that returned without throwing + // as proof of anything on a File Provider domain. + return !fileManager.fileExists(atPath: probeURL.path) + } +} diff --git a/Tests/ShotdeckCoreTests/OneDriveLocatorTests.swift b/Tests/ShotdeckCoreTests/OneDriveLocatorTests.swift new file mode 100644 index 0000000..ed843ab --- /dev/null +++ b/Tests/ShotdeckCoreTests/OneDriveLocatorTests.swift @@ -0,0 +1,125 @@ +import Foundation +import Testing +import ShotdeckCore + +@Test +func syncRootsFindsOneDriveDirsMMDFirstIgnoresNonDirsAndOtherProviders() throws { + let home = try makeFakeHome() + defer { try? FileManager.default.removeItem(at: home) } + let cloudStorage = home.appendingPathComponent("Library/CloudStorage", isDirectory: true) + try FileManager.default.createDirectory(at: cloudStorage, withIntermediateDirectories: true) + + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("OneDrive-Flowmaster", isDirectory: true), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("OneDrive-MMDGROUP", isDirectory: true), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("GoogleDrive-x", isDirectory: true), + withIntermediateDirectories: true + ) + // A plain FILE (not a directory) named like a OneDrive root must be ignored. + FileManager.default.createFile( + atPath: cloudStorage.appendingPathComponent("OneDrive-notadir").path, + contents: Data("not a directory".utf8) + ) + + let roots = OneDriveLocator.syncRoots(home: home, fileManager: .default) + + #expect(roots.map(\.lastPathComponent) == ["OneDrive-MMDGROUP", "OneDrive-Flowmaster"]) +} + +@Test +func syncRootsEmptyAndDefaultFolderNilWithNoCloudStorageDirectory() throws { + let home = try makeFakeHome() + defer { try? FileManager.default.removeItem(at: home) } + // No Library/CloudStorage created at all. + + let roots = OneDriveLocator.syncRoots(home: home, fileManager: .default) + #expect(roots.isEmpty) + + let defaultFolder = OneDriveLocator.defaultRedlineFolder(home: home, fileManager: .default) + #expect(defaultFolder == nil) +} + +@Test +func defaultRedlineFolderIsFirstSyncRootPlusRedline() throws { + let home = try makeFakeHome() + defer { try? FileManager.default.removeItem(at: home) } + let cloudStorage = home.appendingPathComponent("Library/CloudStorage", isDirectory: true) + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("OneDrive-MMDGROUP", isDirectory: true), + withIntermediateDirectories: true + ) + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("OneDrive-Flowmaster", isDirectory: true), + withIntermediateDirectories: true + ) + + let defaultFolder = try #require( + OneDriveLocator.defaultRedlineFolder(home: home, fileManager: .default) + ) + // Derive "expected" from syncRoots() itself (already covered by its own dedicated + // test) rather than hand-building the path string — FileManager's directory + // enumeration can canonicalize /var -> /private/var and the two constructions + // otherwise disagree on that even for a URL that already exists. + let expectedRoot = try #require(OneDriveLocator.syncRoots(home: home, fileManager: .default).first) + let expected = expectedRoot.appendingPathComponent("Redline", isDirectory: true) + #expect(defaultFolder.path == expected.path) +} + +@Test +func resolveOneDriveFolderPrefersAnExistingStoredOverride() throws { + let home = try makeFakeHome() + defer { try? FileManager.default.removeItem(at: home) } + let cloudStorage = home.appendingPathComponent("Library/CloudStorage", isDirectory: true) + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("OneDrive-MMDGROUP", isDirectory: true), + withIntermediateDirectories: true + ) + + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let override = try makeTransportTemporaryDirectory(prefix: "shotdeck-onedrive-override") + defer { try? FileManager.default.removeItem(at: override) } + TransportSettings.setOneDriveFolder(override, defaults: suite.defaults) + + let resolved = OneDriveLocator.resolveOneDriveFolder( + defaults: suite.defaults, home: home, fileManager: .default + ) + #expect(resolved?.path == override.path) +} + +@Test +func resolveOneDriveFolderIgnoresAStoredPathThatNoLongerExists() throws { + let home = try makeFakeHome() + defer { try? FileManager.default.removeItem(at: home) } + let cloudStorage = home.appendingPathComponent("Library/CloudStorage", isDirectory: true) + try FileManager.default.createDirectory( + at: cloudStorage.appendingPathComponent("OneDrive-MMDGROUP", isDirectory: true), + withIntermediateDirectories: true + ) + + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let goneOverride = try makeTransportTemporaryDirectory(prefix: "shotdeck-onedrive-gone") + TransportSettings.setOneDriveFolder(goneOverride, defaults: suite.defaults) + try FileManager.default.removeItem(at: goneOverride) + + let resolved = OneDriveLocator.resolveOneDriveFolder( + defaults: suite.defaults, home: home, fileManager: .default + ) + let expectedRoot = try #require(OneDriveLocator.syncRoots(home: home, fileManager: .default).first) + let expected = expectedRoot.appendingPathComponent("Redline", isDirectory: true) + #expect(resolved?.path == expected.path) +} + +private func makeFakeHome() throws -> URL { + let home = FileManager.default.temporaryDirectory + .appendingPathComponent("shotdeck-fake-home-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: home, withIntermediateDirectories: true) + return home +} diff --git a/Tests/ShotdeckCoreTests/ReturnWatcherTests.swift b/Tests/ShotdeckCoreTests/ReturnWatcherTests.swift index 95bce13..679b504 100644 --- a/Tests/ShotdeckCoreTests/ReturnWatcherTests.swift +++ b/Tests/ShotdeckCoreTests/ReturnWatcherTests.swift @@ -230,3 +230,162 @@ func w27_fsEventsCallbackFiresOnRealArrival() async throws { await watcher.stop() } } + +@Test("recordUncommented defaults to true: an unmarked PDF is still recorded (AirDrop behaviour unchanged)") +func recordUncommentedDefaultTrueRecordsAnUnmarkedPDF() async throws { + let (paths, cleanup) = try makeCasePaths() + defer { try? FileManager.default.removeItem(at: cleanup) } + + let ledger = try ReturnLedger(paths: paths) + let watcher = ReturnWatcher(paths: paths, ledger: ledger) + let pdfURL = paths.watchFolder.appendingPathComponent("Redline-20260905-090000.pdf") + + try makePDF(at: pdfURL, pageCount: 1, creator: "Redline", annotations: []) + + let found = try await watcher.scanNow() + #expect(found.count == 1) + #expect(found.first?.isCommented == false) + + let all = try await ledger.all() + #expect(all.count == 1) +} + +@Test("recordUncommented=false: an unmarked PDF is not recorded or returned; marking it up in place gets it recorded") +func recordUncommentedFalseSkipsUnmarkedThenRecordsAfterInPlaceMarkup() async throws { + let (paths, cleanup) = try makeCasePaths() + defer { try? FileManager.default.removeItem(at: cleanup) } + + let ledger = try ReturnLedger(paths: paths) + let watcher = ReturnWatcher(paths: paths, ledger: ledger) + await watcher.setRecordUncommented(false) + let pdfURL = paths.watchFolder.appendingPathComponent("Redline-20260905-091500.pdf") + + // OneDrive mode: the PDF is freshly written here (by "send"), unmarked so far. + try makePDF(at: pdfURL, pageCount: 1, creator: "Redline", annotations: []) + + let beforeMarkup = try await watcher.scanNow() + #expect(beforeMarkup.isEmpty) + let allBefore = try await ledger.all() + #expect(allBefore.isEmpty) + + // What the iPad does: mark it up in place, in the SAME folder, then save. + let document = try #require(PDFDocument(url: pdfURL)) + let page = try #require(document.page(at: 0)) + page.addAnnotation(makeAnnotation(.ink, bounds: CGRect(x: 100, y: 100, width: 120, height: 50))) + #expect(document.write(to: pdfURL)) + + let afterMarkup = try await watcher.scanNow() + #expect(afterMarkup.count == 1) + #expect(afterMarkup.first?.isCommented == true) + + let commented = try await ledger.commented() + #expect(commented.count == 1) + #expect(commented.first?.fileURL.resolvingSymlinksInPath().path == pdfURL.resolvingSymlinksInPath().path) +} + +// MARK: - Launch-paths BLOCKER regression (adversarial review, 20260905) +// +// The bug: AppDelegate.makeLaunchModel() built `paths` via the AirDrop-only +// FolderSettings.resolvedAppSupportPaths(), so ReturnWatcher's internal watchFolder +// (seeded from paths.watchFolder in its own init) was the AirDrop folder even when +// OneDrive was the persisted transport, and bootstrap() never reconciled it before +// starting. Net effect: PDFs went to OneDrive but FSEvents kept watching the stale +// AirDrop folder for the whole session — marked-up returns were never detected. +// The fix: launch paths now come from TransportSettings.resolvedAppSupportPaths() +// (transport-aware), and AppModel.bootstrap() unconditionally reconciles the watcher's +// folder via updateWatchFolder() before it starts. These two tests characterize the +// bug (still reproducible via the old AirDrop-only construction) and prove the fix +// (the real launch-construction path, end to end). + +@Test("Launch regression (fix): OneDrive persisted -> transport-aware launch paths -> bootstrap-style reconcile -> a marked PDF is detected") +func launchStyleConstructionWithOneDriveTransportDetectsAMarkedReturn() async throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let oneDriveFolder = try makeTransportTemporaryDirectory(prefix: "shotdeck-launch-onedrive") + defer { try? FileManager.default.removeItem(at: oneDriveFolder) } + let appSupportRoot = try makeTransportTemporaryDirectory(prefix: "shotdeck-launch-approot") + defer { try? FileManager.default.removeItem(at: appSupportRoot) } + + TransportSettings.setTransport(.oneDrive, defaults: suite.defaults) + TransportSettings.setOneDriveFolder(oneDriveFolder, defaults: suite.defaults) + + // Exactly what AppDelegate.makeLaunchModel() now does: build launch paths from the + // transport-aware resolver — the fix, NOT FolderSettings.resolvedAppSupportPaths(), + // which is AirDrop-only and is the root cause the next test characterizes. + let paths = try TransportSettings.resolvedAppSupportPaths( + root: appSupportRoot, defaults: suite.defaults, fileManager: .default + ) + #expect(paths.outbox.path == oneDriveFolder.path) + #expect(paths.watchFolder.path == oneDriveFolder.path) + + let ledger = try ReturnLedger(paths: paths) + let watcher = ReturnWatcher(paths: paths, ledger: ledger) + + // What AppModel.bootstrap() now does, unconditionally, before watcher.start(): + await watcher.setRecordUncommented(false) // transport == .oneDrive + try await watcher.updateWatchFolder(paths.watchFolder) + + let pdfURL = oneDriveFolder.appendingPathComponent("Redline-20260905-100000.pdf") + try makePDF( + at: pdfURL, pageCount: 1, creator: "Redline", + annotations: [(page: 0, annotation: makeAnnotation( + .ink, bounds: CGRect(x: 100, y: 100, width: 120, height: 50) + ))] + ) + + let found = try await watcher.scanNow() + #expect(found.contains(where: { + $0.fileURL.resolvingSymlinksInPath().path == pdfURL.resolvingSymlinksInPath().path && $0.isCommented + })) + + let commented = try await ledger.commented() + #expect(commented.contains(where: { + $0.fileURL.resolvingSymlinksInPath().path == pdfURL.resolvingSymlinksInPath().path + })) +} + +@Test("Launch regression (characterizes the bug): AirDrop-only launch paths with no reconcile miss an OneDrive-mode return") +func airDropOnlyLaunchPathsWithoutReconcileMissesAMarkedOneDriveReturn() async throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let oneDriveFolder = try makeTransportTemporaryDirectory(prefix: "shotdeck-buggy-onedrive") + defer { try? FileManager.default.removeItem(at: oneDriveFolder) } + // A configured AirDrop watch-folder override, isolated to a temp dir — NOT the real + // ~/Downloads, which may already hold real marked-up Redline PDFs from actual use + // and would make this test's "found.isEmpty" assertion depend on the state of + // Ben's real Downloads folder instead of the isolated fixture under test. + let staleAirDropFolder = try makeTransportTemporaryDirectory(prefix: "shotdeck-buggy-airdrop-stale") + defer { try? FileManager.default.removeItem(at: staleAirDropFolder) } + let appSupportRoot = try makeTransportTemporaryDirectory(prefix: "shotdeck-buggy-approot") + defer { try? FileManager.default.removeItem(at: appSupportRoot) } + + TransportSettings.setTransport(.oneDrive, defaults: suite.defaults) + TransportSettings.setOneDriveFolder(oneDriveFolder, defaults: suite.defaults) + FolderSettings.setWatchFolder(staleAirDropFolder, defaults: suite.defaults) + + // The BUG's exact construction: FolderSettings.resolvedAppSupportPaths() ignores + // the persisted transport entirely and always resolves the AirDrop folders. + let buggyPaths = try FolderSettings.resolvedAppSupportPaths(root: appSupportRoot, defaults: suite.defaults) + #expect(buggyPaths.watchFolder.path == staleAirDropFolder.path) + #expect(buggyPaths.watchFolder.path != oneDriveFolder.path) + + let ledger = try ReturnLedger(paths: buggyPaths) + let watcher = ReturnWatcher(paths: buggyPaths, ledger: ledger) + // The old bootstrap(): recordUncommented was set, but there was NO + // updateWatchFolder() call before start() to reconcile the folder. + await watcher.setRecordUncommented(false) + + let pdfURL = oneDriveFolder.appendingPathComponent("Redline-20260905-100100.pdf") + try makePDF( + at: pdfURL, pageCount: 1, creator: "Redline", + annotations: [(page: 0, annotation: makeAnnotation( + .ink, bounds: CGRect(x: 100, y: 100, width: 120, height: 50) + ))] + ) + + // The watcher is still pointed at the stale (configured-AirDrop) watch folder, so + // scanning it — NOT the OneDrive folder the PDF actually landed in — finds nothing. + // This is the exact BLOCKER the fix above closes. + let found = try await watcher.scanNow() + #expect(found.isEmpty) +} diff --git a/Tests/ShotdeckCoreTests/TransportSettingsTests.swift b/Tests/ShotdeckCoreTests/TransportSettingsTests.swift new file mode 100644 index 0000000..3eb787f --- /dev/null +++ b/Tests/ShotdeckCoreTests/TransportSettingsTests.swift @@ -0,0 +1,227 @@ +import Foundation +import Testing +import ShotdeckCore + +@Test +func transportDefaultsToAirDropWhenUnset() throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + + #expect(TransportSettings.transport(defaults: suite.defaults) == .airDrop) +} + +@Test +func setTransportRoundTrips() throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + + TransportSettings.setTransport(.oneDrive, defaults: suite.defaults) + #expect(TransportSettings.transport(defaults: suite.defaults) == .oneDrive) + + TransportSettings.setTransport(.airDrop, defaults: suite.defaults) + #expect(TransportSettings.transport(defaults: suite.defaults) == .airDrop) +} + +@Test +func garbageStoredTransportFallsBackToAirDrop() throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + + suite.defaults.set("not-a-real-transport", forKey: TransportSettings.transportDefaultsKey) + #expect(TransportSettings.transport(defaults: suite.defaults) == .airDrop) +} + +@Test +func oneDriveFolderStoreAndReset() throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let folder = try makeTransportTemporaryDirectory(prefix: "shotdeck-onedrive-folder") + defer { try? FileManager.default.removeItem(at: folder) } + + #expect(TransportSettings.storedOneDriveFolderPath(defaults: suite.defaults) == nil) + + TransportSettings.setOneDriveFolder(folder, defaults: suite.defaults) + #expect(TransportSettings.storedOneDriveFolderPath(defaults: suite.defaults) == folder.path) + + TransportSettings.resetOneDriveFolder(defaults: suite.defaults) + #expect(TransportSettings.storedOneDriveFolderPath(defaults: suite.defaults) == nil) +} + +@Test +func effectiveFoldersForAirDropMatchesFolderSettings() throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let outbox = try makeTransportTemporaryDirectory(prefix: "shotdeck-effective-outbox") + defer { try? FileManager.default.removeItem(at: outbox) } + FolderSettings.setOutbox(outbox, defaults: suite.defaults) + + let effective = TransportSettings.effectiveFolders(defaults: suite.defaults) + let expected = FolderSettings.resolve(defaults: suite.defaults) + + #expect(effective.transport == .airDrop) + #expect(effective.outbox.path == expected.outbox.path) + #expect(effective.watch.path == expected.watch.path) +} + +@Test +func effectiveFoldersForOneDriveWithAResolvableFolderUsesItForBoth() throws { + let suite = try makeTransportDefaultsSuite() + defer { tearDownTransportSuite(suite) } + let folder = try makeTransportTemporaryDirectory(prefix: "shotdeck-effective-onedrive") + defer { try? FileManager.default.removeItem(at: folder) } + + TransportSettings.setTransport(.oneDrive, defaults: suite.defaults) + TransportSettings.setOneDriveFolder(folder, defaults: suite.defaults) + + let effective = TransportSettings.effectiveFolders(defaults: suite.defaults) + + #expect(effective.transport == .oneDrive) + #expect(effective.outbox.path == folder.path) + #expect(effective.watch.path == folder.path) + #expect(effective.outbox.path == effective.watch.path) +} + +@Test +func oneDriveFolderUnavailableErrorDescriptionContainsThePath() throws { + let path = "/Users/example/Library/CloudStorage/OneDrive-Example/Redline" + let error = ShotdeckError.oneDriveFolderUnavailable(path: path) + let description = try #require(error.errorDescription) + #expect(!description.isEmpty) + #expect(description.contains(path)) +} + +@Test +func isWritableDirectoryTrueForAnOrdinaryWritableDirectory() throws { + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-writable") + defer { try? FileManager.default.removeItem(at: dir) } + #expect(OneDriveLocator.isWritableDirectory(at: dir)) +} + +@Test +func isWritableDirectoryFalseForAnExistingButUnwritableDirectory() throws { + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-unwritable") + defer { + // Restore perms BEFORE removal — an unwritable dir can't otherwise be cleaned up. + try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: dir.path) + try? FileManager.default.removeItem(at: dir) + } + #expect(OneDriveLocator.isWritableDirectory(at: dir)) // sanity check before chmod + + try FileManager.default.setAttributes([.posixPermissions: 0o500], ofItemAtPath: dir.path) + #expect(!OneDriveLocator.isWritableDirectory(at: dir)) +} + +@Test +func isWritableDirectoryFalseForAPlainFileAndForANonexistentPath() throws { + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-writable-check-parent") + defer { try? FileManager.default.removeItem(at: dir) } + let filePath = dir.appendingPathComponent("plain-file.txt") + FileManager.default.createFile(atPath: filePath.path, contents: Data("x".utf8)) + + #expect(!OneDriveLocator.isWritableDirectory(at: filePath)) + #expect(!OneDriveLocator.isWritableDirectory(at: dir.appendingPathComponent("does-not-exist"))) +} + +@Test +func probeWritableTrueForAnOrdinaryWritableDirectoryAndLeavesNoProbeFileBehind() throws { + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-probe-writable") + defer { try? FileManager.default.removeItem(at: dir) } + + #expect(OneDriveLocator.probeWritable(at: dir)) + + let leftovers = try FileManager.default.contentsOfDirectory(atPath: dir.path) + #expect(leftovers.isEmpty) +} + +@Test +func probeWritableFalseForAChmod500Directory() throws { + // The File Provider edge case this probe exists for: isWritableDirectory can be + // true (as verified by the isWritableDirectory tests above) while an actual write + // still fails. A chmod 500 directory reproduces that "looks writable, isn't" + // shape closely enough to prove the probe itself does a real write, not just + // another permissions-bit check. + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-probe-unwritable") + defer { + try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: dir.path) + try? FileManager.default.removeItem(at: dir) + } + try FileManager.default.setAttributes([.posixPermissions: 0o500], ofItemAtPath: dir.path) + + #expect(!OneDriveLocator.probeWritable(at: dir)) +} + +@Test +func probeWritableFalseForAPlainFilePath() throws { + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-probe-file-parent") + defer { try? FileManager.default.removeItem(at: dir) } + let filePath = dir.appendingPathComponent("plain-file.txt") + FileManager.default.createFile(atPath: filePath.path, contents: Data("x".utf8)) + + #expect(!OneDriveLocator.probeWritable(at: filePath)) +} + +/// The write itself succeeds (a real probe file lands on disk via AtomicFile.write, +/// which never touches this injected FileManager — it uses raw POSIX calls), but the +/// FIRST call to `removeItem(at:)` throws, simulating a transient File Provider +/// removal failure. probeWritable's own defer-based cleanup must retry and succeed +/// (the second call through this same override falls through to `super`), so no +/// probe file is left behind even though the function correctly still reports false +/// (the removal it explicitly attempted did fail). +private final class ThrowOnceOnRemoveFileManager: FileManager, @unchecked Sendable { + private let lock = NSLock() + private var hasThrown = false + + override func removeItem(at URL: URL) throws { + lock.lock() + let shouldThrow = !hasThrown + hasThrown = true + lock.unlock() + if shouldThrow { + throw NSError(domain: "ShotdeckCoreTests.ThrowOnceOnRemove", code: 1) + } + try super.removeItem(at: URL) + } +} + +@Test +func probeWritableFalseAndLeavesNoProbeFileWhenRemoveItemThrowsOnce() throws { + // Directory fsync failure (the OTHER way probeWritable's cleanup can be needed) has + // no injectable seam: AtomicFile.write's directory fsync is a raw Darwin fsync(2) + // call on an already-open file descriptor, not parameterized by any FileManager or + // other dependency this test can substitute, and there is no portable way to make + // fsync(2) itself fail via chmod or other standard test techniques (fsync failures + // are OS/filesystem/hardware-level events). Covering the removeItem-throws path + // (below) is what this test does; the fsync-throws path is covered by code + // inspection only — the same `defer` block guards both. + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-probe-remove-throws") + defer { try? FileManager.default.removeItem(at: dir) } + let injectedFileManager = ThrowOnceOnRemoveFileManager() + + #expect(!OneDriveLocator.probeWritable(at: dir, fileManager: injectedFileManager)) + + let leftovers = try FileManager.default.contentsOfDirectory(atPath: dir.path) + #expect(leftovers.isEmpty) +} + +struct TransportDefaultsSuite { + let name: String + let defaults: UserDefaults +} + +func makeTransportDefaultsSuite() throws -> TransportDefaultsSuite { + let name = "shotdeck-transport-test-\(UUID().uuidString)" + let defaults = try #require(UserDefaults(suiteName: name)) + defaults.removePersistentDomain(forName: name) + return TransportDefaultsSuite(name: name, defaults: defaults) +} + +func tearDownTransportSuite(_ suite: TransportDefaultsSuite) { + suite.defaults.removePersistentDomain(forName: suite.name) +} + +func makeTransportTemporaryDirectory(prefix: String) throws -> URL { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("\(prefix)-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + return url +} diff --git a/Tests/ShotdeckTests/LaunchWiringTests.swift b/Tests/ShotdeckTests/LaunchWiringTests.swift new file mode 100644 index 0000000..e45d498 --- /dev/null +++ b/Tests/ShotdeckTests/LaunchWiringTests.swift @@ -0,0 +1,142 @@ +import AppKit +import Foundation +import PDFKit +import Testing +import ShotdeckCore +@testable import Shotdeck + +/// Coverage gap closed (adversarial review, rounds 3 and 4): the Core-level regression +/// tests in ShotdeckCoreTests hand-replicate what `AppDelegate.makeLaunchModel()` and +/// `AppModel.bootstrap()` do, rather than calling them — so a future revert of +/// `makeLaunchModel()` back to the AirDrop-only resolver, or a dropped +/// `updateWatchFolder` call inside `bootstrap()`, would NOT fail `swift test`. This +/// test goes through the real, unmodified call sites in the `Shotdeck` executable +/// target via `@testable import`, which `ShotdeckCoreTests` cannot reach (it only +/// depends on `ShotdeckCore`) — hence this separate `ShotdeckTests` target. +/// +/// Round 4 correction: the first version of this test asserted only +/// `model.watchFolderURL`, which `AppModel.init` computes independently via +/// `TransportSettings.effectiveFolders()` — so it stayed correct (and the test kept +/// passing) even when `makeLaunchModel()` was reverted to the AirDrop-only resolver, +/// because `bootstrap()`'s own unconditional `updateWatchFolder` reconcile papered +/// over the reverted resolver. That made the "verified this catches the blocker" +/// claim in the previous round's commit message empirically false. This version +/// asserts `model.paths`/the watcher's `currentWatchFolder` BEFORE `bootstrap()` runs, +/// which actually depends on what `makeLaunchModel()` built — see this file's git +/// history (or the round-4 commit message) for the verbatim before/after +/// `swift test --filter` output proving it now discriminates correctly. +@MainActor +@Test("Real wiring: AppDelegate.makeLaunchModel() + AppModel.bootstrap() detect a marked OneDrive return") +func realLaunchModelAndBootstrapDetectAMarkedOneDriveReturn() async throws { + let fm = FileManager.default + + // UserDefaults.standard is the ONLY defaults instance makeLaunchModel()/bootstrap() + // actually read — there is no defaults-threading through AppModel/AppDelegate (the + // same reasoning documented in PickerSelfTest.swift's ONEDRIVE-SELFTEST phase). + // "Isolated" here means snapshot-and-restore around the real keys, not a separate + // UserDefaults(suiteName:) instance that these real, unmodified call sites would + // never actually consult. + let defaults = UserDefaults.standard + let previousTransport = defaults.string(forKey: TransportSettings.transportDefaultsKey) + let previousFolder = defaults.string(forKey: TransportSettings.oneDriveFolderDefaultsKey) + defer { + if let previousTransport { + defaults.set(previousTransport, forKey: TransportSettings.transportDefaultsKey) + } else { + defaults.removeObject(forKey: TransportSettings.transportDefaultsKey) + } + if let previousFolder { + defaults.set(previousFolder, forKey: TransportSettings.oneDriveFolderDefaultsKey) + } else { + defaults.removeObject(forKey: TransportSettings.oneDriveFolderDefaultsKey) + } + } + + let oneDriveFolderRaw = fm.temporaryDirectory + .appendingPathComponent("shotdeck-real-wiring-onedrive-\(UUID().uuidString)", isDirectory: true) + try fm.createDirectory(at: oneDriveFolderRaw, withIntermediateDirectories: true) + defer { try? fm.removeItem(at: oneDriveFolderRaw) } + // FileManager's directory enumeration (inside the real ReturnWatcher/AppSupportPaths + // call sites this test exercises) can canonicalize /var -> /private/var for a path + // that actually exists; resolve here so every comparison below agrees. + let oneDriveFolder = oneDriveFolderRaw.resolvingSymlinksInPath() + + let appSupportRoot = fm.temporaryDirectory + .appendingPathComponent("shotdeck-real-wiring-approot-\(UUID().uuidString)", isDirectory: true) + defer { try? fm.removeItem(at: appSupportRoot) } + + TransportSettings.setTransport(.oneDrive, defaults: defaults) + TransportSettings.setOneDriveFolder(oneDriveFolder, defaults: defaults) + + // Best-effort: keeps AppModel.bootstrap()'s real update-check schedule (a real + // HTTP GET after 10s, plus a RunLoop timer) from starting during this test. + // ProcessInfo.processInfo.environment on Darwin reads `environ` fresh each call, + // so a setenv() here is visible to bootstrap()'s own check immediately. + setenv("SHOTDECK_ONEDRIVE_SELFTEST", "1", 1) + defer { unsetenv("SHOTDECK_ONEDRIVE_SELFTEST") } + + // The REAL, unmodified call sites — not a reimplementation. This is exactly what + // launching Redline with OneDrive as the persisted transport does. + let model = AppDelegate.makeLaunchModel(appSupportRoot: appSupportRoot) + // bootstrap() registers a REAL, process-wide Carbon global hotkey (capture combo, + // e.g. Option-Shift-2). Carbon registrations are not scoped to this test/model — + // they must be released before this test ends, or ShotdeckCoreTests' + // HotkeyCenterCarbonTests (a separate test target, same test process) can find the + // combo already taken / the global hotkey table in an unexpected state. + defer { model.hotkeys.unregisterAll() } + + // PRE-bootstrap assertions — this is the actual proof of the launch RESOLVER + // (AppDelegate.makeLaunchModel() -> TransportSettings.resolvedAppSupportPaths()), + // independent of bootstrap()'s own reconcile. `model.watchFolderURL` alone does + // NOT prove this: AppModel.init computes it separately via + // TransportSettings.effectiveFolders(), so it would read as correct even if + // makeLaunchModel's `paths` were built by the AirDrop-only resolver — which is + // exactly how the first version of this test was empirically shown to be vacuous + // for the launch-resolver path (see this commit's message). `model.paths` is + // `internal` on AppModel, so @testable import already exposes it without any + // production API change; `currentWatchFolder` is the one new (internal-facing, + // `public` on the actor) seam added to ReturnWatcher for this purpose. + #expect(model.paths.watchFolder.path == oneDriveFolder.path) + #expect(model.paths.outbox.path == oneDriveFolder.path) + let seededWatchFolder = await model.watcher.currentWatchFolder + #expect(seededWatchFolder.path == oneDriveFolder.path) + + #expect(model.transport == .oneDrive) + #expect(model.watchFolderURL.path == oneDriveFolder.path) + + await model.bootstrap() + + // Drop a marked-up Redline PDF into the folder in place — what OneDrive syncing + // down an already-marked copy after a relaunch looks like. + let pdfURL = oneDriveFolder.appendingPathComponent("Redline-realwiring-\(UUID().uuidString).pdf") + let document = PDFDocument() + let page = PDFPage() + page.setBounds(CGRect(x: 0, y: 0, width: 612, height: 792), for: .mediaBox) + document.insert(page, at: 0) + document.documentAttributes = [ + PDFDocumentAttribute.creatorAttribute: "Redline", + PDFDocumentAttribute.subjectAttribute: UUID().uuidString, + ] + let ink = PDFAnnotation( + bounds: CGRect(x: 20, y: 20, width: 60, height: 60), forType: .ink, withProperties: nil + ) + let stroke = NSBezierPath() + stroke.move(to: NSPoint(x: 20, y: 20)) + stroke.line(to: NSPoint(x: 80, y: 80)) + ink.add(stroke) + page.addAnnotation(ink) + let written = document.write(to: pdfURL) + #expect(written) + guard written else { return } + + // .resolvingSymlinksInPath().path — not plain URL equality — matching how the rest + // of the suite compares a temp-dir-derived expected URL against a returned one. + let expectedPath = pdfURL.resolvingSymlinksInPath().path + let found = try await model.watcher.scanNow() + #expect(found.first(where: { $0.fileURL.resolvingSymlinksInPath().path == expectedPath })?.isCommented == true) + + let commented = try await model.ledger.commented() + #expect(commented.contains(where: { $0.fileURL.resolvingSymlinksInPath().path == expectedPath })) + + await model.watcher.stop() +}