Compare commits
6
Commits
209921f084
...
70231574c9
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
70231574c9 | ||
|
|
8121738188 | ||
|
|
6484fde530 | ||
|
|
bd11ba96c5 | ||
|
|
ef0d8712f7 | ||
|
|
20de467e87 |
@@ -132,6 +132,13 @@ public final class AppModel {
|
||||
}
|
||||
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
|
||||
func rememberLastComposedPDF(_ url: URL) { lastComposedPDFURL = url }
|
||||
|
||||
/// True when a last-composed PDF path is known this run, or the newest
|
||||
@@ -211,6 +218,17 @@ public final class AppModel {
|
||||
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 }
|
||||
|
||||
@@ -223,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)
|
||||
guard fm.fileExists(atPath: pending.fileURL.path) else {
|
||||
sendTruthFail("PDF was not written")
|
||||
}
|
||||
@@ -476,18 +476,13 @@ enum PickerSelfTest {
|
||||
exit(1)
|
||||
}
|
||||
|
||||
/// Real sync root this Mac has; the phase proves the transport against the actual
|
||||
/// OneDrive file provider, never a fake home tree (that is what
|
||||
/// OneDriveLocatorTests in ShotdeckCoreTests are for).
|
||||
private static let realOneDriveSyncRoot = URL(
|
||||
fileURLWithPath: "/Users/benjaminhippler/Library/CloudStorage/OneDrive-MMDGROUP",
|
||||
isDirectory: true
|
||||
)
|
||||
|
||||
/// Phase 5: proves the OneDrive transport end to end against the real sync root.
|
||||
/// 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).
|
||||
/// 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 {
|
||||
@@ -514,9 +509,16 @@ enum PickerSelfTest {
|
||||
}
|
||||
|
||||
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()
|
||||
let folder = try await executeOneDriveSelfTest(syncRoot: syncRoot)
|
||||
print("ONEDRIVE-SELFTEST PASS path=\(folder.path)")
|
||||
fflush(stdout)
|
||||
exit(0)
|
||||
@@ -528,19 +530,30 @@ enum PickerSelfTest {
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds a session, sends it through the OneDrive branch of `send(anchor: nil)`
|
||||
/// against a NEW folder under the real OneDrive sync root, 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. Never deletes anything under OneDrive — the created folder and
|
||||
/// PDF are left in place for Ben to inspect / for the real iPad round trip.
|
||||
private static func executeOneDriveSelfTest() async throws -> URL {
|
||||
/// 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 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
|
||||
guard fm.fileExists(atPath: realOneDriveSyncRoot.path) else {
|
||||
throw OneDriveSelfTestError.detail(
|
||||
"real OneDrive sync root not found at \(realOneDriveSyncRoot.path)"
|
||||
)
|
||||
}
|
||||
|
||||
// UserDefaults.standard is the ONLY defaults instance send()/TransportSettings
|
||||
// actually read at runtime (there is no defaults-threading through AppModel), so
|
||||
@@ -563,8 +576,7 @@ enum PickerSelfTest {
|
||||
}
|
||||
|
||||
let stamp = DubaiTime.fileStamp(Date())
|
||||
let selftestFolder = realOneDriveSyncRoot
|
||||
.appendingPathComponent("Redline-selftest-\(stamp)", isDirectory: true)
|
||||
let selftestFolder = syncRoot.appendingPathComponent("Redline-selftest-\(stamp)", isDirectory: true)
|
||||
try fm.createDirectory(at: selftestFolder, withIntermediateDirectories: true)
|
||||
|
||||
TransportSettings.setTransport(.oneDrive, defaults: defaults)
|
||||
@@ -629,8 +641,77 @@ enum PickerSelfTest {
|
||||
}
|
||||
|
||||
// What the iPad does: mark it up in place with a real ink annotation, then save.
|
||||
guard let document = PDFDocument(url: pdfURL), let page = document.page(at: 0) else {
|
||||
throw OneDriveSelfTestError.detail("could not reopen \(pdfURL.path) to annotate it")
|
||||
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 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),
|
||||
@@ -642,20 +723,27 @@ enum PickerSelfTest {
|
||||
stroke.line(to: NSPoint(x: 80, y: 80))
|
||||
ink.add(stroke)
|
||||
page.addAnnotation(ink)
|
||||
guard document.write(to: pdfURL) else {
|
||||
throw OneDriveSelfTestError.detail("could not save the annotated PDF back to \(pdfURL.path)")
|
||||
guard document.write(to: url) else {
|
||||
throw OneDriveSelfTestError.detail("could not save the annotated PDF back to \(url.path)")
|
||||
}
|
||||
}
|
||||
|
||||
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")
|
||||
/// 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)")
|
||||
}
|
||||
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")
|
||||
}
|
||||
|
||||
return selftestFolder
|
||||
}
|
||||
|
||||
private static func oneDriveFail(_ detail: String) -> Never {
|
||||
|
||||
@@ -16,17 +16,28 @@ extension AppModel: SendCapable {
|
||||
guard !session.isEmpty, !isSending else { return }
|
||||
setSending(true)
|
||||
|
||||
// Snapshot BOTH the transport AND the destination folder into local `let`s
|
||||
// ONCE, before any `await` in this function. chooseTransport/chooseOneDriveFolder
|
||||
// now refuse (status "Finish the current send first.") while isSending is true,
|
||||
// but this snapshot is the actual fix for the race: even without that guard,
|
||||
// everything below operates on these frozen values — composePDFForSend(outbox:)
|
||||
// takes the folder as a parameter and never re-reads `self.outboxURL` 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 (which
|
||||
// switches on the same frozen `transport` local) runs the other's.
|
||||
let transport = TransportSettings.transport()
|
||||
let destinationFolder: URL
|
||||
|
||||
// OneDrive mode: verify the real destination exists RIGHT NOW, before composing
|
||||
// anything. `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 since then (OneDrive
|
||||
// signed out, external volume unmounted, folder deleted) is caught instead of
|
||||
// silently writing into whatever `outboxURL` happens to hold.
|
||||
// OneDrive mode: verify the real destination exists AND is writable RIGHT NOW,
|
||||
// before composing anything. `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(),
|
||||
Self.directoryExists(at: folder)
|
||||
OneDriveLocator.isWritableDirectory(at: folder)
|
||||
else {
|
||||
let path = OneDriveLocator.resolveOneDriveFolder()?.path
|
||||
?? TransportSettings.storedOneDriveFolderPath()
|
||||
@@ -36,16 +47,19 @@ extension AppModel: SendCapable {
|
||||
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)
|
||||
} 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.
|
||||
@@ -93,19 +107,14 @@ extension AppModel: SendCapable {
|
||||
}
|
||||
}
|
||||
|
||||
private static func directoryExists(at url: URL) -> Bool {
|
||||
var isDirectory: ObjCBool = false
|
||||
let exists = FileManager.default.fileExists(atPath: url.path, isDirectory: &isDirectory)
|
||||
return exists && isDirectory.boolValue
|
||||
}
|
||||
|
||||
/// 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` is passed in (a value `send(anchor:)` snapshotted before any await)
|
||||
/// rather than read from `self.outboxURL` here, so a concurrent transport switch
|
||||
/// mid-send can never redirect an in-flight compose to a different folder.
|
||||
func composePDFForSend(outbox outboxDir: URL) 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)
|
||||
|
||||
@@ -82,21 +82,20 @@ struct SettingsView: View {
|
||||
model.chooseOneDriveFolder()
|
||||
}
|
||||
} else {
|
||||
HStack(spacing: 8) {
|
||||
Text("No OneDrive folder found — sign in to OneDrive or choose a folder.")
|
||||
.font(.caption)
|
||||
.foregroundStyle(.secondary)
|
||||
.fixedSize(horizontal: false, vertical: true)
|
||||
.frame(maxWidth: .infinity, alignment: .leading)
|
||||
Button("Choose…") { model.chooseOneDriveFolder() }
|
||||
// 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()
|
||||
}
|
||||
.frame(minHeight: 22)
|
||||
}
|
||||
}
|
||||
|
||||
GridRow {
|
||||
Text(
|
||||
"The PDF is saved here and this same folder is watched for the marked-up copy. On the iPad open it from Files > OneDrive."
|
||||
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)
|
||||
@@ -249,7 +248,18 @@ extension AppModel: SettingsWindowPresenting {
|
||||
/// 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.
|
||||
func chooseTransport(_ value: SendTransport) {
|
||||
guard !isSending else {
|
||||
setStatus("Finish the current send first.")
|
||||
return
|
||||
}
|
||||
guard value != transport else { return }
|
||||
TransportSettings.setTransport(value)
|
||||
setTransport(value)
|
||||
@@ -261,11 +271,17 @@ extension AppModel: SettingsWindowPresenting {
|
||||
}
|
||||
setFolderURLs(outbox: folders.outbox, watch: folders.watch)
|
||||
setResolvedOneDriveFolder(OneDriveLocator.resolveOneDriveFolder())
|
||||
|
||||
reconcileGeneration += 1
|
||||
let generation = reconcileGeneration
|
||||
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."
|
||||
)
|
||||
@@ -273,7 +289,13 @@ extension AppModel: SettingsWindowPresenting {
|
||||
}
|
||||
}
|
||||
|
||||
/// 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)
|
||||
@@ -281,11 +303,17 @@ extension AppModel: SettingsWindowPresenting {
|
||||
setResolvedOneDriveFolder(OneDriveLocator.resolveOneDriveFolder())
|
||||
guard transport == .oneDrive else { return }
|
||||
setFolderURLs(outbox: url, watch: url)
|
||||
|
||||
reconcileGeneration += 1
|
||||
let generation = reconcileGeneration
|
||||
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."
|
||||
)
|
||||
|
||||
@@ -50,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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -75,6 +75,25 @@ public enum TransportSettings {
|
||||
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
|
||||
@@ -142,4 +161,19 @@ public enum OneDriveLocator {
|
||||
}
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -282,3 +282,110 @@ func recordUncommentedFalseSkipsUnmarkedThenRecordsAfterInPlaceMarkup() async th
|
||||
#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)
|
||||
}
|
||||
|
||||
@@ -90,6 +90,38 @@ func oneDriveFolderUnavailableErrorDescriptionContainsThePath() throws {
|
||||
#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")))
|
||||
}
|
||||
|
||||
struct TransportDefaultsSuite {
|
||||
let name: String
|
||||
let defaults: UserDefaults
|
||||
|
||||
Reference in New Issue
Block a user