diff --git a/pmacs-protocol/src/message.rs b/pmacs-protocol/src/message.rs index ea9d91c..5b478c3 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -705,6 +705,43 @@ pub enum InstanceMessage { /// Inline bytes or a URI the frontend resolves itself. body: ResourceBody, }, + /// Daemon-side input-dispatcher idleness signal. Tells a + /// `crdt_replica` frontend whether the next key event would be + /// **intercepted** by the daemon (minibuffer prompt active or + /// dispatcher holds a pending multi-key prefix) versus would + /// self-insert into the active buffer. + /// + /// Frontends running the optimistic-apply layer (`src/optimistic.rs`) + /// gate the local apply on `idle == true`: when the daemon is not + /// idle, plain-char keystrokes must round-trip as + /// [`FrontendEvent::Key`] so the daemon's minibuffer / prefix + /// dispatcher receives them. Without this signal, characters typed + /// into a minibuffer prompt would be applied as `CrdtOp`s to the + /// previously-active document instead — the surfacing of which + /// motivated this wire addition. + /// + /// Emission contract (daemon side): + /// + /// - Once after `AttachRequest` is accepted, before any other + /// non-handshake message, so a fresh frontend starts from a + /// known idle state regardless of the daemon's current input + /// condition. + /// - At every transition: minibuffer begin / dismiss; dispatcher + /// `pending` empty↔non-empty. + /// - Coalesced where consecutive transitions land on the same + /// value (no spurious back-to-back identical signals). + /// + /// Gated on negotiated `crdt_replica` — frontends without the + /// optimistic-apply layer have no use for this signal and the + /// daemon's per-session filter drops it for them. + DispatchIdle { + /// `true` → the next key event would self-insert into the + /// active buffer (no minibuffer, no pending prefix); optimistic + /// apply is correct. + /// `false` → the daemon would intercept the next key; the + /// frontend must round-trip via [`FrontendEvent::Key`]. + idle: bool, + }, } /// Flat selection state for the wire. @@ -922,7 +959,15 @@ pub enum ResourceBody { /// unchanged, and the new variants simply existing in the enums is /// not a wire-compat issue for non-semantic sessions because the /// daemon never emits them to those sessions. -pub const PROTOCOL_VERSION: u32 = 3; +/// +/// T M11.6: bumped from 3 to 4. v1, v2, v3 wires remain accepted; +/// [`InstanceMessage::DispatchIdle`] is filtered per-session for +/// sessions whose negotiated wire is `<= 3`. Same gating shape as the +/// `CrdtOp` / semantic-frontend bumps: the daemon's per-tick emission +/// checks the session's negotiated version and skips the variant for +/// older peers. An old peer would hard-error on decode of an unknown +/// postcard variant; gating prevents that. +pub const PROTOCOL_VERSION: u32 = 4; /// T M10.5: the set of protocol versions a v1.0 binary accepts on /// the wire. v0.1 binaries only accepted `[1]`; v1.0 binaries accept @@ -938,7 +983,11 @@ pub const PROTOCOL_VERSION: u32 = 3; /// `InstanceMessage::CrdtOp` / `PresenceUpdate` messages even from /// a v3 daemon, and only sessions that negotiated `semantic_render` /// receive the `SemanticFrame` variant family. -pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[1, 2, 3]; +/// +/// T M11.6: extended to `[1, 2, 3, 4]`. v4 sessions receive +/// `InstanceMessage::DispatchIdle`; older sessions are filtered out +/// of that emission. +pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[1, 2, 3, 4]; /// T M10.5: predicate for the handshake check. Returns `true` if /// `peer_version` is in [`SUPPORTED_PROTOCOL_VERSIONS`]. diff --git a/src/attach.rs b/src/attach.rs index 72c6a2a..31245d9 100644 --- a/src/attach.rs +++ b/src/attach.rs @@ -670,6 +670,18 @@ pub(crate) fn run_attach_pair( #[cfg(feature = "crdt")] let mut buffer_mirror = crate::buffer_mirror::BufferMirror::new(assigned_id); + // T M11.6 — daemon-side input-dispatcher idleness. `false` = + // pessimistic default before the first `DispatchIdle` arrives; + // the daemon emits an initial value once per attach so this + // converges to the true state on the first frame. While `false`, + // every keystroke round-trips as `FrontendEvent::Key` regardless + // of optimistic-apply eligibility — this is the fix for the + // M10.10-era latent bug where chars typed into a minibuffer + // prompt were optimistically applied to the previously-active + // document. + #[cfg(feature = "crdt")] + let mut dispatch_idle = false; + // Closure-and-call: any `return` from this closure still falls // through to `kick()` and `reader_handle.join()` below. Without // this wrapping a writer-side IO error inside the loop would skip @@ -763,6 +775,14 @@ pub(crate) fn run_attach_pair( } } } + #[cfg(feature = "crdt")] + Ok(InstanceMessage::DispatchIdle { idle }) => { + // T M11.6 — daemon's input-dispatcher state. + // Consumed *here only*: no frontend rendering + // depends on the value; the optimistic-apply + // gate reads `dispatch_idle` below. + dispatch_idle = idle; + } Ok(msg) => batch.push(msg), Err(mpsc::TryRecvError::Empty) => break, Err(mpsc::TryRecvError::Disconnected) => { @@ -813,7 +833,16 @@ pub(crate) fn run_attach_pair( // keys) fall through to the existing forward_event path. #[cfg(feature = "crdt")] let optimistic_handled = if let Event::Key(k) = &ev { - if matches!(k.kind, KeyEventKind::Press | KeyEventKind::Repeat) { + // T M11.6 — gate on daemon idleness. When the daemon + // would intercept this key (minibuffer prompt active + // or dispatcher holds a pending prefix), force the + // keystroke to round-trip via `FrontendEvent::Key` + // regardless of its optimistic-apply classification. + // The orchestrator below still runs for non-key + // events; only the Press/Repeat→CrdtOp path is gated. + if !dispatch_idle { + false + } else if matches!(k.kind, KeyEventKind::Press | KeyEventKind::Repeat) { let timestamp_ns = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map_or(0, |d| u64::try_from(d.as_nanos()).unwrap_or(0)); diff --git a/src/daemon.rs b/src/daemon.rs index cc9b883..7f7b063 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -836,6 +836,11 @@ fn dispatcher_loop( HashMap::new(); let mut streams: HashMap = HashMap::new(); let mut term_sizes: HashMap = HashMap::new(); + // T M11.6 — last `DispatchIdle` value broadcast per `crdt_replica` + // frontend. Absence means "never sent" — the first tick after + // attach emits an initial `DispatchIdle` so the frontend starts + // from a known idle state (its default is pessimistic-`false`). + let mut last_dispatch_idle_sent: HashMap = HashMap::new(); let mut session_registry = SessionRegistry::new(); // T M10.11 Q8 — jitter PRNG, seeded once so the // convergence-under-jitter scenario is deterministically @@ -979,9 +984,37 @@ fn dispatcher_loop( let snapshot = build_presence_snapshot(editor, *fid); let broadcasts = session_registry.sweep(&[(*fid, snapshot)]); - // Write frame messages to this frontend's stream. + // T M11.6 — DispatchIdle signal. `crdt_replica` frontends + // gate their optimistic-apply path on this; we ship it + // before the frame's other messages so a frontend that + // wakes mid-tick sees the gate flip first. Diff-suppressed + // — initial-after-attach (`last_dispatch_idle_sent` absent) + // and value-change emissions only. let mut write_failed = false; - if let Some(stream) = streams.get_mut(fid) { + if session_registry.session_state(*fid).is_some_and(|s| { + // Filter on both the `crdt_replica` capability (only + // optimistic-apply frontends care) and the negotiated + // wire version (>= 4 means peer knows the variant). + s.negotiated_capabilities.crdt_replica && s.negotiated_protocol_version >= 4 + }) && let Some(stream) = streams.get_mut(fid) + { + let idle_now = editor.dispatch_idle(); + if last_dispatch_idle_sent.get(fid) != Some(&idle_now) { + if let Err(e) = + write_message(stream, &InstanceMessage::DispatchIdle { idle: idle_now }) + { + eprintln!("pmacs: write DispatchIdle for {fid:?} failed: {e}"); + write_failed = true; + } else { + last_dispatch_idle_sent.insert(*fid, idle_now); + } + } + } + + // Write frame messages to this frontend's stream. + if let Some(stream) = streams.get_mut(fid) + && !write_failed + { for msg in &messages { // T M10.10 Day 4 / M10.11 F2 — the criterion-1 // jitter site: render-write latency. @@ -1061,6 +1094,7 @@ fn dispatcher_loop( render_states.remove(fid); semantic_states.remove(fid); term_sizes.remove(fid); + last_dispatch_idle_sent.remove(fid); session_registry.unregister_session(*fid); editor.core.borrow_mut().unregister_frontend_view(*fid); } @@ -1100,6 +1134,7 @@ fn dispatcher_loop( &mut semantic_states, &mut streams, &mut term_sizes, + &mut last_dispatch_idle_sent, &mut session_registry, ); // Drain a burst of immediately-available events to @@ -1114,6 +1149,7 @@ fn dispatcher_loop( &mut semantic_states, &mut streams, &mut term_sizes, + &mut last_dispatch_idle_sent, &mut session_registry, ); } @@ -1209,6 +1245,7 @@ fn handle_session_established( editor.core.borrow_mut().active_frontend = frontend_id; } +#[allow(clippy::too_many_arguments)] fn handle_dispatcher_event( event: DispatcherEvent, editor: &mut EditorState, @@ -1216,6 +1253,7 @@ fn handle_dispatcher_event( semantic_states: &mut HashMap, streams: &mut HashMap, term_sizes: &mut HashMap, + last_dispatch_idle_sent: &mut HashMap, session_registry: &mut SessionRegistry, ) { match event { @@ -1337,6 +1375,7 @@ fn handle_dispatcher_event( semantic_states.remove(&frontend_id); streams.remove(&frontend_id); term_sizes.remove(&frontend_id); + last_dispatch_idle_sent.remove(&frontend_id); session_registry.unregister_session(frontend_id); editor .core diff --git a/src/editor.rs b/src/editor.rs index c3d86f5..6b20371 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -427,6 +427,32 @@ impl EditorState { /// dispatcher, and invoke the resolved command (or the /// self-insert fallback for an unbound printable chord). /// + /// Whether the daemon's key-dispatch path is currently "idle" in + /// the sense that the *next* key event would self-insert into the + /// active buffer rather than being intercepted. + /// + /// `false` when either: + /// + /// - the dispatcher holds a pending multi-key prefix (e.g. the + /// user has typed `C-x` and the daemon is waiting for the next + /// chord), or + /// - a minibuffer prompt is active and absorbing keys. + /// + /// Used by the daemon to drive the `InstanceMessage::DispatchIdle` + /// wire signal that gates `crdt_replica` frontends' optimistic-apply + /// path. Without this signal the optimistic layer would Insert a + /// plain-char keystroke into the active document while the + /// daemon's actual intent is to route the keystroke into the + /// minibuffer prompt — the M10.10 "documented limitation" that + /// surfaced during session-5 manual validation. + #[must_use] + pub fn dispatch_idle(&self) -> bool { + if !self.dispatcher.pending().is_empty() { + return false; + } + !self.core.borrow().minibuffer.is_active() + } + /// `frontend_id` records which frontend produced the event. v0.1 /// uses [`FrontendId::LOCAL`] uniformly; the parameter is /// load-bearing for v0.3 multi-frontend scenarios where the @@ -1582,6 +1608,71 @@ mod tests { assert!(s.core.borrow().status.contains("no file")); } + // ---- T M11.6 — DispatchIdle --------------------------------------------- + + #[test] + fn dispatch_idle_true_on_fresh_editor() { + let s = fresh_with(b""); + assert!(s.dispatch_idle()); + } + + #[test] + fn dispatch_idle_false_while_prefix_pending() { + let mut s = fresh_with(b""); + s.dispatch_key(FrontendId::LOCAL, ctrl('x')); + assert!( + !s.dispatch_idle(), + "C-x prefix should put dispatcher in non-idle state" + ); + } + + #[test] + fn dispatch_idle_true_after_prefix_resolves() { + let mut s = fresh_with(b""); + s.dispatch_key(FrontendId::LOCAL, ctrl('x')); + assert!(!s.dispatch_idle()); + // C-x C-c resolves the prefix into the quit command. After + // the second chord arrives the dispatcher's `pending` is + // cleared regardless of whether the command succeeded. + s.dispatch_key(FrontendId::LOCAL, ctrl('c')); + assert!(s.dispatch_idle(), "prefix cleared ⇒ idle again"); + } + + #[test] + fn dispatch_idle_false_while_minibuffer_active() { + use crate::minibuffer::{CompletionSource, MinibufferSession}; + + let s = fresh_with(b""); + assert!(s.dispatch_idle()); + + // Open a synthetic minibuffer session — same shape Lua's + // `pmacs.minibuffer.read` produces. + let lua = mlua::Lua::new(); + let on_accept: mlua::Function = lua + .create_function(|_, _: String| Ok(())) + .expect("create on_accept"); + s.core.borrow_mut().minibuffer.begin(MinibufferSession { + prompt: "test: ".into(), + initial: String::new(), + history_bucket: String::new(), + source: CompletionSource::None, + on_accept, + on_cancel: None, + candidates: Vec::new(), + selected: None, + history_index: None, + typed_before_history_nav: None, + }); + assert!( + !s.dispatch_idle(), + "active minibuffer prompt should put dispatcher in non-idle state" + ); + + // Dismissing returns to idle. + let _ = s.core.borrow_mut().minibuffer.cancel(); + assert!(s.dispatch_idle(), "dismissed minibuffer ⇒ idle again"); + } + #[test] fn repeat_key_events_dispatch_like_press() { // Some terminals deliver auto-repeated keys as KeyEventKind::Repeat diff --git a/src/frontend.rs b/src/frontend.rs index 974d1e2..fc5be2b 100644 --- a/src/frontend.rs +++ b/src/frontend.rs @@ -383,7 +383,11 @@ impl Frontend { | InstanceMessage::BlockAdornments { .. } | InstanceMessage::FoldState { .. } | InstanceMessage::FileStyleSummary { .. } - | InstanceMessage::ResourceOffer { .. } => { + | InstanceMessage::ResourceOffer { .. } + // T M11.6 — DispatchIdle is consumed by `attach.rs`'s + // optimistic-apply gate; if any reaches this render path + // (shouldn't, given the attach drain), drop silently. + | InstanceMessage::DispatchIdle { .. } => { // v0.1 TUI ignores these; v0.3 GUI consumes them. } } diff --git a/src/protocol.rs b/src/protocol.rs index 8caa2e2..6ed3bcd 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1683,27 +1683,29 @@ mod tests { // --- M5.5a handshake & postcard round-trips --- #[test] - fn protocol_version_is_three_for_v11() { + fn protocol_version_is_four_for_dispatch_idle() { // Pin the value: T M10.5 bumped 1→2 (v1.0 wire: CrdtOp / // PresenceUpdate). T M11.1 bumped 2→3 (v1.1 wire: the - // SemanticFrame family + FrontendEvent::Viewport). The v1.1 - // binary serves v1, v2, and v3 sessions — the slice-membership - // handshake makes the relaxation symmetric, exactly as M10.5 - // did for §sec:m10-backward-compat. - assert_eq!(PROTOCOL_VERSION, 3); + // SemanticFrame family + FrontendEvent::Viewport). T M11.6 + // bumped 3→4 (DispatchIdle for the optimistic-apply gate). + // The current binary serves v1..=v4 sessions — the slice- + // membership handshake makes the relaxation symmetric. + assert_eq!(PROTOCOL_VERSION, 4); } #[test] - fn supported_protocol_versions_includes_one_two_three() { + fn supported_protocol_versions_includes_one_through_four() { // T M10.5: v1.0 binaries accept v1+v2. T M11.1: v1.1 binaries - // accept v1+v2+v3. The check is slice membership, not strict - // equality, so v0.1/v1.0 binaries keep connecting to v1.1 - // binaries unchanged. v4+ is rejected until the next bump. + // accept v1+v2+v3. T M11.6: v4 binaries accept v1+v2+v3+v4. + // The check is slice membership, not strict equality, so + // older binaries keep connecting to current binaries + // unchanged. v5+ is rejected until the next bump. assert!(is_supported_protocol_version(1)); assert!(is_supported_protocol_version(2)); assert!(is_supported_protocol_version(3)); + assert!(is_supported_protocol_version(4)); assert!(!is_supported_protocol_version(0)); - assert!(!is_supported_protocol_version(4)); + assert!(!is_supported_protocol_version(5)); assert!(!is_supported_protocol_version(u32::MAX)); } @@ -1851,6 +1853,21 @@ mod tests { } } + #[test] + fn dispatch_idle_round_trips_through_postcard() { + // T M11.6 — the wire variant. Round-trip both polarities so + // the postcard encoding of bool is verified in both states. + for idle in [true, false] { + let msg = InstanceMessage::DispatchIdle { idle }; + let bytes = postcard::to_allocvec(&msg).expect("encode"); + let decoded: InstanceMessage = postcard::from_bytes(&bytes).expect("decode"); + match decoded { + InstanceMessage::DispatchIdle { idle: got } => assert_eq!(got, idle), + other => panic!("expected DispatchIdle, got {other:?}"), + } + } + } + #[test] fn key_event_to_crossterm_round_trips() { // Build a protocol KeyEvent, translate to crossterm, translate @@ -2056,11 +2073,11 @@ mod tests { #[test] fn m10_5_handshake_matrix_versions_outside_range_rejected() { // v1 daemon's strict-equality behavior is documented at the - // v0.1 code level (different binary); the v1.1 daemon's range - // check accepts v1/v2/v3 (T M11.1 added v3) and rejects v4+ - // until the next protocol bump. + // v0.1 code level (different binary); the current daemon's + // range check accepts v1/v2/v3/v4 (T M11.1 added v3; T M11.6 + // added v4) and rejects v5+ until the next protocol bump. assert!(!is_supported_protocol_version(0)); - assert!(!is_supported_protocol_version(4)); + assert!(!is_supported_protocol_version(5)); assert!(!is_supported_protocol_version(u32::MAX)); }