Compare commits

...
Author SHA1 Message Date
Claude Fable 5 49c6a41033 release: v0.3.0 2026-09-05 10:16:39 +04:00
Claude Fable 5 706726a3d4 fix(test): LaunchWiringTests was empirically vacuous for the makeLaunchModel path
The reviewer patched main.swift back to FolderSettings.resolvedAppSupportPaths()
(the AirDrop-only resolver) and `swift test --filter LaunchWiringTests` STILL
PASSED, because the only thing the test asserted — model.watchFolderURL — is
computed independently by AppModel.init() via TransportSettings.effectiveFolders(),
not from the `paths` makeLaunchModel() built. bootstrap()'s own unconditional
updateWatchFolder reconcile then papered over the reverted resolver, so the
test only ever proved the bootstrap reconcile, never the launch resolver
itself. The "verified this catches the blocker" claim in the previous
commit was empirically false.

Fix: added ReturnWatcher.currentWatchFolder (public var, actor-isolated —
the folder a watcher is CURRENTLY seeded to scan, readable without calling
scanNow()/updateWatchFolder first). The test now asserts, BEFORE
bootstrap() runs: model.paths.watchFolder/outbox (already internal-visible
via @testable import, no production API change needed there) equal the
OneDrive folder, AND the watcher's currentWatchFolder equals it too — both
of which genuinely depend on what makeLaunchModel() built.

Verified properly this time (both outputs below are verbatim from
`swift test --filter LaunchWiringTests`, main.swift's makeLaunchModel()
temporarily reverted to FolderSettings.resolvedAppSupportPaths() then
restored — the revert itself is not part of this commit):

FAILURE (reverted resolver):
    Expectation failed: (model.paths.watchFolder.path -> "/Users/benjaminhippler/Downloads")
      == (oneDriveFolder.path -> ".../shotdeck-real-wiring-onedrive-<uuid>")
    Expectation failed: (model.paths.outbox.path -> "/Users/benjaminhippler/Desktop")
      == (oneDriveFolder.path -> ".../shotdeck-real-wiring-onedrive-<uuid>")
    Expectation failed: (seededWatchFolder.path -> "/Users/benjaminhippler/Downloads")
      == (oneDriveFolder.path -> ".../shotdeck-real-wiring-onedrive-<uuid>")
    Test ... failed after 0.324 seconds with 3 issues.

PASS (resolver restored):
    Test "Real wiring: AppDelegate.makeLaunchModel() + AppModel.bootstrap()
      detect a marked OneDrive return" passed after 0.295 seconds.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
2026-09-05 10:10:14 +04:00
Claude Fable 5 e1f569cc3e fix(core): probeWritable(at:) always cleans up the probe file, even on partial failure
Two leak paths: (a) AtomicFile.write renames the temp file onto the probe
path and THEN fsyncs the containing directory — if that last fsync throws,
the probe file already exists on disk but the old code returned false
without ever attempting removal; (b) if the explicit removeItem call itself
threw, there was no retry, so a transient File Provider removal failure left
the file behind permanently.

Fix: an unconditional `defer` now checks fileExists and retries removeItem
regardless of which branch returned early. The function only reports true
when the explicit write, fsync (inside AtomicFile.write), AND removal all
succeeded AND the file is confirmed gone afterward.

New test: a FileManager subclass whose removeItem(at:) throws on its first
call (AtomicFile.write itself never touches this injected FileManager — it
uses raw Darwin/POSIX calls, not FileManager, so this only intercepts the
explicit removal + the defer's retry) asserts the function returns false AND
no probe file remains — verified this actually needs the defer by
temporarily removing it and confirming the same test then fails with a
leftover ".redline-probe-<uuid>" file (see this branch's history for the
discarded revert). The directory-fsync failure path has no injectable seam
(raw fsync(2) on an already-open fd, not parameterized by any FileManager or
other substitutable dependency, and not reproducible via chmod or other
standard test techniques) — documented in the test rather than simulated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
2026-09-05 10:10:00 +04:00
kua-agentandClaude Fable 5.1 3abb8dc6ca test: add ShotdeckTests target — real launch/bootstrap wiring regression (coverage gap)
The Core-level regression tests added for the launch-paths BLOCKER
(ReturnWatcherTests.swift) hand-replicate what AppDelegate.makeLaunchModel()
and AppModel.bootstrap() do, rather than calling them — a future revert of
either would not fail `swift test`. Neither lives in ShotdeckCore, so
ShotdeckCoreTests cannot reach them; this new test target depends on the
Shotdeck executable target itself and uses @testable import (confirmed this
works cleanly with SwiftPM despite Shotdeck's main.swift top-level-code entry
point — no separate main-symbol conflict).

realLaunchModelAndBootstrapDetectAMarkedOneDriveReturn persists
transport=.oneDrive (UserDefaults.standard, snapshot-and-restore — the same
established pattern PickerSelfTest's ONEDRIVE-SELFTEST phase already uses,
since neither of these real call sites has any defaults-threading to plug an
isolated suite into), calls AppDelegate.makeLaunchModel(appSupportRoot: <temp>)
and awaits model.bootstrap() UNMODIFIED, then drops a marked-up Redline PDF
into the temp OneDrive folder and asserts the watcher reports it.

Verified this test actually catches the original blocker: temporarily
reverted makeLaunchModel() to the AirDrop-only resolver, and separately
reverted bootstrap()'s updateWatchFolder reconcile — both reproduce the
failure (the discarded revert is not part of this commit).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
2026-09-05 09:55:31 +04:00
kua-agentandClaude Fable 5.1 8d68c836cd test(selftest): ONEDRIVE-SELFTEST sub-step 1c — toggle immediately followed by Send
Adds a third rapid-toggle assertion alongside the existing folder-reconcile
check: chooseTransport(.airDrop) immediately followed by
chooseTransport(.oneDrive), with NO sleep, then an immediate send(anchor: nil)
— proving send() correctly awaits the pending reconcile Task
(SendController.swift/AppModel.swift in this series) rather than racing
ahead with a stale recordUncommented flag. Asserts the freshly-sent,
still-unmarked PDF is never itself reported as an already-returned document.

Also updates composePDFForSend's call site for its new transport: parameter.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
2026-09-05 09:55:22 +04:00
kua-agentandClaude Fable 5.1 04d1e73532 fix: store pendingReconcileTask handle; skip real Carbon hotkey binding on self-test runs
AppModel gains pendingReconcileTask (the most recent watcher-reconcile Task
spawned by chooseTransport/chooseOneDriveFolder), which send() now awaits —
see the SendController.swift commit in this series. chooseTransport/
chooseOneDriveFolder store their Task's handle into it instead of firing an
untracked `Task { }`.

bootstrap() now also skips binding the real, process-wide Carbon global
capture hotkey on the same env-var-flagged self-test/headless runs that
already skip the update-check schedule (PickerSelfTest's phases,
PanelSnapshot, and the new ShotdeckTests launch-wiring regression test).
Real Carbon hotkey registration is not safe to exercise in an automated test
process — it can collide with ShotdeckCoreTests' own
HotkeyCenterCarbonTests running in the same test binary — and bootstrap()'s
hotkey step had never actually been exercised by any self-test before (none
of them call bootstrap() directly) until the new real-wiring test in this
series does. A real user launch never sets these env vars, so production
behavior is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
2026-09-05 09:55:16 +04:00
kua-agentandClaude Fable 5.1 d7984add2d 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
2026-09-05 09:55:08 +04:00
kua-agentandClaude Fable 5.1 723cd7dcea fix(core): File Provider write probe — OneDriveLocator.probeWritable(at:)
isWritableDirectory (permissions bits) 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. probeWritable
writes a small ".redline-probe-<uuid>" file into the folder via
AtomicFile.write (open+write+fsync+rename+directory-fsync), then removes it;
any failure at write, fsync, or removal means false.

Three unit tests: an ordinary writable directory (true, and no probe file
left behind), a chmod 500 directory (false; permissions restored in
teardown), and a plain file path (false).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ZiTXPbPCSjzPVsfoweAbp
2026-09-05 09:54:58 +04:00
10 changed files with 423 additions and 31 deletions
+2 -2
View File
@@ -13,9 +13,9 @@
<key>CFBundlePackageType</key> <key>CFBundlePackageType</key>
<string>APPL</string> <string>APPL</string>
<key>CFBundleShortVersionString</key> <key>CFBundleShortVersionString</key>
<string>0.2.0</string> <string>0.3.0</string>
<key>CFBundleVersion</key> <key>CFBundleVersion</key>
<string>2</string> <string>3</string>
<key>LSMinimumSystemVersion</key> <key>LSMinimumSystemVersion</key>
<string>14.0</string> <string>14.0</string>
<key>LSUIElement</key> <key>LSUIElement</key>
+9
View File
@@ -27,5 +27,14 @@ let package = Package(
dependencies: ["ShotdeckCore"], dependencies: ["ShotdeckCore"],
swiftSettings: [.swiftLanguageMode(.v6)] 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)]
),
] ]
) )
+26 -8
View File
@@ -139,6 +139,14 @@ public final class AppModel {
/// lets the LAST choice win instead of applying stale, superseded work. Not /// lets the LAST choice win instead of applying stale, superseded work. Not
/// `@Observable`-relevant state pure internal bookkeeping, never read by a View. /// `@Observable`-relevant state pure internal bookkeeping, never read by a View.
var reconcileGeneration = 0 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<Void, Never>` never throws; awaiting an already-completed task's `.value`
/// returns immediately. Not `@Observable`-relevant pure internal bookkeeping.
var pendingReconcileTask: Task<Void, Never>?
func rememberLastComposedPDF(_ url: URL) { lastComposedPDFURL = url } func rememberLastComposedPDF(_ url: URL) { lastComposedPDFURL = url }
/// True when a last-composed PDF path is known this run, or the newest /// True when a last-composed PDF path is known this run, or the newest
@@ -241,19 +249,29 @@ public final class AppModel {
setStatus((error as? ShotdeckError)?.errorDescription ?? "Could not watch the return folder.") setStatus((error as? ShotdeckError)?.errorDescription ?? "Could not watch the return folder.")
} }
let pref = HotkeyPreference.load() // Env-var-flagged self-test/headless runs (PickerSelfTest's phases, PanelSnapshot,
captureHotkey = pref // and the new ShotdeckTests launch-wiring regression test) skip two real-world
if !bindCaptureHotkey(pref) { // side effects that are unsafe or meaningless in that context: the update-check
setStatus("\(pref.displayString) is already used by another app — capture only works from the menu.") // 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
let skipSchedule = // 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_PICKER_SELFTEST"] != nil
|| ProcessInfo.processInfo.environment["SHOTDECK_SNAPSHOT_DIR"] != nil || ProcessInfo.processInfo.environment["SHOTDECK_SNAPSHOT_DIR"] != nil
|| ProcessInfo.processInfo.environment["SHOTDECK_UPDATE_SELFTEST"] != nil || ProcessInfo.processInfo.environment["SHOTDECK_UPDATE_SELFTEST"] != nil
|| ProcessInfo.processInfo.environment["SHOTDECK_ONEDRIVE_SELFTEST"] != nil || ProcessInfo.processInfo.environment["SHOTDECK_ONEDRIVE_SELFTEST"] != nil
|| ProcessInfo.processInfo.environment["REDLINE_SELFTEST_PHASE"] != nil || ProcessInfo.processInfo.environment["REDLINE_SELFTEST_PHASE"] != nil
if !skipSchedule {
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() updateChecker.startSchedule()
} }
} }
+54 -1
View File
@@ -223,7 +223,7 @@ enum PickerSelfTest {
sendTruthFail("seeded session was empty") sendTruthFail("seeded session was empty")
} }
let pending = try await model.composePDFForSend(outbox: model.outboxURL) let pending = try await model.composePDFForSend(outbox: model.outboxURL, transport: .airDrop)
guard fm.fileExists(atPath: pending.fileURL.path) else { guard fm.fileExists(atPath: pending.fileURL.path) else {
sendTruthFail("PDF was not written") sendTruthFail("PDF was not written")
} }
@@ -543,6 +543,13 @@ enum PickerSelfTest {
/// does let the last choice win instead of an earlier, superseded call applying its /// does let the last choice win instead of an earlier, superseded call applying its
/// stale folder after a later one already won. /// 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 /// 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 /// 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, /// model the same way the real app launches (`AppDelegate.makeLaunchModel()` itself,
@@ -673,6 +680,52 @@ enum PickerSelfTest {
) )
} }
// 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. // Sub-step 2: relaunch simulation see the doc comment above this function.
let relaunchAppSupportRoot = fm.temporaryDirectory let relaunchAppSupportRoot = fm.temporaryDirectory
.appendingPathComponent("shotdeck-onedrive-relaunch-\(UUID().uuidString)", isDirectory: true) .appendingPathComponent("shotdeck-onedrive-relaunch-\(UUID().uuidString)", isDirectory: true)
+53 -18
View File
@@ -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)
+5 -2
View File
@@ -255,6 +255,9 @@ extension AppModel: SettingsWindowPresenting {
/// the live value before every mutating step, so rapid toggling (this function or /// 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 /// 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. /// 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) { func chooseTransport(_ value: SendTransport) {
guard !isSending else { guard !isSending else {
setStatus("Finish the current send first.") setStatus("Finish the current send first.")
@@ -274,7 +277,7 @@ extension AppModel: SettingsWindowPresenting {
reconcileGeneration += 1 reconcileGeneration += 1
let generation = reconcileGeneration let generation = reconcileGeneration
Task { pendingReconcileTask = Task {
guard generation == self.reconcileGeneration else { return } guard generation == self.reconcileGeneration else { return }
await watcher.setRecordUncommented(value == .airDrop) await watcher.setRecordUncommented(value == .airDrop)
guard generation == self.reconcileGeneration else { return } guard generation == self.reconcileGeneration else { return }
@@ -306,7 +309,7 @@ extension AppModel: SettingsWindowPresenting {
reconcileGeneration += 1 reconcileGeneration += 1
let generation = reconcileGeneration let generation = reconcileGeneration
Task { pendingReconcileTask = Task {
guard generation == self.reconcileGeneration else { return } guard generation == self.reconcileGeneration else { return }
do { do {
try await watcher.updateWatchFolder(url) try await watcher.updateWatchFolder(url)
@@ -17,6 +17,15 @@ public actor ReturnWatcher {
/// A document that IS commented is always recorded, regardless of this flag. /// A document that IS commented is always recorded, regardless of this flag.
public var recordUncommented: Bool = true 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 /// Watch folder is `paths.watchFolder`, which production constructs from
/// `FolderSettings.resolve().watch`. This type never calls FolderSettings; /// `FolderSettings.resolve().watch`. This type never calls FolderSettings;
/// `updateWatchFolder` is invoked by the UI layer only. /// `updateWatchFolder` is invoked by the UI layer only.
@@ -176,4 +176,46 @@ public enum OneDriveLocator {
guard exists, isDirectory.boolValue else { return false } guard exists, isDirectory.boolValue else { return false }
return fileManager.isWritableFile(atPath: url.path) 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)
}
} }
@@ -122,6 +122,87 @@ func isWritableDirectoryFalseForAPlainFileAndForANonexistentPath() throws {
#expect(!OneDriveLocator.isWritableDirectory(at: dir.appendingPathComponent("does-not-exist"))) #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 { struct TransportDefaultsSuite {
let name: String let name: String
let defaults: UserDefaults let defaults: UserDefaults
+142
View File
@@ -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()
}