prism branch T-2231/bugfix-…-manager-key commits 6 files 7 touched lines +1643 / -71 production code 1 file, +267 / -19 tests 23 in 2 suites, all green

Pre-push review: T-2231 clipboard Save As notes migration

Round-four gate on PR #401 (head 5c0f3d11). A clipboard Save As could finish with the session on the saved file while NotesManager stayed keyed to the clipboard, silently writing every later note to a record the reopened file never reads. The fix gives a migration ownership of every identity that still addresses the document it is moving, for the whole call.

At a glance

  • The bug. migrateNotes changes identity in two halves around await store.save: the container is republished under the target before the save, the cached identity rebound and the source record deleted after it. For the length of that await every existing guard still admitted a load of the source — current generation, cached path still naming the source — so such a load republished the pre-migration snapshot, the post-save guard read it as "a newer document replaced me", and the migration returned true without doing its second half.
  • The fix. A migration claims the identities that still address its document for the whole call (migrationClaims), released by ticket in a defer. A load for a claimed identifier is retired; a creation is redirected onto the claim's target. The asymmetry is the design: the load wanted to publish a snapshot the migration has already moved; the creation wants to add a note to the document in front of the reader, which is the document being saved.
  • Two identities, not one. The last commit is the substantive one: migrateNotes keys off documentNotes.identifier, while every load and creation resolves through session.source. Those coincide on attempt 1 of a save chain and diverge on attempt 2 — which migrates from attempt 1's file while the session still says .clipboard. Claiming only the source left the entire bug standing one chain-step deeper.
  • Failure path. The revert now re-keys the live container instead of republishing the pre-save snapshot, so a note the redirect appended inside the window is not dropped. Only the identity is the migration's to undo.
  • Not a blocker, but fix it: the code docstring, the decision log, the agent note and the report all assert that overlapping migrations happen in production. ClipboardSaveFlow serialises attempts (await previous?.value) and documents that as a rule, and it is the only caller. The claim stack is defence-in-depth, not a production shape — which is exactly what this PR's own earlier commit said before the wording changed.

Verdict

Ready to push

No blocking or correctness defect found. The claim lifecycle is sound — both beginMigration calls are synchronous, the defer is registered immediately after with no early return in between, and every exit (chain refusal before the claim, save failure, post-save give-up, success) unwinds through it. The single-hop migrationDestination is adequate because the last commit also claims the clipboard identity, so the innermost claim on the identity a caller can actually resolve already names the final target. The escalated round-three finding — claim key vs. addressing identity diverging on a consecutive Save As chain — is genuinely closed, and pinned by three tests each of which fails under its own named mutation.

Verification: make lint 0 violations; make build-ios and make build-macos both succeed with zero compiler warnings; the 23 tests of ConsecutiveSaveAsTests + SaveAsMigrationClaimTests pass across all four locale configurations; make test-quick reported 4642/4685 with 3 failures, all in unrelated WebKit/perf suites (WebScrollabilityReportingTests, WebDetailsNavigationOrderingTests, HTMLCommentStrippingGrowthTests) that pass 38/38 on a targeted re-run — load-induced flakes, not this change.

Two documentation findings are worth correcting before merge (they are cheap, and this project treats a stale agent note as worse than none): a production-reachability overclaim repeated in four places, and a test roster that is stale by three tests and never names the new suite. Neither blocks the push.

Review findings

11 raised · 0 fixed · 11 skipped

Jump to findings →

Commits

Three-level explanation

What changed

Prism lets you paste text and read it as a document, and lets you attach notes to it. When you then Save As to a real file, the notes have to move from the pasted copy to the file. That move is not instant — it has to write to iCloud in the middle — and during that gap the rest of the app still believed the document was the pasted copy.

If anything touched the notes during that gap (the app re-reading them, or you typing a new note), the move got confused, decided someone else had opened a different document, and quietly stopped half-finished. The save still said it worked. From then on the document on screen was the file, but every note you wrote went to the pasted copy — so reopening the saved file showed none of it.

Why it matters

Nothing looked wrong at the time. Both copies held the same notes, so the only symptom was later work vanishing. That is the worst shape a data bug can take.

Key concepts

  • Identity — every document has a name the notes are filed under. Moving a document means changing that name in two places that are not changed at the same moment.
  • A claim — the fix has the move put up a sign saying "this name is mine right now, and here is where it is going". Anything that arrives during the move reads the sign.
  • Reading vs. writing — the sign is answered two different ways. Something trying to re-read the old copy is told to give up, because its information is already out of date. Something trying to add a note is sent to the new file instead, because the note belongs on the document you are looking at, which is the one being saved.

Architecture

NotesManager is a @MainActor @Observable per-reader object. migrateNotes(fromClipboardSession:previousDestination:toFileURL:) straddles await store.save(notes): documentNotes is republished under the target identifier before the save; cachedDocumentPath, cachedDocumentIdentifier, documentPath and the source-record delete all happen after it.

The window between those halves was ordered against nothing. loadGeneration (T-1556) is not bumped by a migration; the cached-path guard (T-369) still names the source; and applicableNotes (T-2089) actively made it worse — it sees a container whose identifier differs from the load's, correctly classifies that as the T-1811 hazard, and applies the load's own pre-migration snapshot.

The pattern

A new piece of MainActor state, migrationClaims: [DocumentIdentifier: [MigrationClaim]], holds a ticketed stack of claims per identifier. beginMigration(from:to:displayName:) -> UUID pushes; endMigration(from:ticket:) retires exactly its own entry via removeAll { $0.ticket == ticket } and nils the key when the last one goes. The target rides on the claim because the two consumers want opposite things from the same fact:

  • loadNotes checks isMigratingAway twice — at entry (before the prologue rebinds the T-839 cache namespace, which no later guard would undo) and again after store.load (for a load that started before the window and resumes inside it).
  • noteContainer(for:) consults migrationDestination and swaps the context, so a creation resolving .clipboard joins the migration's own container.

Trade-offs

The claim list is derived rather than authoritative: it enumerates the identities today's callers can resolve. That is stated honestly in Decision 8's Consequences, and the last commit is the proof it matters — claiming only notes.identifier covered attempt 1 of a save chain and missed attempt 2, where the notes are filed under attempt 1's file while session.source still says .clipboard. Both identities are now claimed, de-duplicated when they coincide.

The failure branch follows from the redirect rather than being independent of it: once a creation can land inside the window, the pre-save local snapshot is stale by exactly that note, so the revert re-keys the live container instead. This PR's own bug shape — a stale snapshot clobbering fresher state — on the failure path.

Why the post-save guard needed a premise, not a patch

The interesting move here is that the fix does not touch the guard that was misfiring. guard documentNotes?.identifier == targetIdentifier else { return true } is correct — its premise ("a mismatch means a genuinely different document") was simply false for one writer: the migration's own source. Every rejected alternative in Decision 8 tries to recover from the false premise (widen to notesBelongToSaveChain, re-adopt on a source-identifier match, bump loadGeneration); the claim restores it instead, which is what lets the guard keep giving the T-1811/T-1812 answer it was written for without also mis-firing on the migration's own document.

The asymmetry inside loadNotes

The second post-await checkpoint (after backupIfNeeded) deliberately re-checks the generation and not isMigratingAway. Everything between the first checkpoint and that await is synchronous, so a migration of this identifier can only have begun inside the await: either it finished (claim released, the check is a no-op) or it is parked in its own store.save — and there the check would be wrong. saveCurrentState reads the live documentNotes, which the migration already rebound to its target, so it persists the migration's own container and leaves the post-save guard satisfied; what it uniquely adds is folding relocated block ids back from anchoredNotes (Req 4.5), done nowhere else. loadResumingInsideAMigrationStillPersistsItsRelocation fails when the guard is added "for symmetry" — a pin on an absence, which is rare and worth noticing.

Ordering correctness

Two invariants carry the design and both are external to this file. NotesStore is an actor and save/load contain no internal await, so store writes are indivisible and totally ordered — a redirected creation's persistNotes enqueues behind the migration's in-flight save and therefore wins on disk, which is why the creation test can assert both notes in the target record. ClipboardSaveFlow chains attempts (task = Task { await previous?.value; await run(…) }) and is @State 1:1 with the manager on DocumentReaderView, so no two migrateNotes calls actually overlap in production. The claim stack is therefore defence-in-depth rather than a shape the app produces — see the finding below.

Residual

noteContainer resolves the redirect once, before its store.load await, and never re-checks after it — unlike the container re-read at the same site, which exists precisely because that await is a window. A creation that entered with a nil or foreign container, parked in store.load(clipboard), and resumed after a whole migration completed would republish the stale clipboard container it read before the delete. Narrow (the fast path returns before the await whenever a migration holds documentNotes), pre-existing rather than introduced, and a one-line fix.

Completeness assessment

Fully implemented: every clause of Requirement 4.8 — two identities claimed and de-duplicated, load checked at entry and after store.load, creation redirected, failure reverting identity only. All 13 tests named across Decisions 7 and 8 exist verbatim, and each of the eleven T-2231 tests fails under its own named mutation.

Partially: the noteContainer post-await redirect gap above; and toggleImportedNoteStatus writes ImportedNoteResolvedCache keyed by cachedDocumentPath, which still names the source for the whole migration — pre-existing and out of scope, but it is an existing third way of addressing the migrating document that the docstring's "exactly the identities" wording does not admit.

Missing: nothing required by the spec.

Important changes — detailed

migrateNotes: claim both addressing identities for the whole call

prism/Services/NotesManager.swift

Why it matters. This is the fix. Without it the post-save identity guard's premise is false and the migration reports success for a rebind it never did. The second claimed identity is what closes the consecutive-Save-As chain case that round three reopened.

What to look at. NotesManager.swift:1332-1371 (claim + defer), :1236-1284 (isMigratingAway / migrationDestination / beginMigration / endMigration)

Takeaway. When a two-phase identity change straddles a suspension point, the durable fix is often to restore an existing guard's premise rather than widen the guard. Widening (here: substituting notesBelongToSaveChain) would have re-admitted the very identities that belong to a different in-flight chain.
Rationale. Decision 8: the claim is the narrowest thing that orders the migration against everything else resolving its context from the source. It leaves T-1811/T-1812 for other documents untouched and needs no new state on the load side.

The claim key and the addressing identity diverge on a save chain

prism/Services/NotesManager.swift

Why it matters. The escalated round-three finding. migrateNotes keys off documentNotes.identifier; every load and creation resolves through session.source. They agree on attempt 1 and part company on attempt 2, which migrates from attempt 1's file while the session still says .clipboard — so a claim on the file alone left the whole bug standing one chain-step deeper.

What to look at. NotesManager.swift:1360-1366; pinned by SaveAsMigrationClaimTests at ConsecutiveSaveAsTests.swift:1192 and :1247

Takeaway. "What the data is filed under" and "what the callers ask for" are two different identities the moment a supersession chain exists. A claim has to be taken on the caller's spelling, not only the owner's.
Rationale. Commit 5c0f3d11: attempt 1 migrates clipboard/<S> onto its file, is superseded, and returns without session.didSave(to:), so attempt 2 migrates from that file (T-1812) while the session it serves still says .clipboard.

loadNotes retired at entry as well as after the store await

prism/Services/NotesManager.swift

Why it matters. Two distinct windows. The entry guard covers a load that starts inside the migration — its prologue would rebind cachedDocumentPath/cachedDocumentIdentifier to the source, reverting the T-839 cache namespace with no later guard to undo it. The post-await guard covers a load that started before the window and resumes inside it, and that one was untested for two rounds because every other test lands its load inside the window, where the entry guard catches it first.

What to look at. NotesManager.swift:341-343 (documentPath), :354-363 (entry), :403-411 (post-await)

Takeaway. A guard whose siblings all fire first is indistinguishable from dead code. The test that pins it has to arrange the one interleaving the siblings do not cover — here, parking the load in store.load before any migration exists, then releasing it inside one.
Rationale. Commit fe7441ac states it outright: deleting the post-await guard left the whole suite green until sourceLoadStartedBeforeTheMigrationIsRetiredWhenItResumesInside was written.

noteContainer redirects a creation onto the migration's target

prism/Services/NotesManager.swift

Why it matters. The creation half. Callers resolve context from DocumentSource, which still says .clipboard for the whole migration, so unredirected a mid-save note cleared the migration's container, reloaded the not-yet-deleted source record and republished it — the identical end state from the other side.

What to look at. NotesManager.swift:1505-1521

Takeaway. A load and a creation want opposite things from the same fact. Retiring both would have been the symmetric-looking answer and the wrong one: the creation is about the document in front of the reader, which is the document being saved.
Rationale. Decision 8's Decision section, and the docstring on migrationClaims which argues the asymmetry explicitly.

Failure revert re-keys the live container, not the pre-save snapshot

prism/Services/NotesManager.swift

Why it matters. Once the redirect can land a creation inside the window, the local snapshot taken before store.save is stale by exactly that note. Republishing it would drop the note from the container the reader is looking at while the creation's own persistNotes may already have written it to the target's record.

What to look at. NotesManager.swift:1379-1397

Takeaway. A revert should undo what its own call did, not restore a snapshot. Here that is the identity and nothing else — the notes were never the migration's to undo.
Rationale. Commit 7a43d377: this PR's own bug shape (a stale snapshot clobbering fresher state) reappearing on the failure path.

Test harness: destination-keyed save hooks, an onLoad hook, and a claim-stack pin

prismTests/ConsecutiveSaveAsTests.swift

Why it matters. The interleavings are driven, not timed — latch pairs and store hooks, no sleeps. HookedNotesStore runs save/delete hooks after the write (production NotesStore.save has no internal await, so hooking first models something that cannot happen), but the onLoad hook runs before the read, because a load has nothing to write and what is wanted is a caller suspended inside the store.

What to look at. ConsecutiveSaveAsTests.swift:44-96 (hooks), :1165-1374 (SaveAsMigrationClaimTests)

Takeaway. Where a test double's hook fires is part of the model it asserts. A hook placed on the wrong side of the write lets a reentrant save be silently clobbered on the way out, which would have made the creation test assert a harness artefact.
Rationale. Commits 44eca404 and fe7441ac state both hook placements and why they differ.

Key decisions

Restore the post-save guard's premise rather than widen the guard.

Decision 8. The guard documentNotes?.identifier == targetIdentifier was correct; its premise ("a mismatch means a different document") was false for the migration's own source. Widening to notesBelongToSaveChain would re-admit clipboard/previous-destination identities belonging to a different in-flight chain — T-2231 from the other side. Bumping loadGeneration would retire every in-flight load, reintroducing T-1811.

Answer loads and creations differently.

A load for a claimed identifier is retired; a creation is redirected. Recorded in Req 4.8 and Decision 8, and argued at length on the migrationClaims docstring.

Claim two identities, de-duplicated.

The clipboard identifier is derived from the sessionID parameter and claimed alongside notes.identifier, because the two diverge on attempt 2 of a save chain. The rejected alternative "claim the source alone" is recorded in Decision 8 together with the reason it failed.

A ticketed stack per identifier rather than a single claim or a depth count.

Releases key on the ticket because defer orders releases within one call, not across concurrent ones. Both simplifications (single claim, removeLast()) are pinned by outerMigrationReleaseKeepsTheInnerClaimOnTheClipboardIdentity. See the finding on the production-reachability wording.

The second load checkpoint deliberately omits the migration guard.

Adding it would skip saveCurrentState(), which is the only place relocated block ids are folded back into documentNotes (Req 4.5). Pinned by a test that fails when the guard is added.

Split the new tests into a second suite in the same file.

SaveAsMigrationClaimTests keeps ConsecutiveSaveAsTests inside SwiftLint's type_body_length budget. Stated in commit 5c0f3d11 and in the suite's own doc comment.

cachedBlocks is retained ahead of the entry guard.

