fix: send() probes real writability before composing; write/rename failures map to oneDriveFolderUnavailable
send()'s OneDrive pre-flight check now also calls OneDriveLocator.probeWritable(at:) alongside isWritableDirectory — closing the File Provider edge case where a signed-out OneDrive domain reports its folder as existing and writable while a real write fails. composePDFForSend(outbox:transport:) now takes the frozen transport too (not just the folder): a write/rename failure specifically at the destination folder — Darwin.rename, AtomicFile.fsyncDirectory, or the post-write existence check — is reported as ShotdeckError.oneDriveFolderUnavailable instead of the generic pdfCompositionFailed when transport is .oneDrive. composer.compose()'s own session/image-content failures are left as generic pdfCompositionFailed regardless of transport — those aren't about the destination folder. Also: send() now `await`s `pendingReconcileTask` (AppModel.swift, set by chooseTransport/chooseOneDriveFolder in SettingsView.swift) before snapshotting transport/folder, closing the toggle-then-immediate-send race — without this, a Send issued right after a transport toggle could run before the watcher's recordUncommented flag finished catching up. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
This commit is contained in:
@@ -16,28 +16,44 @@ extension AppModel: SendCapable {
|
|||||||
guard !session.isEmpty, !isSending else { return }
|
guard !session.isEmpty, !isSending else { return }
|
||||||
setSending(true)
|
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<Void, Never>.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
|
// Snapshot BOTH the transport AND the destination folder into local `let`s
|
||||||
// ONCE, before any `await` in this function. chooseTransport/chooseOneDriveFolder
|
// ONCE, before any further `await` in this function. chooseTransport/
|
||||||
// now refuse (status "Finish the current send first.") while isSending is true,
|
// chooseOneDriveFolder also refuse outright (status "Finish the current send
|
||||||
// but this snapshot is the actual fix for the race: even without that guard,
|
// first.") while isSending is true, but this snapshot is the actual fix for the
|
||||||
// everything below operates on these frozen values — composePDFForSend(outbox:)
|
// send-vs-switch race: even without that guard, everything below operates on
|
||||||
// takes the folder as a parameter and never re-reads `self.outboxURL` after a
|
// these frozen values — composePDFForSend(outbox:transport:) takes both as
|
||||||
// suspension point, so a concurrent transport switch mid-send can no longer land
|
// parameters and never re-reads `self.outboxURL`/`self.transport` after a
|
||||||
// the PDF under one transport's folder while the archive/status branch (which
|
// suspension point, so a concurrent transport switch mid-send can no longer
|
||||||
// switches on the same frozen `transport` local) runs the other's.
|
// land the PDF under one transport's folder while the archive/status branch
|
||||||
|
// runs the other's.
|
||||||
let transport = TransportSettings.transport()
|
let transport = TransportSettings.transport()
|
||||||
let destinationFolder: URL
|
let destinationFolder: URL
|
||||||
|
|
||||||
// OneDrive mode: verify the real destination exists AND is writable RIGHT NOW,
|
// OneDrive mode: verify the real destination exists, is writable, AND actually
|
||||||
// before composing anything. `outboxURL` is kept in sync with the resolved
|
// accepts a real write RIGHT NOW, before composing anything. `isWritableDirectory`
|
||||||
// OneDrive folder by bootstrap/chooseTransport/chooseOneDriveFolder, but this is
|
// 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
|
// re-resolved fresh here (never trusted stale) so a folder that vanished or lost
|
||||||
// its permissions since then (OneDrive signed out, external volume unmounted,
|
// its permissions since then (OneDrive signed out, external volume unmounted,
|
||||||
// folder deleted, chmod'd unwritable) is caught instead of silently attempted
|
// folder deleted, chmod'd unwritable) is caught instead of silently attempted
|
||||||
// and surfacing as a generic PDF-composition failure.
|
// and surfacing as a generic PDF-composition failure.
|
||||||
if transport == .oneDrive {
|
if transport == .oneDrive {
|
||||||
guard let folder = OneDriveLocator.resolveOneDriveFolder(),
|
guard let folder = OneDriveLocator.resolveOneDriveFolder(),
|
||||||
OneDriveLocator.isWritableDirectory(at: folder)
|
OneDriveLocator.isWritableDirectory(at: folder),
|
||||||
|
OneDriveLocator.probeWritable(at: folder)
|
||||||
else {
|
else {
|
||||||
let path = OneDriveLocator.resolveOneDriveFolder()?.path
|
let path = OneDriveLocator.resolveOneDriveFolder()?.path
|
||||||
?? TransportSettings.storedOneDriveFolderPath()
|
?? TransportSettings.storedOneDriveFolderPath()
|
||||||
@@ -59,7 +75,7 @@ extension AppModel: SendCapable {
|
|||||||
|
|
||||||
let pending: ComposedSend
|
let pending: ComposedSend
|
||||||
do {
|
do {
|
||||||
pending = try await composePDFForSend(outbox: destinationFolder)
|
pending = try await composePDFForSend(outbox: destinationFolder, transport: transport)
|
||||||
} catch {
|
} catch {
|
||||||
// Never unlink the published PDF, and never unlink the temp file either:
|
// Never unlink the published PDF, and never unlink the temp file either:
|
||||||
// a rename failure would leave the complete document at the temp name.
|
// a rename failure would leave the complete document at the temp name.
|
||||||
@@ -109,10 +125,16 @@ extension AppModel: SendCapable {
|
|||||||
|
|
||||||
/// Writes the PDF to `outboxDir` 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.
|
/// and does not present AirDrop — that happens only after the share completes.
|
||||||
/// `outboxDir` is passed in (a value `send(anchor:)` snapshotted before any await)
|
/// `outboxDir`/`transport` are passed in (values `send(anchor:)` snapshotted before
|
||||||
/// rather than read from `self.outboxURL` here, so a concurrent transport switch
|
/// any await) rather than read from `self.outboxURL`/`self.transport` here, so a
|
||||||
/// mid-send can never redirect an in-flight compose to a different folder.
|
/// concurrent transport switch mid-send can never redirect an in-flight compose to
|
||||||
func composePDFForSend(outbox outboxDir: URL) async throws -> ComposedSend {
|
/// 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 workingSession = session
|
||||||
let composer = self.composer
|
let composer = self.composer
|
||||||
let sourceDir = paths.sessionDirectory(workingSession.id)
|
let sourceDir = paths.sessionDirectory(workingSession.id)
|
||||||
@@ -134,14 +156,27 @@ extension AppModel: SendCapable {
|
|||||||
// POSIX rename onto `finalURL` replaces any same-name file in one
|
// POSIX rename onto `finalURL` replaces any same-name file in one
|
||||||
// directory operation; there is never a window where the PDF is gone.
|
// directory operation; there is never a window where the PDF is gone.
|
||||||
if Darwin.rename(tempURL.path, finalURL.path) != 0 {
|
if Darwin.rename(tempURL.path, finalURL.path) != 0 {
|
||||||
|
if transport == .oneDrive {
|
||||||
|
throw ShotdeckError.oneDriveFolderUnavailable(path: outboxDir.path)
|
||||||
|
}
|
||||||
throw ShotdeckError.pdfCompositionFailed(
|
throw ShotdeckError.pdfCompositionFailed(
|
||||||
reason: "could not publish the PDF: \(String(cString: strerror(errno)))"
|
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
|
}.value
|
||||||
|
|
||||||
guard FileManager.default.fileExists(atPath: finalURL.path) else {
|
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")
|
throw ShotdeckError.pdfCompositionFailed(reason: "the PDF was not written to disk")
|
||||||
}
|
}
|
||||||
rememberLastComposedPDF(finalURL)
|
rememberLastComposedPDF(finalURL)
|
||||||
|
|||||||
Reference in New Issue
Block a user