From 2fd88b601e80273e3cc77be91f1426f3ce02cbd2 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Fri, 17 Jul 2026 04:06:17 +0800 Subject: [PATCH] fix(web): drop redundant external-apply dirty bump The `if undoable { mark_document_changed() }` added to the live-sync glue was redundant: `replace_document_with_undo` already bumps the content revision via `history_push_past` -> `mark_document_changed`, so an undoable external apply is dirty by construction. Remove the glue's manual bump (and its now-wrong comment); the post-apply pair capture + `note_synced` stay put, since that pair already carries the history bump. Retarget the editor-core test to lock the real invariant: `replace_document_with_undo` ALONE leaves the state dirty (revision != saved), while plain `replace_document` stays clean -- so a future refactor of `history_push_past` can't silently break external-apply dirtiness. --- crates/op-editor-core/src/state.rs | 20 +++++++++------ crates/op-host-web/src/live_sync_glue.rs | 31 ++++++++++-------------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/crates/op-editor-core/src/state.rs b/crates/op-editor-core/src/state.rs index f75c06d4c..9ed8c24fd 100644 --- a/crates/op-editor-core/src/state.rs +++ b/crates/op-editor-core/src/state.rs @@ -594,19 +594,23 @@ mod tests { } #[test] - fn external_apply_bump_marks_dirty_while_open_stays_clean() { - // Locks the editor-core semantics the wasm-only live-sync glue relies - // on (Finding 2): an undoable external apply (AI turn / MCP client) - // followed by `mark_document_changed` must read dirty, while the plain - // open path (`replace_document`) must stay clean. The glue ordering - // itself is compile-gated wasm-only; this test guards the primitives. + fn external_apply_marks_dirty_while_open_stays_clean() { + // Locks the editor-core invariant the wasm-only live-sync glue relies + // on: an undoable external apply (AI turn / MCP client) reads dirty ON + // ITS OWN — `replace_document_with_undo` bumps the content revision via + // `history_push_past` → `mark_document_changed`, so the glue needs no + // manual bump — while the plain open path (`replace_document`) stays + // clean. Locking the invariant here means a future refactor of + // `history_push_past` can't silently break external-apply dirtiness. + // The glue ordering itself is compile-gated wasm-only; this guards the + // primitives it composes. let mut s = EditorState::new(); s.replace_document(empty_document()); // mount-time pull: clean baseline assert!(!s.is_dirty()); - // Undoable external apply + the glue's post-apply content bump. + // Undoable external apply ALONE (no manual mark_document_changed) is + // dirty by construction. s.replace_document_with_undo(empty_document()); - s.mark_document_changed(); assert!(s.is_dirty()); assert_ne!(s.document_revision(), s.saved_revision()); diff --git a/crates/op-host-web/src/live_sync_glue.rs b/crates/op-host-web/src/live_sync_glue.rs index 3c2241531..e0dd71c9d 100644 --- a/crates/op-host-web/src/live_sync_glue.rs +++ b/crates/op-host-web/src/live_sync_glue.rs @@ -262,27 +262,22 @@ fn apply_document_response( if let Ok(mut last_selection_key) = last_selection_key.try_borrow_mut() { *last_selection_key = None; } + // Commit the sync-gate baseline AFTER the apply closure has returned + // (its mutable borrow above is released) and using the POST-apply pair. + // // An external post-bootstrap apply (`undoable` — an AI turn or an MCP // client write) is an UNSAVED daemon-side edit: the tab must read dirty // so closing it prompts to save (spec: MCP edits mark the tab dirty). - // `replace_document_with_undo` leaves revision == saved_revision - // (falsely clean), so bump the content revision now. The bump MUST - // precede the `post_apply` capture below: `note_synced` then records the - // POST-bump pair as the gate baseline, so the very next push tick sees - // current == baseline (no spurious echo-push of the just-applied doc - // back to the daemon) while `is_dirty()` stays true (revision 1 vs - // saved 0). The bootstrap apply (`undoable == false`, the mount-time - // starter→daemon pull) skips the bump and stays clean. - if undoable { - inner_ref - .host_mut() - .editor_state_mut() - .mark_document_changed(); - } - // Commit the sync-gate baseline AFTER the apply closure has returned - // (its mutable borrow above is released) and using the POST-apply - // pair (`replace_document` bumped generation; the dirty bump above - // bumped revision). + // That dirtiness is already established by construction: + // `replace_document_with_undo` runs `history_push_past`, which bumps the + // content revision via `mark_document_changed` — so no manual bump is + // needed here (it would only be a redundant second revision tick). + // `note_synced` records this post-bump pair as the gate baseline, so the + // very next push tick sees current == baseline (no spurious echo-push of + // the just-applied doc back to the daemon) while `is_dirty()` stays true + // (revision != saved 0). The bootstrap apply (`undoable == false`, the + // mount-time starter→daemon pull) goes through the plain + // `replace_document`, which resets revision to 0 and stays clean. let post_apply = current_pair(inner_ref); sync.borrow_mut() .gate