Same reason a skipped guard iCloudAvailable load retains them (T-1811): the blocks describe the document in front of the reader and a migration changes only the identity it is filed under, not the content.

Review findings

SeverityAreaFindingResolution
majorNotesManager.swift:96-110 + decision_log.md:302 + notes-system.md:116 + report.md:171-177Four places assert that overlapping migrations are a production shape: "Overlapping migrations really can hold one identifier at once … an attempt starting inside its predecessor's save stacks a second claim on that identity", and Decision 8's "This is not hypothetical". ClipboardSaveFlow.start chains attempts (task = Task { await previous?.value; await run(...) }) and documents that ordering as a load-bearing rule; saveFlow and notesManager are both @State on DocumentReaderView, so they are 1:1, and migrateNotes has no other caller. Two migrateNotes calls therefore never overlap in production, and the only exerciser of the stack is a test that constructs the nesting by hand from inside a store hook. notes-system.md contradicts itself on this, saying at line 95 that the flow serialises attempts and at 116 that they overlap. This PR's own earlier commit (7a43d377) had the honest wording: "No production path reaches a depth above one today (ClipboardSaveFlow serialises its attempts); the stack is what makes the docstring's claim true without depending on that." The last commit replaced it with the overclaim. Overclaiming comments are the exact defect class rounds two and three flagged.Not fixed — this review is read-only. Keep the stack as defence-in-depth, but restore the earlier commit's wording in all four places: the docstring, Decision 8's rejected alternative, notes-system.md, and report.md.
majordocs/agent-notes/notes-system.md:127The regression roster is stale by exactly the commit that closes the PR. It says "ConsecutiveSaveAsTests.swift — 20 tests" and "The eight T-2231 tests were mutation-checked"; the file now holds 23 tests in two suites (ConsecutiveSaveAsTests at :281 and SaveAsMigrationClaimTests at :1167) and 11 T-2231 tests. The three added by 5c0f3d11 are absent from the enumeration, and SaveAsMigrationClaimTests is never named anywhere in the note — so a future session running -only-testing:prismTests/ConsecutiveSaveAsTests silently skips the three tests that pin the second claimed identity and the claim stack. The report itself states 23/11/two-suites, so the two documents contradict each other, and this same file warns twice that "the roster has been written down with a wrong count more than once".Not fixed — read-only. Update to 23 tests / two suites / eleven T-2231 tests, name SaveAsMigrationClaimTests, and add the three test names.
minorprism/Services/NotesManager.swift:1518-1521 vs :1538noteContainer resolves the migration redirect once, before its store.load await, and never re-checks after it — unlike the container re-read at :1538, which exists precisely because that await is a window. A creation that entered with a nil or foreign container, fell past the fast path into store.load(clipboard), and resumed after a whole migration had completed (including store.delete of the source) would republish the stale clipboard container it read before the delete, resurrecting the record and re-keying the manager to it. Narrow — the fast path at :1523-1524 returns before the await whenever a migration holds documentNotes — and pre-existing rather than introduced here, but it is a gap in the guarantee Req 4.8 now states.Not fixed — read-only. One line: re-run migrationDestination(for:) after the await, before :1538.
minorprism/Services/NotesManager.swift:1381-1391The failure-revert comment's load-bearing word is wrong. It says a redirected creation "appends to documentNotes and republishes it synchronously", but noteContainer has two suspension points before any creation publishes (awaitPendingNotesReload at :1503, store.load at :1531). What actually makes the revert safe is the fast path at :1523-1524: while a migration holds documentNotes under the target, a redirected creation matches it and returns without suspending. As written, the comment invites someone to add a suspension point to noteContainer believing the revert is unaffected.Not fixed — read-only. Re-anchor the comment on the fast path rather than on "synchronously".
minorprismTests/ConsecutiveSaveAsTests.swift:783, :966, :1297Three latch-driven tests hang rather than fail on a regression, and neither suite is time-limited. Each waits on a latch that only opens if the code under test reaches a specific point: :1334 needs the inner migration to reach store.save under secondIdentifier; :823 needs the load to reach backupIfNeeded, so any relocation regression hangs; :987 needs the load to reach store.load, which an over-broad isMigratingAway would prevent — precisely the regression class the test exists to catch. Swift Testing applies no default timeout and both suites are .serialized, so a wedged test stalls the whole run instead of reporting a failure.Not fixed — read-only, and the skill forbids editing tests except to fix a genuine bug. Add .timeLimit(.minutes(1)); the house pattern already exists at prismTests/WebViewPoolTests.swift:56.
minorprismTests/ConsecutiveSaveAsTests.swift:687, :867, :1071, :1195, :1300Five new tests inline a verbatim ~12-line copy of the file's own makeSessionWithClipboardNote(store:) helper (defined at :236) — roughly 60 duplicated lines. Five sibling new tests do call the helper. The only thing forcing the copy is needing `block` as a local, which the helper could return (or callers could read as session.parsedBlocks[0], as the helper-using tests already do). Separately, Latch at :181 is the fourth private copy of a one-shot async signal in prismTests (AsyncSignal in RemoteContentCoordinatorTests.swift:166 and RemoteRefreshFlowTests.swift:544, Gate in LoadGenerationTests.swift:84), and HookedNotesBackupStore at :139 is a near-verbatim third copy of MockNotesBackupStore.swift differing only by an onBackup hook.Not fixed — read-only. Return the block from the helper; consider lifting Latch into prismTests/Support/ and adding onBackup to the shared MockNotesBackupStore.
minorspecs/bugfixes/clipboard-save-as-notes-manager-key/report.md:397 vs :423-426Two runs described as covering "the same targeted set" report different totals: 137/137 after the second fix, then 120/120 in round two. One of the two was a different set.Not fixed — read-only. Say which, or drop "the same".
nitprism/Services/NotesManager.swift:356-358 and :354"A load ... writes nothing at all — not even the prologue" sits three lines below `cachedBlocks = blocks`, which a retired load does write, deliberately, with its own comment saying so. The two comments contradict each other in adjacent hunks.Not fixed — read-only. "writes nothing but cachedBlocks" would settle it.
nitprism/Services/NotesManager.swift:90-95 and :748-774The docstring says "Claimed are exactly the identities that still address the document being migrated", and Decision 8's Consequences invites a future third addressing route to be added to the list. toggleImportedNoteStatus is an existing third route: it writes ImportedNoteResolvedCache keyed by cachedDocumentPath, which still names the source for the whole migration, so a resolved-toggle during a Save As is filed under clipboard/<id> and lost at :1425. Pre-existing (the migration never migrates cache entries at all) and out of scope, but the absolute wording does not admit it.Not fixed — read-only. List it as knowingly out of scope, or soften "exactly".
nitspecs/clipboard-notes/decision_log.md:306 and :310Two citation slips in Decision 8. The Consequences cite Decision 5 for "an orphaned copy of that note in the destination's record", but Decision 5 is about orphaned clipboard note *files* accumulating in iCloud, not stray notes inside a live document's record. And the Impact roster omits loadResumingInsideAMigrationStillPersistsItsRelocation, which is the only pin that fails when someone adds the second-checkpoint guard "for symmetry" — the most load-bearing absence in the change.Not fixed — read-only. Qualify the Decision 5 reference; add the missing test name.
nitprism/Services/NotesManager.swift:1261, :1364-1371beginMigration(from:to:displayName:) — displayName belongs to `to`, not `from`; targetDisplayName: would remove the ambiguity. And zip(claimedIdentifiers, migrationTickets) is sound (map preserves count, no await between) but collecting [(DocumentIdentifier, UUID)] in the single map would remove the need for a reader to verify that.Not fixed — read-only. Cosmetic.

Per-file diffs

Click to expand.

