Pre-push review: T-1698 auto light/dark switch

diff --git a/prism/Theme/SystemColorSchemeObserver.swift b/prism/Theme/SystemColorSchemeObserver.swiftindex 67b92e5..570c9cc 100644--- a/prism/Theme/SystemColorSchemeObserver.swift+++ b/prism/Theme/SystemColorSchemeObserver.swift@@ -12,11 +12,17 @@ import AppKit /// environment value. By reading the system appearance directly, we get the /// correct fallback for auto mode. ///-/// On macOS: reads `NSApp.effectiveAppearance` and listens for the+/// On macOS: reads the `AppleInterfaceStyle` user default and listens for the /// `AppleInterfaceThemeChangedNotification` distributed notification. ///-/// On iOS: reads `UITraitCollection.current` and listens for-/// `UIScreen.modeDidChangeNotification` and trait collection changes.+/// On iOS: reads `UITraitCollection.current` and registers for+/// `UITraitUserInterfaceStyle` trait changes on the app's window (T-1698).+///+/// The iOS registration deliberately targets the `UIWindow`: SwiftUI applies+/// `ThemeModifier`'s `.preferredColorScheme` to the root `UIHostingController`,+/// not to the window, so the window's own `userInterfaceStyle` keeps tracking+/// the real system appearance and this observer never sees Prism's own+/// appearance-mode changes reflected back at it. @MainActor @Observable final class SystemColorSchemeObserver {@@ -27,6 +33,35 @@ final class SystemColorSchemeObserver {     @ObservationIgnored     private var observer: Any? +    #if os(iOS)+    /// The window the interface-style trait registration is currently+    /// attached to, and the token identifying that registration.+    ///+    /// This deliberately tracks *which* window holds the registration rather+    /// than a "registration ever succeeded" flag. Prism opens a+    /// `WindowGroup` scene per document, so several `UIWindow`s coexist on+    /// iPadOS; a trait registration dies with the specific window it was+    /// made on. A boolean latch would stay true after that window closed and+    /// permanently no-op the re-arm below — silently reinstating the+    /// "requires a restart" bug this observer exists to fix. The reference is+    /// weak so it self-clears when the window is deallocated.+    @ObservationIgnored+    private weak var registeredWindow: UIWindow?++    @ObservationIgnored+    private var interfaceStyleRegistration: UITraitChangeRegistration?++    /// Notification observer tokens that drive (re-)registration.+    ///+    /// These are held for this observer's whole lifetime, not torn down after+    /// the first successful registration: they are the re-arm mechanism, not+    /// a one-shot retry. Registration must follow whichever window is current+    /// as document windows open and close, so the observer has to keep+    /// listening indefinitely.+    @ObservationIgnored+    private var windowObservers: [any NSObjectProtocol] = []+    #endif+     init() {         self.systemColorScheme = Self.readCurrentSystemScheme() @@ -47,15 +82,35 @@ final class SystemColorSchemeObserver {             }         }         #else-        // On iOS, listen for trait collection changes via UIScreen notifications.-        observer = NotificationCenter.default.addObserver(-            forName: UIScreen.modeDidChangeNotification,-            object: nil,-            queue: .main-        ) { [weak self] _ in-            Task { @MainActor [weak self] in-                self?.refresh()-            }+        // On iOS, register for `UITraitUserInterfaceStyle` changes on the+        // app's window via the modern trait-change API (iOS 17+).+        //+        // T-1698: this previously listened for `UIScreen.modeDidChangeNotification`,+        // which fires for screen resolution/mode changes — NOT Dark Mode toggles.+        // That notification never fired on an automatic system appearance switch,+        // so `refresh()` never ran and only an app restart (which re-reads the+        // system scheme in `init()`) picked up the change.+        registerForInterfaceStyleChangesIfPossible()++        // Two reasons registration has to be revisited, not done once:+        // the app's window may not exist yet this early in launch (this+        // observer is created before the first window is), and the window+        // holding the registration can later close while the app keeps+        // running (iPadOS opens a scene per document). Both surface as+        // another window becoming key, or as the owning scene disconnecting.+        for name in [UIWindow.didBecomeKeyNotification, UIScene.didDisconnectNotification] {+            windowObservers.append(+                NotificationCenter.default.addObserver(+                    forName: name,+                    object: nil,+                    queue: .main+                ) { [weak self] _ in+                    Task { @MainActor [weak self] in+                        self?.registerForInterfaceStyleChangesIfPossible()+                        self?.refresh()+                    }+                }+            )         }         #endif     }@@ -68,7 +123,80 @@ final class SystemColorSchemeObserver {             NotificationCenter.default.removeObserver(observer)             #endif         }+        #if os(iOS)+        for windowObserver in windowObservers {+            NotificationCenter.default.removeObserver(windowObserver)+        }+        // `interfaceStyleRegistration` is deliberately not unregistered here.+        // `unregisterForTraitChanges` is main-actor work that a `deinit`+        // can't do, and the handler captures `self` weakly, so once this+        // observer is gone the registration is inert.+        //+        // It is not, however, short-lived: the key window normally OUTLIVES+        // the observer, so each discarded observer leaves one inert closure+        // behind on that window. That is acceptable only because there is+        // exactly one observer per app lifetime (`@State` on `PrismApp`).+        // Constructing this class per view would make the accumulation+        // unbounded — the `#Preview` in `SettingsView` and the tests each+        // build one, which is harmless at their scale but is the pattern to+        // keep out of production code.+        #endif+    }++    #if os(iOS)+    /// Registers for `UITraitUserInterfaceStyle` trait changes on the app's+    /// currently active window, moving the registration there if it is+    /// presently on a different (or deallocated) window. A no-op when the+    /// active window is already the registered one, or when no window exists+    /// yet.+    ///+    /// Following the active window is what keeps this alive across iPadOS+    /// multi-window teardown: closing the document window that happened to+    /// carry the registration leaves the observer registered on whichever+    /// window survives, instead of silently observing nothing.+    ///+    /// Registering on the window (rather than e.g. the `UIWindowScene`) is+    /// what makes this observable in a live test: nothing sets the *window's*+    /// `overrideUserInterfaceStyle` in production — SwiftUI puts+    /// `.preferredColorScheme` on the root `UIHostingController` instead, and+    /// overrides propagate downward only — so the window's trait always tracks+    /// the real system value, but tests can still force a trait change by+    /// setting the override directly on the same window.+    private func registerForInterfaceStyleChangesIfPossible() {+        guard let window = Self.activeWindow(), window !== registeredWindow else { return }++        // Drop the previous registration if that window is still around —+        // otherwise it died with it.+        if let previousWindow = registeredWindow, let interfaceStyleRegistration {+            previousWindow.unregisterForTraitChanges(interfaceStyleRegistration)+        }++        // UIKit sets the `UITraitCollection.current` thread-local to the new+        // traits before invoking this handler and restores it afterwards, so+        // `refresh()` — which reads `current` — sees this window's updated+        // style. That guarantee is documented for the trait-change handler+        // specifically, and holds only for its dynamic extent; `refresh()`+        // called from anywhere else reads the screen's value instead, which+        // is what the other call sites want.+        interfaceStyleRegistration = window.registerForTraitChanges(+            [UITraitUserInterfaceStyle.self]+        ) { [weak self] (_: UIWindow, _: UITraitCollection) in+            self?.refresh()+        }+        registeredWindow = window+    }++    /// The app's currently active window, used as the trait-change+    /// observation target. Returns `nil` if no scene/window is connected yet.+    ///+    /// Reuses `WebViewPool.selectIOSWindow(from:)` (T-745) rather than+    /// `connectedScenes.first`, which is non-deterministic (`connectedScenes`+    /// is a `Set`) and can pick a background/inactive scene.+    private static func activeWindow() -> UIWindow? {+        let scenes = UIApplication.shared.connectedScenes.compactMap { $0 as? UIWindowScene }+        return WebViewPool.selectIOSWindow(from: scenes)     }+    #endif      /// Re-reads the current system appearance and updates the published value.     func refresh() {

