Pre-push review: T-2089 — initial note load overwrites notes created during loading

diff --git a/prism/Services/NotesManager.swift b/prism/Services/NotesManager.swiftindex 3c081e5..a4d664c 100644--- a/prism/Services/NotesManager.swift+++ b/prism/Services/NotesManager.swift@@ -23,6 +23,17 @@ final class NotesManager {     // MARK: - Published State      /// The loaded notes for the current document.+    ///+    /// **Invariant: bump `notes.modifiedAt` before publishing a mutated+    /// container.** Every mutator does, and `applicableNotes` (T-2089) rests on+    /// it: that guard spots a mutation that raced a load by comparing this+    /// container against the one the load captured before its store await, using+    /// `DocumentNotes`' synthesised `Equatable`. A mutator that publishes without+    /// the bump, and whose other fields happen to land back where they started,+    /// compares equal — the guard misses the mutation and the load's stale+    /// snapshot wins, which is the bug T-2089 fixed. The opposite error is+    /// harmless: a container that differs when it need not makes memory win,+    /// already the safe direction.     private(set) var documentNotes: DocumentNotes?      /// Whether notes are currently being loaded.@@ -284,6 +295,9 @@ final class NotesManager {         isLoading = true         defer { isLoading = false } +        // The container as this load found it, for the mutation check below.+        let baselineNotes = documentNotes+         // Load from iCloud (async, non-blocking per 12.3)         let loadedNotes = await store.load(for: identifier) @@ -294,7 +308,13 @@ final class NotesManager {         guard generation == loadGeneration else { return }         guard cachedDocumentPath == identifier.path else { return } -        guard let notes = loadedNotes else { clearNoteState(); return }+        // Neither guard above orders this load against a *mutation*: creating a+        // note bumps no generation and moves no cached path, so both pass and+        // the pre-await snapshot gets published over a note that did not exist+        // when it was read (T-2089).+        guard let notes = applicableNotes(+            loaded: loadedNotes, baseline: baselineNotes, identifier: identifier+        ) else { clearNoteState(); return }          // Partition notes into document-level and block notes         let documentLevelNotes = notes.notes.filter { $0.isDocumentLevel }@@ -322,7 +342,9 @@ final class NotesManager {         if notesWereRelocated {             // Back up the pre-relocation notes store before the new renderer's             // first relocation write (webview-rendering Req 5.7). The backup is-            // taken from the loaded `notes` (pre-relocation state); the store+            // taken from `notes` — the container this load resolved to apply,+            // which since T-2089 may be the in-memory one rather than the loaded+            // snapshot, and is pre-relocation either way; the store             // keeps one backup per document and no-ops if one already exists.             // A backup write failure blocks the relocation write and surfaces             // the notes-error path (design Error Handling).@@ -345,6 +367,101 @@ final class NotesManager {         }     } +    /// The notes a resuming load should apply: its own snapshot, unless the+    /// in-memory container moved on while the load was suspended (T-2089).+    ///+    /// A mutation that lands in that window is invisible to `loadGeneration` and+    /// to the cached-path check, so without this the load republishes state that+    /// predates the mutation: the note disappears from `documentNotes`/+    /// `anchoredNotes`, and a load that also relocates writes the stale set back+    /// to the store from `saveCurrentState()`, losing the note on disk too.+    ///+    /// Deferring to memory is safe because memory is *derived from* the store+    /// rather than a divergent branch of it. Check a writer of `documentNotes`+    /// against the question, not against a roster — the roster has been written+    /// down with a wrong count more than once. The question is: *does what it+    /// publishes reach the store, or does it carry an identifier this load does+    /// not own?* The writers that can move the container inside this load's+    /// window answer it three ways. (The remaining ones need no argument: this+    /// function and `clearNoteState` are the load itself, ordered by the+    /// generation guard; `noteContainer` publishes what it just read from the+    /// store; `saveCurrentState` ends in `persistNotes`.)+    ///+    /// - The creation paths — `createAndPersistNote`, `createReply` and+    ///   `handleDocumentNoteCreation` — take their baseline from+    ///   `noteContainer(for:)`, which reads the store itself, and persist what+    ///   they append before returning. A container they moved therefore equals+    ///   what this load read plus mutations already on disk.+    /// - The in-place mutators — `updateNote`, `deleteNote`, `toggleStatus`,+    ///   `clearResolved` and `reattachNote` — never read the store; their+    ///   baseline *is* the in-memory container. Each publishes its mutation and+    ///   then ends in `await persistNotes(notes)`, so what it published is on+    ///   its way to the store: memory is the store's near future, and this+    ///   load's snapshot is its past. That, and not their `documentNotes == nil`+    ///   early return, is what covers them — they really can run inside a load.+    ///   `loadNotes` clears the container *before its store await* only on its+    ///   `guard iCloudAvailable` branch (its other `clearNoteState()` runs after+    ///   that await), so a document switch runs the new document's *first* load+    ///   with the previous container still in memory; and a creation inside this+    ///   load's own window publishes a container (`noteContainer`) for a mutator+    ///   to work on. Neither is gated on `isLoading`.+    /// - `migrateNotes` answers the second half instead: it republishes the+    ///   container under a *different* `identifier` (before its save, so the+    ///   convergence argument does not reach it). It needs neither — the+    ///   identifier check below declines that container and this load applies+    ///   its own result, which is the correct T-1811 behaviour. Its save-failure+    ///   revert answers *neither* half — it republishes under the *source*+    ///   identifier and never persists — but it restores the container to the+    ///   value it held before the migration, so a source-document load in flight+    ///   finds `current == baseline`, the guard declines, and that load applies+    ///   its own result. That is the one writer here whose safety rests on the+    ///   equality rather than on the question.+    ///+    /// For the first two groups this is the mirror of `noteContainer`'s own+    /// re-read across its store await — whichever of the two resumes last defers+    /// to what the other published. A future writer that can answer neither half+    /// — publishing state under this load's own identifier that never reaches+    /// the store — would reopen this bug and needs its own argument here.+    ///+    /// **Constraint: `NotesStore.load` and `save` must not gain a suspension+    /// point.** The converse premise — memory *unchanged* means the loaded+    /// snapshot is at least as fresh as memory — holds only because both are+    /// `async` yet contain no `await`, so actor isolation serialises them and a+    /// save enqueued before a load is always visible to that load. Give either an+    /// internal await — an `NSFileCoordinator` for iCloud is the live candidate,+    /// proposed by the multi-window notes work in T-1723/T-1895 — and a mutation+    /// that publishes *before* this load captures its baseline but whose write+    /// lands *after* the load's read reopens this bug in a shifted window:+    /// `current == baseline`, the guard does not fire, the stale snapshot wins.+    /// `NotesStore.loadFromCurrentPath` carries the sibling caution for the same+    /// reason; the two have to be maintained together.+    ///+    /// The comparison is against a captured baseline rather than a mutation+    /// counter deliberately: `documentNotes` is assigned from many places, and a+    /// hand-bumped counter reopens this exact bug silently the first time one of+    /// them is forgotten. A `didSet` *could* automate the bump —+    /// `@Observable` keeps observing a stored property that carries+    /// `willSet`/`didSet`; only genuinely computed get/set properties drop out of+    /// observation, which is why `AppSettings` re-adds `access`/`withMutation` by+    /// hand for those — but an automated counter carries no information the value+    /// comparison does not already carry, so it would be a second piece of state+    /// to keep in step for no gain.+    ///+    /// The identifier check keeps this narrow: a container for *another*+    /// document is not fresher truth about this one, it is the T-1811 hazard,+    /// and the existing behaviour of applying this load's own result is correct+    /// there.+    private func applicableNotes(+        loaded: DocumentNotes?,+        baseline: DocumentNotes?,+        identifier: DocumentIdentifier+    ) -> DocumentNotes? {+        guard let current = documentNotes,+              current.identifier == identifier,+              current != baseline else { return loaded }+        return current+    }+     /// Check if any notes were relocated during relocation.     private func clearNoteState() {         documentNotes = nil

diff --git a/prism/Services/NotesStore.swift b/prism/Services/NotesStore.swiftindex 12486cb..ce7c211 100644--- a/prism/Services/NotesStore.swift+++ b/prism/Services/NotesStore.swift@@ -87,6 +87,10 @@ actor NotesStore: NotesStoreProtocol {     /// legacy file using the old slash-to-underscore encoding and migrates it     /// to the new filename (T-459).     /// - Requirement: 1.4+    /// - Important: `async` but with no internal `await` — two callers depend on+    ///   that. Before adding a suspension point here (an `NSFileCoordinator` for+    ///   T-1723/T-1895 is the live candidate), read the T-2089 caution in+    ///   `loadFromCurrentPath`; the same applies to `save`.     func load(for identifier: DocumentIdentifier) async -> DocumentNotes? {         guard let url = fileURL(for: identifier) else { return nil } @@ -115,6 +119,15 @@ actor NotesStore: NotesStoreProtocol {             // Synchronous method — no suspension points between decode failure and             // quarantine, so actor isolation keeps load and save mutually exclusive.             // Promoting this method to `async` would invalidate that guarantee.+            //+            // A second caller depends on the same property: `load` and `save`+            // being `async` with no internal `await` is what lets+            // `NotesManager.applicableNotes` (T-2089) read "in-memory container+            // unchanged" as "the loaded snapshot is at least as fresh". Adding a+            // suspension point here or in `save` — an `NSFileCoordinator` for the+            // multi-window/iCloud work in T-1723/T-1895 is the live candidate —+            // reopens that race in a shifted window and needs a fix on that side+            // too, not just here.             logger.error(                 "Corrupt notes file at current path \(identifier.path, privacy: .public): \(String(describing: error), privacy: .public)"             )@@ -186,6 +199,10 @@ actor NotesStore: NotesStoreProtocol {     /// Save notes to iCloud.     /// Also cleans up any orphaned legacy file for this identifier (T-459).     /// - Requirement: 1.5+    /// - Important: `async` but with no internal `await` — two callers depend on+    ///   that. Before adding a suspension point here (an `NSFileCoordinator` for+    ///   T-1723/T-1895 is the live candidate), read the T-2089 caution in+    ///   `loadFromCurrentPath`; the same applies to `load`.     func save(_ notes: DocumentNotes) async throws {         guard let url = fileURL(for: notes.identifier) else {             throw NSError(

diff --git a/prismTests/NotesManagerLoadRaceTests.swift b/prismTests/NotesManagerLoadRaceTests.swiftindex 4084794..7719afc 100644--- a/prismTests/NotesManagerLoadRaceTests.swift+++ b/prismTests/NotesManagerLoadRaceTests.swift@@ -49,6 +49,12 @@ struct NotesManagerLoadRaceTests {         }          func load(for identifier: DocumentIdentifier) async -> DocumentNotes? {+            // Snapshot BEFORE the delay: a load returns what the store held when it+            // was called, and the delay models a slow return, not a late read.+            // Reading after the sleep would silently launder writes that landed+            // during it into the "loaded" result and hide every load-versus-write+            // race this file exists to pin (T-2089).+            let snapshot = storedNotes[identifier.path]             if identifier.path == delayPath {                 if delayFirstLoadOnly {                     if !hasDelayedFirstLoad {@@ -59,7 +65,7 @@ struct NotesManagerLoadRaceTests {                     try? await Task.sleep(for: delayDuration)                 }             }-            return storedNotes[identifier.path]+            return snapshot         }          func save(_ notes: DocumentNotes) async throws {@@ -397,4 +403,251 @@ struct NotesManagerLoadRaceTests {         #expect(manager.anchoredNotes[oldBlock.id] == nil)         #expect(manager.importedNotes[newBlock.id]?.count == 1)     }++    // MARK: - Load vs. Mutation Race (T-2089)++    /// T-2089: `loadGeneration` only orders load against *load*. A note created+    /// while a load is parked in `store.load` leaves the generation untouched, so+    /// the load's guards pass and it publishes a snapshot that predates the note —+    /// dropping it from memory, and (when the load relocates) writing the stale set+    /// over it in the store.+    ///+    /// This is the nil-result half: the store held nothing for the document, so the+    /// resuming load takes the `clearNoteState()` branch and wipes the new note.+    @Test("Note created during an in-flight load survives a nil store result")+    @MainActor+    func noteCreatedDuringLoadSurvivesNilStoreResult() async {+        let store = DelayedNotesStore()++        let url = URL(fileURLWithPath: "/Users/test/project/specs/doc.md")+        let path = "project/specs/doc.md"+        let block = makeBlock("Document content")++        // Nothing stored for this document — the load resolves to nil.+        // Only the load's own read is delayed; the creation's read resolves at once.+        await store.setFirstLoadDelay(for: path)++        let manager = NotesManager.makeForTesting(store: store)+        let sessionID = UUID()++        let load = Task {+            await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])+        }++        // Let the load reach its delayed store read, then create a note.+        try? await Task.sleep(for: .milliseconds(50))+        await manager.createDocumentNote(+            content: "Created while loading", source: .file(url: url), sessionID: sessionID+        )+        #expect(manager.documentNotes?.notes.count == 1)++        await load.value++        // The load must not publish its pre-await nil over the newer note.+        #expect(manager.documentNotes?.notes.count == 1)+        #expect(manager.documentNotes?.notes.first?.content == "Created while loading")+        #expect(manager.anchoredNotes[BlockNote.documentSentinelId]?.count == 1)+    }++    /// T-2089: the loaded-snapshot half. The store held a note, so the resuming+    /// load publishes that snapshot — which predates the note created during the+    /// await — and the new note disappears from memory.+    @Test("Note created during an in-flight load is not overwritten by the loaded snapshot")+    @MainActor+    func noteCreatedDuringLoadSurvivesLoadedSnapshot() async {+        let store = DelayedNotesStore()++        let url = URL(fileURLWithPath: "/Users/test/project/specs/doc.md")+        let path = "project/specs/doc.md"+        let block = makeBlock("Document content")++        let storedNote = makeNote(blockId: block.id, content: "Document content")+        await store.preload(makeDocumentNotes(+            path: path, displayName: "doc.md", notes: [storedNote]+        ))+        await store.setFirstLoadDelay(for: path)++        let manager = NotesManager.makeForTesting(store: store)+        let sessionID = UUID()++        let load = Task {+            await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])+        }++        try? await Task.sleep(for: .milliseconds(50))+        await manager.createDocumentNote(+            content: "Created while loading", source: .file(url: url), sessionID: sessionID+        )++        await load.value++        // Both the stored note and the one created mid-load must be present.+        #expect(manager.documentNotes?.notes.count == 2)+        #expect(manager.documentNotes?.notes.contains { $0.content == "Created while loading" } == true)+        #expect(manager.anchoredNotes[BlockNote.documentSentinelId]?.count == 1)+        #expect(manager.anchoredNotes[block.id]?.count == 1)+    }++    /// T-2089, the data-loss half: the resuming load relocates the stale snapshot+    /// and runs `saveCurrentState()`, which writes that snapshot over the store+    /// entry the mid-load creation had already persisted. The note is then gone+    /// from disk too, not just from memory.+    @Test("Relocation save from a resuming load does not delete a note created during it")+    @MainActor+    func relocationSaveDoesNotDeleteNoteCreatedDuringLoad() async {+        let store = DelayedNotesStore()++        let url = URL(fileURLWithPath: "/Users/test/project/specs/doc.md")+        let path = "project/specs/doc.md"+        let block = makeBlock("Old block content")++        // Anchored to a stale block ID but quoting text close enough to fuzzy-match,+        // so the load relocates and takes the backup + saveCurrentState tail.+        let storedNote = makeNote(blockId: "stale-block-id", content: "block content")+        await store.preload(makeDocumentNotes(+            path: path, displayName: "doc.md", notes: [storedNote]+        ))+        await store.setFirstLoadDelay(for: path)++        let manager = NotesManager.makeForTesting(store: store)+        let sessionID = UUID()++        let load = Task {+            await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])+        }++        try? await Task.sleep(for: .milliseconds(50))+        await manager.createDocumentNote(+            content: "Created while loading", source: .file(url: url), sessionID: sessionID+        )++        // The creation persisted both notes before the load resumes.+        let persistedDuringLoad = await store.storedNotes[path]?.notes.count+        #expect(persistedDuringLoad == 2)++        await load.value++        // Guard against a vacuous pass: if the stored note stopped relocating, the+        // save tail would never run and the assertion below would hold regardless.+        #expect(manager.anchoredNotes[block.id]?.count == 1)++        let persistedAfterLoad = await store.storedNotes[path]?.notes ?? []+        #expect(persistedAfterLoad.count == 2)+        #expect(persistedAfterLoad.contains { $0.content == "Created while loading" })+    }++    /// T-2089, the in-place-mutator half. The creation paths take their baseline+    /// from `noteContainer(for:)`, which reads the store; the in-place mutators+    /// (`updateNote`, `deleteNote`, `toggleStatus`, `clearResolved`,+    /// `reattachNote`) never read the store and take their baseline from+    /// `documentNotes`. Their safety argument is therefore a different one —+    /// each ends in `persistNotes`, so what they publish is on its way to the+    /// store — and it needs its own coverage. A delete landing during a *reload*+    /// is the shape: the load's snapshot still holds the deleted note, so+    /// republishing it resurrects one the user just removed.+    ///+    /// What this pins is the **deferral** — return `loaded` instead of `current`+    /// (pre-fix behaviour) and the reload brings `doomed` back. It does *not*+    /// pin the `current != baseline` clause: dropping that clause makes memory+    /// win unconditionally, which is exactly what the assertions below want, so+    /// this test would still pass. `reloadWithoutMutationAppliesStoreSnapshot`+    /// is the one that pins that clause. (The commit that added this test+    /// claimed the opposite; the two tests' roles are not interchangeable.)+    @Test("Note deleted during an in-flight reload is not resurrected by the loaded snapshot")+    @MainActor+    func noteDeletedDuringReloadIsNotResurrected() async {+        let store = DelayedNotesStore()++        let url = URL(fileURLWithPath: "/Users/test/project/specs/doc.md")+        let path = "project/specs/doc.md"+        let block = makeBlock("Document content")++        let keep = makeNote(blockId: block.id, content: "Document content")+        let doomed = makeNote(blockId: block.id, content: "Document content")+        await store.preload(makeDocumentNotes(+            path: path, displayName: "doc.md", notes: [keep, doomed]+        ))++        let manager = NotesManager.makeForTesting(store: store)+        let sessionID = UUID()++        // First load is undelayed, so the reload below starts with a non-nil+        // `documentNotes` — the state the in-place mutators require.+        await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])+        #expect(manager.documentNotes?.notes.count == 2)++        // Delay the reload's store read so the delete lands inside its window.+        await store.setDelay(for: path)++        let reload = Task {+            await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])+        }++        try? await Task.sleep(for: .milliseconds(50))++        // Guard against a vacuous pass: the window is a 200 ms store delay+        // against this 50 ms sleep, and if the delete landed *after* the reload+        // had applied its snapshot the sequence would be [keep, doomed] →+        // delete → [keep] and every assertion below would still hold, pinning+        // nothing. `isLoading` is true only between `loadNotes`' iCloud guard+        // and its return, so this fails rather than passes silently if the+        // delete misses the window. It is exact here only because the stored+        // notes already anchor to `block`, so the reload does not relocate and+        // has no await between publishing its snapshot and returning — `isLoading`+        // therefore still means "parked in `store.load`". Give this test a+        // relocating fixture and the flag would stay true across the backup tail+        // too, weakening the guard to "has not returned yet".+        #expect(manager.isLoading)++        await manager.deleteNote(doomed.id)+        #expect(manager.documentNotes?.notes.count == 1)++        await reload.value++        // The reload read both notes before the delete, so applying its snapshot+        // would bring the deleted note back in memory and, on the next write, on+        // disk. Memory must win.+        #expect(manager.documentNotes?.notes.count == 1)+        #expect(manager.documentNotes?.notes.contains { $0.id == doomed.id } == false)+        #expect(manager.documentNotes?.notes.contains { $0.id == keep.id } == true)+        #expect(manager.anchoredNotes[block.id]?.count == 1)++        let persisted = await store.storedNotes[path]?.notes ?? []+        #expect(persisted.count == 1)+        #expect(persisted.contains { $0.id == doomed.id } == false)+    }++    /// The T-2089 guard defers to memory only when memory actually moved. A plain+    /// reload with no concurrent mutation must still apply what the store holds,+    /// or the fix would turn every reload into a no-op.+    @Test("Reload without a concurrent mutation still applies the store snapshot")+    @MainActor+    func reloadWithoutMutationAppliesStoreSnapshot() async {+        let store = MockNotesStore()++        let url = URL(fileURLWithPath: "/Users/test/project/specs/doc.md")+        let path = "project/specs/doc.md"+        let block = makeBlock("Document content")++        let first = makeNote(blockId: block.id, content: "Document content")+        await store.preload(makeDocumentNotes(+            path: path, displayName: "doc.md", notes: [first]+        ))++        let manager = NotesManager.makeForTesting(store: store)+        let sessionID = UUID()+        await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])+        #expect(manager.documentNotes?.notes.count == 1)++        // The store gains a second note out of band (e.g. an iCloud sync), then the+        // document is reloaded with no in-memory mutation in between.+        let second = makeNote(blockId: block.id, content: "Document content")+        await store.preload(makeDocumentNotes(+            path: path, displayName: "doc.md", notes: [first, second]+        ))+        await manager.loadNotes(source: .file(url: url), sessionID: sessionID, blocks: [block])++        #expect(manager.documentNotes?.notes.count == 2)+        #expect(manager.anchoredNotes[block.id]?.count == 2)+    } }

diff --git a/docs/agent-notes/notes-system.md b/docs/agent-notes/notes-system.mdindex d582180..54b4f25 100644--- a/docs/agent-notes/notes-system.md+++ b/docs/agent-notes/notes-system.md@@ -155,6 +155,25 @@ Regression coverage is in `NotesManageriCloudSignInTests`, one test per guard: `  Its store read is a suspension point, so two creations that both arrive with nothing loaded (or with another document's container loaded) both reach it and both resume with a baseline read before the other's note existed. Whichever saves last would delete the other's note. The fix is the re-read of `documentNotes` *after* the store await, before `stored` is used: every creation path publishes its appended container to `documentNotes` synchronously — no `await` between `noteContainer` returning and the assignment — so a second caller resuming there sees the first note and builds on it. Keep that property when touching a creation path. Deduplicating the store read itself (one shared in-flight task) would save a redundant read but is not what makes this safe. Pinned by `concurrentCreationsWithNothingLoadedKeepBothNotes`. +## Load vs. Mutation (T-2089)++`loadNotes`' guards at the top of the function are all load-versus-load: `cachedDocumentPath` (T-369) separates documents, `loadGeneration` (T-1556/T-1586) orders reloads of one document. None of them sees a *mutation*. Creating a note bumps no generation and moves no cached path, so a note created while a load sat in `store.load` passed every guard and was then overwritten by the load's pre-await snapshot — and, when the load relocated, deleted from the store by `saveCurrentState()`. The window is ordinary: note creation is gated on `iCloudAvailable`, never on `isLoading`, so the document is fully interactive while its notes load.++`applicableNotes(loaded:baseline:identifier:)` closes it. `loadNotes` captures `documentNotes` as `baselineNotes` immediately before the store await; if the container is for the same document and no longer equals that baseline, it superseded the snapshot and is what gets relocated and applied. Four things about it are easy to get wrong:++- **Deferring to memory is safe because memory is derived from the store, not a divergent branch of it — but the reason splits by writer group.** Check a writer against the question rather than against a roster; the roster has been written down with a wrong count more than once. The question: *does what it publishes reach the store, or does it carry an identifier this load does not own?* The *creation* paths (`createAndPersistNote`, `createReply`, `handleDocumentNoteCreation`) take their baseline from `noteContainer(for:)`, which reads the store itself, and persist what they append; a container they moved equals what the load read plus mutations already on disk. The *in-place* mutators (`updateNote`, `deleteNote`, `toggleStatus`, `clearResolved`, `reattachNote`) never touch the store — their baseline is the in-memory container — and converge because each ends in `persistNotes`: what they publish is on its way to the store, so memory is the store's near future and the load's snapshot is its past. Do not check the first argument against `deleteNote` and conclude the guard is broken; it is the second argument that covers it. **Do not reach for "the mutators no-op while `documentNotes == nil`, so they cannot run during an initial load" either — that premise is false in two reachable shapes,** and an earlier version of this note and of the code comment asserted it: `loadNotes` clears the container *before its store await* only on its `guard iCloudAvailable` branch (its other `clearNoteState()` runs after that await), so a document switch runs the new document's *first* load with the previous container still in memory; and a creation inside the load's own window publishes a container via `noteContainer` for a mutator to work on. Neither is gated on `isLoading`. The `persistNotes` convergence covers both, and the nil premise never applied to a *reload* anyway. `migrateNotes` answers the second half instead of the first — it republishes the container under a different identifier, before its save — so the identifier check declines it and the load applies its own result. Its save-failure revert answers *neither* half (it republishes under the *source* identifier and never persists), and is safe for a third reason: it restores the container to the value it held before the migration, so an in-flight source-document load finds `current == baseline` and applies its own result. For the first two groups this mirrors `noteContainer`'s post-await re-read: whichever of the two resumes last defers to what the other published, and between them the window is closed from both ends.+- **It depends on `NotesStore.load`/`save` never suspending internally.** The converse premise — memory *unchanged* means the loaded snapshot is at least as fresh — holds only because both are `async` with no internal `await`, so actor isolation serialises them and a save enqueued before a load is always visible to it. Add a suspension point and the bug reopens in a *shifted* window: a mutation that publishes before the load captures its baseline but whose write lands after the load's read leaves `current == baseline`, the guard does not fire, and the stale snapshot wins. This is a live forward hazard, not a hypothetical — the multi-window notes work in **T-1723/T-1895** proposes iCloud-appropriate `NSFileCoordinator` file coordination, which is exactly that change. `NotesStore.loadFromCurrentPath` carries the sibling caution (quarantine-vs-save exclusivity) and now cross-references this one; maintain the two together.+- **Merging the two by note id is wrong, not merely redundant.** A delete that lands after the load's read leaves the note present in the snapshot and absent from memory, so a union resurrects it. There is no per-note version information to arbitrate with.+- **Capture-and-compare, not a mutation counter.** `documentNotes` is assigned from many places and a hand-bumped counter reopens this bug silently the first time one is forgotten. A `didSet` *could* automate the bump — verified on Swift 6.3.3 / Xcode 26.6, `@Observable` keeps observing a stored property carrying `willSet`/`didSet` and the observer still fires; only genuinely computed get/set properties drop out, which is why `AppSettings` re-adds `access(keyPath:)`/`withMutation(keyPath:)` by hand for those. (An earlier version of this note and of the code comment claimed the opposite — do not reuse that claim.) The counter is rejected on a different ground: automated or not, it carries no information the value comparison does not already carry, so it is a second piece of state to keep in step for no gain.++Equality is what makes the compare reliable, and it rests on an unwritten-until-now invariant: **every mutator stamps `notes.modifiedAt` before publishing.** `DocumentNotes` is synthesised `Equatable` over an order-sensitive array, so a mutator that publishes without the bump and whose other fields land back where they started compares equal and defeats the guard. The invariant is recorded on the `documentNotes` declaration, where a mutator author will see it. A false *positive* (differing when it need not) is harmless — memory wins, already the safe direction.++The identifier check keeps it narrow: a container for another document is not fresher truth about this one, it is the T-1811 hazard, and applying the load's own result is right there. It also cannot be defeated by a mid-load document switch, because a switch runs `loadNotes`, whose prologue bumps `loadGeneration` before any await — the switch case is retired by the generation guard well before `applicableNotes` runs. `migrateNotes` is the one writer that moves cached identity *without* bumping the generation. It does rewrite `cachedDocumentPath`, so a load for the *source* document that resumes after that rewrite is retired by the path guard — but **the two writes straddle the save**: in `migrateNotes`, the container is rebound to the target identifier (`documentNotes = notes`, right after `notes.identifier = targetIdentifier`) *before* `await store.save`, while `cachedDocumentPath = targetIdentifier.path` runs only *after* it, past the T-1812 re-check. (Line numbers were cited here originally and were stale within the same commit that added them — describe the anchors, not the lines.) A source-identifier load resuming inside that window passes both guards, and `applicableNotes` returns `loaded` (the container's identifier is now the target, so the identifier check declines it), applying the pre-migration snapshot over the rebinding. That is unchanged from pre-T-2089 behaviour — the old code overwrote unconditionally — so it is a pre-existing window this guard neither opens nor closes, not a regression; do not cite the path guard as covering it. The identifier-versus-generation asymmetry is the same one already called out for T-1811.++Regression coverage in `NotesManagerLoadRaceTests`: `noteCreatedDuringLoadSurvivesNilStoreResult` (the `clearNoteState()` branch), `noteCreatedDuringLoadSurvivesLoadedSnapshot` (the publish branch), `relocationSaveDoesNotDeleteNoteCreatedDuringLoad` (the disk loss), `noteDeletedDuringReloadIsNotResurrected` (the in-place-mutator group, via `deleteNote` — every other test drives a creation path), and `reloadWithoutMutationAppliesStoreSnapshot` so the guard cannot be widened into "loads never apply anything". The last two pin *different halves* and are easy to conflate: the deletion test pins the **deferral** (return `loaded` and the deleted note comes back), while only `reloadWithoutMutationAppliesStoreSnapshot` pins the **`current != baseline` clause** — dropping that clause makes memory win unconditionally, which the deletion test asserts anyway, so it would still pass.++**`DelayedNotesStore.load` must snapshot before it sleeps.** It used to read `storedNotes` after the artificial delay, which models a load that observes writes made while it was suspended — laundering the mid-load creation into the "loaded" result and hiding every load-versus-write race in the file. All three T-2089 tests passed against the unfixed code until that was corrected.+ ## Session Change State Reset (T-407)  Shared session-scoped state lives in `DocumentLayoutCoordinator` and is reset via `coordinator.resetSessionState()`, called from both layouts' `.onChange(of: session.id)` handlers. Add new shared session-scoped state to `DocumentLayoutCoordinator.resetSessionState()`. Layout-specific state (e.g. sidebar visibility, search overlay) remains in each layout's `onChange` handler.

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 2814e59..4de52ef 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -92,6 +92,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Notes: rapidly reloading the same document no longer lets an older reload back up or persist notes state that a newer reload has already replaced. The load-generation guard is now re-checked after the backup step, before the state is saved. - Opening a file from the Recents list while a pasted (clipboard) document has unsaved content now shows the Unsaved Changes confirmation instead of silently replacing the unsaved document. Previously only bundled and URL-sourced recents were guarded; regular file recents skipped the dialog. Save and Discard both continue on to open the chosen recent file (T-1630). - HTML comments embedded in paragraph text (and in headings, list items, and table cells) render again as dimmed inline annotations when "Show HTML comments" is on — they were silently dropped by the new rendering engine (T-1638). Stand-alone comments nested inside blockquotes or list items now get the same toggleable annotation treatment instead of leaking as always-visible plain text, and comment annotations regained their info indicator glyph. Search now stays aligned with what is actually rendered: comment shapes that are never displayed (conditional comments, note-infrastructure tags) no longer count as matches when the toggle is on, and hidden comment text nested inside blockquotes or list items no longer produces phantom highlights.+- A note written while a document's notes are still loading is no longer thrown away when the load finishes (T-2089). Notes can be added the moment a document appears, but loading them from iCloud takes a moment longer — and a note added in that gap vanished from the screen as soon as the load landed, because the load put back the set of notes it had read before your note existed. Where that load also had to re-attach notes to moved text, it saved that older set back to iCloud too, so the note was gone for good rather than just off-screen until the next reopen. The load now recognises when the notes have moved on beneath it and keeps what is on screen — your note and everything already stored — instead of replacing it. A reload with nothing added still picks up whatever iCloud holds, as before.  ### Security