SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(web): drop redundant external-apply dirty bump

Browse source 
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.
This commit is contained in:
Kayshen-X 2026-07-17 04:06:17 +08:00
parent ba9efd43fb
commit 2fd88b601e
2 changed files with 25 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
crates/op-editor-core/src/state.rs
View file

@ -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());

31
crates/op-host-web/src/live_sync_glue.rs
View file

@ -262,27 +262,22 @@ fn apply_document_response<C: RepaintContext + 'static>(
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