prism/Services/NotesManager.swift Modified +267 / -19
diff --git a/prism/Services/NotesManager.swift b/prism/Services/NotesManager.swiftindex e89535f1..3ec00ed6 100644--- a/prism/Services/NotesManager.swift+++ b/prism/Services/NotesManager.swift@@ -106,6 +106,75 @@ final class NotesManager {     @ObservationIgnored     private var signInReloadTask: Task<Void, Never>? +    /// Where a `migrateNotes` call is moving an identifier's notes *to*, for the+    /// length of that call. Each call pushes a separately ticketed claim per+    /// identity it is moving away from, and retires exactly its own on the way+    /// out (see the stack note at the end of this comment).+    ///+    /// A migration republishes `documentNotes` under the *target* identifier+    /// before its `store.save` and only rebinds the cached identity after it, so+    /// for the length of that await the manager is mid-transition and the cached+    /// identity still names the source. That is a window no other guard covers:+    /// a load *of the source document itself* resuming inside it has a current+    /// generation and a matching cached path, so it republishes the pre-migration+    /// snapshot. The migration then finds an identifier that is not its target,+    /// reads that (correctly, for T-1812) as "a newer document replaced me", and+    /// returns success without rebinding the cached identity or retiring the+    /// source — so a clipboard Save As finishes with the session on the file and+    /// `NotesManager` still keyed to the clipboard, and every later note is+    /// written to a record the reopened file will never read (T-2231).+    ///+    /// So a migration owns its source identity for the whole of that window, and+    /// the two things that can walk into it are answered differently because they+    /// *want* different things:+    ///+    /// - A **load** for the identifier being migrated away is superseded by the+    ///   migration and abandons its result, exactly as it would for a newer+    ///   generation. It wanted to publish the source's stored snapshot, and that+    ///   snapshot is what the migration has already moved.+    /// - A **creation** for it is not superseded, it is *redirected*: it wants to+    ///   add a note to the document in front of the reader, and that document is+    ///   the one being saved. Its caller resolves the context from+    ///   `DocumentSource`, which still says clipboard — `ClipboardSaveFlow` only+    ///   calls `session.didSave(to:)` once the migration returns — so+    ///   `noteContainer(for:)` swaps in the target this claim records. Without+    ///   that it would clear the migration's container, reload the still-present+    ///   source record, and republish it, reproducing the bug above from the+    ///   creation side.+    ///+    /// Claimed are exactly the identities that still address the document being+    /// migrated: the one its notes are filed under, and — when a supersession+    /// chain has moved those apart — the clipboard session the callers still+    /// resolve (see `migrateNotes`). Nothing else is: a load for any other+    /// document is the T-1811 hazard the post-save guard exists for, and must+    /// still win.+    ///+    /// A *stack* of claims per identifier rather than a single claim or a depth+    /// count. Overlapping migrations really can hold one identifier at once:+    /// consecutive Save As attempts in one chain migrate from each other's+    /// destinations (T-1812) and each also claims the chain's clipboard+    /// identity, so an attempt starting inside its predecessor's save stacks a+    /// second claim on that identity. The innermost claim is the one a+    /// redirected creation has to join — it is the migration currently holding+    /// `documentNotes` — and when it retires, the claim it shadowed has to come+    /// back with its own target. A single claim would be overwritten by the+    /// inner migration and then removed outright when the *outer* one released,+    /// leaving a creation unredirected; `removeLast()` would pop the wrong+    /// entry whenever the outer one released first, sending that creation to a+    /// finished migration's destination. Both mutations are pinned by+    /// `SaveAsMigrationClaimTests`.+    @ObservationIgnored+    private var migrationClaims: [DocumentIdentifier: [MigrationClaim]] = [:]++    /// One in-flight `migrateNotes` call's hold on the identifier it is moving+    /// notes off, and where it is moving them to — the destination a redirected+    /// creation joins. `ticket` identifies the claim to its own owner's release.+    private struct MigrationClaim {+        var ticket: UUID+        var target: DocumentIdentifier+        var targetDisplayName: String+    }+     // MARK: - Initialization      /// Observation token for iCloud availability changes.@@ -263,12 +332,36 @@ final class NotesManager {     ///   - blocks: Current document blocks for relocation.     func loadNotes(source: DocumentSource, sessionID: UUID, blocks: [MarkdownBlock]) async {         let context = resolveNoteContext(source: source, sessionID: sessionID)-        documentPath = context.displayName+        // `documentPath` is half of the identity a migration of this document is+        // rebinding — `migrateNotes` sets it to the file's name after its save —+        // so a load the migration has already superseded must not write the+        // source's name back over it (T-2231). The inner overload's entry guard+        // retires the rest of this load; this is the one piece of the prologue+        // that lives out here.+        if !isMigratingAway(from: context.identifier) {+            documentPath = context.displayName+        }         await loadNotes(identifier: context.identifier, blocks: blocks)     }      /// Shared loading logic for file, clipboard, and bundled sources.     private func loadNotes(identifier: DocumentIdentifier, blocks: [MarkdownBlock]) async {+        // Retained ahead of every guard below, for the same reason a skipped+        // `guard iCloudAvailable` load retains them (T-1811): the blocks describe+        // the document in front of the reader, a sign-in reload needs them to+        // relocate against, and a migration does not change the content — only+        // the identity it is filed under.+        cachedBlocks = blocks++        // A load for a document `migrateNotes` is moving right now is superseded+        // before it starts, so it writes nothing at all — not even the prologue+        // (T-2231). The post-await guard below stops it *publishing* the source's+        // snapshot, but the prologue runs before any await: it would rebind+        // `cachedDocumentPath`/`cachedDocumentIdentifier` to the source, reverting+        // the T-839 cache namespace the migration has already moved to the file,+        // and no later guard undoes that.+        guard !isMigratingAway(from: identifier) else { return }+         // A switch to a different document retires any sign-in reload still         // pending for the previous one, so `noteContainer` does not join a         // reload whose result is already irrelevant (T-1811).@@ -288,7 +381,6 @@ final class NotesManager {         let generation = loadGeneration         cachedDocumentPath = identifier.path         cachedDocumentIdentifier = identifier-        cachedBlocks = blocks          guard iCloudAvailable else { clearNoteState(); return } @@ -308,6 +400,16 @@ final class NotesManager {         guard generation == loadGeneration else { return }         guard cachedDocumentPath == identifier.path else { return } +        // Nor against a Save As *migrating this very document* while suspended+        // in the store: it moves the cached identity only after that save, so+        // both guards above still pass and this load would republish the+        // pre-migration snapshot over the migration's own container — which+        // makes the migration's post-save guard read as "replaced by another+        // document" and give up its rebind (T-2231, `migrationClaims`). The+        // entry guard covers a load that *starts* inside the window; this one+        // covers a load that started before it and resumes inside.+        guard !isMigratingAway(from: identifier) else { 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@@ -363,6 +465,23 @@ final class NotesManager {             // is about to supersede (T-1586).             guard generation == loadGeneration else { return } +            // Deliberately NOT re-checking `isMigratingAway` here, unlike the+            // generation. The asymmetry is load-bearing, not an oversight+            // (T-2231): everything between that guard and the backup await is+            // synchronous, so a migration of *this* identifier can only have+            // begun inside the await — either it has already finished (the claim+            // is released, so the check would be a no-op), or it is still parked+            // in its own `store.save`, and that is the case the check would get+            // wrong. `saveCurrentState` reads the live `documentNotes`, which the+            // migration has already rebound to its target, so it persists the+            // migration's own container — never the source, never a resurrected+            // record — and republishes it under the same identifier, leaving the+            // migration's post-save guard satisfied. What it adds is the+            // relocation: `documentNotes.notes[].blockId` is folded in from+            // `anchoredNotes` here and nowhere else, so skipping it would strand+            // memory with pre-relocation ids that the next `rebuildAnchoredNotes`+            // reads back, undoing the relocation this branch exists to persist+            // (Req 4.5).             await saveCurrentState()         }     }@@ -391,7 +510,13 @@ final class NotesManager {     ///   `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.+    ///   what this load read plus mutations already on disk. That still holds+    ///   when `noteContainer` redirects the creation onto an in-flight+    ///   migration's target (T-2231): it persists what it appends either way,+    ///   and the container it publishes then carries the *target* identifier,+    ///   which the identifier check below declines for a load of any other+    ///   document — including the source, which cannot be here at all (next+    ///   bullet).     /// - 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@@ -405,17 +530,17 @@ final class NotesManager {     ///   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.+    /// - `migrateNotes` is answered *before* this function rather than by it+    ///   (T-2231). It republishes the container under a *different* `identifier`+    ///   before its save, and its save-failure revert republishes under the+    ///   *source* identifier and never persists — neither half of the question,+    ///   both halves of a migration. The ordering is what settles it: for the+    ///   whole of that call it claims the source identity, and a load for the+    ///   source is retired by `isMigratingAway` — at entry, or at the guard+    ///   immediately above this call for one already in flight — so it never+    ///   reaches here to be arbitrated. A load for *another* document does reach+    ///   here, and the identifier check below declines the migration's container+    ///   as not being about that document, which is the correct T-1811 behaviour.     ///     /// 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@@ -1111,6 +1236,49 @@ final class NotesManager {         return notes.identifier == expectedIdentifier || notes.identifier == supersededIdentifier     } +    /// Whether a `migrateNotes` call is currently moving this document's notes+    /// onto another identifier. See `migrationClaims`.+    private func isMigratingAway(from identifier: DocumentIdentifier) -> Bool {+        migrationClaims[identifier] != nil+    }++    /// The identity an in-flight migration is moving this identifier's notes to,+    /// for a caller that should follow it rather than be retired by it. The+    /// innermost claim wins — it is the migration currently holding+    /// `documentNotes`.+    private func migrationDestination(+        for identifier: DocumentIdentifier+    ) -> (identifier: DocumentIdentifier, displayName: String)? {+        guard let claim = migrationClaims[identifier]?.last else { return nil }+        return (claim.target, claim.targetDisplayName)+    }++    /// Pushes a claim and returns the ticket its owner passes back to+    /// `endMigration`. Releases are keyed on the ticket rather than assumed+    /// LIFO: `defer` orders releases within one call, not across concurrent+    /// ones, so a migration finishing while another still holds the same source+    /// must retire *its* claim and leave the other one's destination standing.+    private func beginMigration(+        from identifier: DocumentIdentifier,+        to target: DocumentIdentifier,+        displayName: String+    ) -> UUID {+        let ticket = UUID()+        migrationClaims[identifier, default: []].append(+            MigrationClaim(ticket: ticket, target: target, targetDisplayName: displayName)+        )+        return ticket+    }++    /// Retires one claim, restoring whichever claim is innermost afterwards. The+    /// key is removed only when the last claim goes, so `isMigratingAway` stays+    /// true for another migration still parked in its own save.+    private func endMigration(from identifier: DocumentIdentifier, ticket: UUID) {+        guard var claims = migrationClaims[identifier] else { return }+        claims.removeAll { $0.ticket == ticket }+        migrationClaims[identifier] = claims.isEmpty ? nil : claims+    }+     /// Migrate notes from a clipboard session to a file-based identifier.     ///     /// Handles the clipboard-to-file transition when a user saves a pasted document.@@ -1161,6 +1329,47 @@ final class NotesManager {         let targetIdentifier = identifierResolver.resolve(from: url)         let targetDisplayName = url.lastPathComponent +        // Claim, for the rest of this call, every identity a caller can still+        // address this document by, so nothing resolving one of them — a load of+        // the document being migrated, or a note created on it while+        // `session.source` still says clipboard — can land inside the save (or+        // the delete below) and undo the rebind (T-2231). Each claim carries the+        // target because those two want opposite things: the load is retired,+        // the creation is redirected onto the target. Released on every exit,+        // including the failure revert — which restores the container under+        // `sourceIdentifier` with no await in between, so no load can observe a+        // half-reverted state.+        //+        // Two identities, not one, because the migration's own key and the+        // caller's diverge on a supersession chain. This claims what the *notes*+        // are filed under (`notes.identifier`), while every load and creation+        // resolves its identifier from `session.source` via `resolveNoteContext`.+        // Those agree on the first attempt in a chain and part company on the+        // next one: attempt 1 migrated `clipboard/<S>` onto its file, then found+        // itself superseded and returned *without* `session.didSave(to:)`+        // (`ClipboardSaveFlow`), so attempt 2 migrates from that file (T-1812)+        // while the session it serves still says `.clipboard`. Claiming only the+        // file left the clipboard identity unclaimed and reproduced T-2231 one+        // chain-step deeper: a load or creation resolved through the still-+        // clipboard session fell through both checkpoints, cleared this+        // migration's container, found the deleted clipboard record, and+        // republished or resurrected it — after which the post-save guard below+        // read "a newer document replaced me" and abandoned the rebind. So the+        // clipboard identity is derived from `sessionID` and claimed alongside,+        // and de-duplicated because on the first attempt they are the same.+        let clipboardIdentifier = identifierResolver.resolve(forClipboardSession: sessionID)+        let claimedIdentifiers = clipboardIdentifier == sourceIdentifier+            ? [sourceIdentifier]+            : [sourceIdentifier, clipboardIdentifier]+        let migrationTickets = claimedIdentifiers.map {+            beginMigration(from: $0, to: targetIdentifier, displayName: targetDisplayName)+        }+        defer {+            for (identifier, ticket) in zip(claimedIdentifiers, migrationTickets) {+                endMigration(from: identifier, ticket: ticket)+            }+        }+         notes.identifier = targetIdentifier         notes.displayName = targetDisplayName         documentNotes = notes@@ -1169,11 +1378,21 @@ final class NotesManager {             try await store.save(notes)         } catch {             logger.error("Failed to save migrated notes: \(error, privacy: .public)")-            // Revert in-memory state — but only if it is still ours to revert.-            guard documentNotes?.identifier == targetIdentifier else { return false }-            notes.identifier = sourceIdentifier-            notes.displayName = sourceDisplayName-            documentNotes = notes+            // Revert in-memory state — but only if it is still ours to revert,+            // and revert the *live* container rather than republishing the local+            // snapshot taken before the save. That snapshot is stale by exactly+            // the thing this claim invites in: a creation redirected onto+            // `targetIdentifier` by `noteContainer(for:)` appends to+            // `documentNotes` and republishes it synchronously, inside this very+            // window, without changing its identifier — so the guard below still+            // passes, and overwriting would drop that note from the container the+            // reader is looking at while its own `persistNotes` may already have+            // written it to the target's record. Only the identity is this+            // migration's to undo; the notes are not.+            guard var live = documentNotes, live.identifier == targetIdentifier else { return false }+            live.identifier = sourceIdentifier+            live.displayName = sourceDisplayName+            documentNotes = live             return false         } @@ -1187,6 +1406,17 @@ final class NotesManager {         // predicate also accepts belong to a different in-flight load, and         // rebinding onto one of those would reproduce T-2231 from the other         // side (do not substitute the chain predicate here).+        //+        // What makes "give up quietly, report success" the right response is+        // that a mismatch here means a genuinely *different* document. That is+        // not free — it is what the claim taken above buys, on both of the paths+        // that could otherwise republish the *source* inside this window: a load+        // of the source is retired by `isMigratingAway`, and a creation on it is+        // redirected onto `targetIdentifier` by `noteContainer(for:)`, so it+        // leaves this guard satisfied rather than defeated (T-2231). Before that+        // claim either of them read as a replacement here, and this guard+        // abandoned the rebind — leaving the manager keyed to the clipboard while+        // the session named the file.         guard documentNotes?.identifier == targetIdentifier else { return true }          // Update cached document identity so imported-note resolved toggles@@ -1272,6 +1502,24 @@ extension NotesManager {     ) async -> DocumentNotes {         await awaitPendingNotesReload() +        // A creation whose context names a document `migrateNotes` is moving+        // right now belongs to where it is being moved *to* (T-2231). The+        // callers resolve this context from `DocumentSource`, and a clipboard+        // Save As only calls `session.didSave(to:)` after the migration returns,+        // so for the length of the migration the source still says clipboard+        // while the container has already been rebound to the file. Left+        // unredirected, the identity check below reads that container as another+        // document's, clears it, reloads the source record the migration has not+        // deleted yet, and republishes it — which is exactly the state the+        // migration's post-save guard reads as "a newer document replaced me",+        // so it abandons its rebind and the manager stays keyed to the clipboard.+        // Resolved *after* the reload join above, so a claim taken during that+        // await is still seen.+        var context = context+        if let destination = migrationDestination(for: context.identifier) {+            context = destination+        }+         if let notes = documentNotes {             if notes.identifier == context.identifier { return notes }             // Loaded state belongs to another document — drop it rather than
prismTests/ConsecutiveSaveAsTests.swift Modified +843 / -49
diff --git a/prismTests/ConsecutiveSaveAsTests.swift b/prismTests/ConsecutiveSaveAsTests.swiftindex b2ea4390..6a93daec 100644--- a/prismTests/ConsecutiveSaveAsTests.swift+++ b/prismTests/ConsecutiveSaveAsTests.swift@@ -24,31 +24,73 @@ import SwiftUI import Testing @testable import prism -/// A store that runs a hook inside `save`, so a test can land other work while-/// a migration is suspended in the store.+/// A store that runs a hook inside `save` or `delete`, so a test can land other+/// work while a migration is suspended in the store.+///+/// Each hook runs *after* its own write, deliberately. The production+/// `NotesStore.save` contains no `await` (the invariant `applicableNotes`+/// depends on), so its write is indivisible and the only observable suspension+/// is the caller resuming afterwards. Running the hook first would model+/// something that cannot happen and would let a reentrant save from inside the+/// hook be silently overwritten by the outer one on the way out. private actor HookedNotesStore: NotesStoreProtocol {     var storedNotes: [String: DocumentNotes] = [:]     var isAvailable: Bool { true }     private var onSave: (@Sendable () async -> Void)?+    private var onSaveByPath: [String: @Sendable () async -> Void] = [:]+    private var onDelete: (@Sendable () async -> Void)?+    private var onLoad: (@Sendable () async -> Void)?      func setOnSave(_ hook: @escaping @Sendable () async -> Void) {         onSave = hook     } +    /// Arms a hook for one destination only, so a test can land work inside the+    /// *second* attempt's save in a chain without the setup note's save or the+    /// first attempt's consuming the one-shot hook above.+    func setOnSave(forPath path: String, _ hook: @escaping @Sendable () async -> Void) {+        onSaveByPath[path] = hook+    }++    /// Parks a `load` *before* its read — the one hook that runs first, because+    /// a load has nothing to write and what a test needs here is the caller+    /// suspended inside the store while other work happens around it. What it+    /// then reads is deliberately the store as it stands at resume.+    func setOnLoad(_ hook: @escaping @Sendable () async -> Void) {+        onLoad = hook+    }++    /// Lands work inside `migrateNotes`' source delete — the part of its window+    /// that comes *after* it has already rebound the cached identity.+    func setOnDelete(_ hook: @escaping @Sendable () async -> Void) {+        onDelete = hook+    }+     func load(for identifier: DocumentIdentifier) async -> DocumentNotes? {-        storedNotes[identifier.path]+        if let hook = onLoad {+            onLoad = nil+            await hook()+        }+        return storedNotes[identifier.path]     }      func save(_ notes: DocumentNotes) async throws {+        storedNotes[notes.identifier.path] = notes         if let hook = onSave {             onSave = nil             await hook()         }-        storedNotes[notes.identifier.path] = notes+        if let hook = onSaveByPath.removeValue(forKey: notes.identifier.path) {+            await hook()+        }     }      func delete(for identifier: DocumentIdentifier) async {         storedNotes.removeValue(forKey: identifier.path)+        if let hook = onDelete {+            onDelete = nil+            await hook()+        }     } } @@ -58,17 +100,30 @@ private actor DestinationFailingNotesStore: NotesStoreProtocol {     var storedNotes: [String: DocumentNotes] = [:]     var isAvailable: Bool { true }     private var failingPath: String?+    private var onFailingSave: (@Sendable () async -> Void)?      func setFailingPath(_ path: String?) {         failingPath = path     } +    /// Lands work inside a save that is about to fail — the window in which+    /// `migrateNotes` has already rebound the container onto its target but has+    /// not yet run its revert. Fires *before* the throw because a failing save+    /// writes nothing, so there is no post-write state for the hook to observe.+    func setOnFailingSave(_ hook: @escaping @Sendable () async -> Void) {+        onFailingSave = hook+    }+     func load(for identifier: DocumentIdentifier) async -> DocumentNotes? {         storedNotes[identifier.path]     }      func save(_ notes: DocumentNotes) async throws {         if notes.identifier.path == failingPath {+            if let hook = onFailingSave {+                onFailingSave = nil+                await hook()+            }             throw CocoaError(.fileWriteNoPermission)         }         storedNotes[notes.identifier.path] = notes@@ -79,63 +134,151 @@ private actor DestinationFailingNotesStore: NotesStoreProtocol {     } } -@Suite("Consecutive Save As", .serialized)-@MainActor-struct ConsecutiveSaveAsTests {+/// A backup store that runs a hook inside `backupIfNeeded`, so a test can land+/// other work while a load is suspended in its relocation-write tail.+private actor HookedNotesBackupStore: NotesBackupStoreProtocol {+    private var backups: [String: DocumentNotes] = [:]+    private var onBackup: (@Sendable () async -> Void)? -    private let firstURL = URL(fileURLWithPath: "/Users/test/first.md")-    private let secondURL = URL(fileURLWithPath: "/Users/test/second.md")-    private let thirdURL = URL(fileURLWithPath: "/Users/test/third.md")+    func setOnBackup(_ hook: @escaping @Sendable () async -> Void) {+        onBackup = hook+    } -    // MARK: - Helpers+    func hasBackup(for identifier: DocumentIdentifier) async -> Bool {+        backups[identifier.path] != nil+    } -    private func makeStructure(from blocks: [MarkdownBlock]) -> DocumentStructure {-        MarkdownSectionBuilder.build(from: blocks)+    func backupIfNeeded(_ notes: DocumentNotes) async throws {+        if let hook = onBackup {+            onBackup = nil+            await hook()+        }+        guard backups[notes.identifier.path] == nil else { return }+        backups[notes.identifier.path] = notes     } -    /// A clipboard session with one note already stored under the clipboard-    /// identifier, i.e. the state a Save As starts from.-    private func makeSessionWithClipboardNote(-        store: any NotesStoreProtocol-    ) async -> (session: DocumentSession, manager: NotesManager) {-        let session = DocumentSession(clipboardContent: "# Pasted")-        let block = MarkdownBlock.paragraph(markdown: "Pasted")-        session.parsedBlocks = [block]+    func loadBackup(for identifier: DocumentIdentifier) async -> DocumentNotes? {+        backups[identifier.path]+    } -        let manager = NotesManager.makeForTesting(store: store)-        await manager.createNote(-            content: "Clipboard note",-            for: block,-            sourceIndex: 0,-            in: makeStructure(from: [block]),-            source: .clipboard,-            sessionID: session.id-        )-        return (session, manager)+    func availableBackups() async -> [DocumentNotes] {+        Array(backups.values)     } -    /// Runs two overlapping Save As attempts: the second is requested before-    /// the first attempt's notes work has finished.-    private func runOverlappingSaves(-        session: DocumentSession,-        manager: NotesManager,-        onCompleted: @escaping (URL) -> Void = { _ in },-        onFailed: @escaping (URL?) -> Void = { _ in }-    ) async {-        let flow = ClipboardSaveFlow()+    func restore(for identifier: DocumentIdentifier, into store: any NotesStoreProtocol) async throws {+        guard let backup = backups[identifier.path] else { throw NotesBackupError.noBackup }+        try await store.save(backup)+    } -        session.prepareSave(to: firstURL)-        flow.start(session: session, notesManager: manager, onCompleted: { _, attempt in onCompleted(attempt.url) },-                   onFailed: { _, _, notesRemainAt in onFailed(notesRemainAt) })+    func deleteBackup(for identifier: DocumentIdentifier) async {+        backups.removeValue(forKey: identifier.path)+    }+} -        // The exporter's second callback lands while the first attempt is still-        // suspended in the notes store.-        session.prepareSave(to: secondURL)-        flow.start(session: session, notesManager: manager, onCompleted: { _, attempt in onCompleted(attempt.url) },-                   onFailed: { _, _, notesRemainAt in onFailed(notesRemainAt) })+/// A one-shot latch: `wait()` suspends until someone calls `open()`, and never+/// suspends afterwards. Lets two suspended halves of an interleaving hand off to+/// each other without a sleep.+private actor Latch {+    private var isOpen = false+    private var waiters: [CheckedContinuation<Void, Never>] = []++    func open() {+        guard !isOpen else { return }+        isOpen = true+        let resuming = waiters+        waiters.removeAll()+        for continuation in resuming { continuation.resume() }+    } -        await flow.drain()+    func wait() async {+        guard !isOpen else { return }+        await withCheckedContinuation { waiters.append($0) }     }+}++/// Hands a task started *inside* a store hook back to the test that has to+/// await it. A hook is `@Sendable` and cannot write to a local in the test body.+private actor TaskBox {+    private var task: Task<Bool, Never>?+    private var waiters: [CheckedContinuation<Task<Bool, Never>, Never>] = []++    func hold(_ task: Task<Bool, Never>) {+        self.task = task+        let resuming = waiters+        waiters.removeAll()+        for continuation in resuming { continuation.resume(returning: task) }+    }++    func value() async -> Bool {+        let held: Task<Bool, Never>+        if let task {+            held = task+        } else {+            held = await withCheckedContinuation { waiters.append($0) }+        }+        return await held.value+    }+}++// Shared by both suites in this file: the T-2231 migration-claim tests live in+// their own suite below, and drive the same fixtures.+private let firstURL = URL(fileURLWithPath: "/Users/test/first.md")+private let secondURL = URL(fileURLWithPath: "/Users/test/second.md")+private let thirdURL = URL(fileURLWithPath: "/Users/test/third.md")++private func makeStructure(from blocks: [MarkdownBlock]) -> DocumentStructure {+    MarkdownSectionBuilder.build(from: blocks)+}++/// A clipboard session with one note already stored under the clipboard+/// identifier, i.e. the state a Save As starts from.+@MainActor+private func makeSessionWithClipboardNote(+    store: any NotesStoreProtocol+) async -> (session: DocumentSession, manager: NotesManager) {+    let session = DocumentSession(clipboardContent: "# Pasted")+    let block = MarkdownBlock.paragraph(markdown: "Pasted")+    session.parsedBlocks = [block]++    let manager = NotesManager.makeForTesting(store: store)+    await manager.createNote(+        content: "Clipboard note",+        for: block,+        sourceIndex: 0,+        in: makeStructure(from: [block]),+        source: .clipboard,+        sessionID: session.id+    )+    return (session, manager)+}++/// Runs two overlapping Save As attempts: the second is requested before+/// the first attempt's notes work has finished.+@MainActor+private func runOverlappingSaves(+    session: DocumentSession,+    manager: NotesManager,+    onCompleted: @escaping (URL) -> Void = { _ in },+    onFailed: @escaping (URL?) -> Void = { _ in }+) async {+    let flow = ClipboardSaveFlow()++    session.prepareSave(to: firstURL)+    flow.start(session: session, notesManager: manager, onCompleted: { _, attempt in onCompleted(attempt.url) },+               onFailed: { _, _, notesRemainAt in onFailed(notesRemainAt) })++    // The exporter's second callback lands while the first attempt is still+    // suspended in the notes store.+    session.prepareSave(to: secondURL)+    flow.start(session: session, notesManager: manager, onCompleted: { _, attempt in onCompleted(attempt.url) },+               onFailed: { _, _, notesRemainAt in onFailed(notesRemainAt) })++    await flow.drain()+}++@Suite("Consecutive Save As", .serialized)+@MainActor+struct ConsecutiveSaveAsTests {      // MARK: - Regression @@ -535,6 +678,443 @@ struct ConsecutiveSaveAsTests {         #expect(manager.cachedDocumentIdentifier == DocumentIdentifierResolver().resolve(from: otherURL))     } +    // MARK: - T-2231: a load of the migrating document landing mid-save++    @Test("the migrating document's own load landing mid-save does not strand the notes on the clipboard")+    func migrationSurvivesASourceLoadLandingDuringItsSave() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let session = DocumentSession(clipboardContent: "# Pasted")+        let block = MarkdownBlock.paragraph(markdown: "Pasted")+        session.parsedBlocks = [block]+        let manager = NotesManager.makeForTesting(store: store)+        await manager.createNote(+            content: "Clipboard note",+            for: block,+            sourceIndex: 0,+            in: makeStructure(from: [block]),+            source: .clipboard,+            sessionID: session.id+        )+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)++        // The reader's own clipboard load — started before the save, still+        // parked in the store — lands while the migration is suspended in+        // `store.save`. Unlike the load above it is for the document being+        // migrated, so every existing guard passes: the generation is current+        // and the cached path still names the clipboard, because the migration+        // only rebinds it after the save it is parked in.+        await store.setOnSave { [manager] in+            await manager.loadNotes(+                source: .clipboard,+                sessionID: session.id,+                blocks: [block]+            )+        }++        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        // Bug: the resuming load republished the clipboard snapshot, so the+        // migration's post-save identity guard read "someone replaced me" and+        // returned success without rebinding identity or retiring the source.+        #expect(migrated == true)+        #expect(manager.documentNotes?.identifier == resolver.resolve(from: firstURL))+        #expect(manager.cachedDocumentIdentifier == resolver.resolve(from: firstURL))+        #expect(await store.storedNotes[resolver.resolve(from: firstURL).path]?.notes+            .map(\.content) == ["Clipboard note"])+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)+    }++    @Test("a Save As racing the document's own notes load still keys later notes to the saved file")+    func saveAsKeepsNotesOnTheDestinationWhenASourceLoadLandsMidFlight() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let (session, manager) = await makeSessionWithClipboardNote(store: store)+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)++        await store.setOnSave { [manager] in+            await manager.loadNotes(+                source: .clipboard,+                sessionID: session.id,+                blocks: session.parsedBlocks+            )+        }++        let flow = ClipboardSaveFlow()+        session.prepareSave(to: firstURL)+        flow.start(session: session, notesManager: manager, onCompleted: { _, _ in }, onFailed: { _, _, _ in })+        await flow.drain()++        // The session became a file document either way — the migration+        // reported success — so the visible fault is the disagreement.+        #expect(session.source == .file(url: firstURL))+        #expect(manager.documentNotes?.identifier == resolver.resolve(from: firstURL))++        // Bug: `createDocumentNote(content:)` derives its persistence context+        // from the stale container, so a note added after the save was written+        // under the clipboard identifier and vanished when the file was+        // reopened.+        await manager.createDocumentNote(content: "Note added after saving")++        #expect(await store.storedNotes[resolver.resolve(from: firstURL).path]?.notes+            .map(\.content) == ["Clipboard note", "Note added after saving"])+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)+    }++    /// The other half of T-2231: `loadNotes`' second post-await checkpoint (after+    /// `backupIfNeeded`) re-checks the generation but deliberately does *not*+    /// re-check `isMigratingAway`, and this pins why that is correct rather than+    /// an oversight.+    ///+    /// Everything between the first checkpoint and the backup await is+    /// synchronous, so a migration of this identifier can only begin inside that+    /// await — as it does here. When the load resumes, the migration is still+    /// parked in `store.save`, so a `!isMigratingAway` guard would fire and skip+    /// `saveCurrentState()`. That is the wrong answer: `saveCurrentState` reads+    /// the live container, which the migration has already rebound to its target,+    /// so it persists the migration's own notes under the destination — and it is+    /// the only place the relocated block ids are folded back into+    /// `documentNotes`. Skipping it strands memory with pre-relocation ids that+    /// the next `rebuildAnchoredNotes()` reads back, undoing the relocation.+    @Test("a migration landing in a load's backup window keeps the relocation write")+    func loadResumingInsideAMigrationStillPersistsItsRelocation() async {+        let store = HookedNotesStore()+        let backupStore = HookedNotesBackupStore()+        let resolver = DocumentIdentifierResolver()+        let session = DocumentSession(clipboardContent: "# Pasted")+        let block = MarkdownBlock.paragraph(markdown: "Pasted block content")+        session.parsedBlocks = [block]+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let targetIdentifier = resolver.resolve(from: firstURL)++        // A stored note anchored to a stale block id whose context quote still+        // matches `block`, so the load relocates and enters the backup tail.+        let note = BlockNote(+            id: UUID(),+            blockId: "stale-block-id",+            contextQuote: "Pasted block content",+            content: "Clipboard note",+            status: .active,+            createdAt: Date(),+            modifiedAt: Date()+        )+        try? await store.save(DocumentNotes(+            identifier: clipboardIdentifier, displayName: "Pasted", notes: [note]+        ))++        let manager = NotesManager.makeForTesting(store: store, backupStore: backupStore)++        let loadParkedInBackup = Latch()+        let migrationParkedInSave = Latch()++        // The load parks here — past every entry guard, its state published,+        // relocation done, nothing persisted yet.+        await backupStore.setOnBackup {+            await loadParkedInBackup.open()+            await migrationParkedInSave.wait()+        }++        let load = Task { @MainActor in+            await manager.loadNotes(source: .clipboard, sessionID: session.id, blocks: [block])+        }+        await loadParkedInBackup.wait()++        // Only now does the migration claim the source, so the claim lands after+        // the load's own `isMigratingAway` guard — the one window the second+        // checkpoint could ever see. It then parks in `store.save` and stays+        // parked until the load's whole tail has run.+        await store.setOnSave {+            await migrationParkedInSave.open()+            await load.value+        }++        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        // The migration completes exactly as it does without the interleaving.+        #expect(migrated == true)+        #expect(manager.documentNotes?.identifier == targetIdentifier)+        #expect(manager.cachedDocumentIdentifier == targetIdentifier)+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)++        // And the load's tail ran: the relocated block id is folded into the+        // container. Guarding the second checkpoint on `isMigratingAway` would+        // leave this at "stale-block-id".+        #expect(manager.documentNotes?.notes.map(\.blockId) == [block.id])+        #expect(manager.anchoredNotes[block.id]?.count == 1)+    }++    /// The creation side of the same window. A load is *retired* by the claim;+    /// a creation must be *redirected* by it, because it is about the document+    /// in front of the reader and that document is the one being saved.+    ///+    /// `session.source` still says `.clipboard` throughout the migration —+    /// `ClipboardSaveFlow` calls `didSave(to:)` only once `migrateNotes`+    /// returns — so the creation resolves the clipboard context and would+    /// otherwise walk `noteContainer(for:)` straight into the T-2231 end state:+    /// clear the migration's container, reload the source record it has not+    /// deleted yet, republish it, and leave the migration's post-save guard+    /// reading "a newer document replaced me".+    @Test("a note added while the Save As is migrating follows the notes onto the file")+    func noteCreatedDuringMigrationFollowsTheNotesToTheFile() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let session = DocumentSession(clipboardContent: "# Pasted")+        let block = MarkdownBlock.paragraph(markdown: "Pasted")+        session.parsedBlocks = [block]+        let manager = NotesManager.makeForTesting(store: store)+        await manager.createNote(+            content: "Clipboard note",+            for: block,+            sourceIndex: 0,+            in: makeStructure(from: [block]),+            source: .clipboard,+            sessionID: session.id+        )+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let targetIdentifier = resolver.resolve(from: firstURL)++        await store.setOnSave { [manager] in+            await manager.createNote(+                content: "Note added during the save",+                for: block,+                sourceIndex: 0,+                in: MarkdownSectionBuilder.build(from: [block]),+                source: .clipboard,+                sessionID: session.id+            )+        }++        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        #expect(migrated == true)+        #expect(manager.documentNotes?.identifier == targetIdentifier)+        #expect(manager.cachedDocumentIdentifier == targetIdentifier)+        // Both notes are on the file, and neither was left behind or dropped:+        // the creation joined the migration's own container rather than+        // resurrecting the clipboard one beside it.+        #expect(manager.documentNotes?.notes.map(\.content)+            == ["Clipboard note", "Note added during the save"])+        #expect(await store.storedNotes[targetIdentifier.path]?.notes.map(\.content)+            == ["Clipboard note", "Note added during the save"])+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)+    }++    /// The claim covers the *whole* call, including the source delete that runs+    /// after the cached identity has already been rebound — and a load starting+    /// in that tail has to be turned away at the door, not after its `await`.+    ///+    /// `loadNotes`' prologue rebinds `cachedDocumentPath`/+    /// `cachedDocumentIdentifier`/`documentPath` before its first suspension+    /// point, so the post-await guard is too late here: the migration has+    /// already done its rebind and will not redo it, leaving the T-839 cache+    /// namespace and the window title on the clipboard for good.+    @Test("a load starting inside the migration's tail does not revert the cached identity")+    func sourceLoadStartingDuringTheSourceDeleteDoesNotRevertCachedIdentity() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let (session, manager) = await makeSessionWithClipboardNote(store: store)+        let targetIdentifier = resolver.resolve(from: firstURL)++        await store.setOnDelete { [manager] in+            await manager.loadNotes(+                source: .clipboard,+                sessionID: session.id,+                blocks: session.parsedBlocks+            )+        }++        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        #expect(migrated == true)+        #expect(manager.documentNotes?.identifier == targetIdentifier)+        #expect(manager.cachedDocumentIdentifier == targetIdentifier)+        #expect(manager.documentPath == firstURL.lastPathComponent)+    }++    /// The claim has two checkpoints in `loadNotes` and they catch different+    /// loads. The entry guard catches a load that *starts* inside the window;+    /// this pins the other one — a load that started legitimately **before** the+    /// migration (so the entry guard let it through, correctly: nothing was being+    /// migrated yet) and resumes from `store.load` inside it.+    ///+    /// Neither of the guards it sits behind can see that load. `migrateNotes`+    /// bumps no generation, so the T-1556 check passes; it rebinds+    /// `cachedDocumentPath` only *after* its save, so the T-369 check passes too.+    /// `applicableNotes` cannot help either — it finds `documentNotes` already+    /// rebound to the target, reads the identifier mismatch as the T-1811 hazard,+    /// and hands back the loaded source snapshot. Publishing that snapshot is+    /// what makes the migration's post-save guard read "a newer document+    /// replaced me" and abandon its rebind, which is T-2231 exactly.+    ///+    /// Delete the post-`store.load` `isMigratingAway` guard and this test fails;+    /// every other test in this file that lands a load inside a migration starts+    /// that load inside the window, so the entry guard covers them and they all+    /// stay green.+    @Test("a source load that started before the migration and resumes inside it is retired")+    func sourceLoadStartedBeforeTheMigrationIsRetiredWhenItResumesInside() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let (session, manager) = await makeSessionWithClipboardNote(store: store)+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let targetIdentifier = resolver.resolve(from: firstURL)++        // Park the load in the store, before any migration exists.+        let loadIsParked = Latch()+        let migrationIsSaving = Latch()+        await store.setOnLoad {+            await loadIsParked.open()+            await migrationIsSaving.wait()+        }+        let load = Task { @MainActor in+            await manager.loadNotes(+                source: .clipboard,+                sessionID: session.id,+                blocks: session.parsedBlocks+            )+        }+        await loadIsParked.wait()++        // Release it inside the migration's save, and hold the migration there+        // until the load's whole tail has run, so the interleaving is exact+        // rather than probable.+        await store.setOnSave {+            await migrationIsSaving.open()+            await load.value+        }++        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        #expect(migrated == true)+        #expect(manager.documentNotes?.identifier == targetIdentifier)+        #expect(manager.cachedDocumentIdentifier == targetIdentifier)+        #expect(manager.documentPath == firstURL.lastPathComponent)+        #expect(manager.documentNotes?.notes.map(\.content) == ["Clipboard note"])+        #expect(await store.storedNotes[targetIdentifier.path]?.notes.map(\.content)+            == ["Clipboard note"])+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)+    }++    /// The claim is released on the failure path too — `defer`, not a trailing+    /// call on the success branch. A leaked claim would silently retire every+    /// later load of the document the failed save left the notes on.+    @Test("a failed migration releases its claim on the source")+    func failedMigrationReleasesItsClaimOnTheSource() async throws {+        let store = DestinationFailingNotesStore()+        let resolver = DocumentIdentifierResolver()+        let (session, manager) = await makeSessionWithClipboardNote(store: store)+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)++        await store.setFailingPath(resolver.resolve(from: firstURL).path)+        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )+        #expect(migrated == false)+        #expect(manager.documentNotes?.identifier == clipboardIdentifier)++        // A claim is invisible from outside, so drive its only consequence: put+        // something in the source record that only a load can bring in, and load.+        let blockId = try #require(session.parsedBlocks.first).id+        var updated = try #require(manager.documentNotes)+        updated.notes.append(BlockNote(+            blockId: blockId,+            contextQuote: "Pasted",+            content: "Added after the failure",+            status: .active,+            createdAt: Date(),+            modifiedAt: Date()+        ))+        try await store.save(updated)++        await manager.loadNotes(+            source: .clipboard,+            sessionID: session.id,+            blocks: session.parsedBlocks+        )++        #expect(manager.documentNotes?.notes.map(\.content)+            == ["Clipboard note", "Added after the failure"])+    }++    /// The claim that lets a creation land inside a migration also makes the+    /// migration's failure revert dangerous: the note that creation appends goes+    /// into the *live* `documentNotes`, not into the local snapshot+    /// `migrateNotes` took before its save. Reverting by republishing that+    /// snapshot restores the right identity and the wrong notes — the new note+    /// disappears from the container the reader is looking at, which is this+    /// PR's own bug (a stale snapshot clobbering fresher state) wearing its+    /// failure-path face.+    ///+    /// Deliberately the two halves crossed: the success-path creation test+    /// covers a creation landing in a save that succeeds, and+    /// `failedMigrationReleasesItsClaimOnTheSource` covers a failure with no+    /// concurrent creation. Neither of them meets the other.+    @Test("a note created during a migration survives that migration's failure revert")+    func noteCreatedDuringAFailingMigrationSurvivesTheRevert() async {+        let store = DestinationFailingNotesStore()+        let resolver = DocumentIdentifierResolver()+        let session = DocumentSession(clipboardContent: "# Pasted")+        let block = MarkdownBlock.paragraph(markdown: "Pasted")+        session.parsedBlocks = [block]+        let manager = NotesManager.makeForTesting(store: store)+        await manager.createNote(+            content: "Clipboard note",+            for: block,+            sourceIndex: 0,+            in: makeStructure(from: [block]),+            source: .clipboard,+            sessionID: session.id+        )+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let clipboardDisplayName = manager.documentNotes?.displayName++        await store.setFailingPath(resolver.resolve(from: firstURL).path)+        await store.setOnFailingSave { [manager] in+            // Resolves the clipboard context — `session.didSave(to:)` has not run+            // — so the claim redirects it onto the migration's target and it+            // appends to the migration's own, already-rebound container.+            await manager.createNote(+                content: "Note added during the failing save",+                for: block,+                sourceIndex: 0,+                in: MarkdownSectionBuilder.build(from: [block]),+                source: .clipboard,+                sessionID: session.id+            )+        }++        let migrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        #expect(migrated == false)+        // Identity reverted: the save failed, so this is still the clipboard document.+        #expect(manager.documentNotes?.identifier == clipboardIdentifier)+        #expect(manager.documentNotes?.displayName == clipboardDisplayName)+        // …and the note created inside the window came back with it, rather than+        // being overwritten by the pre-save snapshot.+        #expect(manager.documentNotes?.notes.map(\.content)+            == ["Clipboard note", "Note added during the failing save"])+        #expect(manager.anchoredNotes[block.id]?.map(\.content)+            == ["Clipboard note", "Note added during the failing save"])+    }+     @Test("a recent entry never carries another destination's bookmark")     func recentEntryBookmarkMatchesItsURL() throws {         let flow = DocumentFlowCoordinator()@@ -578,3 +1158,217 @@ struct ConsecutiveSaveAsTests {                 "the entry labelled first.md resolved to \(resolved.lastPathComponent)")     } }++/// The T-2231 claim tests that need a two-attempt chain: a separate suite so the+/// original T-1812 one stays inside SwiftLint's type body budget. Shares this+/// file's fixtures and hooked stores.+@Suite("Save As migration claims", .serialized)+@MainActor+struct SaveAsMigrationClaimTests {++    /// The claim key and the *addressing* identity diverge on a supersession+    /// chain, and claiming only the former left T-2231 standing one chain-step+    /// deeper.+    ///+    /// `migrateNotes` claims what the notes are filed under+    /// (`documentNotes.identifier`); every load and creation resolves its+    /// identifier from `session.source` via `resolveNoteContext`. Attempt 1+    /// migrates `clipboard/<S>` onto `first.md` and deletes the clipboard+    /// record, then finds itself superseded and returns *without*+    /// `session.didSave(to:)` — so attempt 2 migrates from `first.md` (T-1812)+    /// while the session still says `.clipboard`. A note created inside attempt+    /// 2's save window therefore resolves the clipboard identifier, which a+    /// claim on `first.md` alone does not cover: unclaimed, it clears the+    /// migration's container, finds nothing at the deleted clipboard record,+    /// fabricates a clipboard-keyed container and resurrects that record — after+    /// which attempt 2's post-save guard reads "a newer document replaced me",+    /// abandons its rebind and reports success. Exactly the reported end state:+    /// the session on the file, `NotesManager` still keyed to the clipboard.+    ///+    /// Drop the clipboard identifier from `migrateNotes`' claim list and this+    /// fails; the single-attempt creation test above stays green, because there+    /// the two identities are the same one.+    @Test("a note added during the second attempt of a chain follows the notes to that file")+    func noteCreatedDuringSecondAttemptFollowsTheNotesToTheFinalFile() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let session = DocumentSession(clipboardContent: "# Pasted")+        let block = MarkdownBlock.paragraph(markdown: "Pasted")+        session.parsedBlocks = [block]+        let manager = NotesManager.makeForTesting(store: store)+        await manager.createNote(+            content: "Clipboard note",+            for: block,+            sourceIndex: 0,+            in: makeStructure(from: [block]),+            source: .clipboard,+            sessionID: session.id+        )+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let firstIdentifier = resolver.resolve(from: firstURL)+        let secondIdentifier = resolver.resolve(from: secondURL)++        // Keyed on the destination so it lands in the *second* attempt's save,+        // not the setup note's or the first attempt's.+        await store.setOnSave(forPath: secondIdentifier.path) { [manager] in+            await manager.createNote(+                content: "Note added during the second attempt",+                for: block,+                sourceIndex: 0,+                in: MarkdownSectionBuilder.build(from: [block]),+                source: .clipboard,+                sessionID: session.id+            )+        }++        await runOverlappingSaves(session: session, manager: manager)++        #expect(session.source == .file(url: secondURL))+        #expect(manager.documentNotes?.identifier == secondIdentifier)+        #expect(manager.cachedDocumentIdentifier == secondIdentifier)+        #expect(manager.documentNotes?.notes.map(\.content)+            == ["Clipboard note", "Note added during the second attempt"])+        #expect(await store.storedNotes[secondIdentifier.path]?.notes.map(\.content)+            == ["Clipboard note", "Note added during the second attempt"])+        // Neither identity the chain passed through is left holding a record.+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)+        #expect(await store.storedNotes[firstIdentifier.path] == nil)+    }++    /// The load half of the same divergence. A load resolved through the+    /// still-`.clipboard` session during attempt 2's save is not retired by a+    /// claim on `first.md`: it reads the deleted clipboard record as "no notes",+    /// `applicableNotes` declines the migration's container as another+    /// document's, and `clearNoteState()` empties memory — so attempt 2's+    /// post-save guard sees no container at all, abandons its rebind, and the+    /// manager finishes keyed to the clipboard with the window title back at+    /// "Untitled".+    @Test("a load during the second attempt of a chain does not revert to the clipboard")+    func sourceLoadDuringSecondAttemptDoesNotRevertToTheClipboard() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let (session, manager) = await makeSessionWithClipboardNote(store: store)+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let firstIdentifier = resolver.resolve(from: firstURL)+        let secondIdentifier = resolver.resolve(from: secondURL)++        await store.setOnSave(forPath: secondIdentifier.path) { [manager] in+            await manager.loadNotes(+                source: .clipboard,+                sessionID: session.id,+                blocks: session.parsedBlocks+            )+        }++        await runOverlappingSaves(session: session, manager: manager)++        #expect(session.source == .file(url: secondURL))+        #expect(manager.documentNotes?.identifier == secondIdentifier)+        #expect(manager.cachedDocumentIdentifier == secondIdentifier)+        #expect(manager.documentPath == secondURL.lastPathComponent)+        #expect(manager.documentNotes?.notes.map(\.content) == ["Clipboard note"])+        #expect(await store.storedNotes[secondIdentifier.path]?.notes.map(\.content)+            == ["Clipboard note"])+        #expect(await store.storedNotes[clipboardIdentifier.path] == nil)+        #expect(await store.storedNotes[firstIdentifier.path] == nil)+    }++    /// The claims are a *stack* per identifier, and this is the shape that needs+    /// it. Now that every migration in a chain also claims the chain's clipboard+    /// identity, two overlapping attempts really do hold that one identity at+    /// once: the outer attempt claims it as its source, the inner one — started+    /// inside the outer's `store.save`, migrating from the outer's destination —+    /// claims it again alongside its own source.+    ///+    /// The outer attempt then finishes *first* (it is superseded, so it gives up+    /// its rebind and returns) while the inner one is still parked in its save.+    /// A note created in that gap must join the inner migration's destination,+    /// which is only well-defined if the outer's release retired the outer's own+    /// ticket and left the inner claim standing.+    ///+    /// Two mutations fail this test, and each is the obvious "simplification":+    /// collapse `migrationClaims` to one claim per identifier and the inner+    /// migration overwrites the outer's entry, which the outer's release then+    /// removes outright — the creation is not redirected at all and resurrects+    /// the clipboard record. Release with `removeLast()` instead of by ticket+    /// and the outer's release pops the *inner* claim — the creation is+    /// redirected onto the finished attempt's `first.md`.+    @Test("an outer migration releasing first leaves the inner claim standing")+    func outerMigrationReleaseKeepsTheInnerClaimOnTheClipboardIdentity() async {+        let store = HookedNotesStore()+        let resolver = DocumentIdentifierResolver()+        let session = DocumentSession(clipboardContent: "# Pasted")+        let block = MarkdownBlock.paragraph(markdown: "Pasted")+        session.parsedBlocks = [block]+        let manager = NotesManager.makeForTesting(store: store)+        await manager.createNote(+            content: "Clipboard note",+            for: block,+            sourceIndex: 0,+            in: makeStructure(from: [block]),+            source: .clipboard,+            sessionID: session.id+        )+        let clipboardIdentifier = resolver.resolve(forClipboardSession: session.id)+        let firstIdentifier = resolver.resolve(from: firstURL)+        let secondIdentifier = resolver.resolve(from: secondURL)++        let innerIsParkedInSave = Latch()+        let releaseInner = Latch()+        let innerTask = TaskBox()++        await store.setOnSave(forPath: firstIdentifier.path) { [manager] in+            // The inner attempt parks in its own save and stays there until this+            // test's creation has run.+            await store.setOnSave(forPath: secondIdentifier.path) {+                await innerIsParkedInSave.open()+                await releaseInner.wait()+            }+            await innerTask.hold(Task { @MainActor in+                await manager.migrateNotes(+                    fromClipboardSession: session.id,+                    previousDestination: firstURL,+                    toFileURL: secondURL+                )+            })+            await innerIsParkedInSave.wait()+        }++        let outerMigrated = await manager.migrateNotes(+            fromClipboardSession: session.id,+            toFileURL: firstURL+        )++        // The outer attempt gave up its rebind (the inner one moved the notes+        // past it) and has released its claims; the inner is still in its save.+        #expect(outerMigrated == true)++        await manager.createNote(+            content: "Note added between the two releases",+            for: block,+            sourceIndex: 0,+            in: MarkdownSectionBuilder.build(from: [block]),+            source: .clipboard,+            sessionID: session.id+        )++        await releaseInner.open()+        let innerMigrated = await innerTask.value()++        #expect(innerMigrated == true)+        #expect(manager.documentNotes?.identifier == secondIdentifier)+        #expect(manager.cachedDocumentIdentifier == secondIdentifier)+        #expect(manager.documentNotes?.notes.map(\.content)+            == ["Clipboard note", "Note added between the two releases"])+        #expect(await store.storedNotes[secondIdentifier.path]?.notes.map(\.content)+            == ["Clipboard note", "Note added between the two releases"])+        // The inner attempt retired its own source. The outer one gave up before+        // reaching its delete, so the clipboard record survives as a stale orphan+        // (Decision 5) — what must not happen is the creation writing into it,+        // which is how the unstacked claim fails.+        #expect(await store.storedNotes[firstIdentifier.path] == nil)+        #expect(await store.storedNotes[clipboardIdentifier.path]?.notes.map(\.content)+            == ["Clipboard note"])+    }++}
specs/bugfixes/clipboard-save-as-notes-manager-key/report.md Added +454 / -0
diff --git a/specs/bugfixes/clipboard-save-as-notes-manager-key/report.md b/specs/bugfixes/clipboard-save-as-notes-manager-key/report.mdnew file mode 100644index 00000000..165af729--- /dev/null+++ b/specs/bugfixes/clipboard-save-as-notes-manager-key/report.md@@ -0,0 +1,454 @@+# Bugfix Report: Clipboard Save As can finish with NotesManager still keyed to clipboard++**Date:** 2026-08-29+**Status:** Fixed+**Ticket:** T-2231++## Description of the Issue++A clipboard Save As could complete "successfully" — the session transitioned to+the saved file, the notes were written to the file's record — while+`NotesManager` was left holding the *clipboard* container and the clipboard+cached identity. The two halves of the app then disagreed about which document+was open: the reader named the file, but every note API that derives its+persistence context from `documentNotes` (`createDocumentNote(content:)`,+`createReply(content:to:)`, and the imported-note resolved cache keyed on+`cachedDocumentPath`) wrote to the obsolete clipboard record. The clipboard+notes file was also never retired.++**Reproduction steps:**++1. Paste a document, add a note (so a clipboard-keyed container exists).+2. Let a `loadNotes` for that clipboard document be in flight — the reader's+   initial load, or a re-parse-triggered reload; the Save control is gated only+   on `session.isUnsaved`, never on notes loading, so this window is reachable.+3. Save As to a file while that load is suspended in `NotesStore.load`.+4. `migrateNotes` republishes the container under the file identifier and+   suspends in `store.save`; the clipboard load resumes inside that window.+5. Add a note after the save completes, then reopen the saved file — the note is+   gone.++**Impact:** Silent note loss on a clipboard Save As. High severity, because the+state looks correct: both containers carry the same content, so nothing is+visibly wrong until the file is reopened and the post-save work is missing.++## Investigation Summary++- **Symptoms examined:** `migrateNotes` returning `true` (success) on a path+  that performs no identity rebind, and `ClipboardSaveFlow` treating that as a+  completed save (`session.didSave(to:)`).+- **Code inspected:** `prism/Services/NotesManager.swift` (`loadNotes`,+  `applicableNotes`, `migrateNotes`), `prism/ViewModels/ClipboardSaveFlow.swift`,+  `prism/Services/NotesManager+DocumentLevel.swift` (`currentDocumentContext`),+  `prismTests/ConsecutiveSaveAsTests.swift`.+- **Hypotheses tested and ruled out:**+  - *The existing generation guard covers it* — it does not. `migrateNotes`+    starts no load and bumps no generation, so a load overlapping a migration+    keeps a current generation.+  - *The cached-path guard (T-369) covers it* — it does not. The migration+    rebinds `cachedDocumentPath` only **after** its save, so for the length of+    that await the cached path still names the source and the guard passes.+  - *`applicableNotes` (T-2089) covers it* — it does the opposite. It sees an+    in-memory container whose identifier differs from the load's identifier,+    correctly classifies that as the T-1811 hazard, and applies the load's own+    (pre-migration) snapshot — which here is precisely the wrong choice, because+    the "other document" is this same document one step into its own migration.++## Discovered Root Cause++**Defect type:** Race condition — an identity transition that is not atomic+across a suspension point, with no ordering between it and a concurrent load of+the identity being moved.++`migrateNotes` performs the clipboard→file transition in two halves separated by+`await store.save(notes)`:++- before the save: `documentNotes` is republished under the target identifier;+- after the save: `cachedDocumentPath`, `cachedDocumentIdentifier` and+  `documentPath` are rebound, and the source record is deleted.++For the length of that await, the manager is mid-transition and *every* guard in+`loadNotes` still admits a load of the source document: its generation is+current, and the cached path still names the source. Such a load therefore+republishes the pre-migration snapshot over the migration's container. The+migration then resumes into+`guard documentNotes?.identifier == targetIdentifier else { return true }`,+reads the source's own snapshot as "a newer document replaced me" — the T-1812+case that guard was written for — and returns success without completing its+second half.++**Why it occurred:** the post-save guard's premise was that anything replacing+`documentNotes` during the save belongs to a *different* document. That premise+held for every writer considered when it was written (T-1812) and silently+failed for one that was not: a load of the migration's own source. Neither side+knew about the other, and the two-phase rebind made the window observable.++**Contributing factors:** the Save control is not gated on notes loading, so the+initial iCloud load and a Save As legitimately overlap in production; and both+containers carry identical content, so the fault leaves no visible trace until+new work is written under the retired identifier.++### Second root cause: the claim key and the addressing identity diverge++Found in review, after the claim above was in place. `migrateNotes` claimed+`notes.identifier` — what the notes are *filed under* — while every load and+creation resolves its identifier from `session.source` via `resolveNoteContext`.+Those two are the same thing on the first attempt in a save chain and part+company on the next one (Req 4.7 / T-1812):++1. Attempt 1 migrates `clipboard/<S>` onto `first.md` and deletes the clipboard+   record, then fails `guard session.isCurrentSaveAttempt(attempt)`+   (`ClipboardSaveFlow`) and returns **without** `session.didSave(to:)`.+2. So `session.source` is still `.clipboard`, while attempt 2's claim is on+   `first.md`.+3. Nothing in attempt 2's window covers the clipboard identity: neither+   `isMigratingAway` checkpoint fires and `migrationDestination` returns nil. A+   load resolved through the session reads the deleted clipboard record as "no+   notes" and `clearNoteState()` empties memory; a creation clears the+   migration's container, finds nothing at the deleted record, fabricates a+   clipboard-keyed container and resurrects it.+4. `migrateNotes` then fails its post-save guard and returns success — the+   originally reported end state, one chain-step deeper.++The fix is the same claim, taken on both identities: the clipboard identifier is+derivable inside `migrateNotes` from its `sessionID` parameter, so it is claimed+alongside the source (de-duplicated when they are the same) and loads and+creations addressed through the still-`.clipboard` session are retired or+redirected too.++## Resolution for the Issue++A migration now **owns every identity that still addresses the document it is+moving** for the whole of the call — the identifier its notes are filed under and+the clipboard identifier its callers still resolve — and the two things that can+resolve their context from one of those identities while it is held are answered+differently:++- A **load** for it is superseded by the migration and abandons its result — the+  same treatment a load already gets for a newer generation. It wanted to+  publish the source's stored snapshot, and that snapshot is what the migration+  has already moved.+- A **creation** for it is *redirected* onto the migration's target rather than+  retired. It is about the document in front of the reader, and that document is+  the one being saved.++**Changes made:**++- `prism/Services/NotesManager.swift` — new `migrationClaims`+  (claimed identifier → stack of `MigrationClaim { ticket, target,+  targetDisplayName }`, `@ObservationIgnored`), with+  `beginMigration(from:to:displayName:)` returning the ticket,+  `endMigration(from:ticket:)`, `isMigratingAway(from:)` and+  `migrationDestination(for:)`.+- `prism/Services/NotesManager.swift` — `migrateNotes` claims **both**+  identities that still address the document — `sourceIdentifier` (what its+  notes are filed under) and the clipboard identifier derived from its+  `sessionID` parameter (what `resolveNoteContext` still hands the callers),+  de-duplicated when they coincide — before its `store.save`, and releases each+  by ticket on every exit via `defer`, so the claims also cover the post-save+  rebind and the `store.delete(for: sourceIdentifier)` await.+- `prism/Services/NotesManager.swift` — `loadNotes` is retired for a claimed+  identifier at **two** points: at entry, before its prologue (which rebinds+  `cachedDocumentPath`/`cachedDocumentIdentifier`, and `documentPath` in the+  `source:`-taking overload, all *before* the first await — a post-await guard+  is too late to undo those), and again after `store.load` for a load that was+  already in flight. `cachedBlocks = blocks` moves ahead of the entry guard,+  preserving the T-1811 rule that a skipped load still retains its blocks.+- `prism/Services/NotesManager.swift` — `noteContainer(for:)` resolves a claimed+  source context to the claim's target before its identity check. Creations+  resolve their context from `DocumentSource`, which still says `.clipboard`+  for the whole migration, so without this a note added mid-save cleared the+  migration's container, reloaded the not-yet-deleted source record and+  republished it — the same end state, reached from the creation side.+- Comment on `migrateNotes`' post-save identity guard updated: a mismatch there+  means a different document *because* of the claim, on both paths.++**Approach rationale:** the claim is the narrowest thing that orders the+migration against everything else that resolves its context from the source. It+says only "this identity is mid-transition, and here is where it is going",+leaves the T-1811/T-1812 behaviour for *other* documents completely untouched+(nothing outside the two identities that address the migrating document is+claimed), and needs no new state on the load side. A ticketed *stack* per+identifier rather than a single claim, because the same identity really can be+claimed twice: every attempt in a chain also claims the chain's clipboard+identity, so an attempt starting inside its predecessor's save stacks a second+claim on it, and the two release in either order. The target rides on the claim+because the two callers want opposite things from it: a load is retired by the+claim, a creation follows it.++**Alternatives considered:**++- **Bump `loadGeneration` in `migrateNotes`.** Reuses existing machinery and+  fixes the reported interleaving, but it retires *every* in-flight load, not+  just the source's. A load for another document that started before the+  migration would be discarded, leaving `documentNotes` on the migrated file+  while the reader is elsewhere — the T-1811 hazard, reintroduced.+- **Widen the post-save guard to `notesBelongToSaveChain`.** Accepts the source+  identifier post-save and re-applies the rebind. It also accepts the+  clipboard/previous-destination identities that belong to a *different*+  in-flight save chain, and the existing code comments already rule it out for+  that reason.+- **Have the migration re-adopt whatever `documentNotes` holds when it finds the+  source identifier.** Recovers instead of preventing, and loses any note a+  concurrent creation appended under the source identifier before the source+  record is deleted — trading one data-loss shape for another.+- **Gate the Save control on `isLoading`.** Shrinks the window without closing+  it (the migration's own await is unaffected) and makes Save As intermittently+  unavailable for a UI reason the user cannot see.++### The failure revert: identity only, never the notes++The redirect above changes what `migrateNotes`' failure branch is allowed to do.+It used to revert by republishing `notes` — the local copy it took *before*+`store.save`:++```swift+guard documentNotes?.identifier == targetIdentifier else { return false }+notes.identifier = sourceIdentifier      // the pre-save snapshot+documentNotes = notes+```++Once a creation may be redirected into the window, that snapshot is stale by+exactly that note. The creation appends to the **live** `documentNotes` and+republishes it synchronously, without changing its identifier — so the guard+still passes, and the revert drops the note from the container the reader is+looking at while the creation's own `persistNotes` may already have written it+to the destination's record. That is this bug's own shape (a stale snapshot+clobbering fresher state) on the failure path. The revert now re-keys the live+container instead:++```swift+guard var live = documentNotes, live.identifier == targetIdentifier else { return false }+live.identifier = sourceIdentifier+documentNotes = live+```++Only the identity is the migration's to undo; the notes are not.++### The claims are a ticketed stack, released individually++`migrationClaims` holds a stack of ticketed claims per identifier, and each+migration retires exactly its own ticket — never "the last one", and never the+whole key. Two overlapping migrations can hold one identity at once, now that+every attempt in a chain claims the chain's clipboard identity as well as its+own source: an attempt that starts inside its predecessor's `store.save` stacks+a second claim on that identity, and the outer attempt (superseded, so it gives+up its rebind) can release *first*. A single claim per identifier would have+been overwritten by the inner migration and then removed outright by the outer+one's release, leaving a creation unredirected; `removeLast()` would have popped+the inner claim and sent that creation to the finished attempt's destination.+`outerMigrationReleaseKeepsTheInnerClaimOnTheClipboardIdentity` fails under both+mutations.++### The second checkpoint: why the guard is not repeated++`loadNotes` has a second post-await checkpoint, after `backupIfNeeded`, which+re-checks the generation (T-1586) but **not** `isMigratingAway`. Review asked+whether that asymmetry is a gap. It is not, and adding the guard "for+consistency" would be a regression.++Everything between the new first-checkpoint guard and the `backupIfNeeded` await+is synchronous and MainActor-isolated (`applicableNotes`,+`relocationEngine.relocate`, `checkForRelocation`, the state assignments), so a+migration of *this* identifier cannot begin between the two checkpoints. It can+only begin inside the backup await, which leaves two cases:++1. **It began and finished inside the window.** The claim is already released, so+   an `isMigratingAway` check reads false — a no-op.+2. **It is still parked in its own `store.save`.** The check would fire and skip+   `saveCurrentState()`. That is the wrong answer. `saveCurrentState` reads the+   live `documentNotes`, which the migration rebound to its target *before* the+   save it is parked in, so it persists the migration's own container under the+   destination — never the source, never a resurrected clipboard record — and+   republishes it under the same identifier, so the migration's post-save+   identity guard is still satisfied when it resumes. What it uniquely adds is+   the relocation write: `documentNotes.notes[].blockId` is folded in from+   `anchoredNotes` here and nowhere else. Skipping it strands memory with+   pre-relocation block ids that the next `rebuildAnchoredNotes()` (a note+   deletion, a resolve toggle, a note creation) reads straight back, undoing the+   relocation this branch exists to persist (Req 4.5).++The comment at the checkpoint states this, and+`loadResumingInsideAMigrationStillPersistsItsRelocation()` pins it: the test+fails when the guard is added.++## Regression Test++**Test file:** `prismTests/ConsecutiveSaveAsTests.swift` — two suites:+`ConsecutiveSaveAsTests` and, for the three tests that need a two-attempt chain,+`SaveAsMigrationClaimTests` (split out so the original suite stays inside+SwiftLint's `type_body_length` budget; the fixtures and hooked stores are shared+at file scope).++**Test names:**++- `migrationSurvivesASourceLoadLandingDuringItsSave()` — drives `migrateNotes`+  directly with `HookedNotesStore`, landing a full clipboard `loadNotes` inside+  the target save. Asserts the manager ends on the file identifier, that+  `cachedDocumentIdentifier` follows, that the file's record holds the note, and+  that the clipboard record is retired.+- `saveAsKeepsNotesOnTheDestinationWhenASourceLoadLandsMidFlight()` — the same+  interleaving through the real `ClipboardSaveFlow`, then creates a note via+  `createDocumentNote(content:)` and asserts it is persisted under the saved+  file rather than the clipboard. This is the user-visible consequence.+- `noteCreatedDuringMigrationFollowsTheNotesToTheFile()` — the creation side.+  Adds a note from inside the migration's `store.save`, with `session.source`+  still `.clipboard`, and asserts both notes end up on the file and the+  clipboard record is retired. Fails (manager back on the clipboard) when the+  `noteContainer(for:)` redirect is removed.+- `sourceLoadStartingDuringTheSourceDeleteDoesNotRevertCachedIdentity()` — pins+  the *entry* guard specifically, by landing the load in the migration's source+  delete, i.e. after the cached identity has already been rebound and will not+  be rebound again. Fails (cached identity and `documentPath` back on the+  clipboard) when the entry guards are removed; the post-await guard alone does+  not catch it.+- `failedMigrationReleasesItsClaimOnTheSource()` — pins the `defer`. Fails a+  migration, then writes to the source record and loads it, which only succeeds+  if the claim was released. Fails when the release is moved to the success+  tail.+- `loadResumingInsideAMigrationStillPersistsItsRelocation()` — pins the+  *absence* of a guard at the second post-await checkpoint (see "The second+  checkpoint" above). A latch pair lands a whole migration inside a relocating+  load's `backupIfNeeded` window and holds it parked in `store.save` until the+  load's tail has run, then asserts both that the migration completed and that+  the relocated block id reached `documentNotes`. Deterministic — no sleeps.+  Verified to fail when `guard !isMigratingAway(from: identifier)` is added at+  that checkpoint.+- `sourceLoadStartedBeforeTheMigrationIsRetiredWhenItResumesInside()` — pins the+  **post-`store.load`** guard, which nothing else did. Every other test here+  starts its load inside the migration window, so the *entry* guard covers them+  and they all stay green when the post-await one is deleted. This one parks the+  load in `store.load` before any migration exists — so the entry guard passes+  it through, correctly — and releases it inside the migration's save, holding+  the migration there until the load's tail has run. Fails (manager and cached+  identity back on the clipboard, clipboard record still present) when the+  post-`store.load` guard is removed.+- `noteCreatedDuringAFailingMigrationSurvivesTheRevert()` — crosses the two+  halves neither of the tests above meets: a redirected creation lands inside a+  save that then **fails**. Asserts the identity reverts to the clipboard *and*+  that the note created in the window is still in `documentNotes` and+  `anchoredNotes`. Fails when the revert republishes the pre-save local snapshot+  instead of re-keying the live container.+- `noteCreatedDuringSecondAttemptFollowsTheNotesToTheFinalFile()` — the second+  root cause. Drives a real two-attempt chain through `ClipboardSaveFlow` and+  lands a creation (resolving `.clipboard`, as the session still does) inside+  **attempt 2's** save, via a destination-keyed store hook so the setup note's+  and attempt 1's saves do not consume it. Fails — manager back on the+  clipboard, clipboard record resurrected — when the clipboard identifier is+  dropped from `migrateNotes`' claim list.+- `sourceLoadDuringSecondAttemptDoesNotRevertToTheClipboard()` — the load half+  of the same divergence: the load reads the deleted clipboard record as "no+  notes", `applicableNotes` declines the migration's container as another+  document's, and `clearNoteState()` empties memory, so the post-save guard has+  nothing to match. Fails on the same mutation.+- `outerMigrationReleaseKeepsTheInnerClaimOnTheClipboardIdentity()` — the+  executable pin for the claim *stack*. Two overlapping migrations hold the+  chain's clipboard identity at once (the inner one starts inside the outer's+  `store.save`), the outer releases first, and a note created in that gap must+  join the inner migration's destination. Fails when `migrationClaims` is+  collapsed to one claim per identifier **and** when `endMigration` releases with+  `removeLast()` instead of by ticket.++The first two fail on the pre-fix code with exactly the reported symptom (manager+keyed to `clipboard/<session-uuid>`, clipboard record still present, later note+written to it), and the sibling test+`migrationDoesNotRebindIdentityOfADocumentLoadedMidFlight` (T-1812) still+passes, pinning that a load for *another* document still wins. Every one of the+eleven was mutation-checked individually — each fails when, and only when, the+guard it names is removed (or, for the relocation one, added), and each mutation+run failed exactly the expected tests out of the file's 23: dropping the+clipboard claim fails the two chain tests and the stack pin and nothing else;+`removeLast()` and a single claim per identifier each fail the stack pin alone.++`HookedNotesStore` now runs its `save`/`delete` hooks **after** the write rather+than before. Production `NotesStore.save` contains no `await` — the invariant+`applicableNotes` depends on — so its write is indivisible and the only real+suspension is the caller resuming afterwards. Hooking before the write models+something that cannot happen, and lets a reentrant save from inside the hook be+silently overwritten by the outer one on the way out, which would have made the+creation test assert a harness artefact.++**Run command:**++```bash+xcodebuild test -project prism.xcodeproj -scheme prism \+  -destination 'platform=macOS' -testPlan prism \+  -only-test-configuration "en (base)" \+  -only-testing:prismTests/ConsecutiveSaveAsTests \+  -only-testing:prismTests/SaveAsMigrationClaimTests test+```++## Affected Files++| File | Change |+|------|--------|+| `prism/Services/NotesManager.swift` | Migration claims both identities that address the migrating document and records where it is going; `loadNotes` defers to an in-flight migration of its own document at entry and after its store await, `noteContainer(for:)` redirects a creation onto the migration's target, and the second post-await checkpoint states why the same guard must not be repeated there |+| `prismTests/ConsecutiveSaveAsTests.swift` | Eleven regression tests for the load-, creation-, entry-, chain- and failure-path halves of the window, plus delete/load and destination-keyed save hooks on the store, a failing-save hook, a hooked backup store, and a latch and task box for the interleaved ones |+| `specs/clipboard-notes/requirements.md` | Requirement 4.8 — a migration owns, for the whole call, every identity that still addresses the document it is moving, enforced at three sites |+| `specs/clipboard-notes/decision_log.md` | Decision 8 — the claims, the load/creation asymmetry, the identity-only revert, and why the claim covers the clipboard identity as well as the source |+| `docs/agent-notes/notes-system.md` | Records the claim and its load-versus-creation asymmetry; corrects the two passages that described this window as open and uncovered |+| `CHANGELOG.md` | User-facing entry under `[Unreleased] → Fixed` |++## Verification++**Automated:**++- [x] Regression tests pass (and fail on the pre-fix code)+- [x] Targeted run of the notes/save-flow suites after the second fix: 137 tests,+      137 passed, 0 failed, confirmed through `Tools/check-test-results.sh`+- [x] Each of the eleven T-2231 tests mutation-checked against its own guard:+      removing the `noteContainer` redirect, the `loadNotes` entry guards, or the+      `defer` release fails exactly the test that names it, and adding a guard at+      the second checkpoint fails only+      `loadResumingInsideAMigrationStillPersistsItsRelocation`+- [x] `ConsecutiveSaveAsTests` suite passes in full, including the T-1812/T-1811+      tests the fix must not disturb+- [x] Notes suites pass (`NotesManagerLoadRaceTests`, `NotesManagerClipboardTests`,+      `NotesManageriCloudSignInTests`, `SaveFlowIntegrationTests`,+      `LateSaveFinalisationTests`, `DocumentReloadNotesSyncTests`,+      `NotesManagerTests`, `NotesManagerDocumentLevelTests`,+      `NotesManagerThreadingTests`)+- [x] `make lint` — 0 violations+- [!] `make test-quick` did not finish clean on this machine, and not because of+      this change. Several fix agents were running WebKit-heavy suites in+      parallel; the first run aborted the shared test host on WebKit's+      main-thread assertion (287 reported failures, ~283 of which never ran),+      and a retry produced 80 failures that are almost entirely+      `SpikeWebPageHarness … .loadTimedOut` at 46–55 s in the live-WebKit+      suites. The non-WebKit stragglers are timing-sensitive+      (`RawSourceViewModelTests`, `DocumentLayoutCoordinatorReloadTests`,+      the scroll-debounce tests) and pass when run on their own. Nothing in the+      failing set touches notes, clipboard, or Save As; every notes and+      save-flow suite listed above passes in a targeted run. Review round two+      re-ran the same targeted set: one flake+      (`LateSaveFinalisationTests.lateMidChainFailureClearsOnlyTheSavingDocumentsRestorePoint`,+      2.3 s against its usual 0.02 s) that passes on its own and on a rerun of+      the full set, then 120/120 green.++## Prevention++- Treat every two-phase identity change across a suspension point as needing an+  explicit claim on the identity being moved. `documentNotes`,+  `cachedDocumentIdentifier` and `cachedDocumentPath` are rebound in two halves+  around `store.save`; anything that reads them in between must be ordered+  against it, not merely guarded against staleness.+- A guard whose correctness rests on "whoever replaced me is a different+  document" should say so and name what makes it true. The post-save guard in+  `migrateNotes` now does.+- When adding a race guard, enumerate the writers that can land in the window —+  `applicableNotes` already does this for its own window, and the writer this bug+  needed (a load of the migration's source) is exactly the kind a roster misses.+  The first round of this fix then made the same mistake one level down: it+  closed the window for loads and asserted the window was closed, while a *note+  creation* resolving the same source context walked straight back into it. When+  a guard's justification is "nothing else can reach this state", enumerate what+  resolves the state from the same input, not just what the bug report named.++## Related++- T-1812 — consecutive Save As attempts cross-wiring note migration (the case+  the post-save guard was written for; must keep passing)+- T-1811 — sign-in reload rebinding identity onto a document the reader has left+- T-2089 — a load overwriting a note mutation (`applicableNotes`)+- T-1556 / T-369 — the generation and cached-path guards in `loadNotes`+- T-2213, T-2220, T-2245, T-2268 — adjacent open tickets, deliberately untouched
specs/clipboard-notes/decision_log.md Modified +61 / -0
diff --git a/specs/clipboard-notes/decision_log.md b/specs/clipboard-notes/decision_log.mdindex f932511c..a91ef91a 100644--- a/specs/clipboard-notes/decision_log.md+++ b/specs/clipboard-notes/decision_log.md@@ -249,3 +249,64 @@ The user does lose the destination they last chose: they asked for `second.md`, `ClipboardSaveFlow.finaliseFailure`, `DocumentFlowCoordinator.handleSaveFailed(notesRemainAt:)` (which also clears the persisted clipboard state, since the session stops being a clipboard document on this branch), `DocumentReaderView.onSaveFailed`, and Requirement 4.5. Pinned by `failedFirstAttemptStillRevertsToTheClipboard` and `failedMigrationMidChainSettlesOnTheNotesFile` in `prismTests/ConsecutiveSaveAsTests.swift`.  ---++## Decision 8: A Migration Owns Its Source Identity for the Whole Call++**Date**: 2026-08-30+**Status**: accepted++### Context++`migrateNotes` changes document identity in two halves around `await store.save(notes)`: the container is republished under the target identifier *before* the save, and the cached identity (`cachedDocumentPath`, `cachedDocumentIdentifier`, `documentPath`) is rebound and the source record deleted *after* it. For the length of that await the source identifier is still what every other entry point resolves — a clipboard Save As only calls `session.didSave(to:)` once the migration returns, and the Save control is gated on `session.isUnsaved`, never on notes loading, so the initial iCloud load and a Save As legitimately overlap.++Anything that resolved the source identity and landed in that window republished the source's own state over the migration's container. The migration then resumed into `guard documentNotes?.identifier == targetIdentifier`, read the source's snapshot as "a newer document replaced me" — the T-1812 case that guard was written for — and returned **success** without doing its second half. `ClipboardSaveFlow` called `session.didSave(to:)` on that success, and the session and the notes manager parted ways: notes on the file, `NotesManager` still keyed to the clipboard, every later note written to a record the reopened file will never read (T-2231).++Decisions 6 and 7 govern what a *failed* migration settles on. Neither says anything about who owns the identity while the migration is still running, which is the gap this closes.++### Decision++A migration claims, for the whole call, every identifier that still addresses the document it is moving (`migrationClaims`) — taken before the save, released by `defer`, so the claims also cover the post-save rebind and the source delete. That is two identifiers, not one, because the migration's own key and its callers' diverge on a supersession chain: `migrateNotes` keys off what the notes are filed under (`documentNotes.identifier`), while every load and creation resolves its identifier from `session.source` via `resolveNoteContext`. They agree on the first attempt in a chain and part company on the next: attempt 1 migrates the clipboard notes onto its file, is superseded, and returns *without* `session.didSave(to:)`, so attempt 2 migrates from that file (Decision 6, T-1812) while the session it serves still says `.clipboard`. So the clipboard identity is derived from the `sessionID` parameter and claimed alongside the source, de-duplicated when they coincide.++The two things that can walk into that window are answered differently, because they want different things:++- A **load** for the claimed identifier is superseded and abandons its result, exactly as it is by a newer generation. Checked twice: at entry, for a load that starts inside the window, and after `store.load`, for a load that started before the window and resumes inside it.+- A **creation** for it is not superseded but *redirected*: `noteContainer(for:)` swaps in the claim's destination, so the note joins the migration's own container instead of resurrecting the source's.+- A **failure** reverts only the identity, on the *live* container rather than the snapshot taken before the save, so a note the redirect appended during the window survives.++Nothing beyond those two identities is claimed. A load for any other document still wins.++### Rationale++The post-save guard's premise is "a mismatch here means a genuinely different document". That premise was simply false while anything could republish the source inside the window, and every alternative below tries to *recover* from the false premise instead of restoring it. The claim makes it true, which is what lets the guard keep giving the T-1811/T-1812 answer it was written for without also mis-firing on the migration's own document.++Answering loads and creations differently is not a special case: a load wants to publish the source's stored snapshot, and that snapshot is exactly what the migration has already moved, so it has nothing left to say. A creation wants to add a note to the document in front of the reader — which *is* the document being saved, just under a name the session has not adopted yet.++The failure branch follows from the redirect rather than being independent of it. Once a creation may land inside the window, the local snapshot `migrateNotes` took before its save is stale by exactly that note, and reverting by republishing it would drop the note from the container the reader is looking at while the creation's own `persistNotes` may already have written it to the destination's record — this bug's own shape (a stale snapshot clobbering fresher state) on the failure path. Only the identity is the migration's to undo.++### Alternatives Considered++- **Bump `loadGeneration` in `migrateNotes`**: Retires in-flight loads with machinery that already exists — Rejected because it retires *every* in-flight load, including one for another document started before the migration, leaving `documentNotes` on the migrated file while the reader is elsewhere. T-1811, reintroduced.+- **Widen the post-save guard to `notesBelongToSaveChain`**: Accepts the identities a save chain legitimately passes through — Rejected because it also accepts a clipboard/previous-destination identity belonging to a *different* in-flight chain, reproducing T-2231 from the other side. The existing comments already rule it out.+- **Re-adopt whatever `documentNotes` holds when the source identifier is found post-save**: Recovers the rebind without new state — Rejected because it recovers instead of preventing, and loses any note a concurrent creation appended under the source before that record is deleted.+- **Gate the Save control on `isLoading`**: No notes-layer change at all — Rejected because it shrinks the window without closing it; the migration's own await is unaffected.+- **Claim the source identifier alone**: One claim, taken where the migration already knows its key — Rejected because the source and the callers' identity diverge on the second attempt of a save chain, which is exactly where the bug then survived: with the clipboard identity unclaimed, a load or creation resolved through the still-`.clipboard` session walked back into the window and left the migration reporting success for a rebind it never did.+- **Claim with a boolean or a depth count rather than a stack**: Smaller state — Rejected because a release could then pop the wrong entry, or remove the key while another migration still held it, so a creation landing afterwards would follow a *finished* migration's destination or none at all. This is not hypothetical now that every attempt in a chain claims the chain's clipboard identity: an attempt starting inside its predecessor's save stacks a second claim on that identity, and the two release in either order.++### Consequences++**Positive:**+- The post-save guard's premise is true, so `migrateNotes` no longer reports success for a rebind it did not do+- A note created while a Save As is in flight lands on the file, and survives if that save then fails+- T-1811/T-1812/T-1556/T-369 are untouched: a load for a different document still wins, and the claims are scoped to the two identities that address the migrating document++**Negative:**+- Three call sites now consult migration state that used to be local to `migrateNotes` — a load's two checkpoints and `noteContainer(for:)` — so the claim's lifetime is a cross-cutting invariant a reader has to hold+- The claim list is derived, not authoritative: it enumerates the identities today's callers can resolve, so a future caller addressing the document a third way would have to be added to it+- The claim is invisible from outside `NotesManager`, so a test can only drive its consequences, never assert it directly+- A migration that fails after a redirected creation leaves an orphaned copy of that note in the destination's record (the accepted-orphan class of Decision 5)++### Impact++`NotesManager.migrationClaims` / `beginMigration` / `endMigration` / `isMigratingAway` / `migrationDestination`, both `loadNotes` overloads, `noteContainer(for:)`, and `migrateNotes`' failure revert and post-save guard. Requirement 4.8. Pinned by `migrationSurvivesASourceLoadLandingDuringItsSave`, `saveAsKeepsNotesOnTheDestinationWhenASourceLoadLandsMidFlight`, `sourceLoadStartedBeforeTheMigrationIsRetiredWhenItResumesInside`, `sourceLoadStartingDuringTheSourceDeleteDoesNotRevertCachedIdentity`, `noteCreatedDuringMigrationFollowsTheNotesToTheFile`, `noteCreatedDuringAFailingMigrationSurvivesTheRevert`, and `failedMigrationReleasesItsClaimOnTheSource` in `prismTests/ConsecutiveSaveAsTests.swift`. The second claimed identity is pinned by `noteCreatedDuringSecondAttemptFollowsTheNotesToTheFinalFile` and `sourceLoadDuringSecondAttemptDoesNotRevertToTheClipboard` in the same file, and the stack by `outerMigrationReleaseKeepsTheInnerClaimOnTheClipboardIdentity`; `migrationDoesNotRebindIdentityOfADocumentLoadedMidFlight` pins the T-1812 behaviour it must not disturb.++---
docs/agent-notes/notes-system.md Modified +16 / -3
diff --git a/docs/agent-notes/notes-system.md b/docs/agent-notes/notes-system.mdindex ec58b879..047432ec 100644--- a/docs/agent-notes/notes-system.md+++ b/docs/agent-notes/notes-system.md@@ -111,9 +111,22 @@ Two consequences of settling that are easy to misread, both in Decision 7's Cons  Two smaller guards inside `migrateNotes`: it re-checks `documentNotes?.identifier == targetIdentifier` after the store save before rebinding cached document identity (a load for another document can land while it is parked in the store — the T-1811 failure shape), and it skips the source delete when source and target are the same file (re-saving to the same destination would otherwise delete the notes it just wrote). That post-save `targetIdentifier` guard is deliberately narrower than `notesBelongToSaveChain` above it: post-save, only the *target* identifier is acceptable, because substituting the broader chain predicate (which also accepts the clipboard/`previousDestination` identities) would rebind cached document identity onto a container that belongs to a different in-flight load — T-2231's symptom from the other side. Do not "simplify" the two guards to share the wider predicate. +**A migration owns, for the whole call, every identity that still addresses the document it is moving (T-2231).** The post-save guard's premise — "whoever replaced my container is a different document" — was false for one writer: the migration's *own source*. `migrateNotes` rebinds in two halves around `await store.save`, publishing `documentNotes` under the target before it and moving `cachedDocumentPath`/`cachedDocumentIdentifier`/`documentPath` only after it, so for the length of that await every existing `loadNotes` guard admits a load of the source (current generation, cached path still names the source). Such a load republished the pre-migration snapshot, the guard read it as a replacement, and the migration returned `true` without rebinding or retiring the source — session on the file, `NotesManager` on the clipboard, every later note written to a record the reopened file never reads.++`migrationClaims` (`NotesManager`, `@ObservationIgnored`) closes it: `beginMigration(from:to:displayName:)` records claimed identifier → target for the whole call, released by ticket in a `defer` on every exit including the failure revert. **A migration claims two identities, not one** — its source (`documentNotes.identifier`, what the notes are filed under) *and* the clipboard identifier derived from its `sessionID` parameter (what `resolveNoteContext` still hands every load and creation). They coincide on the first attempt in a chain and diverge on the next: attempt 1 migrates the clipboard notes onto its file, is superseded, and returns without `session.didSave(to:)` (`ClipboardSaveFlow`), so attempt 2 migrates from that file while the session still says `.clipboard`. Claiming the source alone left the whole bug standing one chain-step deeper — that was the escalated find on the T-2231 PR, not a hypothetical. It holds a *stack* of ticketed claims per identifier, not one claim or a depth count: with both identities claimed, two overlapping attempts really do hold the chain's clipboard identity at once, and the outer one can release first. A single claim would be overwritten by the inner migration and then removed outright by the outer's release; `removeLast()` would pop the inner claim and send a creation to the finished attempt's destination. Releases key on the ticket for exactly that reason — `defer` orders releases within one call, not across concurrent ones. The two things that can walk into the window are answered *differently*, and that asymmetry is the point:++- A **load** for a claimed identifier is retired — at entry (before `loadNotes`' prologue, which would otherwise revert the T-839 cache namespace and the window title with no later guard to undo it) and again after its `store.load` await, for a load already in flight. `cachedBlocks` is still retained ahead of the entry guard, for the same reason a skipped `guard iCloudAvailable` load retains them (T-1811): the content has not changed, only the identity it is filed under.+- A **creation** for it is redirected, not retired: `noteContainer(for:)` swaps the claimed context for the innermost claim's target. Creations resolve their context from `DocumentSource`, which still says `.clipboard` for the whole migration (`ClipboardSaveFlow` calls `didSave(to:)` only after it returns), so unredirected a note added mid-save would clear the migration's container, reload the not-yet-deleted source record, and republish it — reaching the identical end state from the creation side. Redirected, the note lands on the file the document is becoming.++Nothing outside those two identities is claimed. A load for any other document is the T-1811 hazard the post-save guard exists for and must still win. The claim list is derived rather than authoritative: it enumerates the identities today's callers can resolve, so a future caller addressing the migrating document a third way has to be added to it.++One asymmetry inside `loadNotes` is deliberate and pinned by a test: its **second** post-await checkpoint (after `backupIfNeeded`) re-checks the generation but **not** `isMigratingAway`. Everything between the first checkpoint and that await is synchronous, so a migration of this identifier can only begin inside the await — where the guard would skip `saveCurrentState()`, which is the wrong answer. `saveCurrentState` reads the live container, which the migration has already rebound to its target, so it persists the migration's own notes and leaves the post-save guard satisfied; what it uniquely adds is folding the relocated block ids back into `documentNotes` (Req 4.5), which nothing else does. Adding the guard "for symmetry" fails `loadResumingInsideAMigrationStillPersistsItsRelocation`.+ `DocumentFlowCoordinator.pendingSaveBookmark` carries the session and attempt its bookmark was created for; `completeSaveFlow` uses it only on a match, so a recents entry can never resolve to a different file than the one it names. -Regression coverage: `prismTests/ConsecutiveSaveAsTests.swift` — 12 tests, at least one per guard. The round-one tests were mutation-checked; the two reopening tests have not been (machine contention — see the bugfix report's Verification section), so treat their mutation matrix as intent, not evidence.+Regression coverage: `prismTests/ConsecutiveSaveAsTests.swift` — 20 tests, at least one per guard. The round-one T-1812 tests were mutation-checked; the two reopening tests have not been (machine contention — see the bugfix report's Verification section), so treat their mutation matrix as intent, not evidence. The eight T-2231 tests were mutation-checked one guard at a time: the load-side pair (`migrationSurvivesASourceLoadLandingDuringItsSave`, `saveAsKeepsNotesOnTheDestinationWhenASourceLoadLandsMidFlight`), the entry guard (`sourceLoadStartingDuringTheSourceDeleteDoesNotRevertCachedIdentity`), the post-`store.load` guard (`sourceLoadStartedBeforeTheMigrationIsRetiredWhenItResumesInside`), the `noteContainer` redirect (`noteCreatedDuringMigrationFollowsTheNotesToTheFile`), the identity-only failure revert (`noteCreatedDuringAFailingMigrationSurvivesTheRevert`), the `defer` release (`failedMigrationReleasesItsClaimOnTheSource`), and the *absence* of a second-checkpoint guard (`loadResumingInsideAMigrationStillPersistsItsRelocation`, which fails when one is added). **The post-`store.load` guard was untested for two rounds and nothing noticed**: every other test lands its load *inside* the window, where the entry guard catches it first, so deleting the post-await guard left the whole suite green. A guard needs a test whose load starts before the window and resumes in it, which is what the new one does.++`HookedNotesStore` runs its `save`/`delete` hooks **after** the write, not before. Production `NotesStore.save` has no internal `await` (the invariant `applicableNotes` rests on — see below), so its write is indivisible and the only real suspension is the caller resuming; a hook running first models something that cannot happen, and lets a reentrant save from inside the hook be silently clobbered by the outer one on the way out.  ### Finalising After the Document Has Been Replaced (T-2213) @@ -183,14 +196,14 @@ Its store read is a suspension point, so two creations that both arrive with not  `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.+- **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` is answered *before* this function rather than by it, as of T-2231: it republishes the container under a different identifier before its save, and its save-failure revert re-keys the *live* container back to the source identifier (never the pre-save snapshot, which a redirected creation may already have outrun) and never persists — neither half of the question, both halves of one migration. The ordering settles it instead. For the whole of that call the migration claims the identities that address its document (source and, on a save chain, the clipboard session), and a load for any of them is retired by `isMigratingAway` (at entry, or at the guard immediately above the `applicableNotes` call for one already in flight), so it never reaches here to be arbitrated. A load for *another* document still does, and the identifier check declines the migration's container as not being about that document. (Before that claim, the source load *did* reach here and `applicableNotes` correctly returned `loaded` — which was the wrong outcome for a reason outside this function's remit; see the T-2231 section above.) The creation-path argument is unchanged when `noteContainer` redirects a creation onto a migration's target: it still persists what it appends, and the container it publishes carries the target identifier. 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.+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 passed both guards, and `applicableNotes` returned `loaded` (the container's identifier is now the target, so the identifier check declines it), applying the pre-migration snapshot over the rebinding. That was a pre-existing window this guard neither opened nor closed (pre-T-2089 code overwrote unconditionally), and **it is closed now** — it was T-2231, and what closes it is the migration's claim on its source identity, not anything in `applicableNotes`: a source load is retired before it reaches here, whether it starts inside the window or merely resumes in it. Still do not cite the path guard as covering it; the path guard remains blind to this, which is exactly why a separate claim exists. 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. 
specs/clipboard-notes/requirements.md Modified +1 / -0
diff --git a/specs/clipboard-notes/requirements.md b/specs/clipboard-notes/requirements.mdindex c850cceb..d3191571 100644--- a/specs/clipboard-notes/requirements.md+++ b/specs/clipboard-notes/requirements.md@@ -58,6 +58,7 @@ Prism currently disables the notes feature for documents opened via clipboard pa 5. <a name="4.5"></a>IF migration fails (e.g., write error), the system SHALL retain the notes where they were before the attempt, log the failure, and set the document source to whichever document still holds them — the clipboard for the first save attempt, or the file an earlier attempt in the same save chain migrated them to (Decision 7). The failure message SHALL name that file when it is not the clipboard 6. <a name="4.6"></a>The system SHALL complete migration without requiring user interaction 7. <a name="4.7"></a>WHEN a second Save As starts while an earlier one is still migrating, the system SHALL run the attempts in order and SHALL apply only the latest attempt's outcome to the document source, the recent-files entry, and the note location (T-1812)+8. <a name="4.8"></a>WHILE a migration is in flight, the system SHALL treat every identifier that still addresses the document being migrated as owned by that migration — the identifier its notes are filed under, and the clipboard session identifier the callers still resolve when a superseded attempt has moved the two apart ([4.7](#4.7)). Ownership is enforced at three sites: a note load for a claimed identifier SHALL be checked against the migration on entry and again on resuming from the note store, and abandoned at either check rather than published; a note created on a claimed identifier SHALL be appended to the migration's destination; and a failure SHALL revert only the identity — never the notes, so a note created inside the window survives the revert (Decision 8, T-2231)  --- 
CHANGELOG.md Modified +1 / -0
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex bf353052..8c7c9f71 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0  ### Fixed +- Saving a pasted document to a file no longer leaves your notes attached to the pasted copy (T-2231). Saving moves the document's notes onto the file you chose, and while that move was under way the app could re-read the notes from where they had just been moved *from*, quietly putting the pasted copy back in place behind the move's back. The move then saw a document it no longer recognised, assumed you had opened something else in the meantime, and stopped short — without finishing the handover or clearing the pasted copy away. The save still reported success, so the document became the file you chose while every note you added afterwards went on being written to the pasted copy instead, and was gone the next time you opened the saved file. Nothing looked wrong at the time: both copies held the same notes, so the only visible symptom was later work disappearing. A move now holds the document's identity from the start of the move to the end of it, so a re-read landing in the middle can no longer undo it, and the saved file keeps everything you add to it. Writing a note while the save is still under way went wrong the same way, and is now handled the way you would expect: the note goes onto the file you are saving to, not onto the pasted copy being left behind. The same holds when you save twice in quick succession and the second save takes over from the first: that move keeps hold of the pasted copy's name as well as the file the first save had already moved the notes to, so a re-read or a new note landing in the middle of it still follows the notes to the file you finally chose. Notes belonging to a genuinely different document opened during the save are still left alone, as before. - A remote document opened from a URL is no longer left downloading indefinitely against a server that trickles the body slowly enough to dodge the 30-second timeout (T-2138). The timeout applied only to network inactivity, so a byte sent just before each interval elapsed kept the load open with no end-to-end bound; the whole download is now also bounded by an explicit 30-second deadline covering redirects and streaming together. Implementing that deadline also surfaced, and fixed, a separate, pre-existing problem: accumulating the downloaded body ran on the main thread, where it is roughly 150 times slower. Measured on this project's own build, accumulating a 10 MB body takes 0.88 seconds off the main thread and 133 seconds on it, which could freeze the interface for well over a minute while opening a large document. That accumulation now genuinely runs off the main thread, so a large remote document opens in about a second instead of holding the interface still for minutes; the final UTF-8 decode of the (at most 10 MB) result still runs on the main thread afterwards, at roughly 10 milliseconds, which stays negligible. A file that really is over 10 MB is refused for its size, with the message that says so, rather than as a network timeout — with one trade-off: the new end-to-end deadline applies regardless of why a download is slow, so an honest, otherwise-successful download that used to take longer than 30 seconds to complete now fails with a timeout instead of eventually finishing. - Headings written inside a collapsible section now appear in the table of contents, on iPhone and on iPad/Mac, and choosing one opens the section it lives in before scrolling to it (T-1928). The contents list was built from a model that only ever looked at the top level of the document, so a heading inside a `<details>` block was missing from it entirely and there was no way to navigate to it — even though a separate, unused model in the app had been collecting those headings all along. A nested heading is now listed under whichever heading precedes it, marked with the same chevron the app already uses elsewhere for collapsible content, and is not itself collapsible from the contents list: the collapsible section it sits in is the thing that opens and closes. Following a link to a nested heading's anchor opens its section too, which it previously did not. A document with no collapsible sections is grouped and ordered exactly as before, with two deliberate improvements that also reach it: a heading containing a footnote marker or an HTML comment now lists with those stripped out, matching what the iPhone sheet always showed, and a heading with no text at all now reads "(Empty heading)" instead of appearing as a blank row. Fixing the navigation also uncovered a second problem in the same area, which is fixed here too: from the second collapsible section in a document onwards, the app was identifying those sections by a position that shifts as it counts through the contents of earlier ones, so it could not find them to open. Nothing had noticed because nothing had ever asked it to open one this way. Choosing a heading also no longer competes with the position the app restores when you reopen a document: only a heading you chose yourself is held briefly and re-applied once the section it lives in has opened, and any scroll, wheel flick, key press or click of your own cancels that immediately. - Documents containing HTML comments (`<!--…-->`) no longer stall while opening (T-2147). Several steps that look for comments — in a block of raw HTML, inside a link's label, and in the text Prism searches and exports — cost time in proportion to the *square* of what they were given, so a document that would otherwise open instantly could hold the app for tens of seconds. A run of comment openers with no closing `-->`, which is what a document being written, generated, or truncated mid-comment looks like, was the trigger: every opener read the whole rest of the document looking for a close before giving up. One step was worse than slow. Deciding whether a block of HTML is nothing but comments cost roughly four times as much for every two comments added, so a 145-byte document took 13 milliseconds, a 217-byte one 3.5 seconds, and a 235-byte one 22 seconds, with no upper bound beyond that — and it needed only a handful of ordinary, correctly closed comments followed by a single other character, not a malformed document at all. Removing comments from link labels had two further problems on top of the first. The entire document was rebuilt from scratch once per label carrying a comment, which cost 1.9 seconds for a 480 KB paragraph. And before either of those ran, finding the labels themselves had the same square-law shape on a `[` that is never closed: 32,000 unclosed brackets took 8.4 seconds, and an ordinary 96 KB paragraph that simply opens a few brackets without closing them took 6.2 seconds — so plain prose, not a malformed document, was enough on its own. Every one of these comment-scanning steps, and the label-finding step in front of them, now reads the document once, from left to right, and grows in step with its length rather than with its square. Because a document can be opened from a URL, a file written this way could previously have kept a device busy for a long time on someone else's behalf. Nothing about how comments are displayed changes: each replacement was checked against the exact step it replaced, character for character, over tens of thousands of generated fragments as well as hand-written awkward cases.

