Fix: Make CodeFileDocument's four isolation sites consistent

read(from:ofType:) used a bare MainActor.assumeIsolated, justified by
canConcurrentlyReadDocuments(ofType:) being pinned to false. That
justification was wrong: the pin constrains AppKit's own reads and says
nothing about an in-process caller constructing a document off the main
actor. Commit 1985839 records that exact shape trapping here before and
taking twenty unit tests down with it.

All four nonisolated-override sites now branch on Thread.isMainThread and
assume isolation only on the main-thread side. read and
presentedItemDidChange block with .sync because both must complete before
returning; the LSP notifications and undo registration hop with .async as
they already did.

docs/architecture-decisions.md is corrected too. It previously described
the pin as what made the read path sound, which overstated it.

Author: matthijseikelenboom

CodeEditModules/Sources/CodeEditDocument/CodeFileDocument.swift

@@ -193,20 +193,30 @@ public final class CodeFileDocument: NSDocument, ObservableObject {
             Self.logger.error("Failed to read file from data using encoding: \(rawEncoding)")
             return
         }
-        // `read(from:ofType:)` overrides a nonisolated `NSDocument` method, but everything below
-        // touches main-actor state. Reads are main-thread only, which
-        // `canConcurrentlyReadDocuments(ofType:)` above pins, so stating the isolation is sound.
         let text = nsString as String
-        MainActor.assumeIsolated {
-            self.sourceEncoding = validEncoding
+        let installContents: @MainActor () -> Void = { [self] in
+            sourceEncoding = validEncoding
             if let content {
                 registerContentChangeUndo(fileURL: fileURL, text: text, content: content)
                 content.mutableString.setString(text)
             } else {
-                self.content = NSTextStorage(string: text)
+                content = NSTextStorage(string: text)
             }
             notifyLSPDidOpen()
         }
+
+        // This overrides a nonisolated `NSDocument` method while everything above touches
+        // main-actor state. `canConcurrentlyReadDocuments(ofType:)` keeps AppKit's own reads on the
+        // main thread, but that says nothing about an in-process caller constructing a document off
+        // it, which has happened before and trapped a bare `assumeIsolated` here. So branch, like
+        // ``notifyLSPDidOpen()`` and ``presentedItemDidChange()`` do. Unlike those, this blocks:
+        // `NSDocument` requires the document to be loaded by the time `read` returns, so an async
+        // hop would return an empty document.
+        if Thread.isMainThread {
+            MainActor.assumeIsolated { installContents() }
+        } else {
+            DispatchQueue.main.sync { MainActor.assumeIsolated { installContents() } }
+        }
     }
 
     /// The delegate is main-actor isolated, but document reads and closes can happen off the main

docs/architecture-decisions.md

@@ -140,9 +140,9 @@ That surfaced six pre-existing isolation errors, all in code that is byte-identi
 Both touch main-actor document state.
 Three things now hold that together, and none of them is a static guarantee:
 
-1. `canConcurrentlyReadDocuments(ofType:)` is overridden to return `false`, pinning AppKit's default so reads stay on the main thread. Returning `true` would make the isolation unsound with no compile error.
-2. `read(from:ofType:)` uses `MainActor.assumeIsolated`, which relies on (1).
-3. `presentedItemDidChange()` branches on `Thread.isMainThread`, because it genuinely arrives on the file-presenter thread in production but on the main thread from tests. An unconditional `DispatchQueue.main.sync` deadlocks the second case.
+1. `canConcurrentlyReadDocuments(ofType:)` is overridden to return `false`, pinning AppKit's default so its own reads stay on the main thread. Returning `true` would make that half unsound with no compile error.
+2. All four sites that touch main-actor state from a nonisolated override branch on `Thread.isMainThread` and use `MainActor.assumeIsolated` on the main-thread side: `read(from:ofType:)`, `presentedItemDidChange()`, `notifyLSPDidOpen()`/`notifyLSPDidClose(_:)`, and `registerContentChangeUndo`. The pin is not load-bearing on its own, because it says nothing about an in-process caller constructing a document off the main actor, which has happened here before and trapped a bare `assumeIsolated`.
+3. Two of the four block rather than hop. `read(from:ofType:)` must, because `NSDocument` requires the document loaded by the time it returns; `presentedItemDidChange()` must, or repeated change notifications pile up. Both therefore carry a `DispatchQueue.main.sync`, whose safety rests on no caller blocking the main thread while triggering an off-main read. Nothing enforces that.
 
 This is accepted as a bridge so the extraction can land, not as the end state.
 The real problem is that the type mixes main-actor UI state (`content` is an `NSTextStorage` that SwiftUI observes) with an I/O lifecycle driven from arbitrary threads.