Pre-push review: fix-flaky-macos-tests

diff --git a/prism/Services/DiagramWindowManager.swift b/prism/Services/DiagramWindowManager.swiftindex a721802..ed76273 100644--- a/prism/Services/DiagramWindowManager.swift+++ b/prism/Services/DiagramWindowManager.swift@@ -38,46 +38,53 @@ final class DiagramWindowManager {     /// Position of the most recently opened diagram window (for cascading).     private var lastWindowOrigin: CGPoint? -    /// Observer for window close notifications to auto-cleanup.-    /// Marked `nonisolated(unsafe)` so deinit can remove the observer.-    /// Access is safe: written once in init (MainActor), read once in deinit-    /// (nonisolated), and these are serialized by object lifetime.+    /// Per-window close observers keyed by identifier. Each observer is scoped+    /// to a specific NSWindow (`object: window`) so notifications from+    /// unrelated windows (in other tests, other documents) never reach this+    /// manager. Marked `nonisolated(unsafe)` so `deinit` can drain it.+    /// Lifetime-serialised: writes happen on MainActor in+    /// `registerWindow`/`unregisterWindow`; the only nonisolated read is in+    /// `deinit` after no other reference to `self` remains.     @ObservationIgnored-    private var closeObserver: NSObjectProtocol?+    nonisolated(unsafe) private var closeObservers: [DiagramIdentifier: NSObjectProtocol] = [:]      /// Cascade offset for new windows (20 points per requirement 4.7).     private let cascadeOffset: CGFloat = 20 -    init() {-        setupWindowCloseObserver()-    }+    init() {}      deinit {-        if let observer = closeObserver {-            NotificationCenter.default.removeObserver(observer)+        let notificationCenter = NotificationCenter.default+        for observer in closeObservers.values {+            notificationCenter.removeObserver(observer)         }+        closeObservers.removeAll()     }      // MARK: - Window Close Observer -    /// Sets up observation of window close notifications for auto-cleanup.-    private func setupWindowCloseObserver() {-        closeObserver = NotificationCenter.default.addObserver(+    /// Installs a close observer scoped to `window`. Called from+    /// `registerWindow`; the observer is removed in `unregisterWindow` (or+    /// `deinit`) so it can never outlive the manager or fire for a window the+    /// manager no longer tracks. (T-583)+    private func installCloseObserver(for window: NSWindow, identifier: DiagramIdentifier) {+        let observer = NotificationCenter.default.addObserver(             forName: NSWindow.willCloseNotification,-            object: nil,+            object: window,             queue: .main-        ) { [weak self] notification in-            guard let window = notification.object as? NSWindow else { return }+        ) { [weak self] _ in             Task { @MainActor [weak self] in-                self?.handleWindowClose(window)+                self?.unregisterWindow(for: identifier)             }         }+        closeObservers[identifier] = observer     } -    /// Handles window close notification by finding and unregistering the window.-    private func handleWindowClose(_ window: NSWindow) {-        guard let identifier = openWindows.first(where: { $0.value === window })?.key else { return }-        unregisterWindow(for: identifier)+    /// Removes the close observer for an identifier, if any.+    private func removeCloseObserver(for identifier: DiagramIdentifier) {+        if let observer = closeObservers.removeValue(forKey: identifier) {+            NotificationCenter.default.removeObserver(observer)+        }     }      // MARK: - Pending Opens@@ -118,12 +125,14 @@ final class DiagramWindowManager {         windowOwnership[identifier] = ownerURL         lastWindowOrigin = window.frame.origin         clearPendingOpen(identifier)+        installCloseObserver(for: window, identifier: identifier)     }      /// Unregisters a diagram window when closed.     ///     /// - Parameter identifier: The diagram identifier to unregister.     func unregisterWindow(for identifier: DiagramIdentifier) {+        removeCloseObserver(for: identifier)         openWindows.removeValue(forKey: identifier)         windowOwnership.removeValue(forKey: identifier)         pendingOpens.remove(identifier)@@ -259,6 +268,11 @@ final class DiagramWindowManager {     ///     /// For testing purposes only. Does not close actual windows.     func clearAllTracking() {+        let notificationCenter = NotificationCenter.default+        for observer in closeObservers.values {+            notificationCenter.removeObserver(observer)+        }+        closeObservers.removeAll()         openWindows.removeAll()         windowOwnership.removeAll()         pendingOpens.removeAll()

diff --git a/prism/Services/ImageDetailWindowManager.swift b/prism/Services/ImageDetailWindowManager.swiftindex 0c35d50..b0df879 100644--- a/prism/Services/ImageDetailWindowManager.swift+++ b/prism/Services/ImageDetailWindowManager.swift@@ -29,40 +29,45 @@ final class ImageDetailWindowManager {     /// Identifiers of windows currently being opened (prevents duplicate opens on rapid taps).     private(set) var pendingOpens: Set<ImageDetailIdentifier> = [] -    /// Observer for window close notifications to auto-cleanup.+    /// Per-window close observers keyed by identifier. Scoping the observer+    /// to a specific NSWindow (`object: window`) prevents cross-instance+    /// contamination — notifications from windows owned by another manager+    /// (in another test, another document) never reach this manager.     @ObservationIgnored-    private var closeObserver: NSObjectProtocol?+    nonisolated(unsafe) private var closeObservers: [ImageDetailIdentifier: NSObjectProtocol] = [:] -    init() {-        setupWindowCloseObserver()-    }+    init() {}      deinit {-        if let observer = closeObserver {-            NotificationCenter.default.removeObserver(observer)+        let notificationCenter = NotificationCenter.default+        for observer in closeObservers.values {+            notificationCenter.removeObserver(observer)         }+        closeObservers.removeAll()     }      // MARK: - Window Close Observer -    /// Sets up observation of window close notifications for auto-cleanup.-    private func setupWindowCloseObserver() {-        closeObserver = NotificationCenter.default.addObserver(+    /// Installs a close observer scoped to `window`. Removed in+    /// `unregisterWindow` (or `deinit`) so it never outlives the registration.+    private func installCloseObserver(for window: NSWindow, identifier: ImageDetailIdentifier) {+        let observer = NotificationCenter.default.addObserver(             forName: NSWindow.willCloseNotification,-            object: nil,+            object: window,             queue: .main-        ) { [weak self] notification in-            guard let window = notification.object as? NSWindow else { return }+        ) { [weak self] _ in             Task { @MainActor [weak self] in-                self?.handleWindowClose(window)+                self?.unregisterWindow(for: identifier)             }         }+        closeObservers[identifier] = observer     } -    /// Handles window close notification by finding and unregistering the window.-    private func handleWindowClose(_ window: NSWindow) {-        guard let identifier = openWindows.first(where: { $0.value === window })?.key else { return }-        unregisterWindow(for: identifier)+    /// Removes the close observer for an identifier, if any.+    private func removeCloseObserver(for identifier: ImageDetailIdentifier) {+        if let observer = closeObservers.removeValue(forKey: identifier) {+            NotificationCenter.default.removeObserver(observer)+        }     }      // MARK: - Pending Opens@@ -93,10 +98,12 @@ final class ImageDetailWindowManager {         openWindows[identifier] = window         windowOwnership[identifier] = ownerURL         clearPendingOpen(identifier)+        installCloseObserver(for: window, identifier: identifier)     }      /// Unregisters an image window when closed.     func unregisterWindow(for identifier: ImageDetailIdentifier) {+        removeCloseObserver(for: identifier)         openWindows.removeValue(forKey: identifier)         windowOwnership.removeValue(forKey: identifier)         pendingOpens.remove(identifier)@@ -158,6 +165,11 @@ final class ImageDetailWindowManager {      /// Clears all tracked windows without closing them.     func clearAllTracking() {+        let notificationCenter = NotificationCenter.default+        for observer in closeObservers.values {+            notificationCenter.removeObserver(observer)+        }+        closeObservers.removeAll()         openWindows.removeAll()         windowOwnership.removeAll()         pendingOpens.removeAll()

diff --git a/prism/Services/DocumentIdentifierResolver.swift b/prism/Services/DocumentIdentifierResolver.swiftindex 3c6a209..f63bf9b 100644--- a/prism/Services/DocumentIdentifierResolver.swift+++ b/prism/Services/DocumentIdentifierResolver.swift@@ -96,8 +96,9 @@ struct DocumentIdentifierResolver {         // section of Decision 10 in specs/open-from-url/decision_log.md.         let host = url.host.flatMap({ host in url.port.map { "\(host):\($0)" } ?? host }) ?? "unknown"         // Strip only the leading slash so trailing-slash distinctions are-        // preserved (e.g. `/docs` vs `/docs/`).-        let rawPath = url.path+        // preserved (e.g. `/docs` vs `/docs/`). `URL.path` collapses a+        // trailing slash; `URLComponents.path` preserves it verbatim.+        let rawPath = URLComponents(url: url, resolvingAgainstBaseURL: false)?.path ?? url.path         let path = rawPath.hasPrefix("/") ? String(rawPath.dropFirst()) : rawPath         // Fragments are intentionally excluded from the identifier — they are         // client-side navigation anchors, not part of resource identity.

diff --git a/prism/Services/SearchCoordinator.swift b/prism/Services/SearchCoordinator.swiftindex 15c5ec6..1158ea9 100644--- a/prism/Services/SearchCoordinator.swift+++ b/prism/Services/SearchCoordinator.swift@@ -246,15 +246,6 @@ final class SearchCoordinator: SearchActions {     func recomputeMatchCounts() {         let blocks = blockProvider() -        #if DEBUG-        // blockProvider should return non-empty once DocumentSession.parseContent() completes.-        // An empty result here when the query is active signals a wiring issue.-        assert(-            activeSearchQuery.isEmpty || !blocks.isEmpty,-            "SearchCoordinator: blockProvider returned [] with an active query — was it wired post-init?"-        )-        #endif-         guard activeSearchQuery.count >= SearchService.minimumQueryLength else {             matchCountsPerBlock = []             currentGlobalMatchIndex = nil

diff --git a/prismTests/DiagramWindowManagerTests.swift b/prismTests/DiagramWindowManagerTests.swiftindex 3009134..ddd25c1 100644--- a/prismTests/DiagramWindowManagerTests.swift+++ b/prismTests/DiagramWindowManagerTests.swift@@ -32,6 +32,10 @@ struct DiagramWindowManagerTests {     }      /// Creates a mock NSWindow for testing.+    ///+    /// `isReleasedWhenClosed = false` is critical: NSWindow's default is to+    /// auto-release itself on `close()`, which over-releases the local test+    /// reference and faults `objc_release` when the test scope unwinds.     private func makeMockWindow() -> NSWindow {         let window = NSWindow(             contentRect: CGRect(x: 100, y: 100, width: 600, height: 450),@@ -39,6 +43,7 @@ struct DiagramWindowManagerTests {             backing: .buffered,             defer: true         )+        window.isReleasedWhenClosed = false         return window     } @@ -867,6 +872,7 @@ struct DiagramWindowManagerTests {             backing: .buffered,             defer: true         )+        sentinel.isReleasedWhenClosed = false          // Create a manager, register a window, then let it deallocate.         // We add a secondary observer to count callbacks; the manager's own

diff --git a/Makefile b/Makefileindex d933ce7..b87e8e6 100644--- a/Makefile+++ b/Makefile@@ -92,6 +92,10 @@ build: build-ios build-macos  # Testing .PHONY: test-quick+# Single locale config + worker count 1 to avoid cross-process races in shared+# global state (e.g. MockURLProtocol.handler, NSWindow notifications) that+# surface when xcodebuild runs the locale matrix in parallel workers. Run+# `make test-locales` to exercise the en-AU/en-GB/en-US configurations. test-quick: 	xcodebuild test \ 		-project $(PROJECT) \@@ -99,6 +103,9 @@ test-quick: 		-destination 'platform=macOS' \ 		-configuration Debug \ 		-derivedDataPath $(DERIVED_DATA) \+		-testPlan prism \+		-only-test-configuration "en (base)" \+		-parallel-testing-worker-count 1 \ 		-only-testing:prismTests \ 		$(PIPE_PRETTY) 

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex fe7668c..d0bd3d8 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0  ### Fixed +- macOS unit-test runs no longer crash the xctest runner with a SIGSEGV that masqueraded as ~11,000 "signal trap" collateral failures. Five root causes addressed: (1) `DiagramWindowManager`/`ImageDetailWindowManager` replaced the one global `NSWindow.willCloseNotification` observer (`object: nil`) with per-window observers (`object: window`) installed at `registerWindow` and removed at `unregisterWindow`/`deinit`, so notifications from other documents/tests never reach an unrelated manager; (2) `DocumentIdentifierResolver.resolve(forRemoteURL:)` now derives the path from `URLComponents.path` instead of `URL.path` — the latter silently collapses a trailing slash, causing `https://example.com/docs/` and `https://example.com/docs` to share one identifier; (3) `SearchCoordinator.recomputeMatchCounts()` no longer fires a `#if DEBUG assert(blocks not empty)` that crashed legitimate test scenarios where a search query is set before the document's blocks have been parsed (the guard below already handles empty blocks gracefully); (4) `DiagramWindowManagerTests.makeMockWindow()` and the deinit-observer-test sentinel set `isReleasedWhenClosed = false` because NSWindow auto-releases on `close()` by default, leaving the test's local `let window` as a dangling pointer that faulted `objc_release` when the test scope unwound; (5) `make test-quick` now passes `-testPlan prism -only-test-configuration "en (base)" -parallel-testing-worker-count 1` so the macOS unit run no longer fans out four parallel workers fighting for shared global state (`MockURLProtocol.handler`, `NotificationCenter`, `NSApp.windows`). Locale-matrix coverage is still available via `make test-locales`. - Prism no longer hangs intermittently while scrolling continuously through a document (T-1289). The `.onScrollGeometryChange` action handlers introduced in PR #252 wrote `scrollPercentage`, `pendingRestorePercentage`, and the raw-source `scrollOffset`/`contentHeight` on every scroll frame, but no SwiftUI view reactively reads any of them — they're only sampled at user-action time. More importantly, `KeyboardScrollController.contentHeight` was observed by `DocumentReaderView.body` via `makeDocumentActions` reading `active.canScroll`, so every `LazyVStack` block realisation (each of which grew `contentSize.height` and triggered the inequality-guarded `contentHeight` write) cascaded into a full `DocumentReaderView` body re-evaluation. Two-layer fix: mark the four "no body reader" properties `@ObservationIgnored`, and refactor `KeyboardScrollController` so `contentHeight` is computed over an `@ObservationIgnored` backing store while `canScroll` is a stored observable Bool maintained by the setter — `DocumentReaderView.body` now invalidates only on the two `canScroll` transitions per session, not per realisation. `prismTests/ScrollPositionObservationTests.swift` locks both contracts in with `withObservationTracking`-based assertions so a future refactor can't silently re-introduce the regression. - Switching between rendered and raw markdown views no longer resets the scroll position to the top. The scroll percentage is now snapshotted at toggle time and restored on the receiving view (best-effort; raw lines and rendered blocks aren't 1:1 but the position is approximately preserved in both directions). Same mechanism powers cross-session restore on document re-open. - Localisation build phase produced 298 spurious "declared in catalog but no source reference found" warnings on every macOS build. Two root causes: the script was missing `--scan-roots`, so it ran with the default relative path that didn't resolve under Xcode's build phase CWD; and `ENABLE_USER_SCRIPT_SANDBOXING = YES` granted only `literal` (non-recursive) access to declared input directories, blocking the `*.swift` walk. The build phase now passes `--scan-roots "$SRCROOT/prism"` and target sandboxing is off (the two build scripts only read project sources and write to `$DERIVED_FILE_DIR`).