Things to double-check

Claim lifecycle: no leak path found.

Both beginMigration calls are synchronous and the defer is registered immediately after with no return or throw between. The chain-refusal early return sits before the claim; the save-failure, post-save give-up and success exits all unwind through it, and the claim correctly outlives store.delete. Tickets are UUIDs, and removeAll { $0.ticket == ticket } cannot pop a sibling's entry.

Store ordering is what makes the creation test meaningful.

NotesStore is an actor and save/load contain no internal await (NotesStore.swift:204-207 documents this as a contract two callers depend on). A redirected creation's persistNotes therefore enqueues behind the migration's in-flight save and wins on disk. If T-1723/T-1895 adds NSFileCoordinator here, re-derive this — the agent note already flags it as a live forward hazard.

The failure path can strand a note at the target.

A redirected creation persists the target-keyed container; if the migration's save then fails, memory reverts to the source but the target record keeps the note. Decision 8 does record this in its Consequences — the only quibble is that it cites Decision 5, which covers a different orphan class.

Three test-suite failures in make test-quick were flakes.

WebScrollabilityReportingTests, WebDetailsNavigationOrderingTests and HTMLCommentStrippingGrowthTests failed on the full run (the two WebKit ones after 24-26s, i.e. timeouts under load) and pass 38/38 on a targeted re-run. Unrelated to notes. An earlier targeted run also hit the known en-GB test-host launch failure ("Runningboard has returned error 5"), which cleared on re-run.