PR #350 — wires the previously-dead WebContent-crash recovery to page.navigations, with classification, a bounded retry budget, a stalled-recovery watchdog, and a user-visible give-up banner. Two findings, both small and well-localised; the branch also needs a merge of origin/main before CI can dispatch at all.
handleProcessTermination existed and was unit-tested since the WebKit cutover, but nothing in production ever called it. A WebContent kill left the document permanently blank with no error and no way back but closing the file.WebPage exposes no delegate, no event case and no Observable property for a renderer crash — it throws NavigationError.webContentProcessTerminated from page.navigations, which ends the sequence, and which also throws for ordinary navigation failures. Hence classify-and-resubscribe.ready, then give up with a Reload banner; any ready or any fresh load restores the budget and re-arms observation.RecoveryReason plumbing is inert (major, diagnostics), and the abandonment banner can outlive a successful recovery (minor, user-visible).9643395 — the PR is DIRTY/CONFLICTING on CHANGELOG.md, which suppresses pull_request dispatch. Not the account-level Actions stall. The previous head's runs (Checks, Lint, Localisation Tests) were all green.make lint clean, make build-ios succeeded (one pre-existing unrelated warning), macOS app target compiled via the test run.Needs fixes
The core wiring is right, well-argued, and pinned by the one kind of test that could have caught the original bug — a live controller over a real WebPage. Classification, re-subscription, the recovery-in-flight discrimination, the retry budget and the watchdog all hold up under reading, and every targeted suite is green.
Two things should change before this merges. One is a defect in the diagnostics the second review round was specifically added to provide: attemptRecovery(reason:) accepts a RecoveryReason and never forwards it, so all three recovery causes log “the WebContent process terminated” and the .reloadFailed / .reloadStalled strings are unreachable — while docs/agent-notes/webview-rendering-status.md tells a future maintainer that the log line names which path ran. The other is user-visible: markReady() never clears recoveryAbandoned, and watchdog-driven abandonment leaves the observation loop running, so a document that abandons on a stall and is later auto-recovered renders correctly with the “This document stopped rendering.” banner still on it.
Both fixes are one line each. Separately, the branch must be merged with origin/main (CHANGELOG conflict only) — that conflict is why GitHub has dispatched no workflows for the current head.
e7e932f Fix T-1943: Wire WebContent termination recovery to a real signal 21bc138 Fix T-1943 review round 1: make the recovery's own failure paths work 9643395 Fix T-1943 review round 2: bound the recovery's liveness, and make the re-arm test able to fail Prism draws your document inside a web view. The part of the system that actually paints the page runs in its own separate process, and macOS/iOS are allowed to shut that process down — typically when memory is tight and your document is large or full of images. When that happened, Prism's window went blank. No error, no spinner, nothing: the only fix was to close the file and open it again.
The odd part is that Prism already knew how to recover. There was a function that reloads the document and puts everything back — your theme, where you'd scrolled to, your note markers, your search highlights. It was written, it was tested, and it worked when called. Nothing ever called it. It was, in effect, a fire extinguisher bolted to the wall with no handle.
This is a failure mode that tests are famously bad at catching. Every test called the recovery function directly and asked “does it do the right thing?” — and it did. No test asked the different question: “does anything ever call this?” So a completely dead feature passed review and shipped.
WebDocumentController owns one WebPage per document session. handleProcessTermination(documentURL:) — bump processGeneration, reset readiness, reload, replay the coalesced native-truth snapshot — has existed since the T-1542 WebKit cutover and was covered by WebDocumentControllerTests.terminationReplaysSnapshot. It had no caller.
The reason it had no caller is that WebKit-for-SwiftUI gives you nothing obvious to hang one on. WebPage has no webContentProcessDidTerminate delegate hook, no NavigationEvent case, and no Observable property. The crash arrives as a thrown WebPage.NavigationError.webContentProcessTerminated on the page.navigations async sequence — and throwing ends that sequence.
drainNavigationStream consumes one subscription and returns a NavigationObservationOutcome; applyNavigationOutcome decides and returns whether to continue; observeNavigations joins them in a loop. Both halves are static and generic over the sequence, so the production signal and a test's injected stream travel byte-identical classification code.deinit reachable. The loop holds no strong self across an await; it captures page strongly (so the drain has something to drain) and re-weakens self per callback. deinit then cancels the task, which is only a real fix if page.navigations honours cancellation — pinned by a live test rather than assumed.recoveryInFlight as a disambiguator. A recovery reload can fail provisionally, which the stream reports as an ordinary failedProvisionalNavigation — indistinguishable from a bad link by the error alone. The flag makes the same error legible: inside a recovery it is the recovery failing (charge and retry); outside one it is benign (keep watching, reload nothing).ready produces no event at all. A 20-second generation-guarded timer covers it.The budget counts consecutive attempts in one failing chain, not lifetime failures — so a document that crashes once an hour never abandons, by design. The 20s watchdog is deliberately generous (twice the app's 10s render-timeout convention) because firing early on a merely-slow large document would convert a slow render into a permanent reload loop. Abandonment surfaces as observed state (recoveryAbandoned) rather than a callback, so DocumentScrollContent picks it up with no push.
The classification is deliberately non-exhaustive: drainNavigationStream uses if case against WebPage.NavigationError rather than a switch, because the enum is non-frozen and a future SDK case must degrade to “not a termination” rather than break the build. Anything that isn't webContentProcessTerminated or pageClosed becomes .navigationFailed, which is the conservative direction — worst case you keep observing.
observeNavigations opens each iteration with an unconditional await Task.yield(). The reasoning in the comment is sound and worth preserving: the loop is MainActor-bound, and an await that completes without suspending does not yield the executor, so a hypothetical page that threw synchronously on a fresh subscription would spin the main thread. Rather than resting on “a fresh page.navigations always suspends” — an unpinnable property of a closed-source API — the yield makes the loop safe regardless, at one executor hop per navigation.
The epoch mechanism (observationEpoch) makes the handle-clearing self-evidently correct rather than correct-by-guard: a cancelled loop resuming after a successor was armed compares epochs and declines to nil out its successor's handle. This matters because startNavigationObservation guards on navigationObservationTask == nil, so a stale clear would be indistinguishable from “never armed”, and a stale failure to clear latches observation off for the life of the document — the original bug, one layer up.
Observation is armed from init, which means every WebDocumentController in the app and in the test suite now owns a live MainActor task draining a real navigation stream. That is safe here only because WebDocumentMessageHandler.controller is weak (no retain cycle), the observation task captures weak self, and deinit cancels. Remove any one of those three and you leak one WebPage plus its WebContent process per closed document — which is precisely why cancellingObservationEndsTheRealDrain exists as a live test rather than a comment.
The test file's makeControllerWithParkedObservation is the most instructive thing in the branch. With real observation live, the recovery reload navigates the real page to a fake prism-doc:// URL, which fails; that failure arrives as .navigationFailed during a recovery, so the loop charges and retries it — moving processGeneration with no watchdog involved. The author found this by mutation testing: deleting startRecoveryWatchdog() left every watchdog test green. Parking the loop on a never-yielding stream makes the watchdog the only thing that can move the state under test.
.streamEnded is treated as terminal. If page.navigations ever completed normally after an ordinary successful navigation, recovery would be silently disarmed again — with every test in the file still green, because the wiring pin asserts only that an event was seen. I verified this empirically during review with a throwaway live test (since deleted): after a completed prism-doc:// navigation plus a 1.5s window, lastNavigationOutcome is nil and the observation task is still armed. The assumption holds today; it is unpinned for tomorrow.false, exits observeNavigations, and clears the task handle. Watchdog-driven abandonment discards the return (_ = attemptRecovery(...)) and leaves the loop parked on the live page. The asymmetry is benign for recovery (observation staying armed is strictly better) but it is what makes the stale-banner path below reachable.canScroll is now page-reported over the bridge. resetForNavigation() does not reset it, so between a crash and the reloaded page's first scrollabilityChanged post, Page Up/Down enablement reflects the dead page. Same shape as an ordinary load(), so not a regression from this branch — but it is a new surface the two changes share.prism/ViewModels/WebDocumentController.swift
Why it matters. This is the entire fix. Without the startNavigationObservation() call in init, handleProcessTermination stays unreachable and a WebContent kill leaves a permanently blank document. Everything else in the branch is machinery around this one call.
What to look at. WebDocumentController.swift:208-220 (init + deinit), 793-861 (startNavigationObservation)
prism/ViewModels/WebDocumentController.swift
Why it matters. page.navigations ENDS when it throws, and it throws for ordinary navigation failures too. An observer that does not take a fresh subscription is silently disarmed by the first bad link in a document — with every unit test still green.
What to look at. WebDocumentController.swift:874-933
prism/ViewModels/WebDocumentController.swift
Why it matters. A recovery reload can fail provisionally, which arrives as an ordinary failedProvisionalNavigation — indistinguishable from a bad link by the error alone. Treating it as benign left isReady false forever over a blank document with nothing retrying: the original bug, reached from inside its own fix.
What to look at. WebDocumentController.swift:940-994 (applyNavigationOutcome, attemptRecovery)
prism/ViewModels/WebDocumentController.swift
Why it matters. Failure and re-crash are both events the loop hears. A reload that neither throws nor reaches ready — a wedged scheme handler, a load that never completes, a termination landing in the loop's re-subscription gap — produces nothing, and used to leave recoveryInFlight set forever with nothing scheduled.
What to look at. WebDocumentController.swift:673-735
prism/Views/DocumentScrollContent.swift
Why it matters. Giving up silently reproduces the exact symptom this ticket fixed — a blank document with nothing but an os_log. The banner's Reload is also a fresh user-initiated load, which restores the budget and re-arms observation, so giving up is never permanent while the file stays open.
What to look at. DocumentScrollContent.swift:88-97 (overlay + reduceMotion animation), 304-345 (banner + reloadWebDocument)
prismTests/WebRendering/WebContentTerminationWiringTests.swift
Why it matters. The pre-existing coverage invoked handleProcessTermination directly and was, by construction, blind to whether anything called it — which is exactly how a completely dead recovery path shipped and survived review.
What to look at. WebContentTerminationWiringTests.swift:1-38 (the rationale), 112-158 (the wiring pin), 598-629 (the cancellation pin)
Covers the page's whole life, including a crash before the first document loads. attemptRecovery handles the no-document case by logging and continuing to observe rather than reloading nothing.
WebPage.NavigationError is non-frozen, so a future SDK case must degrade to “not a termination” rather than fail to build. Unknown errors fall to .navigationFailed, the conservative direction — worst case observation continues.
Any recovery reaching ready, and any fresh load, restores the full allowance. A document that crashes once an hour therefore never abandons. Documented in the agent-notes manual-kill procedure as correct behaviour rather than a failure to reproduce.
Changed in review round 1. isReady also charged a crash occurring during an ordinary load, which is not an unproductive recovery — the variable name promised more than the expression delivered.
Asymmetric costs: firing early reloads a merely-slow large document and can convert a slow render into a permanent reload loop; firing late means a wedged reload is retried 20s late instead of never.
Raising a real termination requires killing a launchd-owned system service shared with every other WebKit client on the machine, identifiable only by racy PID set-difference. The signal is injected, and one live test pins that production subscribes to the real source of it.
Safe only because page.navigations honours cancellation — pinned against the real sequence by cancellingObservationEndsTheRealDrain rather than assumed. If that test ever goes red the capture must be weakened; the failure mode is one leaked WebPage and WebContent process per closed document, which nothing else would surface.
The iOS folder-access grant flow already had a same-revision re-fetch. Lifting it out of #if os(iOS) and renaming it from reloadWebDocumentForAccess was the whole cost.
The new banner needed @Environment(\.accessibilityReduceMotion); the sibling banner's unconditional .easeInOut was updated with it rather than left inconsistent.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | WebDocumentController.attemptRecovery — RecoveryReason never forwarded | attemptRecovery(reason:) accepts a RecoveryReason and never uses it. Its only recovery call is handleProcessTermination(documentURL: documentURL), which falls back to the reason: .processTerminated default — so all three causes log "Reloading the document: the WebContent process terminated". RecoveryReason.reloadFailed and .reloadStalled and their logDescription strings are unreachable, and the enum added in review round 2 is inert. The same function's no-document branch logs "WebContent process terminated before any load" even when reached from a stalled or failed reload. This is not cosmetic: docs/agent-notes/webview-rendering-status.md step 5 tells a future maintainer the log line names which path ran ("The same line names `the recovery reload failed` / `the recovery reload never reported ready`"), so the branch ships documentation asserting behaviour the code does not have — on the exact diagnostics a blank-document report would be triaged from. Fix: pass reason through (handleProcessTermination(documentURL: documentURL, reason: reason)) and parameterise the nil-URL log. Not applied: report-only remit, and forwarding an argument changes runtime output. | Reported, not fixed — production behaviour change outside the report-only remit. One-line fix. |
| minor | WebDocumentController.markReady / startRecoveryWatchdog — banner outlives a successful recovery | markReady() clears recoveryInFlight and cancels the watchdog but does not clear recoveryAbandoned; only load() clears it. Meanwhile watchdog-driven abandonment discards attemptRecovery's false return (_ = attemptRecovery(reason: .reloadStalled)), so unlike loop-driven abandonment the observation loop keeps running. Reachable sequence: recovery stalls four times and abandons (banner shown, observation still armed) -> a later genuine termination arrives -> attemptRecovery sees recoveryInFlight == false, restarts the budget at 1 and recovers -> markReady fires -> the document renders correctly with "This document stopped rendering." still overlaid, and its Reload button now pointlessly reloads a healthy page. Fix: clear recoveryAbandoned in markReady() (a page that reported ready is by definition rendering). | Reported, not fixed — user-visible behaviour change outside the report-only remit. One-line fix. |
| minor | WebContentTerminationWiringTests — the wiring pin does not cover stream survival | controllerObservesItsOwnPageNavigationStream asserts that an event was observed, not that observation SURVIVES a completed navigation. .streamEnded is treated as terminal, so if page.navigations ever ended normally after an ordinary successful load, recovery would be silently disarmed again — the exact failure class this ticket exists to fix — with every test in the file green. Verified empirically during this review with a throwaway live test (since deleted): after a completed prism-doc:// navigation plus a 1.5s window, lastNavigationOutcome is nil and test_isObservingNavigations() is true. The assumption holds on the current SDK; it is unpinned against a future one. Two extra assertions on the existing live test would close it at no cost. | Verified as correct today; recommended as a test addition (test change, outside the report-only remit). |
| nit | DocumentScrollContent.recoveryAbandonedBanner — accessibility deviates from its sibling | The banner uses .accessibilityElement(children: .combine), collapsing the Reload button into a single element; the imageAccessBanner directly above it leaves its buttons separately focusable and labelled. The combine is likely fine (SwiftUI merges the child's action onto the combined element) and the hint carries the affordance, but this banner's Reload is the only escape hatch it offers, so the deviation is worth one VoiceOver pass rather than an assumption. | Reported — worth a manual VoiceOver confirmation. |
| nit | DocumentScrollContent.reloadWebDocument — no off-main HTML precompute | The primary load path (.task(id: WebLoadKey…)) awaits WebDocumentControllerFactory.precomputeDocumentHTML so a large document is emitted off the MainActor (T-1681). reloadWebDocument() does not, relying on the per-parseRevision cache on DocumentSession; a cache miss falls back to a synchronous on-main emit. Inherited shape from the iOS folder-access path, now reachable on both platforms, and reachable specifically on the large/image-heavy documents most likely to have had their renderer killed. The cache is not invalidated by a crash, so a hit is the realistic case. | Reported — pre-existing shape, low risk. |
| nit | WebDocumentController.recoveryWatchdogTimeout — ungated test seam | A plain internal var mutated only by tests, while every other test seam on this type (test_markReady, test_isObservingNavigations, test_setInjectedNavigationStream, test_rearmNavigationObservation) is #if DEBUG-gated and test_-prefixed. Also, the watchdog's stall log prints timeout.components.seconds, which is 0 for the sub-second values tests set — harmless for the 20s production value. | Reported — consistency only. |
| info | Interaction with PR #349 (merged into main) — canScroll across a recovery | #349 made canScroll page-reported over the bridge (scrollabilityChanged). resetForNavigation() does not reset it, so between a WebContent crash and the reloaded page's first scrollabilityChanged post, View > Page Up/Down enablement reflects the dead page. Identical shape for an ordinary load(), so not a regression from this branch. No file overlap between the two changes apart from CHANGELOG.md; git merge-tree confirms every source file auto-merges. | Noted — not a regression from this branch. |
| info | Build warning (pre-existing, unrelated) | make build-ios succeeds with one warning: "main actor-isolated conformance of 'ImageDimension' to 'Equatable' cannot be used in nonisolated context; this is an error in the Swift 6 language mode". ImageDimension is untouched by this branch, so the warning pre-dates it — but it is a standing violation of the project's zero-warning pre-push bar and will become a hard error under Swift 6. | Noted — pre-existing, out of scope for this branch. |
Click to expand.
diff --git a/prism/ViewModels/WebDocumentController.swift b/prism/ViewModels/WebDocumentController.swiftindex a6d664b..248aedf 100644--- a/prism/ViewModels/WebDocumentController.swift+++ b/prism/ViewModels/WebDocumentController.swift@@ -208,6 +208,18 @@ final class WebDocumentController { // Wire the handler back to this controller now that `self` is initialised. messageHandler.controller = self++ // Arm WebContent-crash recovery (Req 9.6, T-1943). This call is the whole+ // difference between a recovery path that runs and one that is dead code —+ // `WebContentTerminationWiringTests` is red without it.+ startNavigationObservation()+ }++ deinit {+ // `Task.cancel()` is safe from a nonisolated deinit. The observation task holds+ // no strong `self`, so this is reachable (see `startNavigationObservation`).+ navigationObservationTask?.cancel()+ recoveryWatchdogTask?.cancel() } /// The bridge-world message handler. Retained so its WK registration stays@@ -292,6 +304,11 @@ final class WebDocumentController { private func markReady() { guard !isReady else { return } isReady = true+ // The page came back, so any recovery chain in flight has succeeded: a later+ // navigation failure is an ordinary one again, and the next crash starts a+ // fresh budget (T-1943).+ recoveryInFlight = false+ cancelRecoveryWatchdog() flushPending() } @@ -579,6 +596,19 @@ final class WebDocumentController { /// the new page is ready (design `load` ordering). func load(documentURL: URL, parseRevision: UInt64) { self.parseRevision = parseRevision+ loadedDocumentURL = documentURL+ // A user-initiated navigation (re-parse, file-change reload, URL refresh, the+ // iOS folder-access retry, or the reload the abandonment banner offers) is+ // FRESH EVIDENCE that loading may work, so it retires whatever the previous+ // recovery chain concluded: budget restored, chain broken, banner cleared, and+ // observation re-armed if it had given up. Without this the give-up was+ // permanent for the life of the open document — the original bug, one layer up+ // (T-1943 review).+ unproductiveRecoveries = 0+ recoveryInFlight = false+ recoveryAbandoned = false+ cancelRecoveryWatchdog()+ startNavigationObservation() resetForNavigation() scheduleSnapshotReplay() Task { [page] in@@ -590,15 +620,47 @@ final class WebDocumentController { } } + /// Why a recovery reload is being started.+ ///+ /// Diagnostics only — the recovery itself is identical in all three cases — but a+ /// blank-document report is diagnosed from these log lines alone, so they have to+ /// name what actually happened. A retried navigation failure and a reload that+ /// never became ready are not "the WebContent process terminated" (T-1943 review).+ enum RecoveryReason: Sendable {+ /// The renderer died: `page.navigations` threw `webContentProcessTerminated`.+ case processTerminated+ /// A previous recovery reload failed as an ordinary navigation.+ case reloadFailed+ /// A previous recovery reload neither failed nor reported ready in time.+ case reloadStalled++ /// A fixed, enumerated phrase — never document content, so it logs `.public`.+ var logDescription: String {+ switch self {+ case .processTerminated: "the WebContent process terminated"+ case .reloadFailed: "the recovery reload failed"+ case .reloadStalled: "the recovery reload never reported ready"+ }+ }+ }+ /// Handles WebContent process termination (Req 9.6): bumps the process /// generation (so any in-flight stale message is dropped), reloads the /// current revision, and replays the coalesced snapshot — restoring theme, /// search highlights, note indicators, and scroll position from native truth.- func handleProcessTermination(documentURL: URL) {- Self.logger.error("WebContent process terminated; reloading (category: webcontent)")+ func handleProcessTermination(documentURL: URL, reason: RecoveryReason = .processTerminated) {+ Self.logger.error(+ "Reloading the document: \(reason.logDescription, privacy: .public) (category: webcontent)"+ )+ // A recovery reload is in flight until the reloaded page reports `ready`+ // (`markReady`) or a fresh user-initiated `load` supersedes it. While it is+ // set, a navigation failure is not a benign bad link — it is THIS reload+ // failing, and it is charged and retried as such (see `applyNavigationOutcome`).+ recoveryInFlight = true processGeneration &+= 1 resetForNavigation() scheduleSnapshotReplay()+ startRecoveryWatchdog() Task { [page] in do { for try await _ in page.load(URLRequest(url: documentURL)) {}@@ -608,6 +670,329 @@ final class WebDocumentController { } } + // MARK: - Stalled-recovery watchdog (T-1943 review)++ /// How long a recovery reload may run without reaching `ready` before the watchdog+ /// treats it as stalled.+ ///+ /// Deliberately generous. A large document legitimately takes seconds to emit,+ /// serve, render and lay out, and a watchdog that fired early would reload a page+ /// that was merely slow — turning a slow document into a permanently reloading one.+ /// Twice the app's 10-second render-timeout convention (`MermaidRenderer`,+ /// `SVGRenderer`) is well past anything a healthy load takes, and the cost of+ /// waiting is only that a genuinely wedged reload is retried 20 seconds late+ /// instead of never.+ static let defaultRecoveryWatchdogTimeout: Duration = .seconds(20)++ /// The effective watchdog timeout. Settable so a test can drive the stall path+ /// without waiting out the production value; nothing in the app changes it.+ @ObservationIgnored var recoveryWatchdogTimeout: Duration =+ WebDocumentController.defaultRecoveryWatchdogTimeout++ /// The pending liveness bound on the recovery reload in flight, if any.+ @ObservationIgnored private var recoveryWatchdogTask: Task<Void, Never>?++ /// Arms the stalled-recovery watchdog for the reload just started.+ ///+ /// Every other way a recovery can go wrong is an event: the reload fails, or the+ /// renderer dies again, and either way the observation loop hears about it. A+ /// reload that neither fails nor reaches `ready` — a wedged scheme handler, a load+ /// that simply never completes, or a termination landing in the window between the+ /// stream throwing and the loop re-subscribing — produces NO event at all, and used+ /// to leave `recoveryInFlight` set forever with nothing scheduled: the original+ /// symptom (blank document, no banner, no retry, os_log only) reached from inside+ /// its own fix. The watchdog is the liveness bound that closes it: reach `ready` in+ /// time, or be charged and retried like any other unproductive attempt — and+ /// abandoned, with the banner, once the budget is spent.+ private func startRecoveryWatchdog() {+ recoveryWatchdogTask?.cancel()+ let armedGeneration = processGeneration+ let timeout = recoveryWatchdogTimeout+ recoveryWatchdogTask = Task { @MainActor [weak self] in+ try? await Task.sleep(for: timeout)+ guard !Task.isCancelled, let self else { return }+ // Identity guard: only the reload this watchdog was armed for may be+ // charged. A newer recovery has bumped the generation, `markReady` has+ // lowered `recoveryInFlight`, and a fresh `load` has done both — in every+ // one of those cases this timer is talking about a navigation that is over,+ // and firing would charge the budget for someone else's success.+ guard processGeneration == armedGeneration, recoveryInFlight, !isReady else {+ return+ }+ Self.logger.error(+ "Recovery reload stalled after \(timeout.components.seconds, privacy: .public)s (category: webcontent)"+ )+ _ = attemptRecovery(reason: .reloadStalled)+ }+ }++ /// Disarms the watchdog. Called wherever the reload it watches stops being the+ /// pending one: `markReady` (it arrived), `load` (superseded), and abandonment+ /// (nothing is being retried any more).+ private func cancelRecoveryWatchdog() {+ recoveryWatchdogTask?.cancel()+ recoveryWatchdogTask = nil+ }++ // MARK: - WebContent termination observation (Req 9.6)++ /// Why one subscription to the page's navigation stream stopped.+ ///+ /// `WebPage` reports a WebContent crash the same way it reports an ordinary+ /// failed navigation: `page.navigations` THROWS and the sequence ends. The two+ /// therefore have to be told apart by the thrown error, and only one of them+ /// means "the renderer died" (verified against the real API — see+ /// `startNavigationObservation`).+ enum NavigationObservationOutcome: Equatable, Sendable {+ /// The WebContent process died. Recovery applies.+ case webContentTerminated+ /// An ordinary navigation failure (bad URL, unreachable resource). The page+ /// is alive; nothing to recover, but observation must continue.+ case navigationFailed+ /// The page itself was closed. Terminal — stop observing.+ case pageClosed+ /// The sequence completed without an error. Terminal — stop observing.+ case streamEnded+ }++ /// The document URL of the most recent `load`, so a crash recovery reloads the+ /// same document. Nil until the first load, in which case there is nothing to+ /// recover to.+ @ObservationIgnored private var loadedDocumentURL: URL?++ /// The long-lived task watching `page.navigations`. Cancelled in `deinit`.+ @ObservationIgnored private var navigationObservationTask: Task<Void, Never>?++ /// The last navigation event seen on the observed stream, and the last reason a+ /// subscription ended. Diagnostics — and the only way a test can prove the+ /// production observation is attached to the controller's REAL page rather than+ /// to an injected stub.+ @ObservationIgnored private(set) var lastNavigationEvent: WebPage.NavigationEvent?+ @ObservationIgnored private(set) var lastNavigationOutcome: NavigationObservationOutcome?++ /// Consecutive attempts within one failing recovery chain — a crash or a failed+ /// recovery reload arriving while `recoveryInFlight` is set. Any recovery that+ /// reaches `ready`, and any fresh `load`, breaks the chain and restores the budget.+ @ObservationIgnored private var unproductiveRecoveries = 0++ /// Whether a recovery reload started by `handleProcessTermination` has not yet+ /// reached `ready`. Set by that reload, cleared by `markReady` and by any fresh+ /// `load`. It is what makes a `.navigationFailed` legible: during a recovery it+ /// means the recovery failed, outside one it means a link did.+ @ObservationIgnored private var recoveryInFlight = false++ /// Set once observation has given up on recovering this document. Observed (not+ /// `@ObservationIgnored`) so the view can offer a reload.+ private(set) var recoveryAbandoned = false++ /// How many consecutive recoveries may fail to reach `ready` before observation+ /// gives up. Without a cap, a WebContent process that cannot be relaunched would+ /// have the observer reload in a hot loop forever.+ private static let maxUnproductiveRecoveries = 3++ /// Starts watching the page's navigation stream for a WebContent termination+ /// (Req 9.6). Idempotent; called from `init`, so the observation covers the+ /// page's whole life including a crash before the first load.+ ///+ /// This is the wiring the recovery path was missing (T-1943): `handleProcessTermination`+ /// existed and was unit-tested, but nothing in production ever called it, so a+ /// WebContent kill left the document permanently blank.+ ///+ /// Three properties of the real API shape this loop, all verified directly against+ /// `WebPage` rather than inferred from the docs (which list no failure case on+ /// `NavigationEvent` at all):+ ///+ /// 1. A termination surfaces as `WebPage.NavigationError.webContentProcessTerminated`+ /// THROWN by the stream — there is no event case and no Observable property for it.+ /// 2. The stream ends when it throws, and it throws for ordinary navigation failures+ /// too. So the observer MUST re-subscribe, or the first bad link in a document+ /// would silently disarm crash recovery for the rest of the session.+ /// 3. `page.navigations` HONOURS CANCELLATION: cancelling the task ends an in-flight+ /// drain rather than leaving it suspended. That is what makes the strong `page`+ /// capture below safe — `deinit` cancels, the drain ends, and the page is+ /// released. Pinned by `cancellingObservationEndsTheRealDrain`, which runs the+ /// real sequence; if that test ever goes red the capture must be weakened,+ /// because the failure mode is one leaked `WebPage` (and its WebContent process)+ /// per closed document, which nothing else here would show.+ ///+ /// The loop deliberately holds no strong `self` across its `await`: the drain is+ /// static and the recording callback is weak, so the observation task cannot keep+ /// the controller alive and `deinit` can always cancel it.+ private func startNavigationObservation() {+ guard navigationObservationTask == nil else { return }+ observationEpoch &+= 1+ let epoch = observationEpoch+ navigationObservationTask = Task { @MainActor [weak self, page] in+ let onEvent: @MainActor (WebPage.NavigationEvent) -> Void = { [weak self] event in+ self?.lastNavigationEvent = event+ }+ let applyOutcome: @MainActor (NavigationObservationOutcome) -> Bool = { [weak self] outcome in+ self?.applyNavigationOutcome(outcome) ?? false+ }+ #if DEBUG+ // A test can swap the SIGNAL SOURCE so abandonment travels this exact+ // arming/clearing path rather than being simulated by direct calls+ // (T-1943 review). Everything else — the loop, the decisions, the handle+ // it leaves behind — is the production code.+ let injected = self?.injectedNavigationStream+ #else+ let injected: InjectedNavigationStream? = nil+ #endif+ if let injected {+ await Self.observeNavigations(+ streamProvider: injected, onEvent: onEvent, applyOutcome: applyOutcome+ )+ } else {+ await Self.observeNavigations(+ streamProvider: { page.navigations }, onEvent: onEvent, applyOutcome: applyOutcome+ )+ }+ // The loop has stopped for good (closed page, ended stream, or a spent+ // retry budget). Clearing the handle is what keeps the guard above honest:+ // leaving it set latched observation off for the life of the document, so+ // even a user-initiated reload could never re-arm crash recovery. The epoch+ // makes the clear self-evidently correct rather than correct-by-guard: a+ // loop only ever clears the handle it installed, so a cancelled loop+ // resuming after a fresh one was armed cannot disarm its successor.+ if self?.observationEpoch == epoch { self?.navigationObservationTask = nil }+ }+ }++ /// The source of navigation events a loop subscribes to, when it is not the page's+ /// own `page.navigations`. Test seam only — nil in production and in release builds.+ typealias InjectedNavigationStream =+ @MainActor () -> AsyncThrowingStream<WebPage.NavigationEvent, any Error>++ /// Which arming of the observation the live task belongs to. Bumped on every arm so+ /// a task can tell "my handle" from "my successor's handle" when it exits.+ @ObservationIgnored private var observationEpoch: UInt64 = 0++ #if DEBUG+ @ObservationIgnored private var injectedNavigationStream: InjectedNavigationStream?+ #endif++ /// The loop that joins `drainNavigationStream` to `applyNavigationOutcome`: drain+ /// one subscription, decide, and take a FRESH subscription unless the decision was+ /// terminal.+ ///+ /// Static and generic over a stream PROVIDER for the same reason the drain is+ /// static and generic over a stream: production and tests travel identical code+ /// with only the signal source swapped, and nothing here holds the controller+ /// across an `await` (the closures re-weaken `self` per call), so `deinit` stays+ /// reachable and can always cancel the task.+ static func observeNavigations<S>(+ streamProvider: @MainActor () -> S,+ onEvent: @MainActor (WebPage.NavigationEvent) -> Void = { _ in },+ applyOutcome: @MainActor (NavigationObservationOutcome) -> Bool+ ) async+ where S: AsyncSequence, S.Element == WebPage.NavigationEvent, S.Failure == any Error {+ while !Task.isCancelled {+ // Unconditional, and deliberately not conditional on anything: this loop+ // runs on the MainActor, and an `await` that completes WITHOUT suspending+ // does not yield the executor. So if a page in a permanently-failed state+ // ever threw synchronously on a fresh subscription, the loop would spin the+ // main thread. Rather than resting on "a fresh `page.navigations` always+ // suspends" — a property of a closed-source API that nothing here can pin —+ // the yield makes the loop safe whatever the API does, for the price of one+ // executor hop per subscription (once per navigation, not per event).+ await Task.yield()+ guard !Task.isCancelled else { return }+ let outcome = await drainNavigationStream(streamProvider(), onEvent: onEvent)+ guard !Task.isCancelled else { return }+ guard applyOutcome(outcome) else { return }+ }+ }++ /// Drains ONE subscription to a navigation stream and classifies why it stopped.+ ///+ /// Static on purpose: it must not hold the controller across its `await`, or the+ /// observation task would keep the controller alive for the process's lifetime.+ /// Generic over the sequence so the production signal (`page.navigations`) and a+ /// test's injected stream travel the exact same classification code — the signal+ /// source is the only difference between them.+ static func drainNavigationStream<S>(+ _ sequence: S,+ onEvent: @MainActor (WebPage.NavigationEvent) -> Void = { _ in }+ ) async -> NavigationObservationOutcome+ where S: AsyncSequence, S.Element == WebPage.NavigationEvent, S.Failure == any Error {+ do {+ for try await event in sequence {+ onEvent(event)+ }+ return .streamEnded+ } catch let error as WebPage.NavigationError {+ // `if case` rather than an exhaustive switch: NavigationError is a+ // non-frozen enum, so a future case must degrade to "not a termination"+ // rather than fail to build.+ if case .webContentProcessTerminated = error { return .webContentTerminated }+ if case .pageClosed = error { return .pageClosed }+ return .navigationFailed+ } catch {+ return .navigationFailed+ }+ }++ /// Applies one subscription outcome. Returns whether observation should continue.+ ///+ /// Split from the drain so the recovery decision is testable without a live page,+ /// and so the loop that joins them stays small enough to be pinned by one live+ /// test (`WebContentTerminationWiringTests`).+ @discardableResult+ func applyNavigationOutcome(_ outcome: NavigationObservationOutcome) -> Bool {+ lastNavigationOutcome = outcome+ switch outcome {+ case .navigationFailed:+ // Two different events share this classification, and only the second one+ // is benign. `handleProcessTermination` reloads on the RELAUNCHED process,+ // and that reload can fail provisionally — which the stream reports as an+ // ordinary `failedProvisionalNavigation`, indistinguishable from a bad link+ // by the error alone. Treating it as benign left `isReady` false forever+ // over a blank document, with no retry, no budget charged, and nothing even+ // logged: the original bug, reached by the recovery path itself. The+ // recovery-in-flight flag is what tells the two apart.+ guard recoveryInFlight else { return true }+ return attemptRecovery(reason: .reloadFailed)+ case .pageClosed, .streamEnded:+ return false+ case .webContentTerminated:+ return attemptRecovery(reason: .processTerminated)+ }+ }++ /// Reloads the document to recover the renderer, unless the retry budget is spent.+ /// Returns whether observation continues.+ private func attemptRecovery(reason: RecoveryReason) -> Bool {+ // The budget counts CONSECUTIVE attempts within one failing recovery chain.+ // `recoveryInFlight` is the honest predicate for that: it is set only by a+ // reload this path started and cleared the moment the page reports `ready` (or+ // a fresh `load` supersedes the chain). Using `isReady` instead also charged a+ // crash that happened during an ordinary load, which is not an unproductive+ // recovery at all — the name promised more than the expression delivered.+ unproductiveRecoveries = recoveryInFlight ? unproductiveRecoveries + 1 : 1+ guard unproductiveRecoveries <= Self.maxUnproductiveRecoveries else {+ Self.logger.error(+ "WebContent recovery abandoned after \(Self.maxUnproductiveRecoveries, privacy: .public) unproductive attempts (category: webcontent)"+ )+ recoveryInFlight = false+ cancelRecoveryWatchdog()+ // Reloading in a hot loop is not an option, but neither is silence: from+ // the reader's seat a blank document with nothing but an os_log is exactly+ // the symptom this ticket fixed. The banner offers the reload that used to+ // require closing and reopening the file, and a manual `load` re-arms all+ // of this (see `load`).+ recoveryAbandoned = true+ return false+ }+ guard let documentURL = loadedDocumentURL else {+ // Nothing has been loaded yet, so there is no document to restore.+ // Keep observing: the pending load will arm recovery properly.+ Self.logger.error("WebContent process terminated before any load (category: webcontent)")+ return true+ }+ handleProcessTermination(documentURL: documentURL)+ return true+ }+ /// Clears the per-navigation readiness + pending queue. The snapshot survives /// so it can be replayed once the reloaded page reports ready. private func resetForNavigation() {@@ -655,6 +1040,23 @@ final class WebDocumentController { /// without a live page (task 15). func test_markReady() { markReady() } func test_markLayoutSettled() { markLayoutSettled() }+ /// Whether the navigation observation is currently armed. The give-up path used to+ /// leave the task handle non-nil, which latched the idempotence guard in+ /// `startNavigationObservation` off for the life of the document (T-1943).+ func test_isObservingNavigations() -> Bool { navigationObservationTask != nil }+ /// Swaps the source the observation loop subscribes to. Set before+ /// `test_rearmNavigationObservation()`; pass nil to go back to the real page.+ func test_setInjectedNavigationStream(_ provider: InjectedNavigationStream?) {+ injectedNavigationStream = provider+ }+ /// Cancels the live observation and arms a fresh one, so an injected stream takes+ /// effect on the loop production actually runs — which is what lets a test reach+ /// abandonment through the real arming path instead of simulating it.+ func test_rearmNavigationObservation() {+ navigationObservationTask?.cancel()+ navigationObservationTask = nil+ startNavigationObservation()+ } #endif }
diff --git a/prism/Views/DocumentScrollContent.swift b/prism/Views/DocumentScrollContent.swiftindex cf0c484..e4740e7 100644--- a/prism/Views/DocumentScrollContent.swift+++ b/prism/Views/DocumentScrollContent.swift@@ -64,6 +64,10 @@ struct DocumentScrollContent: View { /// Image services, for the iOS sibling-image folder-access grant flow (Req 3.x). @Environment(\.imageServices) private var imageServices + /// Reduce Motion, honoured by the banner transitions below exactly as both layouts+ /// honour it for theirs (accessibility audit).+ @Environment(\.accessibilityReduceMotion) private var reduceMotion+ /// The web-rendered document surface. Creates the assembly for the session, /// loads the document, and reloads on parseRevision change (Req 2.5). var body: some View {@@ -81,6 +85,17 @@ struct DocumentScrollContent: View { } } .background(context.colors.background)+ // The renderer crashed repeatedly and recovery gave up (T-1943). Without this+ // the reader sees the ORIGINAL symptom — a blank document, no error, close and+ // reopen the only way out — recorded solely in os_log. The reload here is a+ // fresh user-initiated `load`, which is also what restores the retry budget and+ // re-arms crash observation, so one tap returns the document to a fully+ // recoverable state.+ .overlay(alignment: .top) { recoveryAbandonedBanner }+ .animation(+ reduceMotion ? nil : .easeInOut(duration: 0.2),+ value: webController?.recoveryAbandoned ?? false+ ) #if os(iOS) // Sibling-image folder-access grant flow (Req 3.x): a local image that failed for // lack of security-scoped access surfaces this banner. "Grant Access" sets@@ -88,9 +103,12 @@ struct DocumentScrollContent: View { // level (reliable), which grants access + bumps imageAccessReloadToken — observed // here to reload the body so the images retry. .safeAreaInset(edge: .bottom) { imageAccessBanner }- .animation(.easeInOut(duration: 0.2), value: context.coordinator.imageAccessNeededDirectory)+ .animation(+ reduceMotion ? nil : .easeInOut(duration: 0.2),+ value: context.coordinator.imageAccessNeededDirectory+ ) .onChange(of: context.coordinator.imageAccessReloadToken) { _, _ in- reloadWebDocumentForAccess()+ reloadWebDocument() } #endif .task(id: context.session.id) {@@ -274,16 +292,49 @@ struct DocumentScrollContent: View { .buttonStyle(.borderedProminent) } .padding(12)- .background(.regularMaterial, in: RoundedRectangle(cornerRadius: 12))+ .adaptiveMaterialBackground(in: RoundedRectangle(cornerRadius: 12)) .padding(.horizontal, 16) .padding(.bottom, 8) .transition(.move(edge: .bottom).combined(with: .opacity)) } } - /// Reloads the current document so images re-request through the scheme handler after- /// folder access was granted. Same revision — this is a re-fetch, not a re-parse.- private func reloadWebDocumentForAccess() {+ #endif++ /// Banner offering a reload after crash recovery gave up (T-1943). `recoveryAbandoned`+ /// is observed state on the controller, so this appears without any push from here.+ @ViewBuilder+ private var recoveryAbandonedBanner: some View {+ if webController?.recoveryAbandoned == true {+ HStack(spacing: 12) {+ Image(systemName: "exclamationmark.triangle")+ .foregroundStyle(.orange)+ Text("This document stopped rendering.")+ .font(.subheadline)+ .fixedSize(horizontal: false, vertical: true)+ Spacer(minLength: 0)+ Button("Reload") { reloadWebDocument() }+ .buttonStyle(.borderedProminent)+ .controlSize(.small)+ }+ .padding(.horizontal)+ .padding(.vertical, 10)+ // The audited helper, not a bare material: with Reduce Transparency on, a+ // translucent banner over a blank document is exactly finding H4 (T-1044).+ .adaptiveMaterialBackground(in: RoundedRectangle(cornerRadius: 12))+ .shadow(radius: 2)+ .padding()+ .accessibilityElement(children: .combine)+ .accessibilityLabel(LocalizedStringKey("This document stopped rendering."))+ .accessibilityHint(LocalizedStringKey("Reload to try rendering the document again"))+ .transition(.move(edge: .top).combined(with: .opacity))+ }+ }++ /// Reloads the current document at the SAME revision — a re-fetch, not a re-parse.+ /// Used by the iOS folder-access grant flow (images re-request through the scheme+ /// handler once access exists) and by the recovery-abandoned banner.+ private func reloadWebDocument() { guard let webController, context.session.parseRevision > 0 else { return } let revision = context.session.parseRevision let url = WebDocumentControllerFactory.documentURL(@@ -293,7 +344,6 @@ struct DocumentScrollContent: View { webController.load(documentURL: url, parseRevision: revision) restoreScrollPosition(with: webController) }- #endif /// Replays the session's reading position into the freshly-loaded document /// (T-1639), yielding to an undelivered navigation target (T-1775). The
diff --git a/prismTests/WebRendering/WebContentTerminationWiringTests.swift b/prismTests/WebRendering/WebContentTerminationWiringTests.swiftnew file mode 100644index 0000000..de993bb--- /dev/null+++ b/prismTests/WebRendering/WebContentTerminationWiringTests.swift@@ -0,0 +1,630 @@+//+// WebContentTerminationWiringTests.swift+// prismTests+//+// Regression tests for T-1943: the WebContent-termination recovery path existed+// and was fully unit-tested, but NOTHING IN PRODUCTION EVER CALLED IT. A renderer+// kill (memory pressure on a large document) left the document permanently blank.+//+// The lesson this file encodes is about the SHAPE of the tests, not just the fix.+// The pre-existing coverage (`WebDocumentControllerTests.terminationReplaysSnapshot`)+// invokes `handleProcessTermination` directly. Such a test asserts that the handler+// behaves once called and is, by construction, blind to whether anything calls it —+// which is exactly how a completely dead recovery path shipped and survived review.+//+// So the tests here are split along the seam:+//+// 1. `controllerObservesItsOwnPageNavigationStream` is the WIRING pin. It builds a+// REAL controller over a REAL WebPage and asserts the controller sees the real+// page's real navigation events. Delete the `startNavigationObservation()` call+// from the controller's init and this test fails — no other test here does.+// 2. The injected-stream tests pin the CLASSIFICATION and RECOVERY decisions that+// the shared loop makes once a signal arrives. They drive the very same+// `drainNavigationStream` / `applyNavigationOutcome` pair production drives; the+// only thing swapped out is the signal source.+//+// Why the termination itself is injected rather than raised for real: the only way+// to raise a genuine `webContentProcessTerminated` is to kill the WebContent XPC+// process. That was verified to work during this investigation (killing the process+// does make `page.navigations` throw `.webContentProcessTerminated`), but the+// process is a launchd-owned system service shared with every other WebKit client+// on the machine, identifiable only by racy PID set-difference — a test that killed+// the wrong one would take down an unrelated app's web content. The signal is+// injected instead, and test 1 pins that production is subscribed to the real+// source of it. The hand procedure for raising a genuine kill — and the safety+// rules for it — lives in `docs/agent-notes/webview-rendering-status.md`+// ("WebContent termination observation (T-1943)"), so it is repeatable when the+// SDK moves rather than a one-off experiment recorded in a PR description.+//++import Foundation+import Testing+import WebKit+@testable import prism++/// Records the completion of a drain running in its own task, so a test can assert+/// the task actually ENDED rather than merely that it was asked to.+@MainActor+private final class DrainCompletionBox {+ var finished = false+ var outcome: WebDocumentController.NavigationObservationOutcome?+}++@MainActor+struct WebContentTerminationWiringTests {++ // MARK: - Helpers++ private func makeController() -> WebDocumentController {+ WebDocumentController(+ sessionID: "termination-wiring",+ parseRevision: 1,+ schemeHandler: PrismDocSchemeHandler()+ )+ }++ /// A navigation stream that yields `events` and then fails with `error`, matching+ /// the shape of the real `page.navigations` sequence (which ends when it throws).+ private func stream(+ events: [WebPage.NavigationEvent] = [],+ failingWith error: any Error+ ) -> AsyncThrowingStream<WebPage.NavigationEvent, any Error> {+ AsyncThrowingStream { continuation in+ for event in events { continuation.yield(event) }+ continuation.finish(throwing: error)+ }+ }++ /// A controller whose navigation observation is parked on a stream that never+ /// yields and never ends.+ ///+ /// Mandatory for the watchdog tests, and the reason is a trap worth stating: a+ /// controller's REAL `page.navigations` is live from `init`, and the recovery reload+ /// these tests trigger navigates the real page to the fake `prism-doc://` URL, which+ /// fails. That failure arrives as `.navigationFailed` while a recovery is in flight,+ /// so the observation loop charges and retries it — moving `processGeneration` with+ /// no watchdog involved at all. Verified by mutation: with the real observation live,+ /// deleting `startRecoveryWatchdog()` from `handleProcessTermination` left every+ /// watchdog test GREEN. Parking the loop makes the watchdog the only thing that can+ /// move recovery state, which is what these tests claim to measure.+ private func makeControllerWithParkedObservation() -> WebDocumentController {+ let controller = makeController()+ controller.test_setInjectedNavigationStream {+ AsyncThrowingStream { _ in }+ }+ controller.test_rearmNavigationObservation()+ return controller+ }++ /// Waits (up to ~3s) for `condition` to hold. The assertion stays in the test, so a+ /// condition that never holds fails on the expectation rather than on the wait.+ private func poll(+ until condition: @MainActor () -> Bool+ ) async throws {+ for _ in 0..<300 {+ if condition() { return }+ try await Task.sleep(for: .milliseconds(10))+ }+ }++ // MARK: - 1. The production wiring itself++ @Test("The controller observes its own page's real navigation stream")+ func controllerObservesItsOwnPageNavigationStream() async throws {+ // THE fails-when-absent test for T-1943. Everything else in this file works+ // off an injected stream and so stays green even if the controller never+ // subscribes to anything; this one uses the controller's real WebPage, so it+ // is red exactly when the production observation is missing or mis-wired.+ //+ // A successful navigation is the signal deliberately: it needs no failure to+ // engineer, and observing ANY event proves the subscription is attached to+ // the real page. Classifying a termination is tested separately below.+ let session = DocumentSession(clipboardContent: "# Heading\n\nBody text.")+ await session.parseContent()+ let controller = WebDocumentControllerFactory.make(+ session: session, settings: AppSettings()+ )+ #expect(+ controller.lastNavigationEvent == nil,+ "Nothing has navigated yet, so no event can have been observed."+ )++ controller.load(+ documentURL: WebDocumentControllerFactory.documentURL(+ session: session, parseRevision: session.parseRevision+ ),+ parseRevision: session.parseRevision+ )++ // Poll for the navigation to complete (~3s budget).+ var observed: WebPage.NavigationEvent?+ for _ in 0..<60 {+ if controller.lastNavigationEvent == .finished {+ observed = controller.lastNavigationEvent+ break+ }+ try await Task.sleep(for: .milliseconds(50))+ }++ #expect(+ observed == .finished,+ """+ The controller never saw its own page's navigation events, so it is not \+ subscribed to `page.navigations` — which is the ONLY signal WebKit gives \+ for a WebContent process termination. Recovery is dead code again \+ (T-1943). Last event seen: \(String(describing: controller.lastNavigationEvent)).+ """+ )+ }++ // MARK: - 2. Classification of the signal++ @Test("A terminated WebContent process is classified as a termination")+ func terminationErrorIsClassifiedAsTermination() async {+ let outcome = await WebDocumentController.drainNavigationStream(+ stream(failingWith: WebPage.NavigationError.webContentProcessTerminated)+ )+ #expect(outcome == .webContentTerminated)+ }++ @Test("An ordinary failed navigation is not mistaken for a termination")+ func navigationFailureIsNotATermination() async {+ // Verified against the real API: `page.navigations` throws — and ends — for a+ // plain unreachable URL too. Treating that as a crash would reload the+ // document out from under the user every time a link fails to resolve.+ let outcome = await WebDocumentController.drainNavigationStream(+ stream(+ failingWith: WebPage.NavigationError.failedProvisionalNavigation(+ URLError(.cannotFindHost)+ )+ )+ )+ #expect(outcome == .navigationFailed)+ }++ @Test("A closed page ends observation")+ func closedPageIsTerminal() async {+ let outcome = await WebDocumentController.drainNavigationStream(+ stream(failingWith: WebPage.NavigationError.pageClosed)+ )+ #expect(outcome == .pageClosed)+ #expect(makeController().applyNavigationOutcome(.pageClosed) == false)+ }++ @Test("Events reaching the drain are reported before the stream fails")+ func drainReportsEventsBeforeFailing() async {+ var seen: [WebPage.NavigationEvent] = []+ let outcome = await WebDocumentController.drainNavigationStream(+ stream(+ events: [.startedProvisionalNavigation, .committed, .finished],+ failingWith: WebPage.NavigationError.webContentProcessTerminated+ )+ ) { seen.append($0) }+ #expect(seen == [.startedProvisionalNavigation, .committed, .finished])+ #expect(outcome == .webContentTerminated)+ }++ // MARK: - 3. What the controller does with the signal++ @Test("An observed termination runs the recovery the handler implements")+ func observedTerminationTriggersRecovery() {+ let controller = makeController()+ controller.applyTheme(theme: "prism-dark", contrast: .standard, variables: ["--a": "1"])+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ controller.test_markReady()+ controller.test_markLayoutSettled()+ let beforeGeneration = controller.processGeneration++ // The outcome the observation loop hands the controller for a real crash.+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)++ #expect(+ controller.processGeneration == beforeGeneration + 1,+ "A termination must bump the process generation so stale messages are dropped."+ )+ #expect(!controller.isReady)+ #expect(+ controller.pendingCommands.contains(+ .applyTheme(theme: "prism-dark", contrast: .standard, variables: ["--a": "1"], mermaidConfig: "")+ ),+ "Recovery must re-queue the coalesced snapshot so the reloaded page gets native truth."+ )+ }++ @Test("An ordinary navigation failure neither recovers nor stops observation")+ func navigationFailureKeepsObservingWithoutRecovering() {+ // The re-subscription half of the fix. `page.navigations` ends when it throws,+ // and it throws for benign failures, so returning `false` here would let one+ // bad link permanently disarm crash recovery for the rest of the session.+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ let beforeGeneration = controller.processGeneration++ #expect(+ controller.applyNavigationOutcome(.navigationFailed) == true,+ "Observation must continue after a benign navigation failure."+ )+ #expect(controller.processGeneration == beforeGeneration)+ }++ @Test("A termination before any load does not reload, but keeps observing")+ func terminationBeforeFirstLoadIsSurvivable() {+ let controller = makeController()+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(+ controller.processGeneration == 0,+ "There is no document to restore yet, so nothing should be reloaded."+ )+ }++ @Test("Recovery stops after repeated attempts that never reach ready")+ func unproductiveRecoveriesAreCapped() {+ // A WebContent process that cannot be relaunched would otherwise have the+ // observer reload in a hot loop forever.+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ // Each attempt is followed by no `ready`, so none of them counts as progress.+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(+ controller.applyNavigationOutcome(.webContentTerminated) == false,+ "The fourth consecutive unproductive recovery must give up rather than spin."+ )+ }++ @Test("A recovery that reaches ready restores the retry budget")+ func productiveRecoveryResetsTheBudget() {+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)++ // The page comes back this time, so the chain is broken and the budget is+ // whole again: the next three recoveries are permitted from scratch.+ controller.test_markReady()+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(+ controller.applyNavigationOutcome(.webContentTerminated) == false,+ "The budget is restored, not made infinite — the fourth still gives up."+ )+ }++ // MARK: - 4. A recovery reload that fails as an ordinary navigation++ @Test("A recovery reload that fails as an ordinary navigation is retried")+ func failedRecoveryReloadIsRetried() {+ // The sharp edge: `handleProcessTermination` reloads on the relaunched+ // process, and that reload can fail PROVISIONALLY — which the stream reports+ // as an ordinary `failedProvisionalNavigation`, not as a termination. Treating+ // it as "the renderer is alive, keep watching" leaves `isReady` false forever+ // over a blank document with nothing retrying and nothing even logged.+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ let afterFirstRecovery = controller.processGeneration++ #expect(+ controller.applyNavigationOutcome(.navigationFailed) == true,+ "Observation must continue after a failed recovery reload."+ )+ #expect(+ controller.processGeneration == afterFirstRecovery + 1,+ """+ A navigation failure arriving while a recovery reload is in flight IS that \+ recovery failing, so it must be retried — not merely re-observed.+ """+ )+ }++ @Test("Recovery reloads that keep failing as navigation failures exhaust the budget")+ func failedRecoveryReloadsChargeTheBudget() {+ // The retry above must be BOUNDED by the same budget a repeated crash is, or+ // the new path just moves the hot loop rather than removing it.+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.navigationFailed) == true)+ #expect(controller.applyNavigationOutcome(.navigationFailed) == true)+ #expect(+ controller.applyNavigationOutcome(.navigationFailed) == false,+ "Failed recovery reloads must charge the same budget a repeated crash does."+ )+ }++ @Test("An ordinary navigation failure outside a recovery does not charge the budget")+ func benignNavigationFailureDoesNotChargeTheBudget() {+ // A bad link on a healthy, ready page must not consume crash-recovery budget.+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ controller.test_markReady()+ let beforeGeneration = controller.processGeneration++ for _ in 0..<10 {+ #expect(controller.applyNavigationOutcome(.navigationFailed) == true)+ }+ #expect(+ controller.processGeneration == beforeGeneration,+ "A benign failure on a live page reloads nothing."+ )+ // The budget is untouched, so a real crash still gets its full allowance.+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == false)+ }++ @Test("A recovery that reaches ready makes a later navigation failure benign again")+ func readyAfterRecoveryRestoresBenignFailures() {+ // The exact discrimination the whole design rests on, in the direction the other+ // tests do not travel: `recoveryInFlight` must come back DOWN on the recovered+ // page's `ready`, or the next bad link in the document would be misread as a+ // failed recovery and reload the document out from under the reader.+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ controller.test_markReady()+ let afterRecovery = controller.processGeneration++ #expect(controller.applyNavigationOutcome(.navigationFailed) == true)+ #expect(+ controller.processGeneration == afterRecovery,+ """+ The recovery reported ready, so a later navigation failure is an ordinary \+ bad link — reloading on it would make every failed link in a document that \+ once crashed restart the page.+ """+ )+ }++ // MARK: - 5. Abandonment is recoverable, and visible++ @Test("Abandoning recovery raises a user-visible surface")+ func abandonmentIsSurfacedToTheUser() {+ let controller = makeController()+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(!controller.recoveryAbandoned)+ for _ in 0..<3 { _ = controller.applyNavigationOutcome(.webContentTerminated) }+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == false)+ #expect(+ controller.recoveryAbandoned,+ """+ Once the app stops retrying, the user is looking at the ORIGINAL symptom — \+ a blank document with no error. Abandonment has to be observable so the \+ reader is offered a reload instead of being told nothing.+ """+ )+ }++ @Test("A user-initiated load after abandonment restores recovery")+ func loadAfterAbandonmentRestoresRecovery() async throws {+ // `startNavigationObservation` guards on the task handle, so a loop that exits+ // without clearing it latches observation off for the life of the document —+ // the original bug, returning after the budget is spent. A fresh load is fresh+ // evidence that reloading may work.+ //+ // Abandonment is driven through the REAL loop — the controller's own observation+ // task over an injected stream — rather than by calling `applyNavigationOutcome`+ // directly, because the re-arm assertion is about the TASK HANDLE and only the+ // loop clears it. Simulated abandonment left the handle armed from `init` for the+ // whole test, so the final expectation could not fail whatever `load` did+ // (T-1943 review round 2). Mutation-checked: deleting `startNavigationObservation()`+ // from `load()` turns that expectation red.+ let controller = makeController()+ let url = URL(string: "prism-doc://document/x?rev=1")!+ controller.load(documentURL: url, parseRevision: 1)++ controller.test_setInjectedNavigationStream { [self] in+ stream(failingWith: WebPage.NavigationError.webContentProcessTerminated)+ }+ controller.test_rearmNavigationObservation()+ try await poll { controller.recoveryAbandoned }++ #expect(controller.recoveryAbandoned, "A stream that only ever terminates spends the budget.")+ #expect(+ !controller.test_isObservingNavigations(),+ "The loop must clear its own handle when it gives up, or nothing can re-arm it."+ )++ // Back to the real page for the re-arm, so the fresh observation does not+ // immediately re-abandon on the injected stream.+ controller.test_setInjectedNavigationStream(nil)+ controller.load(documentURL: url, parseRevision: 2)++ #expect(!controller.recoveryAbandoned, "A fresh load clears the abandoned state.")+ #expect(+ controller.applyNavigationOutcome(.webContentTerminated) == true,+ "A user-initiated load must restore the retry budget."+ )+ #expect(controller.test_isObservingNavigations(), "…and re-arm the observation.")+ }++ // MARK: - 5b. A recovery reload that stalls (neither fails nor becomes ready)++ @Test("A recovery reload that never reports ready is retried by the watchdog")+ func stalledRecoveryReloadIsRetried() async throws {+ // Every other way a recovery goes wrong is an EVENT the observation loop hears.+ // A reload that neither throws nor reports `ready` — a wedged scheme handler, a+ // load that never completes, a crash landing in the loop's re-subscription gap —+ // produces no event at all, and used to leave `recoveryInFlight` set forever with+ // nothing scheduled: the original symptom, reached from inside its own fix.+ let controller = makeControllerWithParkedObservation()+ controller.recoveryWatchdogTimeout = .milliseconds(20)+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ let afterFirstRecovery = controller.processGeneration++ // Deliberately no `ready` and no further outcome: only the watchdog can move this.+ try await poll { controller.processGeneration > afterFirstRecovery }+ #expect(+ controller.processGeneration > afterFirstRecovery,+ """+ A recovery reload that stalls must be charged and retried. Nothing else can \+ notice it, so without the watchdog the reader is left on a blank document \+ with no banner and no retry (T-1943 review round 2).+ """+ )+ }++ @Test("Recoveries that keep stalling are abandoned rather than reloaded forever")+ func stalledRecoveriesExhaustTheBudget() async throws {+ // The watchdog must charge the SAME budget an observed failure does, or it just+ // moves the hot loop rather than bounding it.+ let controller = makeControllerWithParkedObservation()+ controller.recoveryWatchdogTimeout = .milliseconds(20)+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)++ try await poll { controller.recoveryAbandoned }+ #expect(+ controller.recoveryAbandoned,+ "A permanently stalled recovery must give up and offer the reader a reload."+ )+ }++ @Test("A recovery that reaches ready disarms the watchdog")+ func readyDisarmsTheWatchdog() async throws {+ // The watchdog is a liveness bound, not a reload timer: a recovery that arrived+ // must never be charged for having taken a while.+ let controller = makeControllerWithParkedObservation()+ controller.recoveryWatchdogTimeout = .milliseconds(20)+ controller.load(+ documentURL: URL(string: "prism-doc://document/x?rev=1")!, parseRevision: 1+ )+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)+ let afterRecovery = controller.processGeneration++ controller.test_markReady()+ try await Task.sleep(for: .milliseconds(200))++ #expect(+ controller.processGeneration == afterRecovery,+ "The page came back, so the watchdog must not reload it out from under the reader."+ )+ #expect(!controller.recoveryAbandoned)+ }++ @Test("A fresh load supersedes a pending watchdog")+ func loadSupersedesThePendingWatchdog() async throws {+ // A user-initiated load is a NEWER navigation; a watchdog armed for the one it+ // replaced must not charge the budget against it.+ let controller = makeControllerWithParkedObservation()+ controller.recoveryWatchdogTimeout = .milliseconds(20)+ let url = URL(string: "prism-doc://document/x?rev=1")!+ controller.load(documentURL: url, parseRevision: 1)+ #expect(controller.applyNavigationOutcome(.webContentTerminated) == true)++ controller.load(documentURL: url, parseRevision: 2)+ let afterLoad = controller.processGeneration+ try await Task.sleep(for: .milliseconds(200))++ #expect(+ controller.processGeneration == afterLoad,+ "The stale watchdog fired against a navigation that had already been superseded."+ )+ #expect(!controller.recoveryAbandoned)+ }++ // MARK: - 6. The loop that joins drain and decision++ @Test("The observation loop re-subscribes after a failure and stops on a closed page")+ func observationLoopResubscribesUntilTerminal() async {+ // The composition is the half neither the drain tests nor the decision tests+ // cover: `page.navigations` ENDS when it throws, so a loop that did not take a+ // second subscription would be silently disarmed by the first bad link in a+ // document — with every unit test in this file still green.+ let controller = makeController()+ var subscriptions = 0+ var seen: [WebPage.NavigationEvent] = []++ await WebDocumentController.observeNavigations(+ streamProvider: { [self] in+ subscriptions += 1+ return subscriptions == 1+ ? stream(+ events: [.startedProvisionalNavigation],+ failingWith: WebPage.NavigationError.failedProvisionalNavigation(+ URLError(.cannotFindHost)+ )+ )+ : stream(+ events: [.finished],+ failingWith: WebPage.NavigationError.pageClosed+ )+ },+ onEvent: { seen.append($0) },+ applyOutcome: { controller.applyNavigationOutcome($0) }+ )++ #expect(+ subscriptions == 2,+ """+ A benign navigation failure must be followed by a SECOND subscription. \+ Only one was taken, so observation stopped at the first failure and crash \+ recovery is dead for the rest of the session (T-1943).+ """+ )+ #expect(seen == [.startedProvisionalNavigation, .finished])+ #expect(controller.lastNavigationOutcome == .pageClosed, "A closed page is terminal.")+ }++ // MARK: - 7. Assumed API behaviour, verified rather than assumed++ @Test("Cancelling the observation ends an in-flight drain of a REAL page's stream")+ func cancellingObservationEndsTheRealDrain() async throws {+ // The observation task captures the `WebPage`, so if `page.navigations` did not+ // honour cancellation the task would stay suspended for the process's lifetime+ // holding the page (and its WebContent process) alive after the controller is+ // gone — one leak per closed document. `deinit` cancelling the task is only a+ // fix if cancellation actually ends the drain, so pin that against the REAL+ // sequence rather than assume it.+ let page = WebPage()+ let box = DrainCompletionBox()+ let drain = Task { @MainActor in+ box.outcome = await WebDocumentController.drainNavigationStream(page.navigations)+ box.finished = true+ }+ // Let the task reach its first suspension inside `for try await`.+ try await Task.sleep(for: .milliseconds(200))+ drain.cancel()++ var ended = false+ for _ in 0..<60 {+ if box.finished { ended = true; break }+ try await Task.sleep(for: .milliseconds(50))+ }+ #expect(+ ended,+ """+ `page.navigations` did not end on cancellation, so `deinit`'s \+ `navigationObservationTask?.cancel()` does not actually release the page. \+ The observation must stop capturing `page` strongly (T-1943).+ """+ )+ }+}
diff --git a/docs/agent-notes/webview-rendering-status.md b/docs/agent-notes/webview-rendering-status.mdindex d893b03..4afb08f 100644--- a/docs/agent-notes/webview-rendering-status.md+++ b/docs/agent-notes/webview-rendering-status.md@@ -152,6 +152,75 @@ prints an image title but DROPS a link title, prints the DECODED destination, an and the `[url](url)` discrimination are all defensive against a future non-`format()` source rather than shapes seen today. +## WebContent termination observation (T-1943) — the API facts, and how to re-verify them by hand++`WebPage` gives NO delegate callback, no `NavigationEvent` case, and no Observable+property for a WebContent crash. The only signal is that `page.navigations` **throws**+`WebPage.NavigationError.webContentProcessTerminated`. Three properties follow, and all+three were established by experiment rather than from the documentation (which lists no+failure case on `NavigationEvent` at all):++1. A termination is a THROWN error on the navigation sequence.+2. The sequence **ends when it throws**, and it throws for ordinary navigation failures+ too (an unreachable URL yields `failedProvisionalNavigation` and ends the stream+ identically). An observer that does not re-subscribe is silently disarmed by the+ first bad link in a document.+3. `page.navigations` **honours cancellation** — cancelling the task ends an in-flight+ drain. This is what makes `WebDocumentController`'s strong `page` capture safe;+ without it, every closed document would leak a `WebPage` and its WebContent process.++Fact 3 is pinned in CI by `WebContentTerminationWiringTests.cancellingObservationEndsTheRealDrain`+(real `WebPage`, real sequence). Fact 2's benign half is pinned by+`observationLoopResubscribesUntilTerminal`. Fact 1 is the one CI cannot raise — see+below.++### Manual kill test (the only way to raise a REAL termination)++CI cannot do this, so it is a hand procedure to re-run when the SDK moves or when the+termination branch is edited. **Read the safety note before running it.**++1. Build and run Prism on macOS (`make build-macos`, then launch the app) and open a+ document so a `WebPage` is live and rendering.+2. Note the app's PID: `pgrep -x prism`.+3. Find the WebContent process belonging to **that** app instance:+ `pgrep -f 'com.apple.WebKit.WebContent'` lists every WebKit client's renderer on the+ machine. Narrow to Prism's with+ `ps -Ao pid,ppid,command | grep -i webcontent` and match the one whose command line+ carries Prism's bundle/appPath, or take a PID set difference across launching the app+ (record the set before launch, again after, and use the new PID).+4. Verify the PID before killing it: `ps -p <pid> -o pid,command` must show+ `com.apple.WebKit.WebContent` **and** reference Prism. Then `kill -9 <pid>`.+5. Expected: the document reloads within a moment and comes back with theme, scroll+ position, note markers, and search highlights intact. The log carries+ `Reloading the document: the WebContent process terminated (category: webcontent)` —+ filter with `log stream --predicate 'subsystem CONTAINS "prism"' --info` or Console.app.+ The same line names `the recovery reload failed` / `the recovery reload never reported+ ready` when the reload itself is what went wrong, so the log says which path ran.+6. Exercising the give-up path takes four crashes **without an intervening `ready`** —+ not merely four kills. The budget counts one failing recovery CHAIN: any recovery that+ reports ready breaks the chain and restores the full allowance, so kills spaced far+ enough apart for the document to come back each restart the count at 1 and never+ abandon (that is the correct behaviour, not a failure to reproduce). To reach+ abandonment, kill the relaunched WebContent process again inside the reload window,+ before the recovered document renders — script the PID-verify + `kill` (see the safety+ note: verify each PID, never pattern-kill) and repeat it immediately, or use a document+ large enough that the reload window is comfortably wide. After the fourth unproductive+ attempt the log carries `WebContent recovery abandoned…` and the "This document stopped+ rendering." banner appears with a Reload button. Tapping it must both restore the+ document and re-arm recovery (kill once more — it should recover again).+7. The stall path (a reload that never reports ready) has no kill that raises it by hand;+ it is covered by the watchdog tests in `WebContentTerminationWiringTests`. What is+ worth checking by hand is the negative: open a LARGE document, kill once, and confirm+ the recovered page is not reloaded a second time — a watchdog firing against a slow but+ healthy load would show as a double render. The bound is+ `WebDocumentController.defaultRecoveryWatchdogTimeout` (20s, twice the app's 10s+ render-timeout convention).++**Safety note — do not automate this.** The WebContent process is a launchd-owned system+service shared with every other WebKit client on the machine (Safari, Xcode, Mail, any+Electron-ish app), and identifying Prism's instance is racy. A `pkill` by pattern would+take down unrelated applications' web content. Always PID-verify, never pattern-kill.+ ## Gotchas (don't re-investigate) - **CSP `script-src 'none'` strips `<script type="application/json">` data islands from the DOM on device** (not headless). Data islands must be non-`<script>` (`<div hidden>`, read via `getElementById().textContent`). This caused the entire bridge to be dead on device while headless tests passed (they don't apply the CSP).
diff --git a/prism/Localizable.xcstrings b/prism/Localizable.xcstringsindex 8a1c2e8..2031a1d 100644--- a/prism/Localizable.xcstrings+++ b/prism/Localizable.xcstrings@@ -3865,6 +3865,29 @@ } } },+ "Reload to try rendering the document again": {+ "extractionState": "manual",+ "localizations": {+ "en": {+ "stringUnit": {+ "state": "translated",+ "value": "Reload to try rendering the document again"+ }+ },+ "en-GB": {+ "stringUnit": {+ "state": "translated",+ "value": "Reload to try rendering the document again"+ }+ },+ "en-US": {+ "stringUnit": {+ "state": "translated",+ "value": "Reload to try rendering the document again"+ }+ }+ }+ }, "Rendering diagram": { "extractionState": "manual", "localizations": {@@ -5222,6 +5245,29 @@ } } },+ "This document stopped rendering.": {+ "extractionState": "manual",+ "localizations": {+ "en": {+ "stringUnit": {+ "state": "translated",+ "value": "This document stopped rendering."+ }+ },+ "en-GB": {+ "stringUnit": {+ "state": "translated",+ "value": "This document stopped rendering."+ }+ },+ "en-US": {+ "stringUnit": {+ "state": "translated",+ "value": "This document stopped rendering."+ }+ }+ }+ }, "This will permanently delete all resolved notes. This action cannot be undone.": { "extractionState": "manual", "localizations": {
diff --git a/CLAUDE.md b/CLAUDE.mdindex 3bb9390..2f08372 100644--- a/CLAUDE.md+++ b/CLAUDE.md@@ -50,7 +50,7 @@ The document is rendered by WebKit-for-SwiftUI (`WebView`/`WebPage`). The SwiftU 1. **Parse**: `swift-markdown` → AST → `MarkdownBlock` enum variants (`MarkdownBlockParser`), unchanged from before. T-1558 made the model lossless for nested blockquotes, ordered-list `start`, and rich blocks inside list items (see `specs/web-markdown-fidelity/`). 2. **Emit**: `BlockHTMLEmitter` (`prism/Services/WebRendering/`) is a pure, deterministic function of `[MarkdownBlock]` + `FootnoteData` + `RenderSettings`. It emits one `<section>` per block carrying the content-hash block identity and an occurrence-qualified DOM id (`b-{hash}-{sourceIndex}`, allocated via the shared `BlockDOMID`), escapes by default, and is total (a block that fails to emit falls back to escaped-source `<pre>`, never dropped). `InlineHTMLRenderer` wraps mappable text runs in `<span data-prism-run>` and records a `DocumentSourceMap` (UTF-16 offsets, shipped as an inert `<div hidden>` data island) for selection-anchored notes. `emit` (and the model/service value types it reads) is `nonisolated`, so it runs off the MainActor: `WebDocumentControllerFactory.precomputeDocumentHTML` emits once per `parseRevision` on a `Task.detached` and caches the HTML on `DocumentSession`; the scheme handler serves that cache (synchronous on-main emit only on a miss). Its per-block/inline `HTMLSanitizer` (SwiftSoup) passes are serialized behind a shared `Mutex` because SwiftSoup keeps unsynchronized static pools (T-1681, `specs/offmain-html-emit/`). 3. **Serve**: `PrismDocSchemeHandler` (`prism-doc://` `URLSchemeHandler`) is the single audited I/O path — it serves the document HTML, `document.css` (the only asset fetched through the scheme), and mediates every image subresource through `/img/?src=` (rewritten absolute/relative URLs routed via `ImagePathResolver`/`ImageLoader`/`SVGSourceLoader`). It serves the verbatim CSP (`script-src 'none'`, `connect-src 'none'`, …) as a response header. The document is loaded via the scheme, never `loadHTMLString`.-4. **Host + bridge**: `WebDocumentController`/`WebDocumentView` (`prism/ViewModels/`, `prism/Views/`) own one `WebPage` per session (non-persistent store, `allowsContentJavaScript = false`, all JS injected as user scripts via `WebDocumentControllerFactory.userScripts()`). The native↔JS bridge runs in a dedicated isolated `WKContentWorld`; every message in both directions carries a generation tag (sessionID, parseRevision, processGeneration). `BridgeMessageRouter` validates inbound messages against an enumerated allowlist + exact-generation match and drops forged/stale/malformed ones; `WebDocumentMessageRouter` routes accepted messages onto the existing native session/coordinator state. Outbound commands queue until `ready`; scroll restore waits for `layoutSettled`; on WebContent termination the controller bumps the process generation, reloads, and replays one coalesced state snapshot. `WebDocumentStateSynchronizer` (T-1719) is the single production owner that pushes native truth (sections, details open-state, table modes, notes, typography, comment visibility) to the controller and routes navigation targets (TOC/fragment via `session.pendingAnchorScroll`, notes via `coordinator.noteNavigationTarget`, search current match) through `controller.scrollTo` with `BlockDOMID.navigationDOMID` id translation — Observation-framework driven, so it works with no view mounted; `DocumentScrollContent` mounts the whole assembly via `WebDocumentStateSynchronizer.makeAssembly`. Two inputs are view-fed, because both are view-world environment values: the palette, pushed via `applyTheme(themeKey:contrast:)` — the colorScheme-resolved theme key plus `colorSchemeContrast`, grouped as a `WebPaletteFeed` so a single `.onChange` pushes them together and they can never be applied out of step (T-1829) — and `dynamicTypeSize`, fed in via `start(dynamicTypeSize:)` / `applyDynamicTypeSize(_:)`, which gets NO push of its own: the synchronizer folds it into the typography domain, because `applyTypography` carries one variables dict that wholly replaces the snapshot's typography, so a second pusher would drop the settings-derived half from the recovery replay (T-1828, font-settings Decision 18).+4. **Host + bridge**: `WebDocumentController`/`WebDocumentView` (`prism/ViewModels/`, `prism/Views/`) own one `WebPage` per session (non-persistent store, `allowsContentJavaScript = false`, all JS injected as user scripts via `WebDocumentControllerFactory.userScripts()`). The native↔JS bridge runs in a dedicated isolated `WKContentWorld`; every message in both directions carries a generation tag (sessionID, parseRevision, processGeneration). `BridgeMessageRouter` validates inbound messages against an enumerated allowlist + exact-generation match and drops forged/stale/malformed ones; `WebDocumentMessageRouter` routes accepted messages onto the existing native session/coordinator state. Outbound commands queue until `ready`; scroll restore waits for `layoutSettled`; on WebContent termination the controller bumps the process generation, reloads, and replays one coalesced state snapshot. That termination is observed by `startNavigationObservation()`, armed from the controller's `init` — it was missing entirely until T-1943, so the whole recovery path was dead code in production. `WebPage` offers no delegate callback and no Observable property for a crash: it surfaces as `WebPage.NavigationError.webContentProcessTerminated` **thrown** by `page.navigations`, which ENDS the sequence — and which also throws for ordinary navigation failures — so the observer classifies the error (`drainNavigationStream`) and re-subscribes (`applyNavigationOutcome`), or the first failed navigation would silently disarm crash recovery for the rest of the session. A recovery reload that fails as an ORDINARY navigation is the same failure wearing a different error, so `recoveryInFlight` makes a `.navigationFailed` legible: during a recovery it is charged and retried, outside one it is a benign bad link. Recovery gives up after `maxUnproductiveRecoveries` consecutive attempts that never reach `ready`, rather than reloading in a hot loop — and giving up is neither silent nor permanent: it clears the observation task handle, raises `recoveryAbandoned` (the banner in `DocumentScrollContent`), and any fresh `load` restores the budget and re-arms observation. Because a direct-invocation test cannot see missing wiring (that is exactly how T-1943 survived the cutover and every review), the production subscription is pinned by a live test over a real `WebPage`: `WebContentTerminationWiringTests.controllerObservesItsOwnPageNavigationStream`. `WebDocumentStateSynchronizer` (T-1719) is the single production owner that pushes native truth (sections, details open-state, table modes, notes, typography, comment visibility) to the controller and routes navigation targets (TOC/fragment via `session.pendingAnchorScroll`, notes via `coordinator.noteNavigationTarget`, search current match) through `controller.scrollTo` with `BlockDOMID.navigationDOMID` id translation — Observation-framework driven, so it works with no view mounted; `DocumentScrollContent` mounts the whole assembly via `WebDocumentStateSynchronizer.makeAssembly`. Two inputs are view-fed, because both are view-world environment values: the palette, pushed via `applyTheme(themeKey:contrast:)` — the colorScheme-resolved theme key plus `colorSchemeContrast`, grouped as a `WebPaletteFeed` so a single `.onChange` pushes them together and they can never be applied out of step (T-1829) — and `dynamicTypeSize`, fed in via `start(dynamicTypeSize:)` / `applyDynamicTypeSize(_:)`, which gets NO push of its own: the synchronizer folds it into the typography domain, because `applyTypography` carries one variables dict that wholly replaces the snapshot's typography, so a second pusher would drop the settings-derived half from the recovery replay (T-1828, font-settings Decision 18). 5. **Notes**: `NoteStateFeeder` (`prism/Services/WebRendering/`) maps `NotesManager` state onto `setNoteIndicators`/`setInlineNotes` payloads; `NoteHTMLBuilder` renders the (escaped) bubble/banner HTML natively; `prism-notes.js` (isolated world) injects it as `data-prism-chrome` and posts interaction messages back. Every interactive piece of that chrome is a NATIVE `<button>` or `<a href>` (T-1725) — never a `div`/`span` with `role="button"` — so the user agent supplies focusability, tab order, and Enter/Space activation, and there is no synthetic key handling to keep in sync. Those elements suppress their UA appearance, so `document.css` must reset it; the indicator dot's `font-size: 1em` is load-bearing rather than cosmetic, since the dot's whole gutter geometry is expressed in em. Accessible names are native-owned because the JS cannot reach the string catalog: the indicator's name rides the `setNoteIndicators` payload (`label`, pluralised via `NoteRenderStrings.noteIndicator`), the bubble's action label is baked in by `NoteHTMLBuilder` as visually-hidden text (an `aria-label` there would *replace* the note's own text in the accessible name), and the add-note "+" reads `<main data-prism-add-note-label>`. Both push handlers rebuild all chrome, so each control carries a `data-prism-focus-key` and `prism-notes.js` captures/restores focus around the rebuild. 6. **Search**: counts and navigation order stay in `SearchService`/`SearchCoordinator`. `SearchStateFeeder` translates that into a per-block `setSearchState` payload; `prism-search.js` re-finds the query in each block's rendered text and registers ranges on two named **CSS Custom Highlights** (`prism-search`, `prism-search-current`), windowed to the viewport. The web view's built-in find navigator stays disabled so Cmd+F routes to Prism's search. 7. **Security**: `HTMLSanitizer` (over SwiftSoup) reduces raw HTML embedded in markdown to an allowlist subset on load (Req 1.8/8.1); its `plainText` feeds searchable text. Combined with `allowsContentJavaScript = false` and the served CSP, active-content vectors are blocked by construction.
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 2ba12e8..eda307f 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- A document that goes blank because its rendering process stopped now restores itself (T-1943). The app has always been able to recover from this — it reloads the document and puts back your theme, your reading position, your note markers, and any active search highlights — but nothing was ever watching for the rendering process to stop, so the recovery never actually ran. A large or image-heavy document whose renderer was shut down under memory pressure therefore showed an empty page, with no error and no way back except closing the file and opening it again. The app now watches for it and recovers on the spot. An ordinary failure to load — a link that goes nowhere, an image that cannot be fetched — is told apart from a stopped renderer, so it neither causes a needless reload nor stops the app watching for a real one afterwards. The recovery also covers its own failure: if the reload it starts cannot itself load the document, that counts as the recovery failing and is tried again, instead of leaving the page blank with nothing running. A reload that neither succeeds nor fails — one that simply never finishes — is covered too: it is given a generous time limit, well beyond what even a large document takes to appear, and is then treated as a failed recovery and tried again rather than leaving the page blank indefinitely. If reloading repeatedly fails to bring the document back, the app stops retrying rather than reloading over and over — and says so, with a banner offering to reload. Taking that reload also restores the document's ability to recover on its own again, so giving up is never permanent while the file stays open. - Notes in the document can now be reached with a keyboard, and VoiceOver announces them properly (T-1725). Since the WebKit rendering cutover the note dot beside a block, and the note bubbles shown under one, looked like buttons but were not: Tab walked straight past them, so there was no way to open a note without a pointer, and Enter or Space did nothing even if you got to one. VoiceOver could reach the dot only to announce an unnamed "button", because the dot is drawn rather than written and had no name of its own. The dot and each bubble are now real buttons — focusable, in reading order, and opened by Enter or Space just as by a tap — and the dot announces how many notes it stands for ("Show 3 notes"), while a bubble reads out the note's author, text, and time followed by what activating it does. The "add a document note" control at the top of the document announced itself as a button while answering only Enter; it now presents as the link it is, so what is announced and what the keyboard does agree, and the banner's collapse control now states which notes it hides. Adding, editing, resolving, or deleting a note anywhere in the document used to throw keyboard focus back to the start of the page, because every note control is redrawn each time; focus now stays on the control you were using, and where adding a note replaces a block's **+** button with its note dot, focus moves onto the dot instead of being dropped — as does the reverse, where deleting the last note on a block takes the dot away and brings the **+** back. Focus is only ever moved while you are actually in the document, so saving a note in a sheet no longer risks pulling you out of the text field you are typing in. Two things the announcements turned out to be describing wrongly are fixed with them. The number a dot announces is now the number of notes opening it shows you: a dot on a list said "Show 2 notes" when both notes belonged to items within the list, and then opened an empty panel, because opening a whole block's notes cannot show a note attached to one of its items. Notes attached to a list item or to a table's header row therefore no longer put a dot on the whole block: a list item's note now marks the item itself, and each dot counts only what opening it shows you, while a header-row note is still shown as a bubble under the table and in the notes panel until it gets a dot of its own. And tapping a note bubble now opens that note's own notes rather than the block's, so a note on a list item opens the item's; the hidden text on a bubble says "Show notes" instead of "Edit note", which was never what activation did and is not offered at all on an imported note. Confirming the announcements with VoiceOver and Voice Control on a device is still worth doing by hand. - A note on a list item now shows against that item (T-1745). Adding a note to the second item of a list put the note itself below the whole list, and turned the first item's **+** button into a filled dot — the mark that says "this item has a note" — so the list claimed the note belonged to an item it did not. The note was always attached correctly: copying or exporting notes named the right item, and reopening the document kept it there. Only the display was wrong. A list item's note now appears directly beneath the item it belongs to, the dot appears beside that item, and every other item keeps its **+**. Tapping the dot opens that item's notes rather than the list's, as does tapping the note. Items of a nested list behave the same way, at their own level: adding a note from a nested item's **+** used to file it against that item's parent, which — now that a note is shown against the item it names — would have put it visibly on the wrong line; it now stays on the nested item. A note attached to the list as a whole — one made by selecting text rather than by using an item's **+** — still shows at the list, and no longer takes the first item's **+** away: it sits in its own column beside the items, so every item remains available to note. Table rows are unaffected: their dot already appeared on the right row, and their notes continue to gather below the table, since a note placed inside a cell would distort the table. One place is not covered, and behaves as it did before: for a list inside a collapsible `<details>` section, an item's **+** adds the note to the section rather than to the item, because the app cannot yet tell those items apart — tracked separately (T-2032). - The **Add Note** button that appears when you select text now goes away when the document reloads (T-1852). If a file changed on disk — or a URL document was refreshed — while you had text selected, the selection vanished with the old page but the button stayed floating where it was. Tapping it then opened the note editor quoting text you were no longer looking at, or, if the reload had moved the content around, text from somewhere else entirely. The button is now dismissed the moment a reload starts, including the reload after granting folder access to images and the one that follows a rendering-process restart, and stays away for the rest of the reload: text you drag over while the document is still loading no longer brings the old button back. Each freshly loaded page then confirms for itself that it has no selection.
PR #350 is mergeable: CONFLICTING / mergeStateStatus: DIRTY. GitHub does not dispatch pull_request workflows when the merge ref cannot be computed, which is why head 9643395 shows zero check-runs — not the account-level Actions stall. git merge-tree confirms a single conflict, in CHANGELOG.md; every source file auto-merges. Resolve by keeping both entries under ### Fixed, then re-run the targeted suites on the merged state. I was blocked from performing the merge by the permission classifier.
controllerObservesItsOwnPageNavigationStream polls a real WebPage load for up to 3s and cancellingObservationEndsTheRealDrain for up to 3s more. Green locally on macOS; watch for flakes under simulator load in make test.
Arming from init means every WebDocumentController constructed anywhere in prismTests spawns a MainActor task draining a real navigation stream. Safe here because WebDocumentMessageHandler.controller is weak, the task captures weak self, and deinit cancels — but it is worth confirming on a full make test-quick run that no suite has become slower or flakier from the extra concurrency.
docs/agent-notes/webview-rendering-status.md now carries it, with an explicit “never pattern-kill, always PID-verify” safety note. Worth running once by hand before release, since the classification branch it exercises is the one CI can never raise.