diff --git a/prismTests/SystemColorSchemeObserverTests.swift b/prismTests/SystemColorSchemeObserverTests.swiftnew file mode 100644index 0000000..7f9b1e6--- /dev/null+++ b/prismTests/SystemColorSchemeObserverTests.swift@@ -0,0 +1,200 @@+//+//  SystemColorSchemeObserverTests.swift+//  prismTests+//+//  Regression tests for T-1698: automatic light/dark switching required an+//  app restart on iOS.+//+//  The bug: `SystemColorSchemeObserver` listened for+//  `UIScreen.modeDidChangeNotification` to detect Dark Mode toggles. That+//  notification reports screen resolution/mode changes, not interface-style+//  changes, so it never fired on an automatic system appearance switch —+//  `refresh()` never ran, and only a restart (which re-reads the system+//  scheme in `init()`) picked up the change.+//+//  A direct-invocation test (constructing the observer and calling+//  `refresh()` by hand) can't see this class of bug: the failure is in the+//  production *wiring* — the observer never gets told a change happened in+//  the first place. So this is a live test over the real trait-change+//  registration, mirroring the pattern used for the WebContent-termination+//  wiring regression (T-1943, see `WebContentTerminationWiringTests`).++#if os(iOS)+import Testing+import SwiftUI+import UIKit+@testable import prism++// Serialized: every test here forces `overrideUserInterfaceStyle` on the one+// window the test host owns and then polls across `await` points, so two of+// them running concurrently would see each other's overrides.+//+// `.serialized` only orders the tests *within this suite*; Swift Testing still+// runs other suites in the same host concurrently. `overrideUserInterfaceStyle`+// and window key-ness are process-global, and other iOS suites resolve a window+// through `WebViewPool.selectIOSWindow(from:)`, which prefers the key window —+// so the residual cross-suite risk is real, just not one a suite trait can+// close. Each test restores the override in a `defer` to keep that window as+// narrow as possible.+@Suite("SystemColorSchemeObserver Tests — T-1698 regression", .serialized)+@MainActor+struct SystemColorSchemeObserverTests {+    /// Resolves the window exactly as `SystemColorSchemeObserver` does, so the+    /// test can never force a trait change on a window the observer did not+    /// register on. Using `connectedScenes.first` here instead would be both+    /// non-deterministic (`connectedScenes` is a `Set`) and a *different*+    /// selection rule from the code under test — the tests would go vacuous+    /// rather than fail if the two ever disagreed.+    private static func activeWindow() -> UIWindow? {+        let scenes = UIApplication.shared.connectedScenes.compactMap { $0 as? UIWindowScene }+        return WebViewPool.selectIOSWindow(from: scenes)+    }++    /// The unit test target hosts inside the real `prism.app` (TEST_HOST),+    /// so a `UIWindowScene`/`UIWindow` should be connected by the time tests+    /// run. If this fails, the live test below can't be trusted — the+    /// observer would have silently failed to register for the same reason.+    @Test("Test host has a connected window to observe")+    func testHostHasConnectedWindow() {+        #expect(Self.activeWindow() != nil)+    }++    @Test("Toggling the window's interface style refreshes the observer without a restart")+    func togglingWindowInterfaceStyleRefreshesObserver() async throws {+        let window = try #require(Self.activeWindow())++        // Restore on every exit path, including a thrown cancellation from the+        // polling sleeps below — a leaked override would follow the process+        // into other suites.+        defer { window.overrideUserInterfaceStyle = .unspecified }++        // Reset any override left behind by a previous run, and let the+        // observer establish its baseline from the real (unforced) style.+        window.overrideUserInterfaceStyle = .unspecified+        let observer = SystemColorSchemeObserver()++        // Force the opposite of what the *window* currently resolves to. The+        // target has to come from the window rather than from+        // `observer.systemColorScheme`: the observer reads the+        // `UITraitCollection.current` thread-local, and if the two ever+        // disagreed we would be setting the style the window already has, no+        // trait change would fire, and this would fail after a 2s poll for a+        // reason that has nothing to do with the code under test.+        //+        // This is the same trait (`UITraitUserInterfaceStyle`) a real Dark+        // Mode toggle changes — the production code can't distinguish an+        // override from a genuine system change.+        let target: ColorScheme = window.traitCollection.userInterfaceStyle == .dark ? .light : .dark++        // ...and the observer has to start out disagreeing with it, or the+        // poll below could be satisfied without any trait change having been+        // delivered at all.+        #expect(observer.systemColorScheme != target)++        window.overrideUserInterfaceStyle = target == .dark ? .dark : .light++        // Trait propagation isn't necessarily synchronous with the property+        // set, so poll briefly rather than asserting immediately.+        let deadline = Date().addingTimeInterval(2)+        while observer.systemColorScheme != target, Date() < deadline {+            try await Task.sleep(nanoseconds: 20_000_000)+        }++        #expect(observer.systemColorScheme == target)+    }++    /// The registration is made on one specific `UIWindow` and dies with it.+    /// Prism opens a `WindowGroup` scene per document, so on iPadOS the+    /// window that happened to carry the registration can be closed while+    /// the app keeps running. Tracking "registration ever succeeded" instead+    /// of "which window holds it" leaves the observer permanently deaf from+    /// that point on — the same "requires a restart" symptom, re-triggered by+    /// a window closing rather than by the wrong notification name.+    ///+    /// Scope: this covers the `UIWindow.didBecomeKeyNotification` re-arm+    /// only — the explicit `makeKeyAndVisible()` below is what drives it. The+    /// observer's other re-arm trigger is uncovered; see the note below.+    @Test("Registration re-arms on a surviving window after the registered window goes away")+    func registrationReArmsAfterRegisteredWindowGoesAway() async throws {+        let hostWindow = try #require(Self.activeWindow())+        let scene = try #require(hostWindow.windowScene)+        defer {+            hostWindow.overrideUserInterfaceStyle = .unspecified+            hostWindow.makeKeyAndVisible()+        }+        hostWindow.overrideUserInterfaceStyle = .unspecified++        // A second window, standing in for a second document scene. Making+        // it key means it is the window `activeWindow()` selects, so the+        // observer registers against it rather than the host window.+        var extraWindow: UIWindow? = UIWindow(windowScene: scene)+        extraWindow?.overrideUserInterfaceStyle = .unspecified+        extraWindow?.makeKeyAndVisible()++        // Without this the test would be vacuous: if the observer registered+        // on the host window all along, the re-arm it is meant to prove would+        // never be needed and the assertion below would pass regardless.+        #expect(Self.activeWindow() === extraWindow)++        let observer = SystemColorSchemeObserver()++        // Close that window. The host window becomes key again, exactly as+        // when one of several open document windows is closed.+        extraWindow?.isHidden = true+        extraWindow = nil+        hostWindow.makeKeyAndVisible()++        // Re-registration is dispatched onto the main actor by the+        // notification handler, so let that turn run before provoking the+        // trait change it needs to catch.+        try await Task.sleep(nanoseconds: 200_000_000)++        // Derived from the surviving window, and asserted to differ from the+        // observer's current value, for the same two reasons as the test+        // above: guarantee a real trait change fires, and guarantee the poll+        // cannot be satisfied without one.+        let target: ColorScheme = hostWindow.traitCollection.userInterfaceStyle == .dark ? .light : .dark+        #expect(observer.systemColorScheme != target)++        hostWindow.overrideUserInterfaceStyle = target == .dark ? .dark : .light++        let deadline = Date().addingTimeInterval(2)+        while observer.systemColorScheme != target, Date() < deadline {+            try await Task.sleep(nanoseconds: 20_000_000)+        }++        #expect(observer.systemColorScheme == target)+    }++    // MARK: - Known coverage gap: the UIScene.didDisconnectNotification re-arm+    //+    // `SystemColorSchemeObserver` re-arms its trait registration on two+    // triggers. Only the first, `UIWindow.didBecomeKeyNotification`, is+    // covered (above). The second, `UIScene.didDisconnectNotification`, has+    // no automated coverage, deliberately rather than by oversight.+    //+    // Why it can't be driven here: the unit-test host owns exactly one+    // `UIWindowScene`, and a test cannot connect or disconnect another —+    // `UIApplication.supportsMultipleScenes` is false on the iPhone+    // destination `make test` uses, so a scene-session request is not+    // available either. Posting `UIScene.didDisconnectNotification` by hand+    // does reach the production handler (the observer subscribes with+    // `object: nil` on the default centre), but such a test cannot be shown+    // to prove anything, because it passes just as well with the listener+    // deleted: every route to a stale registration available inside a single+    // scene makes UIKit promote another window to key and post+    // `didBecomeKeyNotification` first, which re-arms the observer on its+    // own. That was measured, not assumed — instrumenting the notification+    // stream showed both deallocating the registered window and calling+    // `resignKey()` on it produce a `UIWindowDidBecomeKeyNotification` for+    // the surviving window before any synthetic post can be made.+    //+    // Manual check on an iPad (the platform where the branch matters): open+    // two Prism document windows, close the one that is currently key, send+    // the app to the background, toggle Light/Dark in Control Centre, then+    // return to the remaining window. Its theme must already match the new+    // system appearance with no relaunch. Repeat with the app left in the+    // foreground as a control — that path is the covered one and should+    // behave identically.+}+#endif

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 338d8c8..8e069c2 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0  ### Fixed +- On iPhone and iPad, Prism now follows the system's light/dark setting the moment it changes, instead of waiting for a restart (T-1698). With appearance set to Auto, switching Dark Mode on or off — by hand in Control Centre, or automatically at sunset — left the app rendering in the previous appearance for the rest of the session; only quitting and reopening it picked the change up, because reopening reads the system setting afresh. The app had been listening for the wrong system signal: one that reports a change to the screen's resolution, not to its appearance, and which therefore never arrived. It now watches the appearance itself. On iPad this also holds across multiple open document windows: the watch follows whichever window is in front, so closing the one it happened to be attached to no longer leaves the remaining windows stuck on the old appearance. Mac was never affected, and the forced Light and Dark modes are unchanged — a system appearance change still leaves them where you set them.+ - Share with Notes on a remote document opened from a host-only or trailing-slash root URL (e.g. `https://example.com`) no longer always fails (T-1822). The display name used for that document falls back to its full absolute URL so a root URL never shows a blank title (T-1177) — but Share with Notes reused that same string, unsanitised, as the temporary export file's name. The URL's `/` and `:` characters were read as path separators, so the write always targeted a nonexistent nested directory and "Export Failed" appeared every time. The export filename is now derived from a sanitised version of the display name, with path separators and control characters stripped rather than passed through.  - A block whose text is exactly a thematic break — `---`, `***`, or `___` — no longer renders empty (T-1669). Rendering re-parses a block's text as its own small markdown document, and that string satisfying markdown's rule for a horizontal rule made it parse as one rather than as plain text; nothing then knew how to turn a rule back into visible text, so it vanished. This is the third defect of this shape (after numbered list markers and `@`-prefixed text, T-1640/T-1641): rendering now knows a rule found this way as text too, and shows the characters as written — in a paragraph, a heading, a list item, or a table cell alike.

