T M11.9 — handle_remote_crdt_op fires buffer.after-edit (closes session-5 root cause) (#49)
Surfaced when PR #48's diag-store stale-flag turned out to have no observable effect on session-5 validation: the wrong-position-color artifact persisted even though the stale-flag suppression chain was in place. ## Root cause (the *actual* one) The M10.10 optimistic-apply layer routes plain-char keystrokes (EOL-eligible, no Ctrl-modifier, etc.) as `FrontendEvent::CrdtOp` rather than `FrontendEvent::Key`. The daemon dispatches CrdtOps via `handle_remote_crdt_op`, which applies the buffer edit and queues the op for broadcast — but **does not fire `buffer.after-edit`**. `buffer.after-edit` was only fired by `dispatch_key` (editor.rs:506) after a Key-path edit. The CrdtOp path bypassed it entirely. The downstream LSP hook in `builtin/runtime/lsp.lua:379` calls `pmacs.lsp.did_change` on every `buffer.after-edit`. With CrdtOp edits not firing the hook, `did_change_full` (and therefore `textDocument/didChange`) was never sent to clangd for the bulk of typing activity. clangd's view of the document silently froze at whatever state the last Key-path edit (find-file, keystrokes through the minibuffer, modifier-combinations) had left it in. Downstream symptoms, all silent: - **Diagnostics frozen at pre-edit byte positions** — the session-5 visible artifact. - **LSP semantic tokens stale** (for grammar-less languages where semantic_render uses LSP not tree-sitter — i.e. C++). - **Inlay hints stale**. - **Hover/go-to-definition/rename can return wrong-position results** if a CrdtOp edit moved positions since the last Key edit. PR #47 (full=true on generation transition) and PR #48 (diag-store stale-flag) were correctness fixes on the producer side, but they depended on `did_change` actually firing to trigger their effects. With did_change silenced, both were dormant for any CrdtOp edit. ## Fix In `handle_remote_crdt_op`, when `edit_opt` is `Some` (the import produced a text delta), after notifying views: 1. Set `active_frontend = source` so the hook's `pmacs.window.buffer()` resolves to the right buffer (matches the pattern `dispatch_key` uses). 2. Fire `buffer.after-edit` via `editor.lua_host.run_hook(...)`. This makes the Lua observer chain (LSP `did_change` and any future consumers) see CrdtOp-path edits identically to Key-path edits. ## Regression test `daemon::tests::handle_remote_crdt_op_fires_after_edit_hook` (crdt-gated): 1. Upgrade the active buffer to CRDT-backed 2. Install a Lua `buffer.after-edit` hook that bumps a global 3. Build a peer LoroDoc from the buffer snapshot, edit on the peer, export the op 4. Call `handle_remote_crdt_op` with the op 5. Assert the global counter is `1` Pre-fix, the counter stays at `0`. ## Gates | Gate | Result | |---|---| | `cargo fmt --check` | clean | | `clippy --features crdt --workspace -D warnings` | clean | | `clippy --workspace -D warnings` (no crdt) | clean | | `cargo test --features crdt --lib` | 1483 (+1) | | `cargo test --lib` (no crdt) | 1319 (test crdt-gated) | | `m4_acceptance --features crdt` | 83 | | `m11_5_semantic_acceptance --features crdt` | 2 | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
886239480e
commit
8db0485839
119
src/daemon.rs
119
src/daemon.rs
|
|
@ -1777,6 +1777,29 @@ fn handle_remote_crdt_op(
|
|||
}
|
||||
|
||||
core.notify_buffer_edit(buffer_id, edit);
|
||||
// T M11.9 — temporarily switch active_frontend to source so
|
||||
// the `buffer.after-edit` hook's Lua observers (notably the
|
||||
// LSP `did_change` glue in `builtin/runtime/lsp.lua`) read
|
||||
// the right buffer via `pmacs.window.buffer()`. Matches the
|
||||
// pattern `dispatch_key` uses (it assigns `active_frontend`
|
||||
// before running its hook).
|
||||
core.active_frontend = source;
|
||||
drop(core);
|
||||
|
||||
// T M11.9 — fire `buffer.after-edit` for replicated edits.
|
||||
// Without this, LSP `textDocument/didChange` is never sent
|
||||
// for keystrokes the M10.10 optimistic-apply layer routed as
|
||||
// `FrontendEvent::CrdtOp` (the bulk of plain-char typing),
|
||||
// so clangd's view of the document drifts behind reality.
|
||||
// Diagnostics, semantic tokens, and inlay hints all silently
|
||||
// freeze at the byte positions they last had when an edit
|
||||
// happened to fall back to the Key path. Closes the actual
|
||||
// root cause of the session-5 wrong-position-color
|
||||
// artifact; the diag-store stale-flag from T M11.8 finally
|
||||
// gets reached.
|
||||
editor
|
||||
.lua_host
|
||||
.run_hook("buffer.after-edit", mlua::MultiValue::new());
|
||||
}
|
||||
|
||||
// Effect 4: queue for broadcast. The source frontend's mirror
|
||||
|
|
@ -1981,4 +2004,100 @@ mod tests {
|
|||
// Test environments are UTF-8, so this should be non-empty.
|
||||
assert!(!id.working_directory.is_empty());
|
||||
}
|
||||
|
||||
/// T M11.9 regression: `handle_remote_crdt_op` fires the
|
||||
/// `buffer.after-edit` Lua hook. Without this, the M10.10
|
||||
/// optimistic-apply path's `FrontendEvent::CrdtOp` route
|
||||
/// bypasses every Lua observer of buffer mutations — most
|
||||
/// importantly the LSP `did_change` notification, which means
|
||||
/// clangd never re-analyzes documents edited via the optimistic
|
||||
/// path. The session-5 wrong-position-color artifact was the
|
||||
/// downstream symptom: diagnostics frozen at pre-edit byte
|
||||
/// positions because clangd had never been told about the edit.
|
||||
#[cfg(feature = "crdt")]
|
||||
#[test]
|
||||
fn handle_remote_crdt_op_fires_after_edit_hook() {
|
||||
use crate::editor::EditorState;
|
||||
use crate::protocol::FrontendId;
|
||||
|
||||
let mut editor = EditorState::new();
|
||||
|
||||
// Install an after-edit hook that bumps a global counter
|
||||
// we can read back from Lua.
|
||||
editor
|
||||
.lua_host
|
||||
.eval(
|
||||
Some("test"),
|
||||
r#"
|
||||
_G.PMACS_TEST_AFTER_EDIT_FIRED = 0
|
||||
pmacs.hook.add("buffer.after-edit", function()
|
||||
_G.PMACS_TEST_AFTER_EDIT_FIRED = (_G.PMACS_TEST_AFTER_EDIT_FIRED or 0) + 1
|
||||
end)
|
||||
"#,
|
||||
)
|
||||
.expect("install after-edit hook");
|
||||
|
||||
// Upgrade the active buffer to CRDT-backed so
|
||||
// `handle_remote_crdt_op` finds a `CrdtState` to apply
|
||||
// against (the non-CRDT path is not exercised here).
|
||||
let buffer_id = editor.core.borrow().active_window().buffer_id;
|
||||
{
|
||||
let core = editor.core.borrow();
|
||||
let mut reg = core.registry.borrow_mut();
|
||||
reg.get_mut(buffer_id)
|
||||
.expect("active buffer")
|
||||
.upgrade_to_crdt(2)
|
||||
.expect("upgrade to crdt");
|
||||
}
|
||||
|
||||
// Build a peer CRDT doc from the buffer's snapshot, perform
|
||||
// an edit on the peer, export the op bytes. This is the
|
||||
// shape `optimistic::apply_local_insert` produces in the
|
||||
// attach loop's optimistic-apply branch.
|
||||
let snapshot_bytes = {
|
||||
let core = editor.core.borrow();
|
||||
let reg = core.registry.borrow();
|
||||
let buf = reg.get(buffer_id).expect("buffer");
|
||||
buf.crdt_state()
|
||||
.expect("crdt-backed")
|
||||
.export_snapshot()
|
||||
.expect("export snapshot")
|
||||
};
|
||||
let peer = loro::LoroDoc::new();
|
||||
peer.set_peer_id(99).expect("set peer id");
|
||||
peer.import(&snapshot_bytes).expect("import snapshot");
|
||||
let v_before = peer.oplog_vv();
|
||||
peer.get_text("body").insert(0, "x").expect("peer insert");
|
||||
let op_bytes = peer
|
||||
.export(loro::ExportMode::updates(&v_before))
|
||||
.expect("export op");
|
||||
|
||||
// Apply the op via `handle_remote_crdt_op`.
|
||||
super::handle_remote_crdt_op(
|
||||
&mut editor,
|
||||
FrontendId(99),
|
||||
buffer_id,
|
||||
crate::rope::CrdtOp {
|
||||
peer_id: 99,
|
||||
bytes: op_bytes,
|
||||
},
|
||||
);
|
||||
|
||||
// The hook should have fired exactly once.
|
||||
let count_val = editor
|
||||
.lua_host
|
||||
.eval(
|
||||
Some("test-readback"),
|
||||
"return _G.PMACS_TEST_AFTER_EDIT_FIRED",
|
||||
)
|
||||
.expect("read counter");
|
||||
let count = match count_val {
|
||||
mlua::Value::Integer(n) => n,
|
||||
other => panic!("expected counter integer, got {other:?}"),
|
||||
};
|
||||
assert_eq!(
|
||||
count, 1,
|
||||
"buffer.after-edit must fire when handle_remote_crdt_op produces a text Edit"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue