Pre-push review: T-1849 security-scoped access lease

diff --git a/prism/Services/SecurityScopedResourceLease.swift b/prism/Services/SecurityScopedResourceLease.swiftnew file mode 100644index 00000000..69a48a59--- /dev/null+++ b/prism/Services/SecurityScopedResourceLease.swift@@ -0,0 +1,63 @@+//+//  SecurityScopedResourceLease.swift+//  prism+//++import Foundation++/// Abstraction over `URL`'s security-scoped resource access, decoupled from+/// `Foundation.URL` itself so the start/stop balancing in+/// `SecurityScopedResourceLease` is unit-testable without a real+/// security-scoped URL — the sandbox behaviour that grants one cannot be+/// exercised from a macOS unit-test host (T-1849). Production always passes+/// a real `URL`; tests inject a fake that records calls.+///+/// `nonisolated`, matching `URL`'s own methods: neither is bound to the+/// MainActor, so a lease can be started, stopped, and torn down (`deinit`)+/// without the actor-isolation ceremony a MainActor-isolated type would need+/// to release resources from its (always-nonisolated) `deinit`.+nonisolated protocol SecurityScopedResourceAccessing {+    func startAccessingSecurityScopedResource() -> Bool+    func stopAccessingSecurityScopedResource()+}++extension URL: SecurityScopedResourceAccessing {}++/// Balances one security-scoped resource's access across a lifetime longer+/// than a single read.+///+/// `URL.startAccessingSecurityScopedResource()` is reference-counted: a+/// caller that has already started its own access can start a second,+/// independent access on the same URL and the resource stays available+/// until BOTH are stopped. `DocumentSession` uses this to hold its own+/// lease on a file source for as long as the session lives — covering its+/// `FileChangeObserver` and any later `reloadContent(from:)` call — separate+/// from whatever transient scope the caller that produced the URL (the file+/// importer, or a resolved recent-file bookmark) held only around its own+/// initial read and released as soon as that read returned (T-1849).+///+/// Started once, at `init`; `isActive` records whether the start actually+/// succeeded, since a URL that was never security-scoped to begin with+/// (an app-sandbox file, or an unsandboxed macOS path) returns `false` and+/// needs no matching stop. `stop()` is idempotent so replacement, explicit+/// close, and `deinit` can all call it without double-releasing.+nonisolated final class SecurityScopedResourceLease {+    private let resource: SecurityScopedResourceAccessing+    private(set) var isActive: Bool++    init(resource: SecurityScopedResourceAccessing) {+        self.resource = resource+        self.isActive = resource.startAccessingSecurityScopedResource()+    }++    /// Releases this lease's share of the access, if it holds one.+    func stop() {+        guard isActive else { return }+        resource.stopAccessingSecurityScopedResource()+        isActive = false+    }++    deinit {+        stop()+    }+}

diff --git a/prism/Models/DocumentSession.swift b/prism/Models/DocumentSession.swiftindex e49218ff..da7a082f 100644--- a/prism/Models/DocumentSession.swift+++ b/prism/Models/DocumentSession.swift@@ -137,6 +137,30 @@ final class DocumentSession: Identifiable {     /// Requirement 2.4: No FileChangeObserver for clipboard-sourced content.     var fileObserver: FileChangeObserver? +    /// This session's own security-scoped access lease on its file source+    /// (nil for non-file sources).+    ///+    /// The caller that produced the URL — the file importer in+    /// `DocumentFlowCoordinator.openFile`, or a bookmark resolved by+    /// `RecentFileEntry.withResolvedURL` — only holds a transient scope+    /// around its own initial read and releases it as soon as that read+    /// returns. Without this, `fileObserver`'s external-change monitoring+    /// and any later `reloadContent(from:)` call ran with no security-scoped+    /// access at all, and could fail even though the initial open succeeded+    /// (T-1849). Starting a second, independent lease here — while the+    /// caller's own scope is still active — nests safely underneath it+    /// (`startAccessingSecurityScopedResource()` is reference-counted), so+    /// this session ends up owning access for as long as it lives, stopped+    /// via `SecurityScopedResourceLease.deinit` when the session itself+    /// deallocates (on close or replacement).+    ///+    /// `didSave(to:)` starts one too, for the same reason: a Save-As'd+    /// session runs well after the file exporter's own transient scope has+    /// ended, with no outer scope left to nest under (T-1849 through the+    /// clipboard-to-file transition). `revertToClipboard()` clears it back+    /// to nil.+    private var fileAccessLease: SecurityScopedResourceLease?+     // MARK: - Parse Generation Tracking (T-718)      /// Monotonic counter incremented at the start of each parse cycle.@@ -287,14 +311,40 @@ final class DocumentSession: Identifiable {     /// - Parameters:     ///   - url: The URL of the file.     ///   - content: The markdown content of the file.-    init(url: URL, content: String) {+    convenience init(url: URL, content: String) {+        self.init(url: url, content: content, fileAccessResource: url)+    }++    /// Designated file initializer, parameterized over the security-scoped+    /// resource so tests can substitute a fake in place of the real `URL`+    /// (see the `#if DEBUG` initializer below). Production always calls+    /// this via `init(url:content:)`, which passes `url` itself.+    private init(url: URL, content: String, fileAccessResource: SecurityScopedResourceAccessing) {         self.id = UUID()         self.source = .file(url: url)         self.content = content         self.fileObserver = FileChangeObserver(fileURL: url)+        self.fileAccessLease = SecurityScopedResourceLease(resource: fileAccessResource)         wireSearchClosures()     } +    #if DEBUG+    /// Test-only initializer that injects a fake security-scoped resource in+    /// place of the real `URL`, so `fileAccessLease`'s start/stop balancing+    /// is unit-testable without a real sandboxed URL — the sandbox behaviour+    /// that grants one cannot be exercised from a macOS unit-test host+    /// (T-1849). Stripped from release builds.+    convenience init(url: URL, content: String, fileAccessResourceForTests resource: SecurityScopedResourceAccessing) {+        self.init(url: url, content: content, fileAccessResource: resource)+    }++    /// Test-only view of whether this session still holds its own+    /// security-scoped access lease. Stripped from release builds.+    var hasActiveFileAccessLeaseForTests: Bool {+        fileAccessLease?.isActive ?? false+    }+    #endif+     /// Creates a session from clipboard content.     ///     /// Initializes with a clipboard source. No FileChangeObserver is created@@ -306,6 +356,7 @@ final class DocumentSession: Identifiable {         self.source = .clipboard         self.content = clipboardContent         self.fileObserver = nil+        self.fileAccessLease = nil         wireSearchClosures()     } @@ -322,6 +373,7 @@ final class DocumentSession: Identifiable {         self.source = .bundled(name: name)         self.content = content         self.fileObserver = nil+        self.fileAccessLease = nil         wireSearchClosures()     } @@ -337,6 +389,7 @@ final class DocumentSession: Identifiable {         self.content = persisted.content         self.scrollPositionID = persisted.scrollPositionID         self.fileObserver = nil+        self.fileAccessLease = nil         wireSearchClosures()     } @@ -353,6 +406,7 @@ final class DocumentSession: Identifiable {         self.source = .url(remote: remoteURL, display: displayURL)         self.content = content         self.fileObserver = nil+        self.fileAccessLease = nil         wireSearchClosures()     } @@ -491,8 +545,33 @@ final class DocumentSession: Identifiable {     ///     /// - Parameter url: The URL where the file was saved.     func didSave(to url: URL) {+        didSave(to: url, fileAccessResource: url)+    }++    #if DEBUG+    /// Test-only variant of `didSave(to:)` that injects a fake+    /// security-scoped resource in place of the real `URL`, mirroring+    /// `init(url:content:fileAccessResourceForTests:)`. Stripped from+    /// release builds.+    func didSave(to url: URL, fileAccessResourceForTests resource: SecurityScopedResourceAccessing) {+        didSave(to: url, fileAccessResource: resource)+    }+    #endif++    /// Shared implementation behind `didSave(to:)`, parameterized over the+    /// security-scoped resource so tests can substitute a fake (mirroring+    /// the designated file initializer above).+    ///+    /// A save-as from a clipboard session has no lease to replace+    /// (`fileAccessLease` is nil until this point). A re-save of an+    /// already-file-backed session (e.g. re-exporting to a new location)+    /// does: assigning the fresh lease here releases the old one first, via+    /// `SecurityScopedResourceLease.deinit`, once this is the only strong+    /// reference to it.+    private func didSave(to url: URL, fileAccessResource: SecurityScopedResourceAccessing) {         source = .file(url: url)         fileObserver = FileChangeObserver(fileURL: url)+        fileAccessLease = SecurityScopedResourceLease(resource: fileAccessResource)         pendingSave = nil     } @@ -504,6 +583,7 @@ final class DocumentSession: Identifiable {     func revertToClipboard() {         source = .clipboard         fileObserver = nil+        fileAccessLease = nil         pendingSave = nil     } 

diff --git a/prismTests/SecurityScopedResourceLeaseTests.swift b/prismTests/SecurityScopedResourceLeaseTests.swiftnew file mode 100644index 00000000..102339e8--- /dev/null+++ b/prismTests/SecurityScopedResourceLeaseTests.swift@@ -0,0 +1,255 @@+//+//  SecurityScopedResourceLeaseTests.swift+//  prismTests+//+//  Regression coverage for T-1849: a file reload run after the URL's+//  initial security-scoped access grant had already been released (by+//  DocumentFlowCoordinator.openFile or RecentFileEntry.withResolvedURL, both+//  of which stop their own scope as soon as their initial read returns).+//+//  DocumentSession now starts its own, independent security-scoped access+//  lease for the whole session lifetime, covering both its FileChangeObserver+//  and any later reloadContent(from:) call. The sandbox behaviour that grants+//  real security-scoped access cannot be exercised from a macOS unit-test+//  host, so these tests exercise the lease-balancing logic itself (and its+//  DocumentSession wiring) through the SecurityScopedResourceAccessing seam,+//  with a fake resource standing in for the real URL.+//++import Foundation+import Testing+@testable import prism++/// Records start/stop calls in place of a real security-scoped `URL`.+private final class MockSecurityScopedResource: SecurityScopedResourceAccessing {+    var startCallCount = 0+    var stopCallCount = 0+    var startReturnValue: Bool++    init(startReturnValue: Bool = true) {+        self.startReturnValue = startReturnValue+    }++    func startAccessingSecurityScopedResource() -> Bool {+        startCallCount += 1+        return startReturnValue+    }++    func stopAccessingSecurityScopedResource() {+        stopCallCount += 1+    }+}++// MARK: - SecurityScopedResourceLease++@Suite("SecurityScopedResourceLease")+struct SecurityScopedResourceLeaseTests {++    @Test("init starts access and records it active when the resource grants it")+    func initStartsAccess() {+        let resource = MockSecurityScopedResource(startReturnValue: true)+        let lease = SecurityScopedResourceLease(resource: resource)++        #expect(resource.startCallCount == 1)+        #expect(lease.isActive)+    }++    @Test("init records no active lease when the resource was never security-scoped")+    func initWithoutGrantIsInactive() {+        let resource = MockSecurityScopedResource(startReturnValue: false)+        let lease = SecurityScopedResourceLease(resource: resource)++        #expect(resource.startCallCount == 1)+        #expect(!lease.isActive)+    }++    @Test("stop releases an active lease exactly once")+    func stopReleasesActiveLease() {+        let resource = MockSecurityScopedResource(startReturnValue: true)+        let lease = SecurityScopedResourceLease(resource: resource)++        lease.stop()++        #expect(resource.stopCallCount == 1)+        #expect(!lease.isActive)+    }++    @Test("stop is idempotent and never double-releases")+    func stopIsIdempotent() {+        let resource = MockSecurityScopedResource(startReturnValue: true)+        let lease = SecurityScopedResourceLease(resource: resource)++        lease.stop()+        lease.stop()+        lease.stop()++        #expect(resource.stopCallCount == 1)+    }++    @Test("stop on a lease that never activated does not call the underlying stop")+    func stopWithoutActiveGrantIsNoOp() {+        let resource = MockSecurityScopedResource(startReturnValue: false)+        let lease = SecurityScopedResourceLease(resource: resource)++        lease.stop()++        #expect(resource.stopCallCount == 0)+    }++    @Test("deinit releases an active lease")+    func deinitReleasesActiveLease() {+        let resource = MockSecurityScopedResource(startReturnValue: true)+        var lease: SecurityScopedResourceLease? = SecurityScopedResourceLease(resource: resource)+        _ = lease // silence "never used" before the reassignment below++        lease = nil++        #expect(resource.stopCallCount == 1)+    }++    @Test("deinit on a never-activated lease does not call the underlying stop")+    func deinitWithoutActiveGrantIsNoOp() {+        let resource = MockSecurityScopedResource(startReturnValue: false)+        var lease: SecurityScopedResourceLease? = SecurityScopedResourceLease(resource: resource)+        _ = lease++        lease = nil++        #expect(resource.stopCallCount == 0)+    }+}++// MARK: - DocumentSession wiring++@Suite("DocumentSession file access lease")+struct DocumentSessionFileAccessLeaseTests {++    @Test("Opening a file session starts its own security-scoped access lease")+    @MainActor+    func openingFileSessionStartsLease() {+        let resource = MockSecurityScopedResource(startReturnValue: true)+        let url = URL(fileURLWithPath: "/tmp/T-1849-test.md")++        let session = DocumentSession(url: url, content: "# Test", fileAccessResourceForTests: resource)++        #expect(resource.startCallCount == 1)+        #expect(session.hasActiveFileAccessLeaseForTests)+    }++    @Test("A file session's lease is not reacquired across a reload")+    @MainActor+    func leaseIsHeldNotReacquiredOnReload() async throws {+        let tempURL = FileManager.default.temporaryDirectory+            .appendingPathComponent("T-1849-reload-\(UUID().uuidString).md")+        try "# Original".write(to: tempURL, atomically: true, encoding: .utf8)+        defer { try? FileManager.default.removeItem(at: tempURL) }++        let resource = MockSecurityScopedResource(startReturnValue: true)+        let session = DocumentSession(url: tempURL, content: "# Original", fileAccessResourceForTests: resource)+        #expect(resource.startCallCount == 1)++        // Simulate the file having changed externally (FileChangeObserver's+        // whole reason for existing) and reload — the scenario T-1849+        // reported failing once the caller's transient access had ended.+        try "# Updated".write(to: tempURL, atomically: true, encoding: .utf8)+        try await session.reloadContent(from: tempURL)++        // The reload must not need (and must not trigger) a fresh start —+        // the session's lease from init is still covering it.+        #expect(resource.startCallCount == 1)+        #expect(session.hasActiveFileAccessLeaseForTests)+        #expect(session.documentTitle == "Updated")+    }++    @Test("A file session releases its lease when deallocated")+    @MainActor+    func sessionReleasesLeaseOnDeinit() {+        let resource = MockSecurityScopedResource(startReturnValue: true)+        let url = URL(fileURLWithPath: "/tmp/T-1849-deinit-test.md")++        var session: DocumentSession? = DocumentSession(+            url: url, content: "# Test", fileAccessResourceForTests: resource+        )+        _ = session++        session = nil++        #expect(resource.stopCallCount == 1)+    }++    @Test("Non-file sessions never touch security-scoped access")+    @MainActor+    func nonFileSessionsHaveNoLease() {+        let clipboardSession = DocumentSession(clipboardContent: "# Clipboard")+        #expect(clipboardSession.hasActiveFileAccessLeaseForTests == false)++        let bundledSession = DocumentSession(bundledResource: "onboarding", content: "# Bundled")+        #expect(bundledSession.hasActiveFileAccessLeaseForTests == false)+    }++    // MARK: - Save As (clipboard → file) transition (T-1849 follow-up)++    @Test("didSave(to:) starts a security-scoped access lease for a clipboard session becoming file-backed")+    @MainActor+    func didSaveStartsLease() {+        let clipboardSession = DocumentSession(clipboardContent: "# Clipboard")+        #expect(clipboardSession.hasActiveFileAccessLeaseForTests == false)++        let resource = MockSecurityScopedResource(startReturnValue: true)+        let savedURL = URL(fileURLWithPath: "/tmp/T-1849-saveas-test.md")+        clipboardSession.didSave(to: savedURL, fileAccessResourceForTests: resource)++        #expect(resource.startCallCount == 1)+        #expect(clipboardSession.hasActiveFileAccessLeaseForTests)+    }++    @Test("revertToClipboard releases the lease didSave(to:) started")+    @MainActor+    func revertToClipboardReleasesLease() {+        let clipboardSession = DocumentSession(clipboardContent: "# Clipboard")+        let resource = MockSecurityScopedResource(startReturnValue: true)+        let savedURL = URL(fileURLWithPath: "/tmp/T-1849-revert-test.md")+        clipboardSession.didSave(to: savedURL, fileAccessResourceForTests: resource)+        #expect(clipboardSession.hasActiveFileAccessLeaseForTests)++        clipboardSession.revertToClipboard()++        #expect(resource.stopCallCount == 1)+        #expect(clipboardSession.hasActiveFileAccessLeaseForTests == false)+    }++    @Test("didSave(to:) tolerates a resource that declines the grant and never stops it")+    @MainActor+    func didSaveToleratesDeclinedGrant() {+        let clipboardSession = DocumentSession(clipboardContent: "# Clipboard")+        let resource = MockSecurityScopedResource(startReturnValue: false)+        let savedURL = URL(fileURLWithPath: "/tmp/T-1849-declined-test.md")+        clipboardSession.didSave(to: savedURL, fileAccessResourceForTests: resource)++        #expect(resource.startCallCount == 1)+        #expect(clipboardSession.hasActiveFileAccessLeaseForTests == false)++        clipboardSession.revertToClipboard()++        // isActive was already false, so releasing must not call the+        // underlying stop — there is nothing to balance.+        #expect(resource.stopCallCount == 0)+    }++    @Test("Construction tolerates a resource that declines the grant and never stops it")+    @MainActor+    func constructionToleratesDeclinedGrant() {+        let resource = MockSecurityScopedResource(startReturnValue: false)+        let url = URL(fileURLWithPath: "/tmp/T-1849-declined-init-test.md")++        var session: DocumentSession? = DocumentSession(+            url: url, content: "# Test", fileAccessResourceForTests: resource+        )+        #expect(resource.startCallCount == 1)+        #expect(session?.hasActiveFileAccessLeaseForTests == false)++        session = nil++        #expect(resource.stopCallCount == 0)+    }+}

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex bb517458..1eafcb70 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0  ### Fixed +- A provider-backed document's reload (external-change banner or a later re-read) could fail even though the initial open succeeded, because security-scoped access was released as soon as the open finished; the document session now holds its own access lease for as long as it stays open, including after a Save As turns a pasted document into a file-backed one (T-1849). - An image that is tiny as a file but enormous as a picture can no longer exhaust memory or terminate Prism, whether it comes from the web, from a file beside the document, or written directly into the markdown (T-2132, T-2149, T-2151, T-1867). A picture is stored compressed, and a plain-coloured one compresses at about a thousand to one — so a 400 KB download can be a 20,000 by 20,000 image that needs about 400 MB the moment anything tries to display it, and four times that if it is in colour. Prism's limits were all written on the wrong side of that: a 50 MB cap on the download said nothing about the picture inside it, and the 2 MB cap on a local SVG was applied only after the whole file had already been read, so a very large one could freeze the app on its way to being refused. The limits that did exist covered only images fetched from the web; the same image referenced from a file next to your document, or embedded inline in the markdown, went straight to the renderer unchecked. Prism now reads the picture's dimensions from its header — a few bytes, before anything is decoded — and decides from that. An ordinary image is displayed as before. A very large one referenced from the web or from a file is scaled down to fit. One beyond any reasonable size is refused outright and shows the usual "Image failed to load" placeholder, rather than being handed to a decoder that would have to build the whole thing first. How large a picture is now also accounts for how much detail each dot of it carries: most pictures store one byte per colour, but some store two or four, and Prism previously assumed the smaller size for all of them and so under-counted the deep ones by half or three quarters. One consequence you may see: a very large deep-colour photograph that used to display at full size is now scaled down, because its true size was always above the limit and is now measured as such. Files are now read up to their limit instead of read whole and then measured — including the copy Prism keeps of a document you have not saved yet, which is restored when the app reopens. How much decoding happens at once is limited by how much memory those pictures actually need rather than by how many of them there are, so a page full of large images no longer overruns while appearing to stay within its bounds. Two things behave differently, both deliberately. An image whose file does not say how big it is, or what kind of dots it stores, now shows the "Image failed to load" placeholder instead of being displayed — there is no way to know what it would cost until it has already cost it. And an image written directly into the markdown is treated more strictly than the same image kept in a file beside the document: it is either small enough to display as it is or refused, never scaled down. That difference is about memory rather than effort. Scaling a picture that is written into the markdown means rebuilding it and writing the smaller version back into the page, where it then stays for as long as the document is open — which costs more memory, for longer, than not showing it. A picture in a file has somewhere else to keep its smaller version, so it can be scaled instead of refused. Animated images are unaffected in either case: they play as before, however many frames they have. - A verification scan that starts during the app's initial entitlement bootstrap can no longer publish a stale result while a newer scan is still in flight (T-2152). While `entitlementState` was still `.loading`, any scan's result was accepted regardless of whether a more recent scan — for example one started right after `AppStore.sync()` — was still reading the world; the older scan finishing first could briefly flip the paywall to locked (or unlocked) ahead of the newer, more current answer. An older result that arrives while a newer scan is still outstanding is now held back rather than published. If the newer scan goes on to answer, its fresher result is published and the held-back one is simply dropped; if instead it is cancelled without ever answering, the held-back result is released, so a cancelled scan cannot leave the paywall stranded on `.loading`. The trade is that the brief loading state now ends when the last overlapping scan answers rather than the first, so it can last marginally longer; every control it gates is disabled meanwhile, so nothing silently does nothing. - Saving a pasted document to a file no longer disturbs whatever document you opened next (T-2213). A save finishes in two parts: the file is written straight away, but the document only becomes that file once its notes have been moved across, and on a slow iCloud connection that second part can still be running after you have closed the document or opened another one. When it finished late, it acted on the document then on screen instead of the one it had saved: the pasted text of that other document was deleted from the place Prism keeps unsaved documents — so it could no longer be recovered after a relaunch — its entry in Recent Files was labelled with the wrong document's title, and an action you had queued behind its own Save prompt could run without you confirming it. A save that failed to move its notes also raised an alert naming a file you were no longer looking at. Each of these now belongs to the document that was actually saved, and the document on screen is left alone. Its Recent Files entry is labelled with its own title rather than the other document's. Where that other document had itself started saving in the meantime, the late save no longer takes over the shortcut that document had prepared for its own file, which can leave the saved file without a Recent Files entry of its own. The file is saved either way, and can be opened from the Files app.