diff --git a/docs/agent-notes/typography-font-settings.md b/docs/agent-notes/typography-font-settings.mdindex 8fe6e97..ce80a95 100644--- a/docs/agent-notes/typography-font-settings.md+++ b/docs/agent-notes/typography-font-settings.md@@ -174,4 +174,5 @@ Do not reinstate per-call-site `.font(.system(size:))` overrides in chrome — e - Settings sheet/scene needs `.applyTheme(settings:systemObserver:)` for ThemePreviewCard to reflect theme colors. - Never pass `nil` to `.preferredColorScheme()` when the view also reads color scheme — always resolve to a concrete value via `SystemColorSchemeObserver`. - `applyTheme()` requires a `SystemColorSchemeObserver` parameter (added in T-429 fix). The observer should be created once at the app level and shared.+- **T-1698**: on iOS the observer's change *signal* is a `registerForTraitChanges([UITraitUserInterfaceStyle.self])` registration held on the app's active window (resolved via `WebViewPool.selectIOSWindow`, T-745), re-armed on `UIWindow.didBecomeKeyNotification` and `UIScene.didDisconnectNotification`. There is no UIKit notification for an interface-style change; the old code used `UIScreen.modeDidChangeNotification`, which reports screen *resolution/mode* changes and never fires on a Dark Mode toggle, so auto light/dark needed an app restart. Two consequences worth remembering. A trait registration is owned by the specific window it was made on and dies with it, so the observer must track *which* window holds it — a "registered once" boolean goes permanently stale when that document window closes on iPadOS, silently restoring the bug. And the window is a safe registration target precisely because SwiftUI lands `.preferredColorScheme` on the root `UIHostingController`, not on the `UIWindow` (measured on the simulator: the host window reports `overrideUserInterfaceStyle == .unspecified` while the hosting controller carries the forced value, and overrides propagate downward only). Register on the `UIWindowScene` instead and the behaviour becomes untestable; register somewhere SwiftUI does override and the observer would echo Prism's own appearance mode back at itself. - SwiftLint rule: `var x: String?` not `var x: String? = nil` (implicit_optional_initialization).